fix the clientX and clientY of touch sensor events

Shopify · Jul 24, 2020 · cf1a703 · cf1a703
1 parent 7a98fcc
commit cf1a703
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 79 deletions.
diff --git a/scripts/test/helpers/constants.js b/scripts/test/helpers/constants.js
@@ -1,6 +1,8 @@
 export const defaultTouchEventOptions = {
   touches: [
     {
+      clientX: 0,
+      clientY: 0,
       pageX: 0,
       pageY: 0,
     },

diff --git a/src/Draggable/Plugins/Mirror/Mirror.js b/src/Draggable/Plugins/Mirror/Mirror.js
@@ -13,7 +13,6 @@ export const onDragMove = Symbol('onDragMove');
 export const onDragStop = Symbol('onDragStop');
 export const onMirrorCreated = Symbol('onMirrorCreated');
 export const onMirrorMove = Symbol('onMirrorMove');
-export const onScroll = Symbol('onScroll');
 export const getAppendableContainer = Symbol('getAppendableContainer');
 
 /**
@@ -67,31 +66,11 @@ export default class Mirror extends AbstractPlugin {
       ...this.getOptions(),
     };
 
-    /**
-     * Scroll offset for touch devices because the mirror is positioned fixed
-     * @property {Object} scrollOffset
-     * @property {Number} scrollOffset.x
-     * @property {Number} scrollOffset.y
-     */
-    this.scrollOffset = {x: 0, y: 0};
-
-    /**
-     * Initial scroll offset for touch devices because the mirror is positioned fixed
-     * @property {Object} scrollOffset
-     * @property {Number} scrollOffset.x
-     * @property {Number} scrollOffset.y
-     */
-    this.initialScrollOffset = {
-      x: window.scrollX,
-      y: window.scrollY,
-    };
-
     this[onDragStart] = this[onDragStart].bind(this);
     this[onDragMove] = this[onDragMove].bind(this);
     this[onDragStop] = this[onDragStop].bind(this);
     this[onMirrorCreated] = this[onMirrorCreated].bind(this);
     this[onMirrorMove] = this[onMirrorMove].bind(this);
-    this[onScroll] = this[onScroll].bind(this);
   }
 
   /**
@@ -131,15 +110,6 @@ export default class Mirror extends AbstractPlugin {
       return;
     }
 
-    if ('ontouchstart' in window) {
-      document.addEventListener('scroll', this[onScroll], true);
-    }
-
-    this.initialScrollOffset = {
-      x: window.scrollX,
-      y: window.scrollY,
-    };
-
     const {source, originalSource, sourceContainer, sensorEvent} = dragEvent;
 
     // Last sensor position of mirror move
@@ -233,13 +203,6 @@ export default class Mirror extends AbstractPlugin {
   }
 
   [onDragStop](dragEvent) {
-    if ('ontouchstart' in window) {
-      document.removeEventListener('scroll', this[onScroll], true);
-    }
-
-    this.initialScrollOffset = {x: 0, y: 0};
-    this.scrollOffset = {x: 0, y: 0};
-
     if (!this.mirror) {
       return;
     }
@@ -261,13 +224,6 @@ export default class Mirror extends AbstractPlugin {
     }
   }
 
-  [onScroll]() {
-    this.scrollOffset = {
-      x: window.scrollX - this.initialScrollOffset.x,
-      y: window.scrollY - this.initialScrollOffset.y,
-    };
-  }
-
   /**
    * Mirror created handler
    * @param {MirrorCreatedEvent} mirrorEvent
@@ -293,7 +249,6 @@ export default class Mirror extends AbstractPlugin {
       source,
       sensorEvent,
       mirrorClass,
-      scrollOffset: this.scrollOffset,
       options: this.options,
       passedThreshX: true,
       passedThreshY: true,
@@ -337,7 +292,6 @@ export default class Mirror extends AbstractPlugin {
       options: this.options,
       initialX: this.initialX,
       initialY: this.initialY,
-      scrollOffset: this.scrollOffset,
       passedThreshX: mirrorEvent.passedThreshX,
       passedThreshY: mirrorEvent.passedThreshY,
       lastMovedX: this.lastMovedX,
@@ -491,7 +445,6 @@ function positionMirror({withFrame = false, initial = false} = {}) {
     mirrorOffset,
     initialY,
     initialX,
-    scrollOffset,
     options,
     passedThreshX,
     passedThreshY,
@@ -511,11 +464,11 @@ function positionMirror({withFrame = false, initial = false} = {}) {
 
         if (mirrorOffset) {
           const x = passedThreshX
-            ? Math.round((sensorEvent.clientX - mirrorOffset.left - scrollOffset.x) / (options.thresholdX || 1)) *
+            ? Math.round((sensorEvent.clientX - mirrorOffset.left) / (options.thresholdX || 1)) *
               (options.thresholdX || 1)
             : Math.round(lastMovedX);
           const y = passedThreshY
-            ? Math.round((sensorEvent.clientY - mirrorOffset.top - scrollOffset.y) / (options.thresholdY || 1)) *
+            ? Math.round((sensorEvent.clientY - mirrorOffset.top) / (options.thresholdY || 1)) *
               (options.thresholdY || 1)
             : Math.round(lastMovedY);
 

diff --git a/src/Draggable/Plugins/Scrollable/Scrollable.js b/src/Draggable/Plugins/Scrollable/Scrollable.js
@@ -49,13 +49,11 @@ export default class Scrollable extends AbstractPlugin {
     };
 
     /**
-     * Keeps current mouse position
-     * @property {Object} currentMousePosition
-     * @property {Number} currentMousePosition.clientX
-     * @property {Number} currentMousePosition.clientY
+     * Keeps current sensor event
+     * @property {SensorEvent} currentSensorEvent
      * @type {Object|null}
      */
-    this.currentMousePosition = null;
+    this.currentSensorEvent = null;
 
     /**
      * Scroll animation frame
@@ -159,18 +157,7 @@ export default class Scrollable extends AbstractPlugin {
       return;
     }
 
-    const sensorEvent = dragEvent.sensorEvent;
-    const scrollOffset = {x: 0, y: 0};
-
-    if ('ontouchstart' in window) {
-      scrollOffset.y = window.pageYOffset || document.documentElement.scrollTop || document.body.scrollTop || 0;
-      scrollOffset.x = window.pageXOffset || document.documentElement.scrollLeft || document.body.scrollLeft || 0;
-    }
-
-    this.currentMousePosition = {
-      clientX: sensorEvent.clientX - scrollOffset.x,
-      clientY: sensorEvent.clientY - scrollOffset.y,
-    };
+    this.currentSensorEvent = dragEvent.sensorEvent;
 
     this.scrollAnimationFrame = requestAnimationFrame(this[scroll]);
   }
@@ -186,15 +173,15 @@ export default class Scrollable extends AbstractPlugin {
     this.scrollableElement = null;
     this.scrollAnimationFrame = null;
     this.findScrollableElementFrame = null;
-    this.currentMousePosition = null;
+    this.currentSensorEvent = null;
   }
 
   /**
    * Scroll function that does the heavylifting
    * @private
    */
   [scroll]() {
-    if (!this.scrollableElement || !this.currentMousePosition) {
+    if (!this.scrollableElement || !this.currentSensorEvent) {
       return;
     }
 
@@ -209,8 +196,7 @@ export default class Scrollable extends AbstractPlugin {
 
     const documentScrollingElement = getDocumentScrollingElement();
     const scrollableElement = this.scrollableElement;
-    const clientX = this.currentMousePosition.clientX;
-    const clientY = this.currentMousePosition.clientY;
+    const {clientX, clientY} = this.currentSensorEvent;
 
     if (scrollableElement !== document.body && scrollableElement !== document.documentElement && !cutOff) {
       const {offsetHeight, offsetWidth} = scrollableElement;

diff --git a/src/Draggable/Sensors/TouchSensor/TouchSensor.js b/src/Draggable/Sensors/TouchSensor/TouchSensor.js
@@ -140,11 +140,11 @@ export default class TouchSensor extends Sensor {
   [startDrag]() {
     const startEvent = this.startEvent;
     const container = this.currentContainer;
-    const touch = touchCoords(startEvent);
+    const {clientX, clientY} = touchCoords(startEvent);
 
     const dragStartEvent = new DragStartSensorEvent({
-      clientX: touch.pageX,
-      clientY: touch.pageY,
+      clientX,
+      clientY,
       target: startEvent.target,
       container,
       originalEvent: startEvent,
@@ -190,12 +190,12 @@ export default class TouchSensor extends Sensor {
     if (!this.dragging) {
       return;
     }
-    const {pageX, pageY} = touchCoords(event);
+    const {clientX, clientY, pageX, pageY} = touchCoords(event);
     const target = document.elementFromPoint(pageX - window.scrollX, pageY - window.scrollY);
 
     const dragMoveEvent = new DragMoveSensorEvent({
-      clientX: pageX,
-      clientY: pageY,
+      clientX,
+      clientY,
       target,
       container: this.currentContainer,
       originalEvent: event,
@@ -227,14 +227,14 @@ export default class TouchSensor extends Sensor {
 
     document.removeEventListener('touchmove', this[onTouchMove]);
 
-    const {pageX, pageY} = touchCoords(event);
+    const {clientX, clientY, pageX, pageY} = touchCoords(event);
     const target = document.elementFromPoint(pageX - window.scrollX, pageY - window.scrollY);
 
     event.preventDefault();
 
     const dragStopEvent = new DragStopSensorEvent({
-      clientX: pageX,
-      clientY: pageY,
+      clientX,
+      clientY,
       target,
       container: this.currentContainer,
       originalEvent: event,

diff --git a/src/Draggable/Sensors/TouchSensor/tests/TouchSensor.test.js b/src/Draggable/Sensors/TouchSensor/tests/TouchSensor.test.js
@@ -97,6 +97,31 @@ describe('TouchSensor', () => {
 
       expect(touchEndEvent.defaultPrevented).toBe(true);
     });
+
+    it('event parameter', () => {
+      const touchEvent = {
+        pageX: 21,
+        pageY: 22,
+        clientX: 11,
+        clientY: 12,
+      };
+
+      function testParameter(event) {
+        expect(event.detail.clientX).toBe(touchEvent.clientX);
+        expect(event.detail.clientY).toBe(touchEvent.clientY);
+      }
+
+      sandbox.addEventListener('drag:start', testParameter);
+
+      sandbox.addEventListener('drag:move', testParameter);
+
+      sandbox.addEventListener('drag:stop', testParameter);
+
+      touchStart(draggableElement, {touches: [touchEvent]});
+      waitForDragDelay();
+      touchMove(draggableElement, {touches: [touchEvent]});
+      touchRelease(draggableElement, {touches: [touchEvent]});
+    });
   });
 
   describe('using distance', () => {