From 3918bdcff78393390097b3c601ff18b7a5833ce1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 1 Mar 2016 10:01:24 -0500 Subject: [PATCH 01/13] lint --- devtools/test_dashboard/buttons.js | 13 +++++-------- devtools/test_dashboard/index.html | 2 ++ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/devtools/test_dashboard/buttons.js b/devtools/test_dashboard/buttons.js index 94f282f97c6..c64d3bd5b44 100644 --- a/devtools/test_dashboard/buttons.js +++ b/devtools/test_dashboard/buttons.js @@ -2,7 +2,7 @@ var Lib = require('@src/lib'); -var plotlist = document.getElementById('plot-list'); +var plotList = document.getElementById('plot-list'); var anchor = document.getElementById('embedded-graph'); var image = document.getElementById('embedded-image'); @@ -14,18 +14,15 @@ anchor.style.height = '600px'; anchor.style.width = '1000px'; function plotButtons(plots, figDir) { - Object.keys(plots).forEach(function(plotname) { - var button = document.createElement('button'); button.style.cssFloat = 'left'; button.style.width = '100px'; button.style.height = '40px'; - button.innerHTML = plotname; - plotlist.appendChild(button); + plotList.appendChild(button); button.addEventListener('click', function() { @@ -58,7 +55,7 @@ function plotButtons(plots, figDir) { snapshot.innerHTML = 'snapshot'; snapshot.style.background = 'blue'; - plotlist.appendChild(snapshot); + plotList.appendChild(snapshot); snapshot.addEventListener('click', function() { @@ -111,7 +108,7 @@ function plotButtons(plots, figDir) { pummelButton.style.marginLeft = '25px'; pummelButton.innerHTML = 'pummel3d'; pummelButton.style.background = 'blue'; - plotlist.appendChild(pummelButton); + plotList.appendChild(pummelButton); var i = 0; var mock = require('@mocks/gl3d_marker-color.json'); @@ -147,7 +144,7 @@ function plotButtons(plots, figDir) { scrapeButton.style.marginLeft = '25px'; scrapeButton.innerHTML = 'scrape SVG'; scrapeButton.style.background = 'blue'; - plotlist.appendChild(scrapeButton); + plotList.appendChild(scrapeButton); scrapeButton.addEventListener('click', function() { Plotly.Snapshot.toSVG(Tabs.get()); diff --git a/devtools/test_dashboard/index.html b/devtools/test_dashboard/index.html index 9d1f78ef45a..20410f98f7b 100644 --- a/devtools/test_dashboard/index.html +++ b/devtools/test_dashboard/index.html @@ -27,6 +27,7 @@ getGraph: function() { return document.getElementById('embedded-graph').children[0]; }, + fresh: function() { var anchor = document.getElementById('embedded-graph'), graphDiv = Tabs.getGraph(); @@ -37,6 +38,7 @@ return graphDiv; }, + plotMock: function(mockName) { var mockURL = '../../test/image/mocks/' + mockName + '.json'; From d29a90f19a8ec18451c4268bccaff4bc2a1098d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 1 Mar 2016 10:01:44 -0500 Subject: [PATCH 02/13] set graph div id to 'graph' --- devtools/test_dashboard/buttons.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/devtools/test_dashboard/buttons.js b/devtools/test_dashboard/buttons.js index c64d3bd5b44..74858401a94 100644 --- a/devtools/test_dashboard/buttons.js +++ b/devtools/test_dashboard/buttons.js @@ -25,17 +25,16 @@ function plotButtons(plots, figDir) { plotList.appendChild(button); button.addEventListener('click', function() { - var myImage = new Image(); myImage.src = figDir + plotname + '.png'; image.innerHTML = ''; image.appendChild(myImage); + gd = document.createElement('div'); + gd.id = 'graph'; anchor.innerHTML = ''; - - gd = document.createElement('div'); anchor.appendChild(gd); var plot = plots[plotname]; From 13952102697a7dda4298a08a04f910941f691aec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 1 Mar 2016 11:07:31 -0500 Subject: [PATCH 03/13] export cleanPlot as part of the Plots module --- src/plots/plots.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index a65162ffc92..b08ee568165 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -492,7 +492,7 @@ plots.supplyDefaults = function(gd) { plots.supplyLayoutModuleDefaults(newLayout, newFullLayout, newFullData); // clean subplots and other artifacts from previous plot calls - cleanPlot(newFullData, newFullLayout, oldFullData, oldFullLayout); + plots.cleanPlot(newFullData, newFullLayout, oldFullData, oldFullLayout); /* * Relink functions and underscore attributes to promote consistency between @@ -520,7 +520,7 @@ plots.supplyDefaults = function(gd) { } }; -function cleanPlot(newFullData, newFullLayout, oldFullData, oldFullLayout) { +plots.cleanPlot = function(newFullData, newFullLayout, oldFullData, oldFullLayout) { var i, j; var plotTypes = Object.keys(subplotsRegistry); @@ -755,9 +755,9 @@ plots.supplyLayoutModuleDefaults = function(layoutIn, layoutOut, fullData) { } }; +// Remove all plotly attributes from a div so it can be replotted fresh +// TODO: these really need to be encapsulated into a much smaller set... plots.purge = function(gd) { - // remove all plotly attributes from a div so it can be replotted fresh - // TODO: these really need to be encapsulated into a much smaller set... // note: we DO NOT remove _context because it doesn't change when we insert // a new plot, and may have been set outside of our scope. From 5bdc38f8176cb8d18af017727e48289fa11bd2c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 1 Mar 2016 11:29:10 -0500 Subject: [PATCH 04/13] add Plotly.purge method: - built on top of Plots.purge. - In addition, clears
- In addition, clears gl contexts using Plots.cleanPlot --- src/core.js | 1 + src/plot_api/plot_api.js | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/core.js b/src/core.js index 286f5e84b17..88bf123dc71 100644 --- a/src/core.js +++ b/src/core.js @@ -28,6 +28,7 @@ exports.prependTraces = Plotly.prependTraces; exports.addTraces = Plotly.addTraces; exports.deleteTraces = Plotly.deleteTraces; exports.moveTraces = Plotly.moveTraces; +exports.purge = Plotly.purge; exports.setPlotConfig = require('./plot_api/set_plot_config'); exports.register = Plotly.register; diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index e4eda4ca963..f76c4bcd25e 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -2394,6 +2394,26 @@ Plotly.relayout = function relayout(gd, astr, val) { }); }; +Plotly.purge = function purge(gd) { + gd = getGraphDiv(gd); + + var fullLayout = gd._fullLayout || {}, + fullData = gd._fullData || []; + + // remove gl contexts + Plots.cleanPlot([], {}, fullData, fullLayout); + + // purge properties + Plots.purge(gd); + + // remove plot container + if(fullLayout._container) fullLayout._container.remove(); + + // other stuff to delete ??? + + return gd; +}; + /** * Reduce all reserved margin objects to a single required margin reservation. * @@ -2493,7 +2513,7 @@ function makePlotFramework(gd) { // Make the svg container fullLayout._paperdiv = fullLayout._container.selectAll('.svg-container').data([0]); fullLayout._paperdiv.enter().append('div') - .classed('svg-container',true) + .classed('svg-container', true) .style('position','relative'); // Initial autosize From 0b120f9b11ff93d1d6579868377eb40cd78ea78c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 1 Mar 2016 11:36:38 -0500 Subject: [PATCH 05/13] make scene2d.destroy clear gl2d canvas, hover svg and mouse div --- src/plots/gl2d/scene2d.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/plots/gl2d/scene2d.js b/src/plots/gl2d/scene2d.js index 0cc7ddb847b..d386b47d58d 100644 --- a/src/plots/gl2d/scene2d.js +++ b/src/plots/gl2d/scene2d.js @@ -292,6 +292,13 @@ proto.cameraChanged = function() { proto.destroy = function() { this.glplot.dispose(); + + this.container.removeChild(this.canvas); + this.container.removeChild(this.svgContainer); + this.container.removeChild(this.mouseContainer); + + this.glplot = null; + this.stopped = true; }; proto.plot = function(fullData, fullLayout) { @@ -405,6 +412,7 @@ proto.plot = function(fullData, fullLayout) { proto.draw = function() { if(this.stopped) return; + requestAnimationFrame(this.redraw); var glplot = this.glplot, From 84fb9fa4c8fd6519222d1def600852ed57280816 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 1 Mar 2016 11:36:58 -0500 Subject: [PATCH 06/13] clear old gl2d in cleanPlot step --- src/plots/gl2d/index.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/plots/gl2d/index.js b/src/plots/gl2d/index.js index 22288ad4d69..d8bb69580b9 100644 --- a/src/plots/gl2d/index.js +++ b/src/plots/gl2d/index.js @@ -65,3 +65,17 @@ exports.plot = function plotGl2d(gd) { scene.plot(fullSubplotData, fullLayout, gd.layout); } }; + +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; + + if(!!oldSubplot._scene2d && (!newFullLayout[xaName] || !newFullLayout[yaName])) { + oldSubplot._scene2d.destroy(); + } + } +}; From 31e040d97e39f3a2d589df687264a9b6ec2f5e9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 1 Mar 2016 11:38:07 -0500 Subject: [PATCH 07/13] rm useless comment --- src/plots/plots.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index b08ee568165..386ee4ee3fd 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -762,9 +762,6 @@ plots.purge = function(gd) { // note: we DO NOT remove _context because it doesn't change when we insert // a new plot, and may have been set outside of our scope. - // clean up the gl and geo containers - // TODO unify subplot creation/update with d3.selection.order - // and/or subplot ids var fullLayout = gd._fullLayout || {}; if(fullLayout._glcontainer !== undefined) fullLayout._glcontainer.remove(); if(fullLayout._geocontainer !== undefined) fullLayout._geocontainer.remove(); From 3ce6c4b3f3c494ae0f76cfac1f232e92a0145a0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 1 Mar 2016 11:52:08 -0500 Subject: [PATCH 08/13] lint --- 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 386ee4ee3fd..6d3693b754c 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -560,7 +560,7 @@ plots.cleanPlot = function(newFullData, newFullLayout, oldFullData, oldFullLayou oldFullLayout._infolayer.selectAll('.cb' + oldUid).remove(); } } -} +}; /** * Relink private _keys and keys with a function value from one layout From fa8b92d990627e2391ebf6bbda758f75940fdaf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 4 Mar 2016 16:31:34 -0500 Subject: [PATCH 09/13] add jsDoc to Plotly.purge --- src/plot_api/plot_api.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index f76c4bcd25e..9267180ee3a 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -2394,6 +2394,12 @@ Plotly.relayout = function relayout(gd, astr, val) { }); }; +/** + * Purge a graph container div back to its initial pre-Plotly.plot state + * + * @param {string id or DOM element} gd + * the id or DOM element of the graph container div + */ Plotly.purge = function purge(gd) { gd = getGraphDiv(gd); From 92f44852ff326d1054815138ce4f9199d2fe5447 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 4 Mar 2016 16:32:03 -0500 Subject: [PATCH 10/13] add Events.purge method, - which all event method from the plot object --- src/lib/events.js | 12 ++++++++++++ src/plot_api/plot_api.js | 3 +++ test/jasmine/tests/events_test.js | 7 +++++++ 3 files changed, 22 insertions(+) diff --git a/src/lib/events.js b/src/lib/events.js index e4d67a53d90..7be09deedb0 100644 --- a/src/lib/events.js +++ b/src/lib/events.js @@ -114,7 +114,19 @@ var Events = { */ return jQueryHandlerValue !== undefined ? jQueryHandlerValue : nodeEventHandlerValue; + }, + + purge: function(plotObj) { + delete plotObj._ev; + delete plotObj.on; + delete plotObj.once; + delete plotObj.removeListener; + delete plotObj.removeAllListeners; + delete plotObj.emit; + + return plotObj; } + }; module.exports = Events; diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 9267180ee3a..8dcd3e3c134 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -2412,6 +2412,9 @@ Plotly.purge = function purge(gd) { // purge properties Plots.purge(gd); + // purge event emitter methods + Events.purge(gd); + // remove plot container if(fullLayout._container) fullLayout._container.remove(); diff --git a/test/jasmine/tests/events_test.js b/test/jasmine/tests/events_test.js index c68ccfd6cb2..98a45e6314a 100644 --- a/test/jasmine/tests/events_test.js +++ b/test/jasmine/tests/events_test.js @@ -181,8 +181,15 @@ describe('Events', function() { expect(eventBaton).toBe(3); expect(result).toBe('pong'); }); + }); + describe('purge', function() { + it('should remove all method from the plotObj', function() { + Events.init(plotObj); + Events.purge(plotObj); + expect(plotObj).toEqual({}); + }); }); }); From 01fe3b2daa2dd80e426a803d57ab7641fb0b8bc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 4 Mar 2016 16:32:30 -0500 Subject: [PATCH 11/13] delete all other _ properties in Plotly.purge --- src/plot_api/plot_api.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 8dcd3e3c134..2c257038927 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -2418,7 +2418,11 @@ Plotly.purge = function purge(gd) { // remove plot container if(fullLayout._container) fullLayout._container.remove(); - // other stuff to delete ??? + delete gd._context; + delete gd._replotPending; + delete gd._mouseDownTime; + delete gd._hmpixcount; + delete gd._hmlumcount; return gd; }; From d30bd1cd663428c3ce2517a8c792efbe898cfb77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 4 Mar 2016 16:32:55 -0500 Subject: [PATCH 12/13] add tests for Plotly.purge and Plots.cleanPlot --- test/jasmine/tests/gl_plot_interact_test.js | 50 +++++++++++++++++++-- test/jasmine/tests/plot_api_test.js | 23 ++++++++++ 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/test/jasmine/tests/gl_plot_interact_test.js b/test/jasmine/tests/gl_plot_interact_test.js index 4388e0a0778..92607cf5d42 100644 --- a/test/jasmine/tests/gl_plot_interact_test.js +++ b/test/jasmine/tests/gl_plot_interact_test.js @@ -36,14 +36,19 @@ describe('Test gl plot interactions', function() { sceneIds = Plots.getSubplotIds(fullLayout, 'gl3d'); sceneIds.forEach(function(id) { - fullLayout[id]._scene.destroy(); + var scene = fullLayout[id]._scene; + + if(scene.glplot) scene.destroy(); }); sceneIds = Plots.getSubplotIds(fullLayout, 'gl2d'); sceneIds.forEach(function(id) { var scene2d = fullLayout._plots[id]._scene2d; - scene2d.stopped = true; - scene2d.destroy(); + + if(scene2d.glplot) { + scene2d.stopped = true; + scene2d.destroy(); + } }); destroyGraphDiv(); @@ -369,4 +374,43 @@ describe('Test gl plot interactions', function() { }); }); + + describe('Plots.cleanPlot', function() { + + it('should remove gl context from the graph div of a gl3d plot', function(done) { + gd = createGraphDiv(); + + var mockData = [{ + type: 'scatter3d' + }]; + + Plotly.plot(gd, mockData).then(function() { + expect(gd._fullLayout.scene._scene.glplot).toBeDefined(); + + Plots.cleanPlot([], {}, gd._fullData, gd._fullLayout); + expect(gd._fullLayout.scene._scene.glplot).toBe(null); + + done(); + }); + }); + + it('should remove gl context from the graph div of a gl2d plot', function(done) { + gd = createGraphDiv(); + + var mockData = [{ + type: 'scattergl', + x: [1,2,3], + y: [1,2,3] + }]; + + Plotly.plot(gd, mockData).then(function() { + expect(gd._fullLayout._plots.xy._scene2d.glplot).toBeDefined(); + + Plots.cleanPlot([], {}, gd._fullData, gd._fullLayout); + expect(gd._fullLayout._plots.xy._scene2d.glplot).toBe(null); + + done(); + }); + }); + }); }); diff --git a/test/jasmine/tests/plot_api_test.js b/test/jasmine/tests/plot_api_test.js index ef506e0dc42..8cbc7637a58 100644 --- a/test/jasmine/tests/plot_api_test.js +++ b/test/jasmine/tests/plot_api_test.js @@ -7,6 +7,8 @@ var Bar = require('@src/traces/bar'); var Legend = require('@src/components/legend'); var pkg = require('../../../package.json'); +var createGraphDiv = require('../assets/create_graph_div'); +var destroyGraphDiv = require('../assets/destroy_graph_div'); describe('Test plot api', function() { 'use strict'; @@ -616,4 +618,25 @@ describe('Test plot api', function() { expect(gd.data).toEqual(cachedData); }); }); + + describe('Plotly.purge', function() { + + afterEach(destroyGraphDiv); + + it('should return the graph div in its original state', function(done) { + var gd = createGraphDiv(); + var initialKeys = Object.keys(gd); + var intialHTML = gd.innerHTML; + var mockData = [{ x: [1,2,3], y: [2,3,4] }]; + + Plotly.plot(gd, mockData).then(function() { + Plotly.purge(gd); + + expect(Object.keys(gd)).toEqual(initialKeys); + expect(gd.innerHTML).toEqual(intialHTML); + + done(); + }); + }); + }); }); From a4e590aba3228d4c2c74a57e7e1eea51b85bc6ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 4 Mar 2016 16:33:18 -0500 Subject: [PATCH 13/13] use Plotly.purge in test-dashboard --- devtools/test_dashboard/buttons.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/devtools/test_dashboard/buttons.js b/devtools/test_dashboard/buttons.js index 74858401a94..3dab386271c 100644 --- a/devtools/test_dashboard/buttons.js +++ b/devtools/test_dashboard/buttons.js @@ -31,6 +31,9 @@ function plotButtons(plots, figDir) { image.innerHTML = ''; image.appendChild(myImage); + var currentGraphDiv = Tabs.getGraph(); + if(currentGraphDiv) Plotly.purge(currentGraphDiv); + gd = document.createElement('div'); gd.id = 'graph';