Range slider fixes (user set y axis types + trace clearance) #1472
Conversation
- so that non-auto type y axes are displayed correctly in the range slide plot
| @@ -376,6 +376,7 @@ function drawRangePlot(rangeSlider, gd, axisOpts, opts) { | |||
| }; | |||
|
|
|||
| mockFigure.layout[oppAxisName] = { | |||
| type: oppAxisOpts.type, | |||
There was a problem hiding this comment.
Before, we were strictly relying on the axis-auto-type routines in Plots.supplyDefaults to determine the range plot y-axis type - which obviously fails when a overrides the y axis type (e.g. in the case of log y axes as presented in #1470)
- scatter, heatmap and contour traces must be cleared
manually as they can't rely on the nasty
`selectAll('g.trace').remove()` in the cartesian plot routine.
- lock down all (I hope) possible combination in new test case.
| @@ -345,6 +345,90 @@ describe('the range slider', function() { | |||
| }) | |||
| .then(done); | |||
| }); | |||
|
|
|||
| it('should clear traces in range plot when needed', function(done) { | |||
There was a problem hiding this comment.
@alexcjohnson This commit fixes part 1
the data is gone from the graph, it should disappear from the range slider too. If you delete any but the last trace, the range slider does update correctly, but not when you delete the last one.
of #1473 in a pretty ugly way, unfortunately.
Most trace clearance (i.e. deletion or visible: false or type-restyle) is handled via this block:
plotinfo.plot.selectAll('g:not(.scatterlayer)').selectAll('g.trace').remove();except for scatter, heatmap and contour traces. That is, part 1 of #1473 actually only affected scatter, heatmap and contour traces.
Scatter traces are exempted from that selectAll('').remove() block since @rreusser's animation PR to smooth out their data updates. They are cleared here. Contour and heatmap groups don't always match one-to-one with trace items (e.g. a contour trace can have a heatmap fill group) so we use their uid to tell which trace to clear here, here here and here.
In brief, this commit ensure that every special .remove() call mentioned in the previous paragraph clear traces in the range slider containers as well.
There was a problem hiding this comment.
ugly but... ya gotta do what ya gotta do. Nice tests. 👍
| if(hasInfoLayer) { | ||
| oldFullLayout._infolayer.selectAll('.cb' + oldUid).remove(); | ||
|
|
||
| oldFullLayout._infolayer.selectAll('g.rangeslider-container') |
There was a problem hiding this comment.
I chose this pattern
fullLayout._infolayer.selectAll('g.rangeslider-container')
over keeping a ref to some underscore key e.g. fullLayout._rangeSliders to make these additions work even when there are no range sliders on the graph without having to resort to Registry.
|
looks good to me - 💃 as soon as you update the test image based on #1475 (finishing the loose end there 🎉 ) |
| @@ -2,7 +2,7 @@ | |||
| "data": [ | |||
| { | |||
| "x": [ 1, 2, 3 ], | |||
| "y": [ 4, 5, 6 ], | |||
| "y": [ 4, 5e5, 6e8 ], | |||
There was a problem hiding this comment.
@alexcjohnson here's bar + log image test ✅
etpinard
changed the title
fixes #1470
Easy.