From 0c62890b6f04e87201c921581d9c9d8abc193f2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 15 May 2017 17:30:59 -0400 Subject: [PATCH 1/5] cache ref to tester div and tester ref on drawing module object - so that Drawing.bBox does not have to query the DOM to fing them on every call. --- src/components/drawing/index.js | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/components/drawing/index.js b/src/components/drawing/index.js index 5eb5be186fa..724d99dec5d 100644 --- a/src/components/drawing/index.js +++ b/src/components/drawing/index.js @@ -579,16 +579,17 @@ drawing.makeTester = function(gd) { tester.node()._cache = {}; } - gd._tester = tester; - gd._testref = testref; + drawing.tester = gd._tester = tester; + drawing.testref = gd._testref = testref; }; // use our offscreen tester to get a clientRect for an element, // in a reference frame where it isn't translated and its anchor // point is at (0,0) // always returns a copy of the bbox, so the caller can modify it safely -var savedBBoxes = [], - maxSavedBBoxes = 10000; +var savedBBoxes = []; +var maxSavedBBoxes = 10000; + drawing.bBox = function(node) { // cache elements we've already measured so we don't have to // remeasure the same thing many times @@ -597,12 +598,13 @@ drawing.bBox = function(node) { return Lib.extendFlat({}, savedBBoxes[saveNum.value]); } - var test3 = d3.select('#js-plotly-tester'), - tester = test3.node(); + var tester3 = drawing.tester; + var tester = tester3.node(); // copy the node to test into the tester var testNode = node.cloneNode(true); tester.appendChild(testNode); + // standardize its position... do we really want to do this? d3.select(testNode).attr({ x: 0, @@ -610,9 +612,10 @@ drawing.bBox = function(node) { transform: '' }); - var testRect = testNode.getBoundingClientRect(), - refRect = test3.select('.js-reference-point') - .node().getBoundingClientRect(); + var testRect = testNode.getBoundingClientRect(); + var refRect = drawing.testref + .node() + .getBoundingClientRect(); tester.removeChild(testNode); From 273a4c37701241d6224a9c4c7fa3c9f84444f66f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 15 May 2017 17:31:38 -0400 Subject: [PATCH 2/5] cache DOMParser instance on svg utils module object - to not have to instantiate one on every appendSVG call --- src/lib/svg_text_utils.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/lib/svg_text_utils.js b/src/lib/svg_text_utils.js index f6d4419dc0c..d1544c1a359 100644 --- a/src/lib/svg_text_utils.js +++ b/src/lib/svg_text_utils.js @@ -17,6 +17,17 @@ var Lib = require('../lib'); var xmlnsNamespaces = require('../constants/xmlns_namespaces'); var stringMappings = require('../constants/string_mappings'); +exports.getDOMParser = function() { + if(exports.domParser) { + return exports.domParser; + } else if(window.DOMParser) { + exports.domParser = new window.DOMParser(); + return exports.domParser; + } else { + throw new Error('Cannot initialize DOMParser'); + } +}; + // Append SVG d3.selection.prototype.appendSVG = function(_svgString) { @@ -27,8 +38,9 @@ d3.selection.prototype.appendSVG = function(_svgString) { '' ].join(''); - var dom = new DOMParser().parseFromString(skeleton, 'application/xml'), - childNode = dom.documentElement.firstChild; + var domParser = exports.getDOMParser(); + var dom = domParser.parseFromString(skeleton, 'application/xml'); + var childNode = dom.documentElement.firstChild; while(childNode) { this.node().appendChild(this.node().ownerDocument.importNode(childNode, true)); From e0050f73c57c54755a6131584b152151bf0c7c06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 16 May 2017 12:11:43 -0400 Subject: [PATCH 3/5] don't expose DOMParser instance on svg utils module object --- src/lib/svg_text_utils.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/lib/svg_text_utils.js b/src/lib/svg_text_utils.js index d1544c1a359..2c9e47d40b8 100644 --- a/src/lib/svg_text_utils.js +++ b/src/lib/svg_text_utils.js @@ -17,12 +17,14 @@ var Lib = require('../lib'); var xmlnsNamespaces = require('../constants/xmlns_namespaces'); var stringMappings = require('../constants/string_mappings'); +var DOM_PARSER; + exports.getDOMParser = function() { - if(exports.domParser) { - return exports.domParser; + if(DOM_PARSER) { + return DOM_PARSER; } else if(window.DOMParser) { - exports.domParser = new window.DOMParser(); - return exports.domParser; + DOM_PARSER = new window.DOMParser(); + return DOM_PARSER; } else { throw new Error('Cannot initialize DOMParser'); } From c3c3b3ab331fc96769885a3c6abaa0e64b2fe846 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 16 May 2017 11:51:18 -0400 Subject: [PATCH 4/5] :hocho: gd._testref (wasn't used anywhere) --- src/components/drawing/index.js | 2 +- src/plots/plots.js | 1 - test/jasmine/tests/plots_test.js | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/components/drawing/index.js b/src/components/drawing/index.js index 724d99dec5d..2ec765c3458 100644 --- a/src/components/drawing/index.js +++ b/src/components/drawing/index.js @@ -580,7 +580,7 @@ drawing.makeTester = function(gd) { } drawing.tester = gd._tester = tester; - drawing.testref = gd._testref = testref; + drawing.testref = testref; }; // use our offscreen tester to get a clientRect for an element, diff --git a/src/plots/plots.js b/src/plots/plots.js index 4efb546fa1d..8e8e21abd98 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1174,7 +1174,6 @@ plots.purge = function(gd) { // these get recreated on Plotly.plot anyway, but just to be safe // (and to have a record of them...) delete gd._tester; - delete gd._testref; delete gd._promises; delete gd._redrawTimer; delete gd.firstscatter; diff --git a/test/jasmine/tests/plots_test.js b/test/jasmine/tests/plots_test.js index b8ea16c9878..f95e98685f5 100644 --- a/test/jasmine/tests/plots_test.js +++ b/test/jasmine/tests/plots_test.js @@ -429,7 +429,6 @@ describe('Test Plots', function() { expect(gd.autoplay).toBeUndefined(); expect(gd.changed).toBeUndefined(); expect(gd._tester).toBeUndefined(); - expect(gd._testref).toBeUndefined(); expect(gd._promises).toBeUndefined(); expect(gd._redrawTimer).toBeUndefined(); expect(gd.firstscatter).toBeUndefined(); From a8fba0336791e92956f8a0da6708ed0ab89329b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 16 May 2017 12:11:03 -0400 Subject: [PATCH 5/5] replace gd._tester with drawing._tester --- src/components/drawing/index.js | 6 +++--- src/components/sliders/draw.js | 4 ++-- src/components/updatemenus/draw.js | 2 +- src/plot_api/plot_api.js | 4 ++-- src/plots/plots.js | 1 - src/traces/carpet/plot.js | 4 ++-- test/jasmine/tests/plots_test.js | 1 - 7 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/components/drawing/index.js b/src/components/drawing/index.js index 2ec765c3458..deaf8b2441b 100644 --- a/src/components/drawing/index.js +++ b/src/components/drawing/index.js @@ -542,11 +542,11 @@ drawing.steps = function(shape) { }; // off-screen svg render testing element, shared by the whole page -// uses the id 'js-plotly-tester' and stores it in gd._tester +// uses the id 'js-plotly-tester' and stores it in drawing.tester // makes a hash of cached text items in tester.node()._cache // so we can add references to rendered text (including all info // needed to fully determine its bounding rect) -drawing.makeTester = function(gd) { +drawing.makeTester = function() { var tester = d3.select('body') .selectAll('#js-plotly-tester') .data([0]); @@ -579,7 +579,7 @@ drawing.makeTester = function(gd) { tester.node()._cache = {}; } - drawing.tester = gd._tester = tester; + drawing.tester = tester; drawing.testref = testref; }; diff --git a/src/components/sliders/draw.js b/src/components/sliders/draw.js index 50c2ef0f35e..3965237d2ce 100644 --- a/src/components/sliders/draw.js +++ b/src/components/sliders/draw.js @@ -117,7 +117,7 @@ function keyFunction(opts) { // Compute the dimensions (mutates sliderOpts): function findDimensions(gd, sliderOpts) { - var sliderLabels = gd._tester.selectAll('g.' + constants.labelGroupClass) + var sliderLabels = Drawing.tester.selectAll('g.' + constants.labelGroupClass) .data(sliderOpts.steps); sliderLabels.enter().append('g') @@ -154,7 +154,7 @@ function findDimensions(gd, sliderOpts) { if(sliderOpts.currentvalue.visible) { // Get the dimensions of the current value label: - var dummyGroup = gd._tester.append('g'); + var dummyGroup = Drawing.tester.append('g'); sliderLabels.each(function(stepOpts) { var curValPrefix = drawCurrentValue(dummyGroup, sliderOpts, stepOpts.label); diff --git a/src/components/updatemenus/draw.js b/src/components/updatemenus/draw.js index 3c9c30968b2..558c4148a2d 100644 --- a/src/components/updatemenus/draw.js +++ b/src/components/updatemenus/draw.js @@ -504,7 +504,7 @@ function findDimensions(gd, menuOpts) { menuOpts.lx = 0; menuOpts.ly = 0; - var fakeButtons = gd._tester.selectAll('g.' + constants.dropdownButtonClassName) + var fakeButtons = Drawing.tester.selectAll('g.' + constants.dropdownButtonClassName) .data(menuOpts.buttons); fakeButtons.enter().append('g') diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index ecb623db380..cf0c268d955 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -93,9 +93,9 @@ Plotly.plot = function(gd, data, layout, config) { d3.select(gd).classed('js-plotly-plot', true); // off-screen getBoundingClientRect testing space, - // in #js-plotly-tester (and stored as gd._tester) + // in #js-plotly-tester (and stored as Drawing.tester) // so we can share cached text across tabs - Drawing.makeTester(gd); + Drawing.makeTester(); // collect promises for any async actions during plotting // any part of the plotting code can push to gd._promises, then diff --git a/src/plots/plots.js b/src/plots/plots.js index 8e8e21abd98..067eb0fb56e 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1173,7 +1173,6 @@ plots.purge = function(gd) { // these get recreated on Plotly.plot anyway, but just to be safe // (and to have a record of them...) - delete gd._tester; delete gd._promises; delete gd._redrawTimer; delete gd.firstscatter; diff --git a/src/traces/carpet/plot.js b/src/traces/carpet/plot.js index 9e62b3221c3..333f9f9c66e 100644 --- a/src/traces/carpet/plot.js +++ b/src/traces/carpet/plot.js @@ -59,8 +59,8 @@ function plotOne(gd, plotinfo, cd) { drawGridLines(xa, ya, boundaryLayer, aax, 'a-boundary', aax._boundarylines); drawGridLines(xa, ya, boundaryLayer, bax, 'b-boundary', bax._boundarylines); - var maxAExtent = drawAxisLabels(gd._tester, xa, ya, trace, t, labelLayer, aax._labels, 'a-label'); - var maxBExtent = drawAxisLabels(gd._tester, xa, ya, trace, t, labelLayer, bax._labels, 'b-label'); + var maxAExtent = drawAxisLabels(Drawing.tester, xa, ya, trace, t, labelLayer, aax._labels, 'a-label'); + var maxBExtent = drawAxisLabels(Drawing.tester, xa, ya, trace, t, labelLayer, bax._labels, 'b-label'); drawAxisTitles(labelLayer, trace, t, xa, ya, maxAExtent, maxBExtent); diff --git a/test/jasmine/tests/plots_test.js b/test/jasmine/tests/plots_test.js index f95e98685f5..4634488c94e 100644 --- a/test/jasmine/tests/plots_test.js +++ b/test/jasmine/tests/plots_test.js @@ -428,7 +428,6 @@ describe('Test Plots', function() { expect(gd.undonum).toBeUndefined(); expect(gd.autoplay).toBeUndefined(); expect(gd.changed).toBeUndefined(); - expect(gd._tester).toBeUndefined(); expect(gd._promises).toBeUndefined(); expect(gd._redrawTimer).toBeUndefined(); expect(gd.firstscatter).toBeUndefined();