c3js / c3

:bar_chart: A D3-based reusable chart library
http://c3js.org
MIT License
9.34k stars 1.39k forks source link

Restore tooltip behavior when grouped #2684

Closed panthony closed 5 years ago

panthony commented 5 years ago

In previous version of c3js when tooltip_grouped was true, the simple act of hovering above a category would highlight/show the data point(s) of that particular category.

But it was broken when c3js started to use a single rect for the whole chart.

This PR aims to restore this important behavior.

It also includes the fix proposed in #2151.

Closes #2362 Closes #1745 Closes #2641 Closes #1201

It removes one of the usage of pathSegList (#2075)

Left to do:

codecov-io commented 5 years ago

Codecov Report

Merging #2684 into master will increase coverage by 1.51%. The diff coverage is 95.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2684      +/-   ##
==========================================
+ Coverage   81.01%   82.52%   +1.51%     
==========================================
  Files          59       59              
  Lines        4640     4659      +19     
==========================================
+ Hits         3759     3845      +86     
+ Misses        881      814      -67
Impacted Files Coverage Δ
src/type.js 98.43% <ø> (-0.05%) :arrow_down:
src/shape.bar.js 100% <100%> (+1.33%) :arrow_up:
src/util.js 98.03% <100%> (+0.26%) :arrow_up:
src/grid.js 74.8% <100%> (+0.76%) :arrow_up:
src/core.js 90.9% <100%> (+0.15%) :arrow_up:
src/data.js 89.13% <100%> (+1.95%) :arrow_up:
src/interaction.js 80.85% <92.15%> (+30.85%) :arrow_up:
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2bc2488...1e66104. Read the comment docs.

panthony commented 5 years ago

I added more tests but codecov did not re-run, not sure if possible to make it re-run manually.

panthony commented 5 years ago

@kt3k I was thinking about fixing #1658 too by moving data_onclick outside the loop and simply calling it once with closest which is supposed to be the closest data point to the mouse.

What do you think?

kt3k commented 5 years ago

@panthony Thank you for tackling this. This was one of the most requested fixes in c3js! The examples seem working nicely on chrome and firefox 👍 Testing with mouse interaction looks so great!

I left a few comments.

I was thinking about fixing #1658 too by moving data_onclick outside the loop and simply calling it once with closest which is supposed to be the closest data point to the mouse.

Sounds good to me.

IceCreamYou commented 4 years ago

Is there a way to avoid this behavior? I have charts with large tooltips, and with this new behavior where tooltips appear when you're not hovering a point/bar, the tooltips get in the way. In particular they get in the way of drag-selections and sometimes even seeing the data point you're trying to hover. I know that I can set tooltip.grouped to false, and I can override tooltip.contents to still display grouped info in that case, but I want to keep the behavior that if a point and bar have the same index, hovering over the bar also gives the point hover styles (and vice versa). If possible, I would prefer not to have to revert to manually toggling classes based on what's hovered or where the mouse is.