Skip to content

Commit

Permalink
Fix buttons getting stuck after scrolling on them (#2693)
Browse files Browse the repository at this point in the history
## Description

I've noticed a weird interaction between Gesture Handler buttons and the
ScrollView wrapped with `NativeViewGestureHandler` after
#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
#2586,
which was fixing stuff that
#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

<details>
<summary>Tested on the Example app (most notably nested buttons
example), code from the issue #2585 and on the following code</summary>

```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 (
    <GestureDetector gesture={scrollGesture}>
      <ScrollView>
        <GestureHandlerRootView>
          {DATA.map((genre, index) => (
            <RectButton
              key={index}
              style={styles.selectableItem}
              onPress={() => {
                console.log('Pressed', genre);
              }}>
              <Text style={[styles.genreText, styles.horizontalMargin]}>
                {genre}
              </Text>
            </RectButton>
          ))}
        </GestureHandlerRootView>
      </ScrollView>
    </GestureDetector>
  );
}

export const styles = StyleSheet.create({
  genreText: {
    fontSize: 18,
    fontWeight: '600',
    marginVertical: 12,
  },
  selectableItem: {
    flexDirection: 'row',
    alignItems: 'center',
    paddingHorizontal: 16,
    paddingVertical: 8,
  },
  horizontalMargin: {
    marginHorizontal: 16,
  },
});
```

</details>

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
  • Loading branch information
j-piasecki authored May 16, 2024
1 parent cbaf6b0 commit bcd9524
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,21 @@ class NativeViewGestureHandler : GestureHandler<NativeViewGestureHandler>() {
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 {
Expand All @@ -104,10 +114,8 @@ class NativeViewGestureHandler : GestureHandler<NativeViewGestureHandler>() {
hook.handleEventBeforeActivation(event)
}
state != STATE_BEGAN -> {
if (hook.canBegin()) {
if (hook.canBegin(event)) {
begin()
} else {
cancel()
}
}
}
Expand Down Expand Up @@ -144,7 +152,7 @@ class NativeViewGestureHandler : GestureHandler<NativeViewGestureHandler>() {
* @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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,15 +270,15 @@ class RNGestureHandlerButtonViewManager : ViewGroupManager<ButtonViewGroup>(), 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)
Expand Down Expand Up @@ -384,7 +384,11 @@ class RNGestureHandlerButtonViewManager : ViewGroupManager<ButtonViewGroup>(), 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
Expand Down

0 comments on commit bcd9524

Please sign in to comment.