Merge pull request #4227 from plotly/treemap-text-transitions-maxdepth

Treemap text positions for out of range cases when transitioning with maxdepth
plotly · Sep 30, 2019 · 4420593 · 4420593
2 parents dfb3987 + 940c83a
commit 4420593
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 47 deletions.
diff --git a/src/traces/treemap/draw_ancestors.js b/src/traces/treemap/draw_ancestors.js
@@ -132,32 +132,25 @@ 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 || ' ') // use one space character instead of a blank string to avoid jumps during transition
             .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,
-            {
-                onPathbar: true
-            }
-        );
+        pt.transform = toMoveInsideSlice(pt, {
+            onPathbar: true
+        });
 
         if(helpers.isOutsideText(trace, pt)) {
             // consider in/out diff font sizes

diff --git a/src/traces/treemap/draw_descendants.js b/src/traces/treemap/draw_descendants.js
@@ -157,39 +157,33 @@ module.exports = function drawDescendants(gd, cd, entry, slices, opts) {
             hovered: false
         });
 
+        if(pt.x0 === pt.x1 || pt.y0 === pt.y1) {
+            pt._text = '';
+        } 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 tx;
-        if(isHeader) {
-            if(noRoomForHeader) return;
-
-            tx = helpers.getPtLabel(pt);
-        } else {
-            tx = formatSliceLabel(pt, entry, trace, cd, fullLayout) || ' ';
-        }
-
-        sliceText.text(tx)
+        sliceText.text(pt._text || ' ') // use one space character instead of a blank string to avoid jumps during transition
             .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(
-            pt.x0,
-            pt.x1,
-            pt.y0,
-            pt.y1,
-            pt.textBB,
-            {
-                isHeader: isHeader
-            }
-        );
+        pt.transform = toMoveInsideSlice(pt, {
+            isHeader: isHeader
+        });
 
         if(helpers.isOutsideText(trace, pt)) {
             // consider in/out diff font sizes

diff --git a/src/traces/treemap/plot.js b/src/traces/treemap/plot.js
@@ -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');
@@ -299,6 +315,11 @@ function plotOne(gd, cd, element, transitionOpts) {
         if(opts.isHeader) {
             x0 += pad.l - TEXTPAD;
             x1 -= pad.r - TEXTPAD;
+            if(x0 >= x1) {
+                var mid = (x0 + x1) / 2;
+                x0 = mid - TEXTPAD;
+                x1 = mid + TEXTPAD;
+            }
 
             // limit the drawing area for headers
             var limY;
@@ -428,16 +449,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
@@ -1208,7 +1208,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', ' ', ' '])) // use one space character instead of a blank string to avoid jumps during transition
         .then(_restyle({textinfo: 'label+text+value'}))
         .then(_assert('show everything', ['Root', 'B', 'A\n1\nnode1', 'b\n3\nnode3']))
         .then(_restyle({textinfo: null}))
@@ -1269,7 +1269,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
@@ -1367,7 +1367,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', [287.375195, 5]);
+            _assert('enter b text to new position', 'transform', 'b', [286.16071428571433, 35714285714286]);
         })
         .catch(failTest)
         .then(done);