rework text transform to move inside slice function

archmoj · archmoj · commit a1a2a2b298fb · 2019-09-26T22:23:09.000-04:00
- cast _text to pt for easier debugging
- pass pt object to the function instead of separated arguments
- do not return in the middle of draw functions
- expand void area for blank text
diff --git a/src/traces/treemap/draw_ancestors.js b/src/traces/treemap/draw_ancestors.js
@@ -132,23 +132,23 @@ module.exports = function drawAncestors(gd, cd, entry, slices, opts) {
             hovered: false
         });
 
+        pt._text = (helpers.getPtLabel(pt) || '').split('<br>').join(' ') || '';
+
         var sliceTextGroup = Lib.ensureSingle(sliceTop, 'g', 'slicetext');
         var sliceText = Lib.ensureSingle(sliceTextGroup, 'text', '', function(s) {
             // prohibit tex interpretation until we can handle
             // tex and regular text together
             s.attr('data-notex', 1);
         });
 
-        var tx = (helpers.getPtLabel(pt) || ' ').split('<br>').join(' ');
-
-        sliceText.text(tx)
+        sliceText.text(pt._text)
             .classed('slicetext', true)
             .attr('text-anchor', 'start')
             .call(Drawing.font, helpers.determineTextFont(trace, pt, fullLayout.font, trace.pathdir))
             .call(svgTextUtils.convertToTspans, gd);
 
         pt.textBB = Drawing.bBox(sliceText.node());
-        pt.transform = toMoveInsideSlice(pt.x0, pt.x1, pt.y0, pt.y1, pt.textBB, {
+        pt.transform = toMoveInsideSlice(pt, {
             onPathbar: true
         });
 
diff --git a/src/traces/treemap/draw_descendants.js b/src/traces/treemap/draw_descendants.js
@@ -12,7 +12,7 @@ var d3 = require('d3');
 var Lib = require('../../lib');
 var Drawing = require('../../components/drawing');
 var svgTextUtils = require('../../lib/svg_text_utils');
-var TEXTPAD = require('../bar/constants').TEXTPAD;
+
 var partition = require('./partition');
 var styleOne = require('./style').styleOne;
 var constants = require('./constants');
@@ -157,45 +157,31 @@ module.exports = function drawDescendants(gd, cd, entry, slices, opts) {
             hovered: false
         });
 
+        if(pt.x0 === pt.x1 && pt.y0 === pt.y1) {
+            pt._text = ' '; // use one space character instead of a blank string in this case
+        } else {
+            if(isHeader) {
+                pt._text = noRoomForHeader ? '' : helpers.getPtLabel(pt) || '';
+            } else {
+                pt._text = formatSliceLabel(pt, entry, trace, cd, fullLayout) || '';
+            }
+        }
+
         var sliceTextGroup = Lib.ensureSingle(sliceTop, 'g', 'slicetext');
         var sliceText = Lib.ensureSingle(sliceTextGroup, 'text', '', function(s) {
             // prohibit tex interpretation until we can handle
             // tex and regular text together
             s.attr('data-notex', 1);
         });
 
-        var _x0 = pt.x0;
-        var _x1 = pt.x1;
-        var _y0 = pt.y0;
-        var _y1 = pt.y1;
-
-        var tx;
-        if(_x0 < _x1 && _y0 < _y1) {
-            if(isHeader) {
-                if(noRoomForHeader) return;
-
-                tx = helpers.getPtLabel(pt);
-            } else {
-                tx = formatSliceLabel(pt, entry, trace, cd, fullLayout) || ' ';
-            }
-        } else { // handle text positions for out of range cases e.g. with maxdepth
-            tx = ' ';
-
-            var expand = 2 * TEXTPAD;
-            _x0 -= expand;
-            _x1 += expand;
-            _y0 -= expand;
-            _y1 += expand;
-        }
-
-        sliceText.text(tx)
+        sliceText.text(pt._text)
             .classed('slicetext', true)
             .attr('text-anchor', hasRight ? 'end' : (hasLeft || isHeader) ? 'start' : 'middle')
             .call(Drawing.font, helpers.determineTextFont(trace, pt, fullLayout.font))
             .call(svgTextUtils.convertToTspans, gd);
 
         pt.textBB = Drawing.bBox(sliceText.node());
-        pt.transform = toMoveInsideSlice(_x0, _x1, _y0, _y1, pt.textBB, {
+        pt.transform = toMoveInsideSlice(pt, {
             isHeader: isHeader
         });
 
diff --git a/src/traces/treemap/plot.js b/src/traces/treemap/plot.js
@@ -162,14 +162,14 @@ function plotOne(gd, cd, element, transitionOpts) {
     var cenX = -vpw / 2 + gs.l + gs.w * (domain.x[1] + domain.x[0]) / 2;
     var cenY = -vph / 2 + gs.t + gs.h * (1 - (domain.y[1] + domain.y[0]) / 2);
 
-    var viewMapX = function(x) { return cenX + x; };
-    var viewMapY = function(y) { return cenY + y; };
+    var viewMapX = function(x) { return cenX + (x || 0); };
+    var viewMapY = function(y) { return cenY + (y || 0); };
 
     var barY0 = viewMapY(0);
     var barX0 = viewMapX(0);
 
-    var viewBarX = function(x) { return barX0 + x; };
-    var viewBarY = function(y) { return barY0 + y; };
+    var viewBarX = function(x) { return barX0 + (x || 0); };
+    var viewBarY = function(y) { return barY0 + (y || 0); };
 
     function pos(x, y) {
         return x + ',' + y;
@@ -273,7 +273,23 @@ function plotOne(gd, cd, element, transitionOpts) {
         );
     };
 
-    var toMoveInsideSlice = function(x0, x1, y0, y1, textBB, opts) {
+    var toMoveInsideSlice = function(pt, opts) {
+        var x0 = pt.x0;
+        var x1 = pt.x1;
+        var y0 = pt.y0;
+        var y1 = pt.y1;
+        var textBB = pt.textBB;
+
+        if(x0 === x1) {
+            x0 -= TEXTPAD;
+            x1 += TEXTPAD;
+        }
+
+        if(y0 === y1) {
+            y0 -= TEXTPAD;
+            y1 += TEXTPAD;
+        }
+
         var hasFlag = function(f) { return trace.textposition.indexOf(f) !== -1; };
 
         var hasBottom = hasFlag('bottom');
@@ -327,20 +343,13 @@ function plotOne(gd, cd, element, transitionOpts) {
             else if(offsetDir === 'right') transform.targetX += deltaX;
         }
 
-        transform.targetX = viewMapX(transform.targetX);
-        transform.targetY = viewMapY(transform.targetY);
-
-        if(isNaN(transform.targetX) || isNaN(transform.targetY)) {
-            return {};
-        }
-
         return {
             scale: transform.scale,
             rotate: transform.rotate,
             textX: transform.textX,
             textY: transform.textY,
-            targetX: transform.targetX,
-            targetY: transform.targetY
+            targetX: viewMapX(transform.targetX),
+            targetY: viewMapY(transform.targetY)
         };
     };
 
@@ -428,16 +437,16 @@ function plotOne(gd, cd, element, transitionOpts) {
         var origin = getOrigin(pt, onPathbar, refRect, size);
 
         Lib.extendFlat(prev, {
-            transform: toMoveInsideSlice(
-                origin.x0,
-                origin.x1,
-                origin.y0,
-                origin.y1,
-                pt.textBB,
-                {
-                    isHeader: helpers.isHeader(pt, trace)
-                }
-            )
+            transform: toMoveInsideSlice({
+                x0: origin.x0,
+                x1: origin.x1,
+                y0: origin.y0,
+                y1: origin.y1,
+                textBB: pt.textBB,
+                _text: pt._text
+            }, {
+                isHeader: helpers.isHeader(pt, trace)
+            })
         });
 
         if(prev0) {
diff --git a/test/jasmine/tests/treemap_test.js b/test/jasmine/tests/treemap_test.js
@@ -1066,7 +1066,7 @@ describe('Test treemap restyle:', function() {
         .then(_restyle({textinfo: 'value'}))
         .then(_assert('show input values', ['Root', 'B', '1', '3']))
         .then(_restyle({textinfo: 'none'}))
-        .then(_assert('no textinfo', ['Root', 'B', ' ', ' '])) // N.B. replaced empty string with space character for better transitions
+        .then(_assert('no textinfo', ['Root', 'B', '', '']))
         .then(_restyle({textinfo: 'label+text+value'}))
         .then(_assert('show everything', ['Root', 'B', 'A\n1\nnode1', 'b\n3\nnode3']))
         .then(_restyle({textinfo: null}))
@@ -1127,7 +1127,7 @@ describe('Test treemap tweening:', function() {
         if(attrName === 'transform') {
             var fake = {attr: function() { return actual; }};
             var xy = Drawing.getTranslate(fake);
-            expect([xy.x, xy.y]).toBeWithinArray(exp, 1, msg2);
+            expect([xy.x, xy.y]).toBeWithinArray(exp, 2, msg2);
         } else {
             // we could maybe to bring in:
             // https://github.com/hughsk/svg-path-parser
@@ -1225,7 +1225,7 @@ describe('Test treemap tweening:', function() {
                 'M284.375,188.5L548.375,188.5L548.375,308.5L284.375,308.5Z'
             );
             _assert('move B text to new position', 'transform', 'B', [220.25126, 0]);
-            _assert('enter b text to new position', 'transform', 'b', [284.66071, 35714285714286]);
+            _assert('enter b text to new position', 'transform', 'b', [286.16071428571433, 35714285714286]);
         })
         .catch(failTest)
         .then(done);
