Merge pull request #2676 from plotly/fix-shift-selection-after-pan

Fix shift selection after pan

Author: etpinard

src/plots/cartesian/dragbox.js

@@ -178,27 +178,34 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
             prepSelect(e, startX, startY, dragOptions, dragModeNow);
         } else {
             dragOptions.clickFn = clickFn;
-
-            if(allFixedRanges) {
-                clearSelect(zoomlayer);
-            } else if(dragModeNow === 'zoom') {
-                dragOptions.moveFn = zoomMove;
-                dragOptions.doneFn = zoomDone;
-
-                // zoomMove takes care of the threshold, but we need to
-                // minimize this so that constrained zoom boxes will flip
-                // orientation at the right place
-                dragOptions.minDrag = 1;
-
-                zoomPrep(e, startX, startY);
-            } else if(dragModeNow === 'pan') {
-                dragOptions.moveFn = plotDrag;
-                dragOptions.doneFn = dragTail;
-                clearSelect(zoomlayer);
+            clearAndResetSelect();
+
+            if(!allFixedRanges) {
+                if(dragModeNow === 'zoom') {
+                    dragOptions.moveFn = zoomMove;
+                    dragOptions.doneFn = zoomDone;
+
+                    // zoomMove takes care of the threshold, but we need to
+                    // minimize this so that constrained zoom boxes will flip
+                    // orientation at the right place
+                    dragOptions.minDrag = 1;
+
+                    zoomPrep(e, startX, startY);
+                } else if(dragModeNow === 'pan') {
+                    dragOptions.moveFn = plotDrag;
+                    dragOptions.doneFn = dragTail;
+                }
             }
         }
     };
 
+    function clearAndResetSelect() {
+        // clear selection polygon cache (if any)
+        dragOptions.plotinfo.selection = false;
+        // clear selection outlines
+        clearSelect(zoomlayer);
+    }
+
     function clickFn(numClicks, evt) {
         removeZoombox(gd);
 
@@ -278,8 +285,6 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
         zb = makeZoombox(zoomlayer, lum, xs, ys, path0);
 
         corners = makeCorners(zoomlayer, xs, ys);
-
-        clearSelect(zoomlayer);
     }
 
     function zoomMove(dx0, dy0) {
@@ -391,9 +396,7 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
             return;
         }
 
-        if(redrawTimer === null) {
-            clearSelect(zoomlayer);
-        }
+        clearAndResetSelect();
 
         // If a transition is in progress, then disable any behavior:
         if(gd._transitioningWithDuration) {

test/jasmine/assets/custom_matchers.js

@@ -203,7 +203,7 @@ function coercePosition(precision) {
 }
 
 function arrayToStr(array) {
-    return '[ ' + array.join(', ') + ' ]';
+    return '[' + array.join(', ') + ']';
 }
 
 beforeAll(function() {

test/jasmine/tests/select_test.js

@@ -42,11 +42,13 @@ function drag(path, options) {
     mouseEvent('mouseup', path[len - 1][0], path[len - 1][1], options);
 }
 
-function assertSelectionNodes(cornerCnt, outlineCnt) {
+function assertSelectionNodes(cornerCnt, outlineCnt, _msg) {
+    var msg = _msg ? ' - ' + _msg : '';
+
     expect(d3.selectAll('.zoomlayer > .zoombox-corners').size())
-        .toBe(cornerCnt, 'selection corner count');
+        .toBe(cornerCnt, 'selection corner count' + msg);
     expect(d3.selectAll('.zoomlayer > .select-outline').size())
-        .toBe(outlineCnt, 'selection outline count');
+        .toBe(outlineCnt, 'selection outline count' + msg);
 }
 
 var selectingCnt, selectingData, selectedCnt, selectedData, deselectCnt, doubleClickData;
@@ -537,16 +539,27 @@ describe('@flaky Test select box and lasso in general:', function() {
         mockCopy.layout.dragmode = 'select';
         mockCopy.config = {scrollZoom: true};
 
-        Plotly.plot(gd, mockCopy).then(function() {
+        function _drag() {
             resetEvents(gd);
             drag(selectPath);
             return selectedPromise;
-        })
-        .then(function() {
+        }
+
+        function _scroll() {
             mouseEvent('mousemove', selectPath[0][0], selectPath[0][1]);
             mouseEvent('scroll', selectPath[0][0], selectPath[0][1], {deltaX: 0, deltaY: -20});
+        }
+
+        Plotly.plot(gd, mockCopy)
+        .then(_drag)
+        .then(_scroll)
+        .then(function() {
+            assertSelectionNodes(0, 0);
         })
+        .then(_drag)
+        .then(_scroll)
         .then(function() {
+            // make sure it works the 2nd time aroung
             assertSelectionNodes(0, 0);
         })
         .catch(failTest)
@@ -721,6 +734,142 @@ describe('@flaky Test select box and lasso in general:', function() {
         .catch(failTest)
         .then(done);
     });
+
+    it('should remember selection polygons from previous select/lasso mode', function(done) {
+        var gd = createGraphDiv();
+        var path1 = [[150, 150], [170, 170]];
+        var path2 = [[193, 193], [213, 193]];
+
+        var fig = Lib.extendDeep({}, mock);
+        fig.layout.margin = {l: 0, t: 0, r: 0, b: 0};
+        fig.layout.width = 500;
+        fig.layout.height = 500;
+        fig.layout.dragmode = 'select';
+        fig.config = {scrollZoom: true};
+
+        // d attr to array of segment [x,y]
+        function outline2coords(outline) {
+            if(!outline.size()) return [[]];
+
+            return outline.attr('d')
+                .replace(/Z/g, '')
+                .split('M')
+                .filter(Boolean)
+                .map(function(s) {
+                    return s.split('L')
+                        .map(function(s) { return s.split(',').map(Number); });
+                })
+                .reduce(function(a, b) { return a.concat(b); });
+        }
+
+        function _assert(msg, exp) {
+            var outline = d3.select(gd).select('.zoomlayer').select('.select-outline-1');
+
+            if(exp.outline) {
+                expect(outline2coords(outline)).toBeCloseTo2DArray(exp.outline, 2, msg);
+            } else {
+                assertSelectionNodes(0, 0, msg);
+            }
+        }
+
+        function _drag(path, opts) {
+            return function() {
+                resetEvents(gd);
+                drag(path, opts);
+                return selectedPromise;
+            };
+        }
+
+        Plotly.plot(gd, fig)
+        .then(function() { _assert('base', {outline: false}); })
+        .then(_drag(path1))
+        .then(function() {
+            _assert('select path1', {
+                outline: [[150, 150], [150, 170], [170, 170], [170, 150], [150, 150]]
+            });
+        })
+        .then(_drag(path2))
+        .then(function() {
+            _assert('select path2', {
+                outline: [[193, 0], [193, 500], [213, 500], [213, 0], [193, 0]]
+            });
+        })
+        .then(_drag(path1))
+        .then(_drag(path2, {shiftKey: true}))
+        .then(function() {
+            _assert('select path1+path2', {
+                outline: [
+                    [170, 170], [170, 150], [150, 150], [150, 170], [170, 170],
+                    [213, 500], [213, 0], [193, 0], [193, 500], [213, 500]
+                ]
+            });
+        })
+        .then(function() {
+            return Plotly.relayout(gd, 'dragmode', 'lasso');
+        })
+        .then(function() {
+            // N.B. all relayout calls clear the selection outline at the moment,
+            // perhaps we could make an exception for select <-> lasso ?
+            _assert('after relayout -> lasso', {outline: false});
+        })
+        .then(_drag(lassoPath, {shiftKey: true}))
+        .then(function() {
+            // merged with previous 'select' polygon
+            _assert('after shift lasso', {
+                outline: [
+                    [170, 170], [170, 150], [150, 150], [150, 170], [170, 170],
+                    [213, 500], [213, 0], [193, 0], [193, 500], [213, 500],
+                    [335, 243], [328, 169], [316, 171], [318, 239], [335, 243]
+                ]
+            });
+        })
+        .then(_drag(lassoPath))
+        .then(function() {
+            _assert('after lasso (no-shift)', {
+                outline: [[316, 171], [318, 239], [335, 243], [328, 169], [316, 171]]
+            });
+        })
+        .then(function() {
+            return Plotly.relayout(gd, 'dragmode', 'pan');
+        })
+        .then(function() {
+            _assert('after relayout -> pan', {outline: false});
+            drag(path2);
+            _assert('after pan', {outline: false});
+            return Plotly.relayout(gd, 'dragmode', 'select');
+        })
+        .then(function() {
+            _assert('after relayout back to select', {outline: false});
+        })
+        .then(_drag(path1, {shiftKey: true}))
+        .then(function() {
+            // this used to merged 'lasso' polygons before (see #2669)
+            _assert('shift select path1 after pan', {
+                outline: [[150, 150], [150, 170], [170, 170], [170, 150], [150, 150]]
+            });
+        })
+        .then(_drag(path2, {shiftKey: true}))
+        .then(function() {
+            _assert('shift select path1+path2 after pan', {
+                outline: [
+                    [170, 170], [170, 150], [150, 150], [150, 170], [170, 170],
+                    [213, 500], [213, 0], [193, 0], [193, 500], [213, 500]
+                ]
+            });
+        })
+        .then(function() {
+            mouseEvent('mousemove', 200, 200);
+            mouseEvent('scroll', 200, 200, {deltaX: 0, deltaY: -20});
+        })
+        .then(_drag(path1, {shiftKey: true}))
+        .then(function() {
+            _assert('shift select path1 after scroll', {
+                outline: [[150, 150], [150, 170], [170, 170], [170, 150], [150, 150]]
+            });
+        })
+        .catch(failTest)
+        .then(done);
+    });
 });
 
 describe('@flaky Test select box and lasso per trace:', function() {