Merge pull request #1683 from plotly/annotation-text-position-fix

Drawing bbox fix for window scroll

Author: etpinard

src/components/drawing/index.js

@@ -557,11 +557,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]);
@@ -594,16 +594,16 @@ drawing.makeTester = function(gd) {
         tester.node()._cache = {};
     }
 
-    gd._tester = tester;
-    gd._testref = testref;
+    drawing.tester = tester;
+    drawing.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
@@ -613,14 +613,13 @@ drawing.bBox = function(node) {
         return Lib.extendFlat({}, savedBBoxes[saveNum.value]);
     }
 
-    if(!drawing.test3) {
-        drawing.test3 = d3.select('#js-plotly-tester');
-        drawing.tester = drawing.test3.node();
-    }
+    var tester3 = drawing.tester;
+    var tester = tester3.node();
 
     // copy the node to test into the tester
     var testNode = node.cloneNode(true);
-    drawing.tester.appendChild(testNode);
+    tester.appendChild(testNode);
+
     // standardize its position... do we really want to do this?
     d3.select(testNode).attr({
         x: 0,
@@ -629,20 +628,19 @@ drawing.bBox = function(node) {
     });
 
     var testRect = testNode.getBoundingClientRect();
-    if(!drawing.refRect) {
-        drawing.refRect = drawing.test3.select('.js-reference-point')
-            .node().getBoundingClientRect();
-    }
+    var refRect = drawing.testref
+        .node()
+        .getBoundingClientRect();
 
-    drawing.tester.removeChild(testNode);
+    tester.removeChild(testNode);
 
     var bb = {
         height: testRect.height,
         width: testRect.width,
-        left: testRect.left - drawing.refRect.left,
-        top: testRect.top - drawing.refRect.top,
-        right: testRect.right - drawing.refRect.left,
-        bottom: testRect.bottom - drawing.refRect.top
+        left: testRect.left - refRect.left,
+        top: testRect.top - refRect.top,
+        right: testRect.right - refRect.left,
+        bottom: testRect.bottom - refRect.top
     };
 
     // make sure we don't have too many saved boxes,

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);

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')

src/lib/svg_text_utils.js

@@ -17,9 +17,20 @@ var Lib = require('../lib');
 var xmlnsNamespaces = require('../constants/xmlns_namespaces');
 var stringMappings = require('../constants/string_mappings');
 
-// Append SVG
+var DOM_PARSER;
+
+exports.getDOMParser = function() {
+    if(DOM_PARSER) {
+        return DOM_PARSER;
+    } else if(window.DOMParser) {
+        DOM_PARSER = new window.DOMParser();
+        return DOM_PARSER;
+    } else {
+        throw new Error('Cannot initialize DOMParser');
+    }
+};
 
-var parser = new DOMParser();
+// Append SVG
 
 d3.selection.prototype.appendSVG = function(_svgString) {
     var skeleton = [
@@ -29,8 +40,9 @@ d3.selection.prototype.appendSVG = function(_svgString) {
         '</svg>'
     ].join('');
 
-    var dom = parser.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));

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

src/plots/plots.js

@@ -1173,8 +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._testref;
     delete gd._promises;
     delete gd._redrawTimer;
     delete gd.firstscatter;

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);
 

test/jasmine/tests/drawing_test.js

@@ -1,16 +1,18 @@
-var Drawing = require('@src/components/drawing');
-
 var d3 = require('d3');
-
 var Plotly = require('@lib/index');
-
+var Drawing = require('@src/components/drawing');
 var createGraphDiv = require('../assets/create_graph_div');
 var destroyGraphDiv = require('../assets/destroy_graph_div');
 var fail = require('../assets/fail_test');
+var customMatchers = require('../assets/custom_matchers');
 
 describe('Drawing', function() {
     'use strict';
 
+    beforeAll(function() {
+        jasmine.addMatchers(customMatchers);
+    });
+
     describe('setClipUrl', function() {
 
         beforeEach(function() {
@@ -348,6 +350,79 @@ describe('Drawing', function() {
             expect(g.attr('transform')).toEqual('translate(8,9) scale(4,5) translate(-8,-9) translate(1, 2)');
         });
     });
+
+    describe('bBox', function() {
+        afterEach(destroyGraphDiv);
+
+        function assertBBox(actual, expected) {
+            expect(actual.height).toEqual(expected.height, 'height');
+            expect(actual.top).toEqual(expected.top, 'top');
+            expect(actual.bottom).toEqual(expected.bottom, 'bottom');
+
+            var TOL = 3;
+            expect(actual.width).toBeWithin(expected.width, TOL, 'width');
+            expect(actual.left).toBeWithin(expected.left, TOL, 'left');
+            expect(actual.right).toBeWithin(expected.right, TOL, 'right');
+        }
+
+        it('should update bounding box dimension on window scroll', function(done) {
+            var gd = createGraphDiv();
+
+            // allow page to scroll
+            gd.style.position = 'static';
+
+            Plotly.plot(gd, [{
+                y: [1, 2, 1]
+            }], {
+                annotations: [{
+                    text: 'hello'
+                }],
+                height: window.innerHeight * 2,
+                width: 500
+            })
+            .then(function() {
+                var node = d3.select('text.annotation').node();
+                assertBBox(Drawing.bBox(node), {
+                    height: 14,
+                    width: 27.671875,
+                    left: -13.671875,
+                    top: -11,
+                    right: 14,
+                    bottom: 3
+                });
+
+                window.scroll(0, 200);
+                return Plotly.relayout(gd, 'annotations[0].text', 'HELLO');
+            })
+            .then(function() {
+                var node = d3.select('text.annotation').node();
+                assertBBox(Drawing.bBox(node), {
+                    height: 14,
+                    width: 41.015625,
+                    left: -20.671875,
+                    top: -11,
+                    right: 20.34375,
+                    bottom: 3
+                });
+
+                window.scroll(200, 0);
+                return Plotly.relayout(gd, 'annotations[0].font.size', 20);
+            })
+            .then(function() {
+                var node = d3.select('text.annotation').node();
+                assertBBox(Drawing.bBox(node), {
+                    height: 22,
+                    width: 66.015625,
+                    left: -32.78125,
+                    top: -18,
+                    right: 33.234375,
+                    bottom: 4
+                });
+            })
+            .catch(fail)
+            .then(done);
+        });
+    });
 });
 
 describe('gradients', function() {

test/jasmine/tests/plots_test.js

@@ -428,8 +428,6 @@ describe('Test Plots', function() {
             expect(gd.undonum).toBeUndefined();
             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();