Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Pressables not being tappable in PanResponder Views on Android #29533

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 72 additions & 1 deletion RNTester/js/examples/Pressable/PressableExample.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ import {
Text,
Platform,
View,
PanResponder,
} from 'react-native';

const {useEffect, useRef, useState} = React;
const {useEffect, useRef, useState, useReducer, useMemo} = React;

const forceTouchAvailable =
(Platform.OS === 'ios' && Platform.constants.forceTouchAvailable) || false;
Expand Down Expand Up @@ -250,6 +251,67 @@ function PressableDisabled() {
);
}

function PressableInPanResponder() {
// We don't put messages in state since it might be updated multiple times
// before a render and older messages would get clobbered. Instead, we rely
// on forceUpdate.
const messages = useRef<string[]>([]);
const [, forceUpdate] = useReducer(x => x + 1, 0);

const appendMessage = (message: string) => {
const lastMessage =
messages.current.length && messages.current[messages.current.length - 1];
if (message === lastMessage) {
return; // Reduce duplicate touchMove messages
}
messages.current = [...messages.current, message];
forceUpdate();
};

const panResponder = useMemo(
() =>
PanResponder.create({
onStartShouldSetPanResponder: (): boolean => {
appendMessage('onStartShouldSetPanResponder, returning false');
return false;
},
onStartShouldSetPanResponderCapture: (): boolean => {
appendMessage('onStartShouldSetPanResponderCapture, returning false');
return false;
},
onMoveShouldSetPanResponder: (): boolean => {
appendMessage('onMoveShouldSetPanResponder, returning true');
return true;
},
onMoveShouldSetPanResponderCapture: (): boolean => {
appendMessage('onMoveShouldSetPanResponderCapture, returning false');
return false;
},
onPanResponderGrant: () => appendMessage('onPanResponderGrant'),
onPanResponderMove: () => appendMessage('onPanResponderMove'),
onPanResponderRelease: () => appendMessage('onPanResponderRelease'),
onPanResponderTerminate: () => appendMessage('onPanResponderTerminate'),
}),
[],
);

return (
<>
<View {...panResponder.panHandlers}>
<Pressable
onPress={() => appendMessage('Pressable onPress')}
onPressIn={() => appendMessage('Pressable onPressIn')}
style={[styles.row, styles.block]}>
<Text style={styles.button}>Pressable</Text>
</Pressable>
</View>
<View style={styles.logBox}>
<Text>{messages.current.join('\n')}</Text>
</View>
</>
);
}

const styles = StyleSheet.create({
row: {
justifyContent: 'center',
Expand Down Expand Up @@ -476,4 +538,13 @@ exports.examples = [
return <PressableDisabled />;
},
},
{
title: 'Pressable in PanResponder',
description: ('<Pressable> components should be pressable within ' +
'PanResponders that only handle touch moves. onPress should trigger' +
"if you press and don't move your finger.": string),
render: function(): React.Node {
return <PressableInPanResponder />;
},
},
];
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ public class JSTouchDispatcher {
private final ViewGroup mRootViewGroup;
private final TouchEventCoalescingKeyHelper mTouchEventCoalescingKeyHelper =
new TouchEventCoalescingKeyHelper();
private final float[] mLastTouchStartCoordinates = new float[2];
private boolean mTouchMoveStarted = false;

public JSTouchDispatcher(ViewGroup viewGroup) {
mRootViewGroup = viewGroup;
Expand Down Expand Up @@ -65,12 +67,7 @@ public void handleTouchEvent(MotionEvent ev, EventDispatcher eventDispatcher) {
ReactConstants.TAG, "Got DOWN touch before receiving UP or CANCEL from last gesture");
}

// First event for this gesture. We expect tag to be set to -1, and we use helper method
// {@link #findTargetTagForTouch} to find react view ID that will be responsible for handling
// this gesture
mChildIsHandlingNativeGesture = false;
mGestureStartTime = ev.getEventTime();
mTargetTag = findTargetTagAndSetCoordinates(ev);
this.markTouchStarted(ev);
eventDispatcher.dispatchEvent(
TouchEvent.obtain(
mTargetTag,
Expand Down Expand Up @@ -104,20 +101,23 @@ public void handleTouchEvent(MotionEvent ev, EventDispatcher eventDispatcher) {
mTargetCoordinates[0],
mTargetCoordinates[1],
mTouchEventCoalescingKeyHelper));
mTargetTag = -1;
mGestureStartTime = TouchEvent.UNSET;
this.markTouchEnded();
} else if (action == MotionEvent.ACTION_MOVE) {
// Update pointer position for current gesture
findTargetTagAndSetCoordinates(ev);
eventDispatcher.dispatchEvent(
// Only dispatch the event if we've already started a move or the move is at least some
// number of pixels away from the starting point
if (this.shouldDispatchTouchMoveEvent()) {
eventDispatcher.dispatchEvent(
TouchEvent.obtain(
mTargetTag,
TouchEventType.MOVE,
ev,
mGestureStartTime,
mTargetCoordinates[0],
mTargetCoordinates[1],
mTouchEventCoalescingKeyHelper));
mTargetTag,
TouchEventType.MOVE,
ev,
mGestureStartTime,
mTargetCoordinates[0],
mTargetCoordinates[1],
mTouchEventCoalescingKeyHelper));
}
} else if (action == MotionEvent.ACTION_POINTER_DOWN) {
// New pointer goes down, this can only happen after ACTION_DOWN is sent for the first pointer
eventDispatcher.dispatchEvent(
Expand Down Expand Up @@ -148,8 +148,7 @@ public void handleTouchEvent(MotionEvent ev, EventDispatcher eventDispatcher) {
ReactConstants.TAG,
"Received an ACTION_CANCEL touch event for which we have no corresponding ACTION_DOWN");
}
mTargetTag = -1;
mGestureStartTime = TouchEvent.UNSET;
this.markTouchEnded();
} else {
FLog.w(
ReactConstants.TAG,
Expand All @@ -163,6 +162,60 @@ private int findTargetTagAndSetCoordinates(MotionEvent ev) {
ev.getX(), ev.getY(), mRootViewGroup, mTargetCoordinates, null);
}

private void markTouchStarted(MotionEvent ev) {
// First event for this gesture. We expect tag to be set to -1, and we use helper method
// {@link #findTargetTagForTouch} to find react view ID that will be responsible for handling
// this gesture
mChildIsHandlingNativeGesture = false;
mGestureStartTime = ev.getEventTime();
mTargetTag = findTargetTagAndSetCoordinates(ev);

// Track the last touch start coordinates to prevent touch moves that are too close to the
// start coordinates from being emitted. See {@link #shouldDispatchTouchMoveEvent} for
// more details
mTouchMoveStarted = false;
mLastTouchStartCoordinates[0] = mTargetCoordinates[0];
mLastTouchStartCoordinates[1] = mTargetCoordinates[1];
}

private void markTouchEnded() {
mTargetTag = -1;
mGestureStartTime = TouchEvent.UNSET;
}

/**
* This function prevents `topTouchMove` events from being dispatched if the user hasn't moved
* their finger from the start of the last touch, to match the iOS behaviour.
*
* On Android, `MotionEvent.ACTION_MOVE` events are dispatched even if the user had moved
* their finger 0 pixels from the starting point. This is unlike on iOS, where move events
* are only dispatched after some distance.
*
* This causes a problem in the following case:
*
* - A parent component has a `PanResponder` that responds to all move events
* - The parent has a child `Pressable` (or `Touchable`)
*
* Because the move event is emitted even without any real move, the parent would take over
* immediately on press down and no presses would ever register on the child.
*/
private boolean shouldDispatchTouchMoveEvent() {
if (mTouchMoveStarted) {
return true;
}

// Logic based on https://github.com/facebook/react-native/blob/v0.63.2/Libraries/Pressability/Pressability.js#L516
// which recognizes only moves of >10 pixels as moves that should cancel a long press. This
// matches the behaviour on iOS and prevents move events that aren't really moves from
// canceling touches on child pressables.
mTouchMoveStarted = Math.hypot(
mLastTouchStartCoordinates[0] - mTargetCoordinates[0],
mLastTouchStartCoordinates[1] - mTargetCoordinates[1]
) > 10;

return mTouchMoveStarted;
}

private void dispatchCancelEvent(MotionEvent androidEvent, EventDispatcher eventDispatcher) {
// This means the gesture has already ended, via some other CANCEL or UP event. This is not
// expected to happen very often as it would mean some child View has decided to intercept the
Expand Down