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

Fix hovermode 'closest' + hoverinfo 'none' with superimposed data bug #495

Merged
merged 2 commits into from
May 2, 2016

Conversation

etpinard
Copy link
Contributor

@etpinard etpinard commented May 2, 2016

fixes #493

@mdtusz this commit c8b05ed in PR #438 introduced the bug in v1.10.0.

In brief

hoverinfo: 'none' traces used to be skipped over in the search-closest-data routine before v1.10.0 which fixed issue #313 . But, as a side-effect, that patch allowed Fx.getClosest to pick hoverinfo: 'none' data points which were then passed to the hover label routine resulting in non apparent hover labels.

More info on Fx.getClosest

Note that hovermode: 'closest' only shows the hover label for the closest point. Previously, if two points were superimposed, only the data point of the first trace (as ordered in data) was passed to the hover label routine. Now, all superimposed points (i.e with the same distance to cursor) are passed to the hover label routine.

etpinard added 2 commits May 2, 2016 10:42
- so that superimposed trace hover data can skip over 'hoverinfo'
  traces upon hover label creation.
@etpinard etpinard added bug something broken status: reviewable labels May 2, 2016
@@ -618,7 +618,7 @@ fx.getClosest = function(cd, distfn, pointData) {
// do this for 'closest'
for(var i=0; i<cd.length; i++) {
var newDistance = distfn(cd[i]);
if(newDistance < pointData.distance) {
if(newDistance <= pointData.distance) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What a fix! 👏

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, it's a little hacky. Ideally hoverinfo: 'none' traces wouldn't be passed to the hover label routine, but that caused a plotly_hover test to fail, so I decided to go with ⏫ .

@mdtusz
Copy link
Contributor

mdtusz commented May 2, 2016

💃

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

Successfully merging this pull request may close these issues.

Hovermode 'closest' should ignore hoverinfo 'none' pts
2 participants