Permalink
Browse files

Handle touchCancel properly in ScrollResponder

Summary:
Touch cancel events are currently being ignored by the ScrollView component. Currently scrollview responds both to scroll events and touchStart/touchMove/touchEnd events.

The reason why ScrollView listens to touchStart/touchEnd is so that it can update its `state.isTouching` param. This parameter then is used in `scrollResponderHandleScrollShouldSetResponder` to make the decision if scrollview should set the responder or not. So if `isTouching` is true (we've received touchStart) then ScrollView want to became a JS responder. This in turn is important for the case where we receive scroll events that does not necessarily need to trigger responder change, e.g. we don't want Scrollview to become JS responder if scroll events have been triggered by `scrollTo` in which case setting responder would put the whole responder system in a bogus state (note that responder can be released only by touchEnd or touchCancel, so if there is no touchEnd that follows scroll event then ScrollView will remain the responder and this would break next touch interaction).

It is therefore crucial for the ScrollView to reset `isTouching` state when touchCancel arrives, as otherwise the next scroll event would incorrectly trigger responder change.

On top of that ScrollView seems to be the only component in RN's core that registers to handle touchEnd but ignores touchCancel, which stands agains the comment added to `RCTRootView.cancelTouches` [here](c14cc12#diff-9cd70243bd2af75c613e29972bb1b41cR127).

This problem is difficult to test with a pure RN native app, as on Android it does not surface because of the `responderIgnoreScroll` flag that is being added to every scroll event, and it essentially makes the responder system ignore scroll events so they would never trigger responder change. On the other hand on iOS the cancel events are pretty rare. With pure RN app they can only be triggered by a "system" level interaction (e.g. when system alert dialog appears or when home button is clicked and there is a touch interaction happening). This issue becomes more prominent when RN app is embedded in a more sophisticated application that may use [`RCTRootView.cancelTouches`](https://github.com/facebook/react-native/blob/1e8f3b11027fe0a7514b4fc97d0798d3c64bc895/React/Base/RCTRootView.h#L130) method to block RNs gesture recognizers in some cases or with third-party libraries that deals with touch events like [react-native-gesture-handler](https://github.com/kmagiera/react-native-gesture-handler) that also calls into the method when native touch interaction is detected.
Closes #16004

Differential Revision: D6003063

Pulled By: shergin

fbshipit-source-id: f6495ffc57a5f996117b5bd80478bb1a58d2d799
  • Loading branch information...
kmagiera authored and facebook-github-bot committed Oct 19, 2017
1 parent 40a9083 commit bae9b2b2067ea096f9228d0c49d5c9dce72fc8c3
Showing with 11 additions and 0 deletions.
  1. +10 −0 Libraries/Components/ScrollResponder.js
  2. +1 −0 Libraries/Components/ScrollView/ScrollView.js
@@ -260,6 +260,16 @@ var ScrollResponderMixin = {
this.props.onTouchEnd && this.props.onTouchEnd(e);
},
/**
* Invoke this from an `onTouchCancel` event.
*
* @param {SyntheticEvent} e Event.
*/
scrollResponderHandleTouchCancel: function(e: Event) {
this.state.isTouching = false;
this.props.onTouchCancel && this.props.onTouchCancel(e);
},
/**
* Invoke this from an `onResponderRelease` event.
*/
@@ -803,6 +803,7 @@ const ScrollView = createReactClass({
onTouchEnd: this.scrollResponderHandleTouchEnd,
onTouchMove: this.scrollResponderHandleTouchMove,
onTouchStart: this.scrollResponderHandleTouchStart,
onTouchCancel: this.scrollResponderHandleTouchCancel,
scrollEventThrottle: hasStickyHeaders ? 1 : this.props.scrollEventThrottle,
sendMomentumEvents: (this.props.onMomentumScrollBegin || this.props.onMomentumScrollEnd) ?
true : false,

0 comments on commit bae9b2b

Please sign in to comment.