Conversation
| calcAxisExpansion(gd, trace, xa, ya, x, y, ppad); | ||
| } | ||
|
|
||
| var hasPeriodX = !!trace.xperiodalignment; |
There was a problem hiding this comment.
interesting that this PR is mostly removals :)
|
Playing with this a bit, I'm noticing that the y-position of the unified label feels a bit weird... I thought it was set to be at the median of all the points it contained, or something like that, no? |
|
Also, if I run my cursor carefully up and down the left-most spikeline, I see the hoverlabel jump around in the y-dimension:
|
No. That's based on the winning point at the moment. plotly.js/src/components/fx/hover.js Lines 1073 to 1076 in 990e260 However we could possibly use median of y values on x unified and median of x on y unified.If that's what you want please open a new issue for that. |
Is this something we changed in the v2 set of changes? I feel like I recall that when we first did this |
Positive: |
|
Now addressed in b44e11d |
|
Ah, much nicer! For some reason the hover for the left-most seems higher with respect to the data than the others... is that normal? |
Look normal to me: demo |
|
Check out the position of the hover wrt the yellow and blue lines for the right-most (sorry, I mistakenly said left-most above) |
That works fine on my ubuntu using Chrome. |
|
Ok, we can fix that particular oddity later I guess :) |
|
Huh, this only happens in Observable, not in CodePen. Annoying! https://codepen.io/nicolaskruchten/pen/JjNJgxE But ✨ from me! |
| it('@gl shows hover info for scattergl', function(done) { | ||
| Plotly.newPlot(gd, require('@mocks/gl2d_period_positioning.json')) | ||
| .then(function() { return Plotly.restyle(gd, 'xperiodalignment', 'start'); }) | ||
| .then(function() { return Plotly.restyle(gd, 'yperiodalignment', 'start'); }) |
There was a problem hiding this comment.
Nonblocking, but these tests can be simplified (and sped up) with the object form
Plotly.restyle(gd,{xperiodalignment: 'start', yperiodalignment: 'start'})
| assertHoverLabelContent({ | ||
| name: 'scatter', | ||
| nums: '(Jan 2001, 1)' | ||
| nums: '(Jul 2001, 1)' |
There was a problem hiding this comment.
So these tests were actually locking down the incorrect behavior previously... This is why in general I think such tests should be self-contained rather than relying on a separate mock: you can read the test carefully and if the dimensions are nice and simple (ie you set round numbers for the size, margins, & axis ranges) you can manually verify that it's the desired behavior.
alexcjohnson
left a comment
There was a problem hiding this comment.
Excellent! Only non-blocking comments. 💃
scatter and scattergl hover on axes with period and adjust the unified hover position in respect to mean
scatter and scattergl hover on axes with period and adjust the unified hover position in respect to mean
Closes #5822.
@plotly/plotly_js