fix(browser): block the thread when handling touch events

oliviertassinari · Feb 6, 2017 · 82bcc19 · 82bcc19
1 parent 6095112
commit 82bcc19
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 25 deletions.
diff --git a/packages/react-swipeable-views/src/SwipeableViews.js b/packages/react-swipeable-views/src/SwipeableViews.js
@@ -4,8 +4,18 @@ import React, { Component, PropTypes, Children } from 'react';
 import warning from 'warning';
 import transitionInfo from 'dom-helpers/transition/properties';
 import addEventListener from 'dom-helpers/events/on';
+import removeEventListener from 'dom-helpers/events/off';
 import { constant, checkIndexBounds, computeIndex, getDisplaySameSlide } from 'react-swipeable-views-core';
 
+function addEventListenerEnhanced(node, event, handler, options) {
+  addEventListener(node, event, handler, options);
+  return {
+    remove() {
+      removeEventListener(node, event, handler, options);
+    },
+  };
+}
+
 let styleInjected = false;
 
 // Support old version of iOS.
@@ -353,10 +363,17 @@ class SwipeableViews extends Component {
 
   componentDidMount() {
     // Subscribe to transition end events.
-    addEventListener(this.containerNode, transitionInfo.end, () => {
+    this.transitionListener = addEventListenerEnhanced(this.containerNode, transitionInfo.end, () => {
       this.handleTransitionEnd();
     });
 
+    // Block the thread to handle that event.
+    this.touchMoveListener = addEventListenerEnhanced(this.rootNode, 'touchmove',
+      this.handleTouchMove, {
+        passive: false,
+      },
+    );
+
     /* eslint-disable react/no-did-mount-set-state */
     this.setState({
       isFirstRender: false,
@@ -385,6 +402,11 @@ class SwipeableViews extends Component {
     }
   }
 
+  componentWillUnmount() {
+    this.transitionListener.remove();
+    this.touchMoveListener.remove();
+  }
+
   rootNode = null;
   containerNode = null;
   ignoreNextScrollEvents = false;
@@ -396,6 +418,8 @@ class SwipeableViews extends Component {
   isSwiping = undefined;
   started = false;
   startIndex = 0;
+  transitionListener = null;
+  touchMoveListener = null;
 
   handleTouchStart = (event) => {
     const {
@@ -439,10 +463,6 @@ class SwipeableViews extends Component {
   };
 
   handleTouchMove = (event) => {
-    if (this.props.onTouchMove) {
-      this.props.onTouchMove(event);
-    }
-
     // The touch start event can be cancel.
     // Makes sure we set a starting point.
     if (!this.started) {
@@ -692,7 +712,6 @@ class SwipeableViews extends Component {
 
     const touchEvents = disabled ? {} : {
       onTouchStart: this.handleTouchStart,
-      onTouchMove: this.handleTouchMove,
       onTouchEnd: this.handleTouchEnd,
     };
 

diff --git a/packages/react-swipeable-views/src/SwipeableViews.spec.js b/packages/react-swipeable-views/src/SwipeableViews.spec.js
@@ -7,6 +7,13 @@ import { assert } from 'chai';
 import { spy } from 'sinon';
 import SwipeableViews, { findNativeHandler, getDomTreeShapes } from './SwipeableViews';
 
+function simulateMouseMove(wrapper, event) {
+  return wrapper.instance().handleTouchMove({
+    preventDefault: () => {},
+    ...event,
+  });
+}
+
 describe('SwipeableViews', () => {
   describe('prop: children', () => {
     it('should render the children', () => {
@@ -43,7 +50,7 @@ describe('SwipeableViews', () => {
           pageY: 50,
         }],
       });
-      wrapper.simulate('touchMove', {
+      simulateMouseMove(wrapper, {
         touches: [{
           pageX: 150,
           pageY: 50,
@@ -57,7 +64,7 @@ describe('SwipeableViews', () => {
     it('should not change slide when swipe was not enough', () => {
       const wrapper = createWrapper();
 
-      wrapper.simulate('touchMove', {
+      simulateMouseMove(wrapper, {
         touches: [{
           pageX: 80,
           pageY: 50,
@@ -71,7 +78,7 @@ describe('SwipeableViews', () => {
     it('should change slide after long swipe', () => {
       const wrapper = createWrapper();
 
-      wrapper.simulate('touchMove', {
+      simulateMouseMove(wrapper, {
         touches: [{
           pageX: 20,
           pageY: 50,
@@ -86,7 +93,7 @@ describe('SwipeableViews', () => {
     it('should change slider hysteresis via prop', () => {
       const wrapper = createWrapper(0.3);
 
-      wrapper.simulate('touchMove', {
+      simulateMouseMove(wrapper, {
         touches: [{
           pageX: 80,
           pageY: 50,
@@ -199,7 +206,7 @@ describe('SwipeableViews', () => {
     });
 
     it('should not detect a swipe when scrolling', () => {
-      wrapper.simulate('touchMove', {
+      simulateMouseMove(wrapper, {
         touches: [{
           pageX: 50,
           pageY: 60,
@@ -209,39 +216,37 @@ describe('SwipeableViews', () => {
     });
 
     it('should detect a swipe when doing a clear movement', () => {
-      wrapper.simulate('touchMove', {
+      simulateMouseMove(wrapper, {
         touches: [{
           pageX: 60,
           pageY: 50,
         }],
-        preventDefault: () => {},
       });
       assert.strictEqual(instance.isSwiping, true, 'Should detect a swipe');
     });
 
     it('should wait for a clear movement to detect a swipe', () => {
-      wrapper.simulate('touchMove', {
+      simulateMouseMove(wrapper, {
         touches: [{
           pageX: 48,
           pageY: 50,
         }],
       });
       assert.strictEqual(instance.isSwiping, undefined, 'We do not know yet');
 
-      wrapper.simulate('touchMove', {
+      simulateMouseMove(wrapper, {
         touches: [{
           pageX: 50,
           pageY: 48,
         }],
       });
       assert.strictEqual(instance.isSwiping, undefined, 'We do not know yet');
 
-      wrapper.simulate('touchMove', {
+      simulateMouseMove(wrapper, {
         touches: [{
           pageX: 40,
           pageY: 50,
         }],
-        preventDefault: () => {},
       });
       assert.strictEqual(instance.isSwiping, true, 'Should detect a swipe');
     });
@@ -299,8 +304,8 @@ describe('SwipeableViews', () => {
         }],
       };
 
-      wrapperNester.simulate('touchMove', touchMoveEvent1);
-      wrapperParent.simulate('touchMove', touchMoveEvent1);
+      simulateMouseMove(wrapperNester, touchMoveEvent1);
+      simulateMouseMove(wrapperParent, touchMoveEvent1);
 
       const touchMoveEvent2 = {
         touches: [{
@@ -309,8 +314,8 @@ describe('SwipeableViews', () => {
         }],
       };
 
-      wrapperNester.simulate('touchMove', touchMoveEvent2);
-      wrapperParent.simulate('touchMove', touchMoveEvent2);
+      simulateMouseMove(wrapperNester, touchMoveEvent2);
+      simulateMouseMove(wrapperParent, touchMoveEvent2);
 
       assert.strictEqual(wrapperNester.state().indexCurrent, 0.025);
       assert.strictEqual(wrapperParent.state().indexCurrent, 1);
@@ -324,8 +329,8 @@ describe('SwipeableViews', () => {
         }],
       };
 
-      wrapperNester.simulate('touchMove', touchMoveEvent1);
-      wrapperParent.simulate('touchMove', touchMoveEvent1);
+      simulateMouseMove(wrapperNester, touchMoveEvent1);
+      simulateMouseMove(wrapperParent, touchMoveEvent1);
 
       const touchMoveEvent2 = {
         touches: [{
@@ -334,8 +339,8 @@ describe('SwipeableViews', () => {
         }],
       };
 
-      wrapperNester.simulate('touchMove', touchMoveEvent2);
-      wrapperParent.simulate('touchMove', touchMoveEvent2);
+      simulateMouseMove(wrapperNester, touchMoveEvent2);
+      simulateMouseMove(wrapperParent, touchMoveEvent2);
 
       assert.strictEqual(wrapperNester.state().indexCurrent, 0);
       assert.strictEqual(wrapperParent.state().indexCurrent, 0.975);