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

compare/unified hover should include all points at same coordinate #4656

Closed
nicolaskruchten opened this issue Mar 17, 2020 · 10 comments · Fixed by #4664
Closed

compare/unified hover should include all points at same coordinate #4656

nicolaskruchten opened this issue Mar 17, 2020 · 10 comments · Fixed by #4664
Assignees
Labels
bug something broken
Milestone

Comments

@nicolaskruchten
Copy link
Contributor

In x-unified and x-compare modes, it should never be possible for there to be two data with the same x-coordinate and only one of them is displayed in a hoverlabel. I know compare mode doesn't currently behave this way but I consider it a bug.

The solution proposed by @alexcjohnson seems good to me:

  • Step 1: find hover points the way we do now
  • Step 2: loop back over traces not in the set of hover points we already found, and see if any of them have a point with position-axis value exactly matching what we found for the other point(s)
@antoinerg
Copy link
Contributor

Demonstration of the issue:
unified

@antoinerg
Copy link
Contributor

antoinerg commented Mar 17, 2020

The approach suggested by @alexcjohnson seem to work fine for the simple case shown above: https://codepen.io/antoinerg/pen/gOpKwPg (ie. scatter and bar).

Now, I wonder if there are cases for which this may lead to undesirable behavior. WIP on branch fix-4656.

@antoinerg
Copy link
Contributor

Now, I wonder if there are cases for which this may lead to undesirable behavior. WIP on branch fix-4656.

For one, categorical axis are not working ATM. Working on a fix.

@antoinerg
Copy link
Contributor

My branch fix-4656 now has fixes for date and categorical axes: https://codepen.io/antoinerg/pen/gOpKwPg

cc @nicolaskruchten

@nicolaskruchten
Copy link
Contributor Author

Looks good! Here's an oddity that I don't think we can easily fix, with two points above a bar, one at a slightly different x-coordinate:

unified

@antoinerg
Copy link
Contributor

Looks good! Here's an oddity that I don't think we can easily fix, with two points above a bar, one at a slightly different x-coordinate:

unified

I'm curious, what behavior would you rather have in that case?

@alexcjohnson
Copy link
Collaborator

two points above a bar, one at a slightly different x-coordinate

There's a reason the title of this issue says "should include all points at same coordinate" 😏

@nicolaskruchten
Copy link
Contributor Author

Indeed :) I was just noticing while testing that as I sweep in an increasing-x direction, I get the hover at 1, then 1.1, then 1 again even though my mouse is now at 1.2 or so. C'est la vie: the bar picks it up and then adds the point from 1. It's more correct than anything else I can think of right now :)

@antoinerg
Copy link
Contributor

antoinerg commented Mar 19, 2020

Some tests are failing with the changes made in branch fix-4656: multiple boxes/violins sharing a category with offsets. See second figure in https://codepen.io/antoinerg/pen/gOpKwPg . It used to hover on 1 box, now it hovers 2 boxes. I imagine we'd rather hover on all of them right? Would it be OK to only select 1 box as we did before?

@alexcjohnson when you have a chance, can you take a look at branch fix-4656 and let me know what you think of this approach (and potential pitfalls).

@nicolaskruchten
Copy link
Contributor Author

For boxes and violins, the 1.52 behaviour for compare was good IMO.

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 a pull request may close this issue.

4 participants