Drawing.bBox "inTester" mode for quicker testing of dummy elements

plotly · alexcjohnson · Jun 30, 2017 · Jun 21, 2017 · Jun 21, 2017 · Jun 22, 2017
commit 539ca224de5d350373782eec85a726a6818fe716
diff --git a/src/components/drawing/index.js b/src/components/drawing/index.js
@@ -604,15 +604,31 @@ drawing.makeTester = function() {
 
 /*
  * 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)
+ * in a reference frame where it isn't translated (or transformed) and
+ * its anchor point is at (0,0)
  * always returns a copy of the bbox, so the caller can modify it safely
+ *
+ * @param {SVGElement} node: the element to measure. If possible this should be
+ *   a <text> or MathJax <g> element that's already passed through
+ *   `convertToTspans` because in that case we can cache the results, but it's
+ *   possible to pass in any svg element.
+ *
+ * @param {boolean} inTester: is this element already in `drawing.tester`?
+ *   If you are measuring a dummy element, rather than one you really intend
+ *   to use on the plot, making it in `drawing.tester` in the first place
+ *   allows us to test faster because it cuts out cloning and appending it.
+ *
+ * @param {string} hash: for internal use only, if we already know the cache key
+ *   for this element beforehand.
+ *
+ * @return {object}: a plain object containing the width, height, left, right,
+ *   top, and bottom of `node`
  */
 drawing.savedBBoxes = {};
 var savedBBoxesCount = 0;
 var maxSavedBBoxes = 10000;
 
-drawing.bBox = function(node, hash) {
+drawing.bBox = function(node, inTester, hash) {
     /*
      * Cache elements we've already measured so we don't have to
      * remeasure the same thing many times
@@ -645,7 +661,7 @@ drawing.bBox = function(node, hash) {
             if(!transform) {
                 // in this case, just varying x and y, don't bother caching
                 // the final bBox because the alteration is quick.
-                var innerBB = drawing.bBox(innerNode, hash);
+                var innerBB = drawing.bBox(innerNode, false, hash);
                 if(x) {
                     innerBB.left += x;
                     innerBB.right += x;
@@ -672,12 +688,17 @@ drawing.bBox = function(node, hash) {
             if(out) return Lib.extendFlat({}, out);
         }
     }
+    var testNode, tester;
+    if(inTester) {
+        testNode = node;
+    }
+    else {
+        tester = drawing.tester.node();
 
-    var tester = drawing.tester.node();
-
-    // copy the node to test into the tester
-    var testNode = node.cloneNode(true);
-    tester.appendChild(testNode);
+        // copy the node to test into the tester
+        testNode = node.cloneNode(true);
+        tester.appendChild(testNode);
+    }
 
     // standardize its position (and newline tspans if any)
     d3.select(testNode)
@@ -689,7 +710,7 @@ drawing.bBox = function(node, hash) {
         .node()
         .getBoundingClientRect();
 
-    tester.removeChild(testNode);
+    if(!inTester) tester.removeChild(testNode);
 
     var bb = {
         height: testRect.height,

diff --git a/src/traces/contour/plot.js b/src/traces/contour/plot.js
@@ -368,7 +368,7 @@ function makeLinesAndLabels(plotgroup, pathinfo, gd, cd0, contours, perimeter) {
             };
         }
 
-        var dummyText = defs.append('text')
+        var dummyText = Drawing.tester.append('text')
             .attr('data-notex', 1)
             .call(Drawing.font, contours.font);
 
@@ -397,7 +397,7 @@ function makeLinesAndLabels(plotgroup, pathinfo, gd, cd0, contours, perimeter) {
             var text = contourFormat(d.level);
             dummyText.text(text)
                 .call(svgTextUtils.convertToTspans, gd);
-            var bBox = Drawing.bBox(dummyText.node());
+            var bBox = Drawing.bBox(dummyText.node(), true);
             var textWidth = bBox.width;
             var textHeight = bBox.height;
             var dy = (bBox.top + bBox.bottom) / 2;

diff --git a/test/jasmine/tests/drawing_test.js b/test/jasmine/tests/drawing_test.js
@@ -1,6 +1,7 @@
 var d3 = require('d3');
 var Plotly = require('@lib/index');
 var Drawing = require('@src/components/drawing');
+var svgTextUtils = require('@src/lib/svg_text_utils');
 var createGraphDiv = require('../assets/create_graph_div');
 var destroyGraphDiv = require('../assets/destroy_graph_div');
 var fail = require('../assets/fail_test');
@@ -422,6 +423,28 @@ describe('Drawing', function() {
             .catch(fail)
             .then(done);
         });
+
+        it('works with dummy nodes created in Drawing.tester', function() {
+            var node = Drawing.tester.append('text')
+                .text('bananas')
+                .call(Drawing.font, '"Open Sans", verdana, arial, sans-serif', 19)
+                .call(svgTextUtils.convertToTspans).node();
+
+            expect(node.parentNode).toBe(Drawing.tester.node());
+
+            assertBBox(Drawing.bBox(node), {
+                height: 21,
+                width: 80,
+                left: 0,
+                top: -17,
+                right: 80,
+                bottom: 4
+            });
+
+            expect(node.parentNode).toBe(Drawing.tester.node());
+
+            node.parentNode.removeChild(node);
+        });
     });
 });