From 8882813f9d4f9dcfc58c851bfdae8615fec7c421 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Wed, 7 Mar 2018 21:07:56 -0500 Subject: [PATCH 1/2] fix #2452 - removing and adding scatter(gl) as not the first module --- src/plots/cartesian/index.js | 30 ++++------------ src/traces/scattergl/index.js | 4 ++- test/jasmine/tests/cartesian_test.js | 52 ++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 24 deletions(-) diff --git a/src/plots/cartesian/index.js b/src/plots/cartesian/index.js index 2d312a50ca3..84d728253bc 100644 --- a/src/plots/cartesian/index.js +++ b/src/plots/cartesian/index.js @@ -216,35 +216,19 @@ exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) var oldModules = oldFullLayout._modules || [], newModules = newFullLayout._modules || []; - var hadScatter, hasScatter, hadGl, hasGl, i, oldPlots, ids, subplotInfo; + var hadScatter, hasScatter, hadGl, hasGl, i, oldPlots, ids, subplotInfo, moduleName; for(i = 0; i < oldModules.length; i++) { - if(oldModules[i].name === 'scatter') { - hadScatter = true; - } - break; - } - - for(i = 0; i < newModules.length; i++) { - if(newModules[i].name === 'scatter') { - hasScatter = true; - break; - } - } - - for(i = 0; i < oldModules.length; i++) { - if(oldModules[i].name === 'scattergl') { - hadGl = true; - } - break; + moduleName = oldModules[i].name; + if(moduleName === 'scatter') hadScatter = true; + else if(moduleName === 'scattergl') hadGl = true; } for(i = 0; i < newModules.length; i++) { - if(newModules[i].name === 'scattergl') { - hasGl = true; - break; - } + moduleName = newModules[i].name; + if(moduleName === 'scatter') hasScatter = true; + else if(moduleName === 'scattergl') hasGl = true; } if(hadScatter && !hasScatter) { diff --git a/src/traces/scattergl/index.js b/src/traces/scattergl/index.js index abe85db5f1e..b7ab521e34d 100644 --- a/src/traces/scattergl/index.js +++ b/src/traces/scattergl/index.js @@ -610,7 +610,9 @@ function sceneUpdate(gd, subplot) { scene.selectBatch = null; scene.unselectBatch = null; - delete subplot._scene; + // we can't just delete _scene, because `destroy` is called in the + // middle of supplyDefaults, before relinkPrivateKeys which will put it back. + subplot._scene = null; }; } diff --git a/test/jasmine/tests/cartesian_test.js b/test/jasmine/tests/cartesian_test.js index 2bcf763642a..09e45bd8f1a 100644 --- a/test/jasmine/tests/cartesian_test.js +++ b/test/jasmine/tests/cartesian_test.js @@ -159,6 +159,58 @@ describe('restyle', function() { .then(done); }); + + it('can legend-hide the second and only scatter trace', function(done) { + Plotly.plot(gd, [ + {y: [1, 2, 3], type: 'bar'}, + {y: [1, 2, 3], xaxis: 'x2', yaxis: 'y2', type: 'scatter'} + ], { + xaxis: {domain: [0, 0.4]}, + xaxis2: {domain: [0.6, 1]}, + yaxis2: {anchor: 'x2'}, + width: 600, + height: 400 + }) + .then(function() { + expect(d3.select('.scatter').size()).toBe(1); + return Plotly.restyle(gd, {visible: 'legendonly'}, 1); + }) + .then(function() { + expect(d3.select('.scatter').size()).toBe(0); + return Plotly.restyle(gd, {visible: true}, 1); + }) + .then(function() { + expect(d3.select('.scatter').size()).toBe(1); + }) + .catch(failTest) + .then(done); + }); + + it('can legend-hide the second and only scattergl trace', function(done) { + Plotly.plot(gd, [ + {y: [1, 2, 3], type: 'bar'}, + {y: [1, 2, 3], xaxis: 'x2', yaxis: 'y2', type: 'scattergl'} + ], { + xaxis: {domain: [0, 0.4]}, + xaxis2: {domain: [0.6, 1]}, + yaxis2: {anchor: 'x2'}, + width: 600, + height: 400 + }) + .then(function() { + expect(!!gd._fullLayout._plots.x2y2._scene).toBe(true); + return Plotly.restyle(gd, {visible: 'legendonly'}, 1); + }) + .then(function() { + expect(!!gd._fullLayout._plots.x2y2._scene).toBe(false); + return Plotly.restyle(gd, {visible: true}, 1); + }) + .then(function() { + expect(!!gd._fullLayout._plots.x2y2._scene).toBe(true); + }) + .catch(failTest) + .then(done); + }); }); }); From 2c2b94a7114368aee559859b9d80dfcb1817d4cc Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Wed, 7 Mar 2018 21:30:17 -0500 Subject: [PATCH 2/2] fix gl2d cleanPlot test --- test/jasmine/tests/cartesian_test.js | 2 +- test/jasmine/tests/gl2d_plot_interact_test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/jasmine/tests/cartesian_test.js b/test/jasmine/tests/cartesian_test.js index 09e45bd8f1a..ab307a2e697 100644 --- a/test/jasmine/tests/cartesian_test.js +++ b/test/jasmine/tests/cartesian_test.js @@ -186,7 +186,7 @@ describe('restyle', function() { .then(done); }); - it('can legend-hide the second and only scattergl trace', function(done) { + it('@gl can legend-hide the second and only scattergl trace', function(done) { Plotly.plot(gd, [ {y: [1, 2, 3], type: 'bar'}, {y: [1, 2, 3], xaxis: 'x2', yaxis: 'y2', type: 'scattergl'} diff --git a/test/jasmine/tests/gl2d_plot_interact_test.js b/test/jasmine/tests/gl2d_plot_interact_test.js index da414dce007..daa3faf0114 100644 --- a/test/jasmine/tests/gl2d_plot_interact_test.js +++ b/test/jasmine/tests/gl2d_plot_interact_test.js @@ -39,7 +39,7 @@ describe('@gl Test removal of gl contexts', function() { expect(gd._fullLayout._plots.xy._scene).toBeDefined(); Plots.cleanPlot([], {}, gd._fullData, gd._fullLayout); - expect(gd._fullLayout._plots.xy._scene).toBeUndefined(); + expect(!!gd._fullLayout._plots.xy._scene).toBe(false); }) .then(done); });