Skip to content

Commit 24deba6

Browse files
etpinardetpinard
authored
Merge pull request #3950 from plotly/1d-2d-drag-bug
1d -> 2d -> 1d zoombox regression fix + drag test maintenance
2 parents 57c1fbd + e676a43 commit 24deba6

18 files changed

+479
-386
lines changed

‎src/plots/cartesian/dragbox.js

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
8585
// graph-wide optimization flags
8686
var hasScatterGl, hasSplom, hasSVG;
8787
// collected changes to be made to the plot by relayout at the end
88-
var updates = {};
88+
var updates;
8989

9090
function recomputeAxisLists() {
9191
xa0 = plotinfo.xaxis;
@@ -415,6 +415,8 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
415415
}
416416

417417
function computeZoomUpdates() {
418+
updates = {};
419+
418420
// TODO: edit linked axes in zoomAxRanges and in dragTail
419421
if(zoomMode === 'xy' || zoomMode === 'x') {
420422
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) {
427429
}
428430

429431
function zoomDone() {
430-
updates = {};
431-
432432
// more strict than dragged, which allows you to come back to where you started
433433
// and still count as dragged
434434
if(Math.min(box.h, box.w) < MINDRAG * 2) {
@@ -654,12 +654,12 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
654654
var ax2 = constrainedAxes[0] || xaHash[axId2] || yaHash[axId2];
655655

656656
if(ax2) {
657-
var rng = ax2.range;
658657
if(out) {
659-
out[ax._name + '.range[0]'] = rng[0];
660-
out[ax._name + '.range[1]'] = rng[1];
658+
// zoombox case - don't mutate 'range', just add keys in 'updates'
659+
out[ax._name + '.range[0]'] = out[ax2._name + '.range[0]'];
660+
out[ax._name + '.range[1]'] = out[ax2._name + '.range[1]'];
661661
} else {
662-
ax.range = rng;
662+
ax.range = ax2.range.slice();
663663
}
664664
}
665665
}
@@ -995,19 +995,14 @@ function zoomAxRanges(axList, r0Fraction, r1Fraction, updates, linkedAxes) {
995995

996996
var axRangeLinear0 = axi._rl[0];
997997
var axRangeLinearSpan = axi._rl[1] - axRangeLinear0;
998-
axi.range = [
999-
axi.l2r(axRangeLinear0 + axRangeLinearSpan * r0Fraction),
1000-
axi.l2r(axRangeLinear0 + axRangeLinearSpan * r1Fraction)
1001-
];
1002-
1003-
updates[axi._name + '.range[0]'] = axi.range[0];
1004-
updates[axi._name + '.range[1]'] = axi.range[1];
998+
updates[axi._name + '.range[0]'] = axi.l2r(axRangeLinear0 + axRangeLinearSpan * r0Fraction);
999+
updates[axi._name + '.range[1]'] = axi.l2r(axRangeLinear0 + axRangeLinearSpan * r1Fraction);
10051000
}
10061001

10071002
// zoom linked axes about their centers
10081003
if(linkedAxes && linkedAxes.length) {
10091004
var linkedR0Fraction = (r0Fraction + (1 - r1Fraction)) / 2;
1010-
zoomAxRanges(linkedAxes, linkedR0Fraction, 1 - linkedR0Fraction, updates, [], []);
1005+
zoomAxRanges(linkedAxes, linkedR0Fraction, 1 - linkedR0Fraction, updates, []);
10111006
}
10121007
}
10131008

‎test/jasmine/assets/drag.js

Lines changed: 133 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,146 @@
11
var isNumeric = require('fast-isnumeric');
2+
23
var mouseEvent = require('./mouse_event');
4+
var touchEvent = require('./touch_event');
35
var getNodeCoords = require('./get_node_coords');
46
var delay = require('./delay');
57

6-
function makeFns(node, dx, dy, opts) {
8+
/**
9+
* Inside `opts`:
10+
*
11+
* @param {array of 2 numbers} pos0 :
12+
* px position of start of drag motion, default computed using `node` and `edge`
13+
* @param {DOM element} node :
14+
* element to drag on, default found using `pos0`
15+
* @param {string} edge :
16+
* combinations of 'n', 's', 'e', 'w' used to find `pos0` of `node`
17+
*
18+
* Set one of:
19+
* @param {array of arrays of numbers} path :
20+
* px position drag path
21+
* @param {array of 2 numbers} dpos :
22+
* px position delta
23+
* @param {array of 2 numbers} posN :
24+
* px position of end of drag motion
25+
*
26+
* If using `dpos` or `posN`
27+
* @param {number} nsteps :
28+
* set number of steps to take between `pos0` and `pos0` + `dpos` or `posN`, default is 1
29+
*
30+
* @param {boolean} touch :
31+
* pass `true` to simulate touch events
32+
* @param {boolean} shiftKey, altKey, ctrlKey ....
33+
* pass `true to simulate <shift>, alt, ctrl drag (see ./mouse_event.js for more info)
34+
*
35+
* @param {function} clearThrottle :
36+
* pass Lib.clearThrottle to clear throttle for all mouse/touch event
37+
* @param {boolean} noCover :
38+
* do not wait for "drag cover" element to start "move" events
39+
*
40+
* @return {object}
41+
* - {function} start
42+
* - {function} end
43+
*/
44+
function makeFns(opts) {
745
opts = opts || {};
846

9-
var nsteps = opts.nsteps || 1;
10-
var edge = opts.edge || '';
11-
var noCover = Boolean(opts.noCover);
47+
var path;
48+
49+
if(Array.isArray(opts.path) && opts.path.length >= 2 &&
50+
Array.isArray(opts.path[0]) && Array.isArray(opts.path[1])) {
51+
path = opts.path;
52+
} else {
53+
var nsteps = opts.nsteps || 1;
54+
var p0, dp;
55+
var msg = [];
56+
57+
if(opts.pos0 && isNumeric(opts.pos0[0]) && isNumeric(opts.pos0[1])) {
58+
p0 = opts.pos0;
59+
} else if(opts.node) {
60+
var coords = getNodeCoords(opts.node, opts.edge || '');
61+
p0 = [coords.x, coords.y];
62+
} else {
63+
msg.push('Cannot determine drag path start position from the given options');
64+
}
65+
66+
if(opts.dpos && isNumeric(opts.dpos[0]) && isNumeric(opts.dpos[1])) {
67+
dp = opts.dpos;
68+
} else if(opts.posN && isNumeric(opts.posN[0]) && isNumeric(opts.posN[1])) {
69+
dp = [opts.posN[0] - opts.pos0[0], opts.posN[1] - opts.pos0[1]];
70+
} else {
71+
msg.push('Cannot determine drag path step from the given options');
72+
}
73+
74+
if(msg.length) {
75+
throw new Error(msg.join('\n'));
76+
}
77+
78+
path = [p0];
79+
80+
for(var i = 1; i <= nsteps; i++) {
81+
path[i] = [
82+
p0[0] + i * dp[0] / nsteps,
83+
p0[1] + i * dp[1] / nsteps
84+
];
85+
}
86+
}
1287

13-
var coords = getNodeCoords(node, edge);
14-
var fromX = isNumeric(opts.x0) ? opts.x0 : coords.x;
15-
var fromY = isNumeric(opts.y0) ? opts.y0 : coords.y;
88+
function extendOpts(patch) {
89+
var out = {};
90+
var k;
91+
for(k in opts) out[k] = opts[k];
92+
for(k in patch) out[k] = patch[k];
93+
return out;
94+
}
1695

1796
var dragCoverNode;
18-
var toX;
19-
var toY;
2097

2198
function start() {
22-
mouseEvent('mousemove', fromX, fromY, {element: node});
23-
mouseEvent('mousedown', fromX, fromY, {element: node});
99+
if(opts.clearThrottle) opts.clearThrottle();
100+
101+
var x0 = path[0][0];
102+
var y0 = path[0][1];
103+
104+
var _opts = extendOpts({element: opts.node});
105+
106+
if(opts.touch) {
107+
touchEvent('touchstart', x0, y0, _opts);
108+
} else {
109+
mouseEvent('mousemove', x0, y0, _opts);
110+
mouseEvent('mousedown', x0, y0, _opts);
111+
}
24112

25-
return (noCover ? Promise.resolve(node) : waitForDragCover())
113+
return (opts.noCover ? Promise.resolve(opts.node) : waitForDragCover())
26114
.then(function(_dragCoverNode) {
27115
dragCoverNode = _dragCoverNode;
28116

29-
for(var i = 1; i <= nsteps; i++) {
30-
toX = fromX + i * dx / nsteps;
31-
toY = fromY + i * dy / nsteps;
32-
mouseEvent('mousemove', toX, toY, {element: dragCoverNode});
33-
}
117+
var _opts = extendOpts({element: dragCoverNode});
118+
119+
path.slice(1).forEach(function(p) {
120+
if(opts.clearThrottle) opts.clearThrottle();
121+
if(opts.touch) {
122+
touchEvent('touchmove', p[0], p[1], _opts);
123+
} else {
124+
mouseEvent('mousemove', p[0], p[1], _opts);
125+
}
126+
});
34127
});
35128
}
36129

37130
function end() {
38-
mouseEvent('mouseup', toX, toY, {element: dragCoverNode});
39-
return noCover || waitForDragCoverRemoval();
131+
var iN = path.length - 1;
132+
var xN = path[iN][0];
133+
var yN = path[iN][1];
134+
135+
var _opts = extendOpts({element: dragCoverNode});
136+
137+
if(opts.touch) {
138+
touchEvent('touchend', xN, yN, _opts);
139+
} else {
140+
mouseEvent('mouseup', xN, yN, _opts);
141+
}
142+
143+
return opts.noCover || waitForDragCoverRemoval();
40144
}
41145

42146
return {
@@ -45,21 +149,17 @@ function makeFns(node, dx, dy, opts) {
45149
};
46150
}
47151

48-
/*
49-
* drag: grab a node and drag it (dx, dy) pixels
50-
* optionally specify an edge ('n', 'se', 'w' etc)
51-
* to grab it by an edge or corner (otherwise the middle is used)
152+
/**
153+
* Inside `opts`:
154+
*
155+
* Same as in makeDragFns plus:
156+
*
157+
* @param {number} timeDelay :
158+
* time delay between drag start promise resolve and drag end call
52159
*/
53-
function drag(node, dx, dy, edge, x0, y0, nsteps, noCover, timeDelay) {
54-
if(!timeDelay) timeDelay = 0;
55-
var fns = makeFns(node, dx, dy, {
56-
edge: edge,
57-
x0: x0,
58-
y0: y0,
59-
nsteps: nsteps,
60-
noCover: noCover
61-
});
62-
160+
function drag(opts) {
161+
var fns = makeFns(opts);
162+
var timeDelay = opts.timeDelay || 0;
63163
return fns.start().then(delay(timeDelay)).then(fns.end);
64164
}
65165

‎test/jasmine/tests/annotations_test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1050,7 +1050,7 @@ describe('annotation effects', function() {
10501050
afterEach(destroyGraphDiv);
10511051

10521052
function dragAndReplot(node, dx, dy, edge) {
1053-
return drag(node, dx, dy, edge).then(function() {
1053+
return drag({node: node, dpos: [dx, dy], edge: edge}).then(function() {
10541054
return Plots.previousPromises(gd);
10551055
});
10561056
}

0 commit comments

Comments
 (0)