From 4240a41702d86d64d2390ae784c9e50d06782bef Mon Sep 17 00:00:00 2001 From: etienne Date: Mon, 28 May 2018 14:27:08 -0400 Subject: [PATCH 1/6] make custom array matcher logs easier to copy-paste into test code --- test/jasmine/assets/custom_matchers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/jasmine/assets/custom_matchers.js b/test/jasmine/assets/custom_matchers.js index 23deb14ef4e..a071a5e3d56 100644 --- a/test/jasmine/assets/custom_matchers.js +++ b/test/jasmine/assets/custom_matchers.js @@ -203,7 +203,7 @@ function coercePosition(precision) { } function arrayToStr(array) { - return '[ ' + array.join(', ') + ' ]'; + return '[' + array.join(', ') + ']'; } beforeAll(function() { From f7dd63e2af13c5d907374249ad739e606ed50d3c Mon Sep 17 00:00:00 2001 From: etienne Date: Mon, 28 May 2018 14:27:31 -0400 Subject: [PATCH 2/6] add extra msg to selection outline node cnt assertions --- test/jasmine/tests/select_test.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/jasmine/tests/select_test.js b/test/jasmine/tests/select_test.js index cd9f60d03cf..55437e392c5 100644 --- a/test/jasmine/tests/select_test.js +++ b/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; From 7784a10880545ef79b5dbc269d5450f337c6807d Mon Sep 17 00:00:00 2001 From: etienne Date: Mon, 28 May 2018 14:28:09 -0400 Subject: [PATCH 3/6] fix #2669 - fix shift selection after pan --- src/plots/cartesian/dragbox.js | 2 + test/jasmine/tests/select_test.js | 125 ++++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+) diff --git a/src/plots/cartesian/dragbox.js b/src/plots/cartesian/dragbox.js index e81be8142f3..3c88ec3599d 100644 --- a/src/plots/cartesian/dragbox.js +++ b/src/plots/cartesian/dragbox.js @@ -178,6 +178,8 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) { prepSelect(e, startX, startY, dragOptions, dragModeNow); } else { dragOptions.clickFn = clickFn; + // clear selection polygon cache (if any) + plotinfo.selection = false; if(allFixedRanges) { clearSelect(zoomlayer); diff --git a/test/jasmine/tests/select_test.js b/test/jasmine/tests/select_test.js index 55437e392c5..21a7f1dbf6e 100644 --- a/test/jasmine/tests/select_test.js +++ b/test/jasmine/tests/select_test.js @@ -723,6 +723,131 @@ 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'; + + // 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] + ] + }); + }) + .catch(failTest) + .then(done); + }); }); describe('@flaky Test select box and lasso per trace:', function() { From ffe1700aa3c38221ed5d57cec08c2b16b88275a3 Mon Sep 17 00:00:00 2001 From: etienne Date: Tue, 29 May 2018 11:16:43 -0400 Subject: [PATCH 4/6] make sure scroll clears selection outline not just the 1st time --- src/plots/cartesian/dragbox.js | 4 +--- test/jasmine/tests/select_test.js | 17 ++++++++++++++--- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/plots/cartesian/dragbox.js b/src/plots/cartesian/dragbox.js index 3c88ec3599d..8d02541207d 100644 --- a/src/plots/cartesian/dragbox.js +++ b/src/plots/cartesian/dragbox.js @@ -393,9 +393,7 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) { return; } - if(redrawTimer === null) { - clearSelect(zoomlayer); - } + clearSelect(zoomlayer); // If a transition is in progress, then disable any behavior: if(gd._transitioningWithDuration) { diff --git a/test/jasmine/tests/select_test.js b/test/jasmine/tests/select_test.js index 21a7f1dbf6e..c2bb39e6513 100644 --- a/test/jasmine/tests/select_test.js +++ b/test/jasmine/tests/select_test.js @@ -539,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) From f682fb8979da56083c4a513fe7cbe642f323a2e3 Mon Sep 17 00:00:00 2001 From: etienne Date: Tue, 29 May 2018 11:24:59 -0400 Subject: [PATCH 5/6] clear selectio polygon cache on scroll --- src/plots/cartesian/dragbox.js | 17 +++++++++++------ test/jasmine/tests/select_test.js | 11 +++++++++++ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/plots/cartesian/dragbox.js b/src/plots/cartesian/dragbox.js index 8d02541207d..47be7993327 100644 --- a/src/plots/cartesian/dragbox.js +++ b/src/plots/cartesian/dragbox.js @@ -178,11 +178,9 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) { prepSelect(e, startX, startY, dragOptions, dragModeNow); } else { dragOptions.clickFn = clickFn; - // clear selection polygon cache (if any) - plotinfo.selection = false; if(allFixedRanges) { - clearSelect(zoomlayer); + clearAndResetSelect(); } else if(dragModeNow === 'zoom') { dragOptions.moveFn = zoomMove; dragOptions.doneFn = zoomDone; @@ -196,11 +194,18 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) { } else if(dragModeNow === 'pan') { dragOptions.moveFn = plotDrag; dragOptions.doneFn = dragTail; - clearSelect(zoomlayer); + clearAndResetSelect(); } } }; + function clearAndResetSelect() { + // clear selection polygon cache (if any) + dragOptions.plotinfo.selection = false; + // clear selection outlines + clearSelect(zoomlayer); + } + function clickFn(numClicks, evt) { removeZoombox(gd); @@ -281,7 +286,7 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) { corners = makeCorners(zoomlayer, xs, ys); - clearSelect(zoomlayer); + clearAndResetSelect(); } function zoomMove(dx0, dy0) { @@ -393,7 +398,7 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) { return; } - clearSelect(zoomlayer); + clearAndResetSelect(); // If a transition is in progress, then disable any behavior: if(gd._transitioningWithDuration) { diff --git a/test/jasmine/tests/select_test.js b/test/jasmine/tests/select_test.js index c2bb39e6513..7709e0fc0bd 100644 --- a/test/jasmine/tests/select_test.js +++ b/test/jasmine/tests/select_test.js @@ -745,6 +745,7 @@ describe('@flaky Test select box and lasso in general:', function() { 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) { @@ -856,6 +857,16 @@ describe('@flaky Test select box and lasso in general:', function() { ] }); }) + .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); }); From 34a0cf7faefa75caffd3c572b80209f9ddc6303d Mon Sep 17 00:00:00 2001 From: etienne Date: Tue, 29 May 2018 11:52:57 -0400 Subject: [PATCH 6/6] :palm_tree: clearAndResetSelect calls --- src/plots/cartesian/dragbox.js | 36 ++++++++++++++++------------------ 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/src/plots/cartesian/dragbox.js b/src/plots/cartesian/dragbox.js index 47be7993327..f4c4f154116 100644 --- a/src/plots/cartesian/dragbox.js +++ b/src/plots/cartesian/dragbox.js @@ -178,23 +178,23 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) { prepSelect(e, startX, startY, dragOptions, dragModeNow); } else { dragOptions.clickFn = clickFn; - - if(allFixedRanges) { - clearAndResetSelect(); - } 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; - clearAndResetSelect(); + 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; + } } } }; @@ -285,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); - - clearAndResetSelect(); } function zoomMove(dx0, dy0) {