Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Show value counts and highlight bar on histogram hover #44

Merged
merged 7 commits into from
Aug 28, 2024

Conversation

jwilber
Copy link
Contributor

@jwilber jwilber commented Aug 17, 2024

A little Friday night PR: adding hover effects to the histogram (see video).

  • Add hover bar
  • Show value counts/percents for the selected bin on hover
quakPRHistHover.mov

In addition, I also created a percentFormatter, that formats percents in a cleaner manner, and added this to the ValueCountsPlot as well.

One note here, hovering over the histogram currently shows the x-axis value -- it may be more appropriate to show the actual bin interval (see image below, which is what Observable does). A different PR can add that in pretty easily given the current setup, if that's desired.

Screenshot 2024-08-16 at 11 12 04 PM

@jwilber jwilber changed the title Histogram hover: show value counts and highlight bar on feat: Histogram hover: show value counts and highlight bar on Aug 17, 2024
@dvdkouril
Copy link
Collaborator

I’ll just comment on the histogram hover value design choice: The reasoning is that you’re still selecting a range start/end even within a single bun, that’s why showing a precise value is more appropriate, imho.
Also I think replicating Observable datatable 1 to 1 is not the goal (but Trevor might have a different opinion)

@jwilber
Copy link
Contributor Author

jwilber commented Aug 17, 2024

Fair points! I have no strong opinions, as I'm not the author of the library, but here's my though process regarding proposed PR changes:

  1. In my opinion, that level of granularity makes more sense in this specific linked view if the corresponding rows are highlighted as well. In general, I suspect visualization consumers expect any interactions in the UI to directly reflect (and act on) the marks and encodings of the viz. That said, the proposed hover here does maintain the precise value in the x-axis, but also highlights the bin it belongs to.

  2. From an outsider looking in, the UI has exact one-to-one parity with Observable datatable. There are some missing features (e.g. time-series display, UI interactivity such as this, etc.) My goal was to help pad these in, but if I have misread the intention of the library, feel free to close this PR -- no harm done!

Copy link
Owner

@manzt manzt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jwilber for another excellent contribution!

To clarify: While I don't have a specific roadmap or goal for 100% parity with Observable's data table, your recent PRs have been filling crucial gaps in our functionality. These additions are valuable and align well with features I've been considering.

With that said, I want to be open and welcoming to new design choices beyond Observable. For example, I've been tinkering with a new UI for column selection/reordering beyond Observable's simple multi-select checkboxes.

The histogram hover effects could be another area of divergence. I see value in both:

  • More precise hover info for selecting ranges (crosshair cursor)
  • Bin hovering info (for consistency with categorical value pills)

For histogram interactions, I've noticed that Observable only shows the highlighted bin when hovering below the bins. When in crosshair selection mode, no hover information is displayed (including bins). I think we could adopt a similar approach, but with an added feature: in crosshair mode, we could display the current precise hover value (as is currently implemented).

This would combine the familiar Observable behavior with context for creating range selection precision. We could tackle this in a separate PR. What do you think?

Example of Observable behavior (bin hovering without crosshair tool):

Screen.Recording.2024-08-19.at.15.18.42.mov

idea: It would actually be kind of cool if a click interaction in the bin hovering mode made a continuous range selection for just that bin, since the continuous cross hair selection is the only way to define ranges currently.

@jwilber
Copy link
Contributor Author

jwilber commented Aug 22, 2024

Sure!

So, I'll move the percent formatting into it's own separate PR, and add that to the valuecounts plot as well.

For histogram interactions, I've noticed that Observable only shows the highlighted bin when hovering below the bins. When in crosshair selection mode, no hover information is displayed (including bins). I think we could adopt a similar approach, but with an added feature: in crosshair mode, we could display the current precise hover value (as is currently implemented).

I'm not sure I follow 100% here. My take is: keep the functionality I've added here, but only trigger when the user hovers below the bins, otherwise, keep the current crosshair functionality? Is that right?

@manzt
Copy link
Owner

manzt commented Aug 22, 2024

So, I'll move the percent formatting into it's own separate PR, and add that to the valuecounts plot as well.

awesome!

My take is: keep the functionality I've added here, but only trigger when the user hovers below the bins, otherwise, keep the current crosshair functionality?

Couldn't have said it better myself (clearly 😂)!

@jwilber
Copy link
Contributor Author

jwilber commented Aug 23, 2024

Sweet, decided to just update in this MR, since it's not much code change.

See:

quakMRhover.mov

@manzt manzt changed the title feat: Histogram hover: show value counts and highlight bar on feat: show value counts and highlight bar on histogram hover Aug 28, 2024
@manzt manzt changed the title feat: show value counts and highlight bar on histogram hover feat: Show value counts and highlight bar on histogram hover Aug 28, 2024
@manzt manzt merged commit fa61ea3 into manzt:main Aug 28, 2024
3 checks passed
@manzt
Copy link
Owner

manzt commented Aug 28, 2024

Thanks @jwilber !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants