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

Mouse event sensors for bar charts #1428

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Conversation

alexmacy
Copy link

I'm really excited for the new version to be compatible with d3 v4/v5! Thanks for the update!

I'd like to propose the addition of mouse-sensor bars for the bar charts. These are 'hidden' bars that span the entire height of the chart, beyond the top of the visible bar. This is particularly helpful for bars with a small enough value that it is barely visible.

Note: this requires update to bar-chart.js as well as a css change that I made in dc.css - but may need to also be made elsewhere? And as you can see, I did not make any changes to dc.js, dc.min.js, or dc.min.css.

Add 'hidden' bars over the existing bars. These act as mouse interaction sensor for bars, and is particularly helpful for bars that have a low enough value to be difficult to click on.

Also need to make a slight change to dc.css and dc.min.css.
@gordonwoodhull
Copy link
Contributor

I'm in favor of this. People have used bar chart labels as a workaround here, but you don't always want that clutter.

A couple things need to be cleaned up:

  • you need a unique class name for the sensor-bars so they don't get mistaken for real bars, by the tests (4 failing) or by user code. Please test with grunt test
  • we use sass (scss) so the new styles belong in style/dc.scss - dc.css is generated

I am conflicted about whether/how the background should be highlighted or not when it's hovered.

image

image

(You can barely see the grey highlight on the first screenshot, but it's easier to see IRL.)

It's kind of nice to have the additional interaction indicating that clicking there is like clicking on the bar. But I found it distracting that it's a different color depending whether the bar is selected or not. I wonder if anyone else has an opinion about this. My instinct it to use a very pale green there instead of blue or grey but that is of course a matter of opinion! Obviously it should be controlled by CSS.

Finally, I think we should put the feature under a flag (say mouseSensor) - it can be defaulted true, but I think people should have the option to turn it off if they don't want it.

@alexmacy
Copy link
Author

Great! I'll take a look when I get a chance.

- Change the class of the sensor bars.
- Test an idea for passing highlighting from the sensor bars to the
main bars.
@alexmacy
Copy link
Author

Made some more changes:

  • Class for the sensor bars is now '.sensor-bar' and it's passing the grunt tests.
  • Added .mousesensor([mousesensor]), defaulted it to true.
  • Took a stab at the highlighting issue that you pointed out. Curious what you might think about it, since it feels kinda hacky. I added an event listener that looks for the bar with the same index in the .main layer and changes the opacity from there. The result is that you never see the sensor bars and visually it looks the same as before, just with the added interaction. Again, kinda hacky, and doesn't feel as clean as using the :hover state of the element...

Testing with the other examples also made me realize two things that I'll look into when I get a chance:

  • it would be incomplete to add this to the bar charts without adding it to the row charts
  • the mouse sensors are not working as intended for stacked bar charts

@grugknuckle
Copy link

I have run into this issue myself and would love to have a feature like this. However, I would point out that that the issue of not being able to hover/click small bars isn't limited just to bar charts. For example, pie charts and row charts have the same problem. In my opinion, a general solution would be better. I don't know what that is, but I think having the labels be clickable / hoverable might be satisfactory.

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