Merge pull request #3950 from plotly/1d-2d-drag-bug

1d -> 2d -> 1d zoombox regression fix + drag test maintenance

Author: etpinard

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, []);
     }
 }
 

test/jasmine/assets/drag.js

@@ -1,42 +1,146 @@
 var isNumeric = require('fast-isnumeric');
+
 var mouseEvent = require('./mouse_event');
+var touchEvent = require('./touch_event');
 var getNodeCoords = require('./get_node_coords');
 var delay = require('./delay');
 
-function makeFns(node, dx, dy, opts) {
+/**
+ * Inside `opts`:
+ *
+ * @param {array of 2 numbers} pos0 :
+ *  px position of start of drag motion, default computed using `node` and `edge`
+ * @param {DOM element} node :
+ *  element to drag on, default found using `pos0`
+ * @param {string} edge :
+ *  combinations of 'n', 's', 'e', 'w' used to find `pos0` of `node`
+ *
+ * Set one of:
+ * @param {array of arrays of numbers} path :
+ *  px position drag path
+ * @param {array of 2 numbers} dpos :
+ *  px position delta
+ * @param {array of 2 numbers} posN :
+ *  px position of end of drag motion
+ *
+ * If using `dpos` or `posN`
+ * @param {number} nsteps :
+ *  set number of steps to take between `pos0` and `pos0` + `dpos` or `posN`, default is 1
+ *
+ * @param {boolean} touch :
+ *  pass `true` to simulate touch events
+ * @param {boolean} shiftKey, altKey, ctrlKey ....
+ * pass `true to simulate <shift>, alt, ctrl drag (see ./mouse_event.js for more info)
+ *
+ * @param {function} clearThrottle :
+ *  pass Lib.clearThrottle to clear throttle for all mouse/touch event
+ * @param {boolean} noCover :
+ *  do not wait for "drag cover" element to start "move" events
+ *
+ * @return {object}
+ *  - {function} start
+ *  - {function} end
+ */
+function makeFns(opts) {
     opts = opts || {};
 
-    var nsteps = opts.nsteps || 1;
-    var edge = opts.edge || '';
-    var noCover = Boolean(opts.noCover);
+    var path;
+
+    if(Array.isArray(opts.path) && opts.path.length >= 2 &&
+        Array.isArray(opts.path[0]) && Array.isArray(opts.path[1])) {
+        path = opts.path;
+    } else {
+        var nsteps = opts.nsteps || 1;
+        var p0, dp;
+        var msg = [];
+
+        if(opts.pos0 && isNumeric(opts.pos0[0]) && isNumeric(opts.pos0[1])) {
+            p0 = opts.pos0;
+        } else if(opts.node) {
+            var coords = getNodeCoords(opts.node, opts.edge || '');
+            p0 = [coords.x, coords.y];
+        } else {
+            msg.push('Cannot determine drag path start position from the given options');
+        }
+
+        if(opts.dpos && isNumeric(opts.dpos[0]) && isNumeric(opts.dpos[1])) {
+            dp = opts.dpos;
+        } else if(opts.posN && isNumeric(opts.posN[0]) && isNumeric(opts.posN[1])) {
+            dp = [opts.posN[0] - opts.pos0[0], opts.posN[1] - opts.pos0[1]];
+        } else {
+            msg.push('Cannot determine drag path step from the given options');
+        }
+
+        if(msg.length) {
+            throw new Error(msg.join('\n'));
+        }
+
+        path = [p0];
+
+        for(var i = 1; i <= nsteps; i++) {
+            path[i] = [
+                p0[0] + i * dp[0] / nsteps,
+                p0[1] + i * dp[1] / nsteps
+            ];
+        }
+    }
 
-    var coords = getNodeCoords(node, edge);
-    var fromX = isNumeric(opts.x0) ? opts.x0 : coords.x;
-    var fromY = isNumeric(opts.y0) ? opts.y0 : coords.y;
+    function extendOpts(patch) {
+        var out = {};
+        var k;
+        for(k in opts) out[k] = opts[k];
+        for(k in patch) out[k] = patch[k];
+        return out;
+    }
 
     var dragCoverNode;
-    var toX;
-    var toY;
 
     function start() {
-        mouseEvent('mousemove', fromX, fromY, {element: node});
-        mouseEvent('mousedown', fromX, fromY, {element: node});
+        if(opts.clearThrottle) opts.clearThrottle();
+
+        var x0 = path[0][0];
+        var y0 = path[0][1];
+
+        var _opts = extendOpts({element: opts.node});
+
+        if(opts.touch) {
+            touchEvent('touchstart', x0, y0, _opts);
+        } else {
+            mouseEvent('mousemove', x0, y0, _opts);
+            mouseEvent('mousedown', x0, y0, _opts);
+        }
 
-        return (noCover ? Promise.resolve(node) : waitForDragCover())
+        return (opts.noCover ? Promise.resolve(opts.node) : waitForDragCover())
         .then(function(_dragCoverNode) {
             dragCoverNode = _dragCoverNode;
 
-            for(var i = 1; i <= nsteps; i++) {
-                toX = fromX + i * dx / nsteps;
-                toY = fromY + i * dy / nsteps;
-                mouseEvent('mousemove', toX, toY, {element: dragCoverNode});
-            }
+            var _opts = extendOpts({element: dragCoverNode});
+
+            path.slice(1).forEach(function(p) {
+                if(opts.clearThrottle) opts.clearThrottle();
+                if(opts.touch) {
+                    touchEvent('touchmove', p[0], p[1], _opts);
+                } else {
+                    mouseEvent('mousemove', p[0], p[1], _opts);
+                }
+            });
         });
     }
 
     function end() {
-        mouseEvent('mouseup', toX, toY, {element: dragCoverNode});
-        return noCover || waitForDragCoverRemoval();
+        var iN = path.length - 1;
+        var xN = path[iN][0];
+        var yN = path[iN][1];
+
+        var _opts = extendOpts({element: dragCoverNode});
+
+        if(opts.touch) {
+            touchEvent('touchend', xN, yN, _opts);
+        } else {
+            mouseEvent('mouseup', xN, yN, _opts);
+        }
+
+        return opts.noCover || waitForDragCoverRemoval();
     }
 
     return {
@@ -45,21 +149,17 @@ function makeFns(node, dx, dy, opts) {
     };
 }
 
-/*
- * drag: grab a node and drag it (dx, dy) pixels
- * optionally specify an edge ('n', 'se', 'w' etc)
- * to grab it by an edge or corner (otherwise the middle is used)
+/**
+ * Inside `opts`:
+ *
+ * Same as in makeDragFns plus:
+ *
+ * @param {number} timeDelay :
+ *  time delay between drag start promise resolve and drag end call
  */
-function drag(node, dx, dy, edge, x0, y0, nsteps, noCover, timeDelay) {
-    if(!timeDelay) timeDelay = 0;
-    var fns = makeFns(node, dx, dy, {
-        edge: edge,
-        x0: x0,
-        y0: y0,
-        nsteps: nsteps,
-        noCover: noCover
-    });
-
+function drag(opts) {
+    var fns = makeFns(opts);
+    var timeDelay = opts.timeDelay || 0;
     return fns.start().then(delay(timeDelay)).then(fns.end);
 }
 

test/jasmine/tests/annotations_test.js

@@ -1050,7 +1050,7 @@ describe('annotation effects', function() {
     afterEach(destroyGraphDiv);
 
     function dragAndReplot(node, dx, dy, edge) {
-        return drag(node, dx, dy, edge).then(function() {
+        return drag({node: node, dpos: [dx, dy], edge: edge}).then(function() {
             return Plots.previousPromises(gd);
         });
     }