From bcd9524e9ccc06eeab6ee1e8c2bdc5cecf2c5b68 Mon Sep 17 00:00:00 2001 From: Jakub Piasecki Date: Thu, 16 May 2024 11:41:38 +0200 Subject: [PATCH] Fix buttons getting stuck after scrolling on them (#2693) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description I've noticed a weird interaction between Gesture Handler buttons and the ScrollView wrapped with `NativeViewGestureHandler` after https://github.com/software-mansion/react-native-gesture-handler/pull/2551 - after starting the scroll on a button, that button (and only that one) would become unresponsive for the next touch. The issue is apparent after touching the same button after the scroll ends, where it doesn't react to the first touch (see the before video). After some debugging, I've found that [cancelAndClearTouchTargets](https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/core/java/android/view/ViewGroup.java;l=2922?q=ViewGroup&ss=android%2Fplatform%2Fsuperproject%2Fmain) was invoked before delivering the event to the children of the `GestureHandlerRootView` when clicking on the same button. This, in a nutshell, was resulting in the following scenario: 1. User touches the button, `MotionEvent.ACTION_DOWN` is dispatched 2. RootView tries to deliver the event to handlers, Button receives the event and sets `lastAction` to `ACTION_DOWN` and `lastEventTime`. 3. No handler is active, RootView calls `super.dispatchTouchEvent` 4. `cancelAndClearTouchTargets` is invoked, `ACTION_CANCEL` event is dispatched to children of the RootView 5. Button receives the cancel event and processes it internally, touch is cancelled 6. Original `ACTION_DOWN` event is dispatched to children of RootView 7. Button receives the `ACTION_DOWN` event but it's the same event that it received before the synthetic cancel event - it's the same action and time as saved, so the event is ignored 8. `ACTION_UP` event is dispatched when finger is removed from the screen, Button ignores it since there is no press active This PR changes the logic in the Button component to also save the last action type and time for `CANCEL` events, so that the second time the `DOWN` event is delivered, it's not being ignored. This is effectively a revert of https://github.com/software-mansion/react-native-gesture-handler/pull/2586, which was fixing stuff that https://github.com/software-mansion/react-native-gesture-handler/pull/1601 broke in a wrong way. Hopefully, this one solves the issue once and for all 🤞. ### From the review > You could also consider adding a few words about ScrollView interactions into PR description 😅 When running the reproducer code below, you may notice that the scrolling will be cancelled by the buttons being pressed. This can be avoided by setting `disallowInterruption` on the native gesture wrapping the `ScrollView`, which is done by default on the `ScrollView` exported by RNGH. ## Test plan
Tested on the Example app (most notably nested buttons example), code from the issue #2585 and on the following code ```jsx import React from 'react'; import { StyleSheet, Text, ScrollView } from 'react-native'; import { GestureDetector, Gesture, RectButton, GestureHandlerRootView, } from 'react-native-gesture-handler'; export const DATA = new Array(100).fill(0).map((_, index) => `Item ${index}`); export default function App() { const scrollGesture = Gesture.Native(); return ( {DATA.map((genre, index) => ( { console.log('Pressed', genre); }}> {genre} ))} ); } export const styles = StyleSheet.create({ genreText: { fontSize: 18, fontWeight: '600', marginVertical: 12, }, selectableItem: { flexDirection: 'row', alignItems: 'center', paddingHorizontal: 16, paddingVertical: 8, }, horizontalMargin: { marginHorizontal: 16, }, }); ```
Before the change: https://github.com/software-mansion/react-native-gesture-handler/assets/21055725/48befc20-b115-445d-8e2f-315f2d32e247 After the change: https://github.com/software-mansion/react-native-gesture-handler/assets/21055725/ae49371d-0adc-4dfd-abcb-08de8a276a28 --- .../core/NativeViewGestureHandler.kt | 24 ++++++++++++------- .../RNGestureHandlerButtonViewManager.kt | 14 +++++++---- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/android/src/main/java/com/swmansion/gesturehandler/core/NativeViewGestureHandler.kt b/android/src/main/java/com/swmansion/gesturehandler/core/NativeViewGestureHandler.kt index 64f230a2c8..411e1b10cf 100644 --- a/android/src/main/java/com/swmansion/gesturehandler/core/NativeViewGestureHandler.kt +++ b/android/src/main/java/com/swmansion/gesturehandler/core/NativeViewGestureHandler.kt @@ -83,11 +83,21 @@ class NativeViewGestureHandler : GestureHandler() { override fun onHandle(event: MotionEvent, sourceEvent: MotionEvent) { val view = view!! if (event.actionMasked == MotionEvent.ACTION_UP) { - view.onTouchEvent(event) - if ((state == STATE_UNDETERMINED || state == STATE_BEGAN) && view.isPressed) { - activate() + if (state == STATE_UNDETERMINED && !hook.canBegin(event)) { + cancel() + } else { + view.onTouchEvent(event) + if ((state == STATE_UNDETERMINED || state == STATE_BEGAN) && view.isPressed) { + activate() + } + + if (state == STATE_UNDETERMINED) { + cancel() + } else { + end() + } } - end() + hook.afterGestureEnd(event) } else if (state == STATE_UNDETERMINED || state == STATE_BEGAN) { when { @@ -104,10 +114,8 @@ class NativeViewGestureHandler : GestureHandler() { hook.handleEventBeforeActivation(event) } state != STATE_BEGAN -> { - if (hook.canBegin()) { + if (hook.canBegin(event)) { begin() - } else { - cancel() } } } @@ -144,7 +152,7 @@ class NativeViewGestureHandler : GestureHandler() { * @return Boolean value signalling whether the handler can transition to the BEGAN state. If false * the gesture will be cancelled. */ - fun canBegin() = true + fun canBegin(event: MotionEvent) = true /** * Called after the gesture transitions to the END state. diff --git a/android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerButtonViewManager.kt b/android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerButtonViewManager.kt index a165ffd109..e47f115bfb 100644 --- a/android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerButtonViewManager.kt +++ b/android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerButtonViewManager.kt @@ -270,15 +270,15 @@ class RNGestureHandlerButtonViewManager : ViewGroupManager(), R * [com.swmansion.gesturehandler.NativeViewGestureHandler.onHandle] */ @SuppressLint("ClickableViewAccessibility") override fun onTouchEvent(event: MotionEvent): Boolean { + val eventTime = event.eventTime + val action = event.action + if (event.action == MotionEvent.ACTION_CANCEL) { tryFreeingResponder() - return super.onTouchEvent(event) } - val eventTime = event.eventTime - val action = event.action // always true when lastEventTime or lastAction have default value (-1) - if (lastEventTime != eventTime || lastAction != action) { + if (lastEventTime != eventTime || lastAction != action || action == MotionEvent.ACTION_CANCEL) { lastEventTime = eventTime lastAction = action return super.onTouchEvent(event) @@ -384,7 +384,11 @@ class RNGestureHandlerButtonViewManager : ViewGroupManager(), R } } - override fun canBegin(): Boolean { + override fun canBegin(event: MotionEvent): Boolean { + if (event.action == MotionEvent.ACTION_CANCEL || event.action == MotionEvent.ACTION_UP || event.actionMasked == MotionEvent.ACTION_POINTER_UP) { + return false + } + val isResponder = tryGrabbingResponder() if (isResponder) { isTouched = true