From 5a83c8431b23c91f08a927703dd02a1db9068360 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 26 May 2016 15:02:01 -0400 Subject: [PATCH 1/4] add fallback for fullLayout_plots : - so that Plots.getSubplots('gl2d' or 'cartesian') does not fail on a blank (i.e. Plotly.plot(gd, [])) plot - I am beginning to doubt that storing both 'cartesian' and 'gl2d' subplot objects in fullLayout._plots is the way to go. --- src/plots/plots.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index d091511bffe..6a33c21fc53 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -193,7 +193,7 @@ plots.getSubplotIds = function getSubplotIds(layout, type) { if(type === 'cartesian' && (!layout._has || !layout._has('cartesian'))) return []; if(type === 'gl2d' && (!layout._has || !layout._has('gl2d'))) return []; if(type === 'cartesian' || type === 'gl2d') { - return Object.keys(layout._plots); + return Object.keys(layout._plots || {}); } var idRegex = _module.idRegex, From dc3ab87561dbd5c7156b9ae01c7c2324a591c4a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 26 May 2016 15:06:09 -0400 Subject: [PATCH 2/4] make gl2d clean-plot step more robust: - delete old 2d scenes that don't nest any 'gl2d' traces, instead of deleting 2d scenes that have no corresp. axes in new fullLayout. - delete item in _plots, so that if a 'gl2d' -> 'cartesian' subplot conversion happens unhindered. --- src/plots/gl2d/index.js | 13 +++++++++---- test/jasmine/tests/gl_plot_interact_test.js | 2 +- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/plots/gl2d/index.js b/src/plots/gl2d/index.js index b07f97341df..5fd16a5bb3a 100644 --- a/src/plots/gl2d/index.js +++ b/src/plots/gl2d/index.js @@ -69,12 +69,17 @@ exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) var oldSceneKeys = Plots.getSubplotIds(oldFullLayout, 'gl2d'); for(var i = 0; i < oldSceneKeys.length; i++) { - var oldSubplot = oldFullLayout._plots[oldSceneKeys[i]], - xaName = oldSubplot.xaxis._name, - yaName = oldSubplot.yaxis._name; + var id = oldSceneKeys[i], + oldSubplot = oldFullLayout._plots[id]; - if(!!oldSubplot._scene2d && (!newFullLayout[xaName] || !newFullLayout[yaName])) { + // old subplot wasn't gl2d; nothing to do + if(!oldSubplot._scene2d) continue; + + // if no traces are present, delete gl2d subplot + var subplotData = Plots.getSubplotData(newFullData, 'gl2d', id); + if(subplotData.length === 0) { oldSubplot._scene2d.destroy(); + delete oldFullLayout._plots[id]; } } }; diff --git a/test/jasmine/tests/gl_plot_interact_test.js b/test/jasmine/tests/gl_plot_interact_test.js index b23b1481c55..b6d3d6ade0d 100644 --- a/test/jasmine/tests/gl_plot_interact_test.js +++ b/test/jasmine/tests/gl_plot_interact_test.js @@ -539,7 +539,7 @@ describe('Test gl plot interactions', function() { expect(gd._fullLayout._plots.xy._scene2d.glplot).toBeDefined(); Plots.cleanPlot([], {}, gd._fullData, gd._fullLayout); - expect(gd._fullLayout._plots.xy._scene2d.glplot).toBe(null); + expect(gd._fullLayout._plots).toEqual({}); done(); }); From 51113b72b55a0dbd3ba322d8609471c357d304b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 26 May 2016 15:06:34 -0400 Subject: [PATCH 3/4] add gl2d replot-from-blank-graph test case --- test/jasmine/tests/gl_plot_interact_test.js | 37 +++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/test/jasmine/tests/gl_plot_interact_test.js b/test/jasmine/tests/gl_plot_interact_test.js index b6d3d6ade0d..5cf318cda82 100644 --- a/test/jasmine/tests/gl_plot_interact_test.js +++ b/test/jasmine/tests/gl_plot_interact_test.js @@ -580,4 +580,41 @@ describe('Test gl plot side effects', function() { }); }); }); + + it('should be able to replot from a blank graph', function(done) { + var gd = createGraphDiv(); + + function assert(cnt) { + var nodes = d3.selectAll('canvas'); + expect(nodes.size()).toEqual(cnt); + } + + var data = [{ + type: 'scattergl', + x: [1, 2, 3], + y: [2, 1, 2] + }]; + + Plotly.plot(gd, []).then(function() { + assert(0); + + return Plotly.plot(gd, data); + }).then(function() { + assert(1); + + return Plotly.purge(gd); + }).then(function() { + assert(0); + + return Plotly.plot(gd, data); + }).then(function() { + assert(1); + + return Plotly.deleteTraces(gd, [0]); + }).then(function() { + assert(0); + + return Plotly.purge(gd); + }).then(done); + }); }); From 5033fd9e83a42ccfc7d0fe8bbf17b288012fd757 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 26 May 2016 15:29:51 -0400 Subject: [PATCH 4/4] rename local assert -> countCanvases --- test/jasmine/tests/gl_plot_interact_test.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/jasmine/tests/gl_plot_interact_test.js b/test/jasmine/tests/gl_plot_interact_test.js index 5cf318cda82..1b274f95e21 100644 --- a/test/jasmine/tests/gl_plot_interact_test.js +++ b/test/jasmine/tests/gl_plot_interact_test.js @@ -584,7 +584,7 @@ describe('Test gl plot side effects', function() { it('should be able to replot from a blank graph', function(done) { var gd = createGraphDiv(); - function assert(cnt) { + function countCanvases(cnt) { var nodes = d3.selectAll('canvas'); expect(nodes.size()).toEqual(cnt); } @@ -596,23 +596,23 @@ describe('Test gl plot side effects', function() { }]; Plotly.plot(gd, []).then(function() { - assert(0); + countCanvases(0); return Plotly.plot(gd, data); }).then(function() { - assert(1); + countCanvases(1); return Plotly.purge(gd); }).then(function() { - assert(0); + countCanvases(0); return Plotly.plot(gd, data); }).then(function() { - assert(1); + countCanvases(1); return Plotly.deleteTraces(gd, [0]); }).then(function() { - assert(0); + countCanvases(0); return Plotly.purge(gd); }).then(done);