Skip to content

Commit b1b525e

Browse files
etpinardetpinard
authored
Merge pull request #1472 from plotly/range-slider-log
Range slider fixes (user set y axis types + trace clearance)
2 parents 88ccce4 + e1d703f commit b1b525e

File tree

9 files changed

+162
-43
lines changed

9 files changed

+162
-43
lines changed

‎src/components/rangeslider/draw.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,7 @@ function drawRangePlot(rangeSlider, gd, axisOpts, opts) {
376376
};
377377

378378
mockFigure.layout[oppAxisName] = {
379+
type: oppAxisOpts.type,
379380
domain: [0, 1],
380381
range: oppAxisOpts.range.slice(),
381382
calendar: oppAxisOpts.calendar

‎src/plot_api/plot_api.js‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,11 +288,19 @@ Plotly.plot = function(gd, data, layout, config) {
288288
uid = trace.uid;
289289

290290
if(!isVisible || !Registry.traceIs(trace, '2dMap')) {
291-
fullLayout._paper.selectAll(
291+
var query = (
292292
'.hm' + uid +
293293
',.contour' + uid +
294294
',#clip' + uid
295-
).remove();
295+
);
296+
297+
fullLayout._paper
298+
.selectAll(query)
299+
.remove();
300+
301+
fullLayout._infolayer.selectAll('g.rangeslider-container')
302+
.selectAll(query)
303+
.remove();
296304
}
297305

298306
if(!isVisible || !trace._module.colorbar) {

‎src/plots/cartesian/index.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,11 @@ exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout)
160160
.remove();
161161
}
162162
}
163+
164+
oldFullLayout._infolayer.selectAll('g.rangeslider-container')
165+
.select('g.scatterlayer')
166+
.selectAll('g.trace')
167+
.remove();
163168
}
164169

165170
var hadCartesian = (oldFullLayout._has && oldFullLayout._has('cartesian'));

‎src/plots/plots.js‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -571,23 +571,25 @@ plots.cleanPlot = function(newFullData, newFullLayout, oldFullData, oldFullLayou
571571
if(oldUid === newTrace.uid) continue oldLoop;
572572
}
573573

574-
// clean old heatmap, contour, and scatter traces
575-
//
576-
// Note: This is also how scatter traces (cartesian and scatterternary) get
577-
// removed since otherwise the scatter module is not called (and so the join
578-
// doesn't register the removal) if scatter traces disappear entirely.
574+
var query = (
575+
'.hm' + oldUid +
576+
',.contour' + oldUid +
577+
',#clip' + oldUid +
578+
',.trace' + oldUid
579+
);
580+
581+
// clean old heatmap, contour traces and clip paths
582+
// that rely on uid identifiers
579583
if(hasPaper) {
580-
oldFullLayout._paper.selectAll(
581-
'.hm' + oldUid +
582-
',.contour' + oldUid +
583-
',#clip' + oldUid +
584-
',.trace' + oldUid
585-
).remove();
584+
oldFullLayout._paper.selectAll(query).remove();
586585
}
587586

588-
// clean old colorbars
587+
// clean old colorbars and range slider plot
589588
if(hasInfoLayer) {
590589
oldFullLayout._infolayer.selectAll('.cb' + oldUid).remove();
590+
591+
oldFullLayout._infolayer.selectAll('g.rangeslider-container')
592+
.selectAll(query).remove();
591593
}
592594
}
593595
};

‎src/traces/contour/plot.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,11 @@ function plotOne(gd, plotinfo, cd) {
5656
heatmapPlot(gd, plotinfo, [cd]);
5757
}
5858
// in case this used to be a heatmap (or have heatmap fill)
59-
else fullLayout._paper.selectAll('.hm' + uid).remove();
59+
else {
60+
fullLayout._paper.selectAll('.hm' + uid).remove();
61+
fullLayout._infolayer.selectAll('g.rangeslider-container')
62+
.selectAll('.hm' + uid).remove();
63+
}
6064

6165
makeCrossings(pathinfo);
6266
findAllPaths(pathinfo);

‎src/traces/heatmap/plot.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ function plotOne(gd, plotinfo, cd) {
3636

3737
// in case this used to be a contour map
3838
fullLayout._paper.selectAll('.contour' + uid).remove();
39+
fullLayout._infolayer.selectAll('g.rangeslider-container')
40+
.selectAll('.contour' + uid).remove();
3941

4042
if(trace.visible !== true) {
4143
fullLayout._paper.selectAll('.' + id).remove();

‎test/image/baselines/range_slider_multiple.png‎

879 Bytes
Loading

‎test/image/mocks/range_slider_multiple.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"data": [
33
{
44
"x": [ 1, 2, 3 ],
5-
"y": [ 4, 5, 6 ],
5+
"y": [ 4, 5e5, 6e8 ],
66
"type": "bar"
77
},
88
{
@@ -31,7 +31,7 @@
3131
},
3232
"yaxis": {
3333
"domain": [ 0.3, 0.8 ],
34-
"type": "linear"
34+
"type": "log"
3535
},
3636
"yaxis2": {
3737
"anchor": "x2",

‎test/jasmine/tests/range_slider_test.js‎

Lines changed: 123 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -294,43 +294,140 @@ describe('the range slider', function() {
294294

295295
it('should not add the slider to the DOM by default', function(done) {
296296
Plotly.plot(gd, [{ x: [1, 2, 3], y: [2, 3, 4] }], {})
297-
.then(function() {
298-
var rangeSlider = getRangeSlider();
299-
expect(rangeSlider).not.toBeDefined();
300-
})
301-
.then(done);
297+
.then(function() {
298+
var rangeSlider = getRangeSlider();
299+
expect(rangeSlider).not.toBeDefined();
300+
})
301+
.then(done);
302302
});
303303

304304
it('should add the slider if rangeslider is set to anything', function(done) {
305305
Plotly.plot(gd, [{ x: [1, 2, 3], y: [2, 3, 4] }], {})
306-
.then(function() { Plotly.relayout(gd, 'xaxis.rangeslider', 'exists'); })
307-
.then(function() {
308-
var rangeSlider = getRangeSlider();
309-
expect(rangeSlider).toBeDefined();
310-
})
311-
.then(done);
306+
.then(function() {
307+
return Plotly.relayout(gd, 'xaxis.rangeslider', 'exists');
308+
})
309+
.then(function() {
310+
var rangeSlider = getRangeSlider();
311+
expect(rangeSlider).toBeDefined();
312+
})
313+
.then(done);
312314
});
313315

314316
it('should add the slider if visible changed to `true`', function(done) {
315317
Plotly.plot(gd, [{ x: [1, 2, 3], y: [2, 3, 4] }], {})
316-
.then(function() { Plotly.relayout(gd, 'xaxis.rangeslider.visible', true); })
317-
.then(function() {
318-
var rangeSlider = getRangeSlider();
319-
expect(rangeSlider).toBeDefined();
320-
expect(countRangeSliderClipPaths()).toEqual(1);
321-
})
322-
.then(done);
318+
.then(function() {
319+
return Plotly.relayout(gd, 'xaxis.rangeslider.visible', true);
320+
})
321+
.then(function() {
322+
var rangeSlider = getRangeSlider();
323+
expect(rangeSlider).toBeDefined();
324+
expect(countRangeSliderClipPaths()).toEqual(1);
325+
})
326+
.then(done);
323327
});
324328

325329
it('should remove the slider if changed to `false` or `undefined`', function(done) {
326-
Plotly.plot(gd, [{ x: [1, 2, 3], y: [2, 3, 4] }], { xaxis: { rangeslider: { visible: true }}})
327-
.then(function() { Plotly.relayout(gd, 'xaxis.rangeslider.visible', false); })
328-
.then(function() {
329-
var rangeSlider = getRangeSlider();
330-
expect(rangeSlider).not.toBeDefined();
331-
expect(countRangeSliderClipPaths()).toEqual(0);
332-
})
333-
.then(done);
330+
Plotly.plot(gd, [{
331+
x: [1, 2, 3],
332+
y: [2, 3, 4]
333+
}], {
334+
xaxis: {
335+
rangeslider: { visible: true }
336+
}
337+
})
338+
.then(function() {
339+
return Plotly.relayout(gd, 'xaxis.rangeslider.visible', false);
340+
})
341+
.then(function() {
342+
var rangeSlider = getRangeSlider();
343+
expect(rangeSlider).not.toBeDefined();
344+
expect(countRangeSliderClipPaths()).toEqual(0);
345+
})
346+
.then(done);
347+
});
348+
349+
it('should clear traces in range plot when needed', function(done) {
350+
351+
function count(query) {
352+
return d3.select(getRangeSlider()).selectAll(query).size();
353+
}
354+
355+
Plotly.plot(gd, [{
356+
type: 'scatter',
357+
x: [1, 2, 3],
358+
y: [2, 1, 2]
359+
}, {
360+
type: 'bar',
361+
x: [1, 2, 3],
362+
y: [2, 5, 2]
363+
}], {
364+
xaxis: {
365+
rangeslider: { visible: true }
366+
}
367+
})
368+
.then(function() {
369+
expect(count('g.scatterlayer > g.trace')).toEqual(1);
370+
expect(count('g.barlayer > g.trace')).toEqual(1);
371+
372+
return Plotly.restyle(gd, 'visible', false);
373+
})
374+
.then(function() {
375+
expect(count('g.scatterlayer > g.trace')).toEqual(0);
376+
expect(count('g.barlayer > g.trace')).toEqual(0);
377+
378+
return Plotly.restyle(gd, 'visible', true);
379+
})
380+
.then(function() {
381+
expect(count('g.scatterlayer > g.trace')).toEqual(1);
382+
expect(count('g.barlayer > g.trace')).toEqual(1);
383+
384+
return Plotly.deleteTraces(gd, [0, 1]);
385+
})
386+
.then(function() {
387+
expect(count('g.scatterlayer > g.trace')).toEqual(0);
388+
expect(count('g.barlayer > g.trace')).toEqual(0);
389+
390+
return Plotly.addTraces(gd, [{
391+
type: 'heatmap',
392+
z: [[1, 2, 3], [2, 1, 3]]
393+
}]);
394+
})
395+
.then(function() {
396+
expect(count('g.imagelayer > g.hm')).toEqual(1);
397+
398+
return Plotly.restyle(gd, 'visible', false);
399+
})
400+
.then(function() {
401+
expect(count('g.imagelayer > g.hm')).toEqual(0);
402+
403+
return Plotly.restyle(gd, {
404+
visible: true,
405+
type: 'contour'
406+
});
407+
})
408+
.then(function() {
409+
expect(count('g.maplayer > g.contour')).toEqual(1);
410+
411+
return Plotly.restyle(gd, 'type', 'heatmap');
412+
})
413+
.then(function() {
414+
expect(count('g.imagelayer > g.hm')).toEqual(1);
415+
expect(count('g.maplayer > g.contour')).toEqual(0);
416+
417+
return Plotly.restyle(gd, 'type', 'contour');
418+
})
419+
.then(function() {
420+
expect(count('g.imagelayer > g.hm')).toEqual(0);
421+
expect(count('g.maplayer > g.contour')).toEqual(1);
422+
423+
return Plotly.deleteTraces(gd, [0]);
424+
})
425+
.then(function() {
426+
expect(count('g.imagelayer > g.hm')).toEqual(0);
427+
expect(count('g.maplayer > g.contour')).toEqual(0);
428+
})
429+
.then(done);
430+
334431
});
335432
});
336433

0 commit comments

Comments
 (0)