From 0cad1fee0323ce7fe4d62cea657668a8f6a990e3 Mon Sep 17 00:00:00 2001 From: etienne Date: Fri, 25 May 2018 12:53:37 -0400 Subject: [PATCH 1/3] lint (breakup prepFn and clickFn from dragOption obj assigment) --- src/plots/cartesian/dragbox.js | 168 +++++++++++++++++---------------- 1 file changed, 85 insertions(+), 83 deletions(-) diff --git a/src/plots/cartesian/dragbox.js b/src/plots/cartesian/dragbox.js index ebd542fdbdb..b83cfa2c42e 100644 --- a/src/plots/cartesian/dragbox.js +++ b/src/plots/cartesian/dragbox.js @@ -144,98 +144,100 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) { var dragOptions = { element: dragger, gd: gd, - plotinfo: plotinfo, - prepFn: function(e, startX, startY) { - var dragModeNow = gd._fullLayout.dragmode; - - recomputeAxisLists(); - - if(!allFixedRanges) { - if(isMainDrag) { - // main dragger handles all drag modes, and changes - // to pan (or to zoom if it already is pan) on shift - if(e.shiftKey) { - if(dragModeNow === 'pan') dragModeNow = 'zoom'; - else if(!isSelectOrLasso(dragModeNow)) dragModeNow = 'pan'; - } - else if(e.ctrlKey) { - dragModeNow = 'pan'; - } + plotinfo: plotinfo + }; + + dragOptions.prepFn = function(e, startX, startY) { + var dragModeNow = gd._fullLayout.dragmode; + + recomputeAxisLists(); + + if(!allFixedRanges) { + if(isMainDrag) { + // main dragger handles all drag modes, and changes + // to pan (or to zoom if it already is pan) on shift + if(e.shiftKey) { + if(dragModeNow === 'pan') dragModeNow = 'zoom'; + else if(!isSelectOrLasso(dragModeNow)) dragModeNow = 'pan'; + } + else if(e.ctrlKey) { + dragModeNow = 'pan'; } - // all other draggers just pan - else dragModeNow = 'pan'; } + // all other draggers just pan + else dragModeNow = 'pan'; + } - if(dragModeNow === 'lasso') dragOptions.minDrag = 1; - else dragOptions.minDrag = undefined; + if(dragModeNow === 'lasso') dragOptions.minDrag = 1; + else dragOptions.minDrag = undefined; - if(isSelectOrLasso(dragModeNow)) { - dragOptions.xaxes = xaxes; - dragOptions.yaxes = yaxes; - prepSelect(e, startX, startY, dragOptions, dragModeNow); - } - else if(allFixedRanges) { - clearSelect(zoomlayer); - } - else if(dragModeNow === 'zoom') { - dragOptions.moveFn = zoomMove; - dragOptions.doneFn = zoomDone; + if(isSelectOrLasso(dragModeNow)) { + dragOptions.xaxes = xaxes; + dragOptions.yaxes = yaxes; + prepSelect(e, startX, startY, dragOptions, dragModeNow); + } + else 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; + // 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); - } - }, - clickFn: function(numClicks, evt) { - removeZoombox(gd); + zoomPrep(e, startX, startY); + } + else if(dragModeNow === 'pan') { + dragOptions.moveFn = plotDrag; + dragOptions.doneFn = dragTail; + clearSelect(zoomlayer); + } + }; - if(numClicks === 2 && !singleEnd) doubleClick(); + dragOptions.clickFn = function(numClicks, evt) { + removeZoombox(gd); - if(isMainDrag) { - Fx.click(gd, evt, plotinfo.id); + if(numClicks === 2 && !singleEnd) doubleClick(); + + if(isMainDrag) { + Fx.click(gd, evt, plotinfo.id); + } + else if(numClicks === 1 && singleEnd) { + var ax = ns ? ya0 : xa0, + end = (ns === 's' || ew === 'w') ? 0 : 1, + attrStr = ax._name + '.range[' + end + ']', + initialText = getEndText(ax, end), + hAlign = 'left', + vAlign = 'middle'; + + if(ax.fixedrange) return; + + if(ns) { + vAlign = (ns === 'n') ? 'top' : 'bottom'; + if(ax.side === 'right') hAlign = 'right'; } - else if(numClicks === 1 && singleEnd) { - var ax = ns ? ya0 : xa0, - end = (ns === 's' || ew === 'w') ? 0 : 1, - attrStr = ax._name + '.range[' + end + ']', - initialText = getEndText(ax, end), - hAlign = 'left', - vAlign = 'middle'; - - if(ax.fixedrange) return; - - if(ns) { - vAlign = (ns === 'n') ? 'top' : 'bottom'; - if(ax.side === 'right') hAlign = 'right'; - } - else if(ew === 'e') hAlign = 'right'; - - if(gd._context.showAxisRangeEntryBoxes) { - d3.select(dragger) - .call(svgTextUtils.makeEditable, { - gd: gd, - immediate: true, - background: gd._fullLayout.paper_bgcolor, - text: String(initialText), - fill: ax.tickfont ? ax.tickfont.color : '#444', - horizontalAlign: hAlign, - verticalAlign: vAlign - }) - .on('edit', function(text) { - var v = ax.d2r(text); - if(v !== undefined) { - Registry.call('relayout', gd, attrStr, v); - } - }); - } + else if(ew === 'e') hAlign = 'right'; + + if(gd._context.showAxisRangeEntryBoxes) { + d3.select(dragger) + .call(svgTextUtils.makeEditable, { + gd: gd, + immediate: true, + background: gd._fullLayout.paper_bgcolor, + text: String(initialText), + fill: ax.tickfont ? ax.tickfont.color : '#444', + horizontalAlign: hAlign, + verticalAlign: vAlign + }) + .on('edit', function(text) { + var v = ax.d2r(text); + if(v !== undefined) { + Registry.call('relayout', gd, attrStr, v); + } + }); } } }; From 20820cb70c4879c91e8ade621c829f1b5ac4c069 Mon Sep 17 00:00:00 2001 From: etienne Date: Fri, 25 May 2018 14:36:41 -0400 Subject: [PATCH 2/3] reassign zoom/pan clickFn when switching from select/lasso mode --- src/plots/cartesian/dragbox.js | 44 ++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/src/plots/cartesian/dragbox.js b/src/plots/cartesian/dragbox.js index b83cfa2c42e..e81be8142f3 100644 --- a/src/plots/cartesian/dragbox.js +++ b/src/plots/cartesian/dragbox.js @@ -174,30 +174,32 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) { if(isSelectOrLasso(dragModeNow)) { dragOptions.xaxes = xaxes; dragOptions.yaxes = yaxes; + // this attaches moveFn, clickFn, doneFn on dragOptions prepSelect(e, startX, startY, dragOptions, dragModeNow); - } - else 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); + } 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); + } } }; - dragOptions.clickFn = function(numClicks, evt) { + function clickFn(numClicks, evt) { removeZoombox(gd); if(numClicks === 2 && !singleEnd) doubleClick(); @@ -240,7 +242,7 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) { }); } } - }; + } dragElement.init(dragOptions); From 1be1742bb2bd9c6d5c59ce20eb47f6726b496041 Mon Sep 17 00:00:00 2001 From: etienne Date: Fri, 25 May 2018 14:37:08 -0400 Subject: [PATCH 3/3] add select vs pan double-click test --- test/jasmine/tests/select_test.js | 108 +++++++++++++++++++++++++++++- 1 file changed, 107 insertions(+), 1 deletion(-) diff --git a/test/jasmine/tests/select_test.js b/test/jasmine/tests/select_test.js index 1a0e6323e97..cd9f60d03cf 100644 --- a/test/jasmine/tests/select_test.js +++ b/test/jasmine/tests/select_test.js @@ -612,9 +612,115 @@ describe('@flaky Test select box and lasso in general:', function() { }) .catch(failTest) .then(done); - }); + it('should clear selected points on double click only on pan/lasso modes', function(done) { + var gd = createGraphDiv(); + var fig = Lib.extendDeep({}, require('@mocks/0.json')); + fig.data = [fig.data[0]]; + fig.layout.xaxis.autorange = false; + fig.layout.xaxis.range = [2, 8]; + fig.layout.yaxis.autorange = false; + fig.layout.yaxis.range = [0, 3]; + + function _assert(msg, exp) { + expect(gd.layout.xaxis.range) + .toBeCloseToArray(exp.xrng, 2, 'xaxis range - ' + msg); + expect(gd.layout.yaxis.range) + .toBeCloseToArray(exp.yrng, 2, 'yaxis range - ' + msg); + + if(exp.selpts === null) { + expect('selectedpoints' in gd.data[0]) + .toBe(false, 'cleared selectedpoints - ' + msg); + } else { + expect(gd.data[0].selectedpoints) + .toBeCloseToArray(exp.selpts, 2, 'selectedpoints - ' + msg); + } + } + + Plotly.plot(gd, fig).then(function() { + _assert('base', { + xrng: [2, 8], + yrng: [0, 3], + selpts: null + }); + return Plotly.relayout(gd, 'xaxis.range', [0, 10]); + }) + .then(function() { + _assert('after xrng relayout', { + xrng: [0, 10], + yrng: [0, 3], + selpts: null + }); + return doubleClick(200, 200); + }) + .then(function() { + _assert('after double-click under dragmode zoom', { + xrng: [2, 8], + yrng: [0, 3], + selpts: null + }); + return Plotly.relayout(gd, 'dragmode', 'select'); + }) + .then(function() { + _assert('after relayout to select', { + xrng: [2, 8], + yrng: [0, 3], + selpts: null + }); + return drag([[100, 100], [400, 400]]); + }) + .then(function() { + _assert('after selection', { + xrng: [2, 8], + yrng: [0, 3], + selpts: [40, 41, 42, 43, 44, 45, 46, 47, 48] + }); + return doubleClick(200, 200); + }) + .then(function() { + _assert('after double-click under dragmode select', { + xrng: [2, 8], + yrng: [0, 3], + selpts: null + }); + return drag([[100, 100], [400, 400]]); + }) + .then(function() { + _assert('after selection 2', { + xrng: [2, 8], + yrng: [0, 3], + selpts: [40, 41, 42, 43, 44, 45, 46, 47, 48] + }); + return Plotly.relayout(gd, 'dragmode', 'pan'); + }) + .then(function() { + _assert('after relayout to pan', { + xrng: [2, 8], + yrng: [0, 3], + selpts: [40, 41, 42, 43, 44, 45, 46, 47, 48] + }); + return Plotly.relayout(gd, 'yaxis.range', [0, 20]); + }) + .then(function() { + _assert('after yrng relayout', { + xrng: [2, 8], + yrng: [0, 20], + selpts: [40, 41, 42, 43, 44, 45, 46, 47, 48] + }); + return doubleClick(200, 200); + }) + .then(function() { + _assert('after double-click under dragmode pan', { + xrng: [2, 8], + yrng: [0, 3], + // N.B. does not clear selection! + selpts: [40, 41, 42, 43, 44, 45, 46, 47, 48] + }); + }) + .catch(failTest) + .then(done); + }); }); describe('@flaky Test select box and lasso per trace:', function() {