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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

More zoomed-in box/violin hover labels fixes #3965

Merged
merged 4 commits into from Jun 25, 2019

Conversation

etpinard
Copy link
Contributor

@plotly/plotly_js This presents a few commits that alters (mostly fixes 馃槒 ) the box/violin hover behaviour when their corresponding value axis range is within the box/violin q1/q3 values. This PR is essentially generalises #3889

Consider,

Plotly.newPlot(gd, [{
  type: 'box',
  y: [1,2,2,3]
}], {
  yaxis: {range: [1.6, 2.4]},
  hovermode: 'x'
})

Currently on master (ever since #3645, released in v1.45.3), hovering over the box gets us an exception on this line

var p1 = g1[0];

as reported in #3962


Commit a4b771c is enough to fix the problem and matches the pre-1.45.3 behaviour:

Peek 2019-06-14 17-02

but notice that min and q3 are shown and q1 and max are not, which looked odd to me.


Commit 1634ec8 made q1 and q3 show instead:

Peek 2019-06-14 17-05

which I think is better as we show both "innermost" values, but maybe we could do even better.


Commit ce5e5f9 made all box labels show i.e. min, q1, median, q3 and max while avoiding overlaps on the y-axis bounds:

Peek 2019-06-14 17-08

which I feel matches similar scenarios (e.g zoomed-in stack bars codepen) better, but please let me know if you disagree.

... to not confuse 'xa' & 'ya' with ax objects
- N.B. in the case where multiple item in a given d3-data bound
  array have identical key-function results,
  *only the first* of those items is considered. This implies that
  the `i` in e.g `selection.each(function(d, i) { })` does not
  always increment with a constant step.
... so that if the min and q1 overlap and have the same
    overlap key-function output, q1 will be rendered, not min
- this makes all box/hover labels render even when their x/y
  positions overlap perfectly.
@antoinerg
Copy link
Contributor

Thank you for fixing #3962 @etpinard !

As far as I'm concerned, this PR deserves a dancer as it improves things. However, I'll let @nicolaskruchten and/or @alexcjohnson comment on the decision of showing all boxes (ie. ce5e5f9).

@alexcjohnson
Copy link
Contributor

Yep, much better! 馃拑

Does look a little weird to have the arrows pointing right at the edge of the plot area, but of course this is what we do in other contexts where you're hovering on something off-plot. So maybe at some point we can alter that somehow to make it clearer they're not actually right there. Maybe as simple as getting rid of the arrow altogether, so these ones only have the box?

@etpinard
Copy link
Contributor Author

Maybe as simple as getting rid of the arrow altogether, so these ones only have the box?

Follow-up in #3985

@etpinard
Copy link
Contributor Author

Ok, let's start merging things for 1.49.0

@etpinard etpinard merged commit fef1c60 into master Jun 25, 2019
@etpinard etpinard deleted the one-more-hoverAvoirOverlap-fix branch June 25, 2019 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants