fix #3943 - don't mutate fullLayout ax ranges during zoombox mousemove

etpinard · etpinard · commit 69cb62849181 · 2019-06-11T10:28:44.000-04:00
- first reason, why don't need to
- second reason, mutation aren't reset meaning going from 2d back to 1d
  zoombox lead to wrong rendering after mouseup and a discrepency
  between gd.layout(x|y).range and gd._fullLayout(x|y).range values
diff --git a/src/plots/cartesian/dragbox.js b/src/plots/cartesian/dragbox.js
@@ -85,7 +85,7 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
     // graph-wide optimization flags
     var hasScatterGl, hasSplom, hasSVG;
     // collected changes to be made to the plot by relayout at the end
-    var updates = {};
+    var updates;
 
     function recomputeAxisLists() {
         xa0 = plotinfo.xaxis;
@@ -415,6 +415,8 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
     }
 
     function computeZoomUpdates() {
+        updates = {};
+
         // TODO: edit linked axes in zoomAxRanges and in dragTail
         if(zoomMode === 'xy' || zoomMode === 'x') {
             zoomAxRanges(xaxes, box.l / pw, box.r / pw, updates, links.xaxes);
@@ -427,8 +429,6 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
     }
 
     function zoomDone() {
-        updates = {};
-
         // more strict than dragged, which allows you to come back to where you started
         // and still count as dragged
         if(Math.min(box.h, box.w) < MINDRAG * 2) {
@@ -654,12 +654,12 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
             var ax2 = constrainedAxes[0] || xaHash[axId2] || yaHash[axId2];
 
             if(ax2) {
-                var rng = ax2.range;
                 if(out) {
-                    out[ax._name + '.range[0]'] = rng[0];
-                    out[ax._name + '.range[1]'] = rng[1];
+                    // zoombox case - don't mutate 'range', just add keys in 'updates'
+                    out[ax._name + '.range[0]'] = out[ax2._name + '.range[0]'];
+                    out[ax._name + '.range[1]'] = out[ax2._name + '.range[1]'];
                 } else {
-                    ax.range = rng;
+                    ax.range = ax2.range.slice();
                 }
             }
         }
@@ -995,19 +995,14 @@ function zoomAxRanges(axList, r0Fraction, r1Fraction, updates, linkedAxes) {
 
         var axRangeLinear0 = axi._rl[0];
         var axRangeLinearSpan = axi._rl[1] - axRangeLinear0;
-        axi.range = [
-            axi.l2r(axRangeLinear0 + axRangeLinearSpan * r0Fraction),
-            axi.l2r(axRangeLinear0 + axRangeLinearSpan * r1Fraction)
-        ];
-
-        updates[axi._name + '.range[0]'] = axi.range[0];
-        updates[axi._name + '.range[1]'] = axi.range[1];
+        updates[axi._name + '.range[0]'] = axi.l2r(axRangeLinear0 + axRangeLinearSpan * r0Fraction);
+        updates[axi._name + '.range[1]'] = axi.l2r(axRangeLinear0 + axRangeLinearSpan * r1Fraction);
     }
 
     // zoom linked axes about their centers
     if(linkedAxes && linkedAxes.length) {
         var linkedR0Fraction = (r0Fraction + (1 - r1Fraction)) / 2;
-        zoomAxRanges(linkedAxes, linkedR0Fraction, 1 - linkedR0Fraction, updates, [], []);
+        zoomAxRanges(linkedAxes, linkedR0Fraction, 1 - linkedR0Fraction, updates, []);
     }
 }
 
diff --git a/test/jasmine/tests/cartesian_interact_test.js b/test/jasmine/tests/cartesian_interact_test.js
@@ -727,6 +727,72 @@ describe('axis zoom/pan and main plot zoom', function() {
         .then(done);
     });
 
+    it('handles y-only to xy back to y-only in single zoombox drag motion', function(done) {
+        function _assert(msg, evtData, xrng, yrng) {
+            expect([evtData['xaxis.range[0]'], evtData['xaxis.range[1]']])
+                    .toBeCloseToArray(xrng, 2, 'x evt - ' + msg);
+            expect([evtData['yaxis.range[0]'], evtData['yaxis.range[1]']])
+                    .toBeCloseToArray(yrng, 2, 'y evt - ' + msg);
+        }
+
+        var relayoutingList = [];
+        var relayoutList = [];
+
+        var xrng0 = [-0.1347, 2.1347];
+        var yrng1 = [1.3581, 1.5];
+        var blank = [undefined, undefined];
+
+        Plotly.plot(gd, [{
+            y: [1, 2, 1]
+        }], {
+            margin: {l: 0, t: 0, r: 0, b: 0},
+            width: 400, height: 400
+        })
+        .then(function() {
+            gd.on('plotly_relayouting', function(d) {
+                relayoutingList.push(d);
+
+                // N.B. should not mutate axis range on mousemove
+                expect(gd._fullLayout.xaxis.range)
+                    .toBeCloseToArray(xrng0, 2, 'full x range| relyouting call #' + relayoutingList.length);
+            });
+            gd.on('plotly_relayout', function(d) { relayoutList.push(d); });
+        })
+        .then(function() {
+            return drag({
+                node: getDragger('xy', 'nsew'),
+                path: [
+                    // start in middle
+                    [200, 200],
+                    // y-only zoombox
+                    [200, 250],
+                    // xy zoombox
+                    [250, 250],
+                    // back to y-only
+                    [200, 250]
+                ]
+            });
+        })
+        .then(delay(100))
+        .then(function() {
+            if(relayoutingList.length === 3) {
+                _assert('relayouting y-only', relayoutingList[0], blank, yrng1);
+                _assert('relayouting xy', relayoutingList[1], [0.9999, 1.2836], yrng1);
+                _assert('relayouting back to y-only', relayoutingList[2], blank, yrng1);
+            } else {
+                fail('did not emit correct number of plotly_relayouting events');
+            }
+
+            if(relayoutList.length === 1) {
+                _assert('relayout', relayoutList[0], blank, yrng1);
+            } else {
+                fail('did not emit correct number of plotly_relayout events');
+            }
+        })
+        .catch(failTest)
+        .then(done);
+    });
+
     it('should compute correct multicategory tick label span during drag', function(done) {
         var fig = Lib.extendDeep({}, require('@mocks/multicategory.json'));
 
@@ -1545,15 +1611,16 @@ describe('axis zoom/pan and main plot zoom', function() {
                 return drag.start()
                     .then(_assert('just after start of zoombox', {
                         nodeCnt: 4,
-                        xrng: [1.5, 1.6880],
+                        xrng: 'previous',
                         hasDragData: true,
                         zoombox: 'M269.5,114.5h-3v41h3ZM300.5,114.5h3v41h-3Z',
                         clipTranslate: [0, 0]
                     }))
                     .then(delay(step))
                     .then(_assert('during zoombox drag', {
                         nodeCnt: 5,
-                        xrng: [2, 2.2507],
+                        // N.B. x autorange for one more node
+                        xrng: [-0.257, 4.257],
                         hasDragData: true,
                         zoombox: 'M269.5,114.5h-3v41h3ZM300.5,114.5h3v41h-3Z',
                         clipTranslate: [0, 0]
