Merge pull request #440 from zjffun/fix/drag-delay

fix: does not trigger `drag:start` sensor event when moved during delay

Author: tsov

index.d.ts

@@ -86,6 +86,12 @@ declare module '@shopify/draggable' {
         ? SnapOutEvent
         : AbstractEvent;
 
+    interface DelayOptions {
+        mouse?: number;
+        drag?: number;
+        touch?: number;
+    }
+
     /**
      * DragEvent
      */
@@ -167,7 +173,7 @@ declare module '@shopify/draggable' {
         draggable?: string;
         distance?: number;
         handle?: string | NodeList | HTMLElement[] | HTMLElement | ((currentElement: HTMLElement) => HTMLElement);
-        delay?: number;
+        delay?: number | DelayOptions;
         plugins?: Array<typeof AbstractPlugin>;
         sensors?: Sensor[];
         classes?: { [key in DraggableClassNames]: string };
@@ -315,7 +321,7 @@ declare module '@shopify/draggable' {
     export class DragPressureSensorEvent extends SensorEvent { }
 
     export interface SensorOptions {
-        delay?: number;
+        delay?: number | DelayOptions;
     }
 
     export class Sensor {

scripts/test/helpers/sensor.js

@@ -1,13 +1,16 @@
 import {DRAG_DELAY, defaultTouchEventOptions, defaultMouseEventOptions} from './constants';
 import {triggerEvent} from './event';
 
-export function waitForDragDelay(dragDelay = DRAG_DELAY) {
+export function waitForDragDelay({dragDelay = DRAG_DELAY, restoreDateMock = true} = {}) {
   const next = Date.now() + dragDelay + 1;
   const dateMock = jest.spyOn(Date, 'now').mockImplementation(() => {
     return next;
   });
   jest.runTimersToTime(dragDelay + 1);
-  dateMock.mockRestore();
+  if (restoreDateMock) {
+    dateMock.mockRestore();
+  }
+  return dateMock;
 }
 
 export function clickMouse(element, options = {}) {

src/Draggable/Draggable.js

@@ -48,7 +48,7 @@ const defaultClasses = {
 export const defaultOptions = {
   draggable: '.draggable-source',
   handle: null,
-  delay: 100,
+  delay: {},
   distance: 0,
   placedTimeout: 800,
   plugins: [],

src/Draggable/README.md

@@ -91,9 +91,19 @@ look for an element with `.draggable-source` class. Default: `.draggable-source`
 Specify a css selector for a handle element if you don't want to allow drag action
 on the entire element. Default: `null`
 
-**`delay {Number}`**  
+**`delay {Number|Object}`**  
 If you want to delay a drag start you can specify delay in milliseconds. This can be useful
-for draggable elements within scrollable containers. Default: `100`
+for draggable elements within scrollable containers. To allow touch scrolling, we set 100ms delay for TouchSensor by default. Default:
+
+```js
+{
+  mouse: 0,
+  drag: 0,
+  touch: 100,
+}
+```
+
+You can set the same delay for all sensors by setting a number, or set an object to set the delay for each sensor separately.
 
 **`distance {Number}`**  
 The distance you want the pointer to have moved before drag starts. This can be useful

src/Draggable/Sensors/DragSensor/DragSensor.js

@@ -211,7 +211,7 @@ export default class DragSensor extends Sensor {
     this.mouseDownTimeout = setTimeout(() => {
       target.draggable = true;
       this.draggableElement = target;
-    }, this.options.delay);
+    }, this.delay.drag);
   }
 
   /**

src/Draggable/Sensors/MouseSensor/MouseSensor.js

@@ -83,7 +83,7 @@ export default class MouseSensor extends Sensor {
       return;
     }
 
-    const {delay = 0} = this.options;
+    const {delay} = this;
     const {pageX, pageY} = event;
 
     Object.assign(this, {pageX, pageY});
@@ -97,7 +97,7 @@ export default class MouseSensor extends Sensor {
 
     this.mouseDownTimeout = window.setTimeout(() => {
       this[onDistanceChange]({pageX: this.pageX, pageY: this.pageY});
-    }, delay);
+    }, delay.mouse);
   }
 
   /**
@@ -134,8 +134,8 @@ export default class MouseSensor extends Sensor {
    */
   [onDistanceChange](event) {
     const {pageX, pageY} = event;
-    const {delay, distance} = this.options;
-    const {startEvent} = this;
+    const {distance} = this.options;
+    const {startEvent, delay} = this;
 
     Object.assign(this, {pageX, pageY});
 
@@ -146,8 +146,12 @@ export default class MouseSensor extends Sensor {
     const timeElapsed = Date.now() - this.onMouseDownAt;
     const distanceTravelled = euclideanDistance(startEvent.pageX, startEvent.pageY, pageX, pageY) || 0;
 
-    if (timeElapsed >= delay && distanceTravelled >= distance) {
-      window.clearTimeout(this.mouseDownTimeout);
+    clearTimeout(this.mouseDownTimeout);
+
+    if (timeElapsed < delay.mouse) {
+      // moved during delay
+      document.removeEventListener('mousemove', this[onDistanceChange]);
+    } else if (distanceTravelled >= distance) {
       document.removeEventListener('mousemove', this[onDistanceChange]);
       this[startDrag]();
     }

src/Draggable/Sensors/MouseSensor/tests/MouseSensor.test.js

@@ -251,14 +251,17 @@ describe('MouseSensor', () => {
       expect(dragFlow).not.toHaveTriggeredSensorEvent('drag:start');
     });
 
-    it('only triggers `drag:start` sensor event once when delay ends after distance is met', () => {
+    it('does not trigger `drag:start` sensor event when moved during delay', () => {
       function dragFlow() {
         clickMouse(draggableElement);
         moveMouse(draggableElement, {pageY: 1, pageX: 0});
+        const dateMock = waitForDragDelay({restoreDateMock: false});
+        moveMouse(draggableElement, {pageY: 2, pageX: 0});
         waitForDragDelay();
         releaseMouse(document.body);
+        dateMock.mockRestore();
       }
-      expect(dragFlow).toHaveTriggeredSensorEvent('drag:start', 1);
+      expect(dragFlow).not.toHaveTriggeredSensorEvent('drag:start', 1);
     });
 
     it('only triggers `drag:start` sensor event once when distance and delay are met at the same time', () => {

src/Draggable/Sensors/Sensor/Sensor.js

@@ -1,3 +1,9 @@
+const defaultDealy = {
+  mouse: 0,
+  drag: 0,
+  touch: 100,
+};
+
 /**
  * Base sensor class. Extend from this class to create a new or custom sensor
  * @class Sensor
@@ -45,6 +51,13 @@ export default class Sensor {
      * @type {Event}
      */
     this.startEvent = null;
+
+    /**
+     * The delay of each sensor
+     * @property delay
+     * @type {Object}
+     */
+    this.delay = calcDelay(options.delay);
   }
 
   /**
@@ -96,3 +109,37 @@ export default class Sensor {
     return sensorEvent;
   }
 }
+
+/**
+ * Calculate the delay of each sensor through the delay in the options
+ * @param {undefined|Number|Object} optionsDelay - the delay in the options
+ * @return {Object}
+ */
+function calcDelay(optionsDelay) {
+  const delay = {};
+
+  if (optionsDelay === undefined) {
+    return {...defaultDealy};
+  }
+
+  if (typeof optionsDelay === 'number') {
+    for (const key in defaultDealy) {
+      if (defaultDealy.hasOwnProperty(key)) {
+        delay[key] = optionsDelay;
+      }
+    }
+    return delay;
+  }
+
+  for (const key in defaultDealy) {
+    if (defaultDealy.hasOwnProperty(key)) {
+      if (optionsDelay[key] === undefined) {
+        delay[key] = defaultDealy[key];
+      } else {
+        delay[key] = optionsDelay[key];
+      }
+    }
+  }
+
+  return delay;
+}

src/Draggable/Sensors/Sensor/tests/Sensor.test.js

@@ -17,6 +17,38 @@ describe('Sensor', () => {
       expect(sensor.containers).toEqual(expectedContainers);
       expect(sensor.options).toEqual(expectedOptions);
     });
+
+    describe('should initialize with correct delay', () => {
+      it('unset', () => {
+        const sensor = new Sensor(undefined, {});
+
+        expect(sensor.delay).toEqual({
+          mouse: 0,
+          drag: 0,
+          touch: 100,
+        });
+      });
+
+      it('number', () => {
+        const sensor = new Sensor(undefined, {delay: 42});
+
+        expect(sensor.delay).toEqual({
+          mouse: 42,
+          drag: 42,
+          touch: 42,
+        });
+      });
+
+      it('object', () => {
+        const sensor = new Sensor(undefined, {delay: {mouse: 42, drag: 142}});
+
+        expect(sensor.delay).toEqual({
+          mouse: 42,
+          drag: 142,
+          touch: 100,
+        });
+      });
+    });
   });
 
   describe('#attach', () => {

src/Draggable/Sensors/TouchSensor/TouchSensor.js

@@ -111,7 +111,8 @@ export default class TouchSensor extends Sensor {
     if (!container) {
       return;
     }
-    const {distance = 0, delay = 0} = this.options;
+    const {distance = 0} = this.options;
+    const {delay} = this;
     const {pageX, pageY} = touchCoords(event);
 
     Object.assign(this, {pageX, pageY});
@@ -130,7 +131,7 @@ export default class TouchSensor extends Sensor {
 
     this.tapTimeout = window.setTimeout(() => {
       this[onDistanceChange]({touches: [{pageX: this.pageX, pageY: this.pageY}]});
-    }, delay);
+    }, delay.touch);
   }
 
   /**
@@ -166,16 +167,21 @@ export default class TouchSensor extends Sensor {
    * @param {Event} event - Touch move event
    */
   [onDistanceChange](event) {
-    const {delay, distance} = this.options;
-    const {startEvent} = this;
+    const {distance} = this.options;
+    const {startEvent, delay} = this;
     const start = touchCoords(startEvent);
     const current = touchCoords(event);
     const timeElapsed = Date.now() - this.onTouchStartAt;
     const distanceTravelled = euclideanDistance(start.pageX, start.pageY, current.pageX, current.pageY);
 
     Object.assign(this, current);
-    if (timeElapsed >= delay && distanceTravelled >= distance) {
-      window.clearTimeout(this.tapTimeout);
+
+    clearTimeout(this.tapTimeout);
+
+    if (timeElapsed < delay.touch) {
+      // moved during delay
+      document.removeEventListener('touchmove', this[onDistanceChange]);
+    } else if (distanceTravelled >= distance) {
       document.removeEventListener('touchmove', this[onDistanceChange]);
       this[startDrag]();
     }