Merge pull request #200 from Shopify/touch-webkit-fix

Touch Sensor fix for scrolling
Shopify · Apr 4, 2018 · 75c773a · 75c773a
2 parents 8f672b4 + 5a898dd
commit 75c773a
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 62 deletions.
diff --git a/src/Draggable/Sensors/TouchSensor/TouchSensor.js b/src/Draggable/Sensors/TouchSensor/TouchSensor.js
@@ -6,16 +6,26 @@ const onTouchStart = Symbol('onTouchStart');
 const onTouchHold = Symbol('onTouchHold');
 const onTouchEnd = Symbol('onTouchEnd');
 const onTouchMove = Symbol('onTouchMove');
-const onScroll = Symbol('onScroll');
 
 /**
- * Adds default document.ontouchmove. Workaround for preventing scrolling on touchmove
+ * Prevents scrolling when set to true
+ * @var {Boolean} preventScrolling
  */
-document.ontouchmove =
-  document.ontouchmove ||
-  function() {
-    return true;
-  };
+let preventScrolling = false;
+
+// WebKit requires cancelable `touchmove` events to be added as early as possible
+window.addEventListener(
+  'touchmove',
+  (event) => {
+    if (!preventScrolling) {
+      return;
+    }
+
+    // Prevent scrolling
+    event.preventDefault();
+  },
+  {passive: false},
+);
 
 /**
  * This sensor picks up native browser touch events and dictates drag operations
@@ -58,7 +68,6 @@ export default class TouchSensor extends Sensor {
     this[onTouchHold] = this[onTouchHold].bind(this);
     this[onTouchEnd] = this[onTouchEnd].bind(this);
     this[onTouchMove] = this[onTouchMove].bind(this);
-    this[onScroll] = this[onScroll].bind(this);
   }
 
   /**
@@ -87,22 +96,12 @@ export default class TouchSensor extends Sensor {
       return;
     }
 
-    document.addEventListener('touchmove', this[onTouchMove], {passive: false});
+    document.addEventListener('touchmove', this[onTouchMove]);
     document.addEventListener('touchend', this[onTouchEnd]);
     document.addEventListener('touchcancel', this[onTouchEnd]);
-
-    // detect if body is scrolling on iOS
-    document.addEventListener('scroll', this[onScroll]);
     container.addEventListener('contextmenu', onContextMenu);
 
     this.currentContainer = container;
-
-    this.currentScrollableParent = closest(container, (element) => element.offsetHeight < element.scrollHeight);
-
-    if (this.currentScrollableParent) {
-      this.currentScrollableParent.addEventListener('scroll', this[onScroll]);
-    }
-
     this.tapTimeout = setTimeout(this[onTouchHold](event, container), this.options.delay);
   }
 
@@ -114,7 +113,7 @@ export default class TouchSensor extends Sensor {
    */
   [onTouchHold](event, container) {
     return () => {
-      if (this.touchMoved || !event.cancelable) {
+      if (this.touchMoved) {
         return;
       }
 
@@ -132,6 +131,7 @@ export default class TouchSensor extends Sensor {
       this.trigger(container, dragStartEvent);
 
       this.dragging = !dragStartEvent.canceled();
+      preventScrolling = this.dragging;
     };
   }
 
@@ -147,10 +147,6 @@ export default class TouchSensor extends Sensor {
       return;
     }
 
-    // Cancels scrolling while dragging
-    event.preventDefault();
-    event.stopPropagation();
-
     const touch = event.touches[0] || event.changedTouches[0];
     const target = document.elementFromPoint(touch.pageX - window.scrollX, touch.pageY - window.scrollY);
 
@@ -172,21 +168,16 @@ export default class TouchSensor extends Sensor {
    */
   [onTouchEnd](event) {
     this.touchMoved = false;
+    preventScrolling = false;
 
     document.removeEventListener('touchend', this[onTouchEnd]);
     document.removeEventListener('touchcancel', this[onTouchEnd]);
-    document.removeEventListener('touchmove', this[onTouchMove], {passive: false});
-
-    document.removeEventListener('scroll', this[onScroll]);
+    document.removeEventListener('touchmove', this[onTouchMove]);
 
     if (this.currentContainer) {
       this.currentContainer.removeEventListener('contextmenu', onContextMenu);
     }
 
-    if (this.currentScrollableParent) {
-      this.currentScrollableParent.removeEventListener('scroll', this[onScroll]);
-    }
-
     clearTimeout(this.tapTimeout);
 
     if (!this.dragging) {
@@ -211,14 +202,6 @@ export default class TouchSensor extends Sensor {
     this.currentContainer = null;
     this.dragging = false;
   }
-
-  /**
-   * Scroll handler, cancel potential drag and allow scroll on iOS or other touch devices
-   * @private
-   */
-  [onScroll]() {
-    clearTimeout(this.tapTimeout);
-  }
 }
 
 function onContextMenu(event) {

diff --git a/src/Draggable/Sensors/TouchSensor/tests/TouchSensor.test.js b/src/Draggable/Sensors/TouchSensor/tests/TouchSensor.test.js
@@ -103,25 +103,6 @@ describe('TouchSensor', () => {
     expect(dragFlow).not.toHaveTriggeredSensorEvent('drag:start');
   });
 
-  it('prevents `drag:start` when browser starts scrolling instead', () => {
-    function dragFlow() {
-      touchStart(draggableElement);
-      triggerEvent(document.body, 'scroll');
-      waitForDragDelay();
-    }
-
-    expect(dragFlow).not.toHaveTriggeredSensorEvent('drag:start');
-  });
-
-  it('prevents `drag:start` when touchstart event is not cancelable', () => {
-    function dragFlow() {
-      touchStart(draggableElement, {cancelable: false});
-      waitForDragDelay();
-    }
-
-    expect(dragFlow).not.toHaveTriggeredSensorEvent('drag:start');
-  });
-
   it('prevents `drag:start` when touch moved before drag delay', () => {
     function dragFlow() {
       touchStart(draggableElement);
@@ -153,16 +134,12 @@ describe('TouchSensor', () => {
 
     expect(touchMoveEvent.defaultPrevented).toBe(false);
 
-    expect(touchMoveEvent.stoppedPropagation).toBeUndefined();
-
     touchStart(draggableElement);
     waitForDragDelay();
     touchMoveEvent = touchMove(draggableElement);
 
     expect(touchMoveEvent.defaultPrevented).toBe(true);
 
-    expect(touchMoveEvent.stoppedPropagation).toBe(true);
-
     touchRelease(draggableElement);
   });