software-mansion / react-native-gesture-handler

Declarative API exposing platform native touch and gesture system to React Native.
https://docs.swmansion.com/react-native-gesture-handler/
MIT License
5.85k stars 954 forks source link

Nested touchables don't work correctly on Android #784

Closed brunolemos closed 2 years ago

brunolemos commented 4 years ago

Expected Behavior

Kapture 2019-10-05 at 16 34 43

Current Behavior

Kapture 2019-10-05 at 16 57 32

Description

When you have nested touchables from react-native-gesture-handler and press the inner touchable, the outer one also gets pressed.

Reproduction (Android): https://snack.expo.io/@brunolemos/rngh-nested-touchable-android-bug

brunolemos commented 4 years ago

Upgrading to react-native@0.61+ and to the latest version of react-native-gesture-handler fixed the issue.

brunolemos commented 4 years ago

Only iOS is working as expected, Android still has the wrong behavior. Both touchables get triggered. @kmagiera Any workaround for this?

I noticed that the onPress handler doesn't receive an event argument. If they did, I could call e.preventDefault() once and check e.isDefaultPrevented() second as a workaround.

https://github.com/kmagiera/react-native-gesture-handler/blob/533f8b042660d5d19eb54e4d99ea3b39a8a84339/touchables/GenericTouchable.js#L211

leteach commented 4 years ago

Any fix for this? I am experiencing it as well. Upgraded our react-native-gesture-handler version to 1.4.1 and still seeing the same problem. Everything works fine in iOS but Android is trigger both onPress events for the two separate touchable events.

leteach commented 4 years ago

I had to create a separate component for Android that imported the parent ToucableWithoutFeedback from react-native and the child TouchableOpacity from react-native-gesture-handler. That had to be conditionally rendered based one Platform.OS. It is hacky as hell but it works for now.

brunolemos commented 4 years ago

@leteach the parent onPress don't get triggered that way? Are you using Swipeable as well? I think I can't use ToucableWithoutFeedback from react-native on a Swipeable because that would cause issues when swiping (unwaned press events).

leteach commented 4 years ago

@brunolemos The issue occurs regardless of whether the parent is a TouchableOpacity or TouchableWithoutFeedback. It is happening specifically inside a component that utilizes a library called react-native-snap-carousel, which I suspect has a Swipeable in it.

And no, with this work around it works in both iOS and Android. The parent event is not triggered at the same time. This is not a solution I want to keep long term but for now it works.

brunolemos commented 4 years ago

The issue happens with any nested Touchables if they are imported from rngh. And only happens on Android (works on iOS and Web). Updated the title and description.

It's not directly related to Swipeable but because of it I can't use a touchable from react-native as a workaround, otherwise it gets triggered on swipe.

@kmagiera here's a small repro: https://snack.expo.io/@brunolemos/rngh-nested-touchable-android-bug

brunolemos commented 4 years ago

486 fixes this bug 🎉

Here's the patch that can be applied using patch-package:

patches/react-native-gesture-handler+1.5.2.patch ```patch diff --git a/node_modules/react-native-gesture-handler/android/build/generated/source/buildConfig/androidTest/debug/com/swmansion/gesturehandler/react/test/BuildConfig.java b/node_modules/react-native-gesture-handler/android/build/generated/source/buildConfig/androidTest/debug/com/swmansion/gesturehandler/react/test/BuildConfig.java new file mode 100644 index 0000000..dc64a01 --- /dev/null +++ b/node_modules/react-native-gesture-handler/android/build/generated/source/buildConfig/androidTest/debug/com/swmansion/gesturehandler/react/test/BuildConfig.java @@ -0,0 +1,13 @@ +/** + * Automatically generated file. DO NOT MODIFY + */ +package com.swmansion.gesturehandler.react.test; + +public final class BuildConfig { + public static final boolean DEBUG = Boolean.parseBoolean("true"); + public static final String APPLICATION_ID = "com.swmansion.gesturehandler.react.test"; + public static final String BUILD_TYPE = "debug"; + public static final String FLAVOR = ""; + public static final int VERSION_CODE = 1; + public static final String VERSION_NAME = "1.0"; +} diff --git a/node_modules/react-native-gesture-handler/android/build/generated/source/buildConfig/debug/com/swmansion/gesturehandler/react/BuildConfig.java b/node_modules/react-native-gesture-handler/android/build/generated/source/buildConfig/debug/com/swmansion/gesturehandler/react/BuildConfig.java new file mode 100644 index 0000000..bd6dac7 --- /dev/null +++ b/node_modules/react-native-gesture-handler/android/build/generated/source/buildConfig/debug/com/swmansion/gesturehandler/react/BuildConfig.java @@ -0,0 +1,18 @@ +/** + * Automatically generated file. DO NOT MODIFY + */ +package com.swmansion.gesturehandler.react; + +public final class BuildConfig { + public static final boolean DEBUG = Boolean.parseBoolean("true"); + public static final String LIBRARY_PACKAGE_NAME = "com.swmansion.gesturehandler.react"; + /** + * @deprecated APPLICATION_ID is misleading in libraries. For the library package name use LIBRARY_PACKAGE_NAME + */ + @Deprecated + public static final String APPLICATION_ID = "com.swmansion.gesturehandler.react"; + public static final String BUILD_TYPE = "debug"; + public static final String FLAVOR = ""; + public static final int VERSION_CODE = 1; + public static final String VERSION_NAME = "1.0"; +} diff --git a/node_modules/react-native-gesture-handler/android/lib/src/main/java/com/swmansion/gesturehandler/NativeViewGestureHandler.java b/node_modules/react-native-gesture-handler/android/lib/src/main/java/com/swmansion/gesturehandler/NativeViewGestureHandler.java index ca69016..0a38f5b 100644 --- a/node_modules/react-native-gesture-handler/android/lib/src/main/java/com/swmansion/gesturehandler/NativeViewGestureHandler.java +++ b/node_modules/react-native-gesture-handler/android/lib/src/main/java/com/swmansion/gesturehandler/NativeViewGestureHandler.java @@ -5,6 +5,8 @@ import android.view.MotionEvent; import android.view.View; import android.view.ViewGroup; +import com.swmansion.gesturehandler.react.RNGestureHandlerButtonViewManager; + public class NativeViewGestureHandler extends GestureHandler { private boolean mShouldActivateOnStart; @@ -34,6 +36,18 @@ public class NativeViewGestureHandler extends GestureHandler { - static class ButtonViewGroup extends ViewGroup { + public static class ButtonViewGroup extends ViewGroup { static TypedValue sResolveOutValue = new TypedValue(); static ButtonViewGroup sResponder; @@ -32,6 +32,7 @@ public class RNGestureHandlerButtonViewManager extends boolean mUseForeground = false; boolean mUseBorderless = false; float mBorderRadius = 0; + private boolean mExclusive = true; boolean mNeedBackgroundUpdate = false; @@ -50,6 +51,10 @@ public class RNGestureHandlerButtonViewManager extends mNeedBackgroundUpdate = true; } + public void setExclusive(boolean exclusive) { + mExclusive = exclusive; + } + public void setRippleColor(Integer color) { mRippleColor = color; mNeedBackgroundUpdate = true; @@ -167,14 +172,20 @@ public class RNGestureHandlerButtonViewManager extends } } + public boolean setResponder() { + if (sResponder == null) { + if (mExclusive) { + sResponder = this; + } + return true; + } + return false; + } + @Override public void setPressed(boolean pressed) { - if (pressed && sResponder == null) { - // first button to be pressed grabs button responder - sResponder = this; - } - if (!pressed || sResponder == this) { - // we set pressed state only for current responder + if (!pressed || sResponder == this || (sResponder == null && !mExclusive)) { + // we set pressed state only for current responder if exclusive super.setPressed(pressed); } if (!pressed && sResponder == this) { @@ -225,6 +236,11 @@ public class RNGestureHandlerButtonViewManager extends view.setRippleColor(rippleColor); } + @ReactProp(name = "exclusive", defaultBoolean = true) + public void setExclusive(ButtonViewGroup view, boolean exclusive) { + view.setExclusive(exclusive); + } + @Override protected void onAfterUpdateTransaction(ButtonViewGroup view) { view.updateBackground(); ```
TuanLinhDang commented 4 years ago

@brunolemos Can you please show me step to fix it bro? Im still struggling :)

brunolemos commented 4 years ago

@TuanLinhDang you need to follow the instructions at the patch-package readme, which is basically installing yarn add patch-package postinstall-postinstall and adding "postinstall": "patch-package" to the package.json at scripts. Then, create a patches folder at your project root and a file named react-native-gesture-handler+1.5.2.patch on it with the contents I pasted above. Then run yarn so the patch gets applied.

micheleb commented 4 years ago

@brunolemos thanks for the patch! I noticed that there are a few changes that are not necessary in order for the patch to work, and they point to paths to your local filesystem anyway, so here's the patch after removing those:

patches/react-native-gesture-handler+1.5.2.patch

```diff diff --git a/node_modules/react-native-gesture-handler/android/build/generated/source/buildConfig/androidTest/debug/com/swmansion/gesturehandler/react/test/BuildConfig.java b/node_modules/react-native-gesture-handler/android/build/generated/source/buildConfig/androidTest/debug/com/swmansion/gesturehandler/react/test/BuildConfig.java new file mode 100644 index 0000000..dc64a01 --- /dev/null +++ b/node_modules/react-native-gesture-handler/android/build/generated/source/buildConfig/androidTest/debug/com/swmansion/gesturehandler/react/test/BuildConfig.java @@ -0,0 +1,13 @@ +/** + * Automatically generated file. DO NOT MODIFY + */ +package com.swmansion.gesturehandler.react.test; + +public final class BuildConfig { + public static final boolean DEBUG = Boolean.parseBoolean("true"); + public static final String APPLICATION_ID = "com.swmansion.gesturehandler.react.test"; + public static final String BUILD_TYPE = "debug"; + public static final String FLAVOR = ""; + public static final int VERSION_CODE = 1; + public static final String VERSION_NAME = "1.0"; +} diff --git a/node_modules/react-native-gesture-handler/android/build/generated/source/buildConfig/debug/com/swmansion/gesturehandler/react/BuildConfig.java b/node_modules/react-native-gesture-handler/android/build/generated/source/buildConfig/debug/com/swmansion/gesturehandler/react/BuildConfig.java new file mode 100644 index 0000000..bd6dac7 --- /dev/null +++ b/node_modules/react-native-gesture-handler/android/build/generated/source/buildConfig/debug/com/swmansion/gesturehandler/react/BuildConfig.java @@ -0,0 +1,18 @@ +/** + * Automatically generated file. DO NOT MODIFY + */ +package com.swmansion.gesturehandler.react; + +public final class BuildConfig { + public static final boolean DEBUG = Boolean.parseBoolean("true"); + public static final String LIBRARY_PACKAGE_NAME = "com.swmansion.gesturehandler.react"; + /** + * @deprecated APPLICATION_ID is misleading in libraries. For the library package name use LIBRARY_PACKAGE_NAME + */ + @Deprecated + public static final String APPLICATION_ID = "com.swmansion.gesturehandler.react"; + public static final String BUILD_TYPE = "debug"; + public static final String FLAVOR = ""; + public static final int VERSION_CODE = 1; + public static final String VERSION_NAME = "1.0"; +} diff --git a/node_modules/react-native-gesture-handler/android/lib/src/main/java/com/swmansion/gesturehandler/NativeViewGestureHandler.java b/node_modules/react-native-gesture-handler/android/lib/src/main/java/com/swmansion/gesturehandler/NativeViewGestureHandler.java index ca69016..0a38f5b 100644 --- a/node_modules/react-native-gesture-handler/android/lib/src/main/java/com/swmansion/gesturehandler/NativeViewGestureHandler.java +++ b/node_modules/react-native-gesture-handler/android/lib/src/main/java/com/swmansion/gesturehandler/NativeViewGestureHandler.java @@ -5,6 +5,8 @@ import android.view.MotionEvent; import android.view.View; import android.view.ViewGroup; +import com.swmansion.gesturehandler.react.RNGestureHandlerButtonViewManager; + public class NativeViewGestureHandler extends GestureHandler { private boolean mShouldActivateOnStart; @@ -34,6 +36,18 @@ public class NativeViewGestureHandler extends GestureHandler { - static class ButtonViewGroup extends ViewGroup { + public static class ButtonViewGroup extends ViewGroup { static TypedValue sResolveOutValue = new TypedValue(); static ButtonViewGroup sResponder; @@ -32,6 +32,7 @@ public class RNGestureHandlerButtonViewManager extends boolean mUseForeground = false; boolean mUseBorderless = false; float mBorderRadius = 0; + private boolean mExclusive = true; boolean mNeedBackgroundUpdate = false; @@ -50,6 +51,10 @@ public class RNGestureHandlerButtonViewManager extends mNeedBackgroundUpdate = true; } + public void setExclusive(boolean exclusive) { + mExclusive = exclusive; + } + public void setRippleColor(Integer color) { mRippleColor = color; mNeedBackgroundUpdate = true; @@ -167,14 +172,20 @@ public class RNGestureHandlerButtonViewManager extends } } + public boolean setResponder() { + if (sResponder == null) { + if (mExclusive) { + sResponder = this; + } + return true; + } + return false; + } + @Override public void setPressed(boolean pressed) { - if (pressed && sResponder == null) { - // first button to be pressed grabs button responder - sResponder = this; - } - if (!pressed || sResponder == this) { - // we set pressed state only for current responder + if (!pressed || sResponder == this || (sResponder == null && !mExclusive)) { + // we set pressed state only for current responder if exclusive super.setPressed(pressed); } if (!pressed && sResponder == this) { @@ -225,6 +236,11 @@ public class RNGestureHandlerButtonViewManager extends view.setRippleColor(rippleColor); } + @ReactProp(name = "exclusive", defaultBoolean = true) + public void setExclusive(ButtonViewGroup view, boolean exclusive) { + view.setExclusive(exclusive); + } + @Override protected void onAfterUpdateTransaction(ButtonViewGroup view) { view.updateBackground(); ```

Also, I noticed that if I click on a nested touchable that does something that doesn't require a re-render (e.g., if a button simply shows a Toast on android), the next time I click on it it will still trigger the outer touchable. Maybe the state does not get reset after the click?

Not a big issue anyway, as usually clicks result in a re-render anyway, but just FYI.

Thanks again!

brunolemos commented 4 years ago

there are a few changes that are not necessary in order for the patch to work, and they point to paths to your local filesystem anyway,

oh didn't notice, it's auto generated, updated!

the next time I click on it it will still trigger the outer touchable. Maybe the state does not get reset after the click?

I didn't verify this but please comment it on #486 so maybe the author can fix this before merging

FRizzonelli commented 4 years ago

@brunolemos Thanks for sharing the patch, life saver <3 @kmagiera Do you think it's possible to merge it in master and release on NPM? I can confirm that without the patch, it's not working on Android. (Tested on version 61.5 and 1.5.2, both latest)

ALexandreM75013 commented 4 years ago

I am also encountering the same problem.

Did the patch has been pushed on master and available on NPM ?

lucastonon commented 4 years ago

I am still having this problem on 1.6.1

msvargas commented 4 years ago

+1 same problem

msvargas commented 4 years ago

I update patch to working with react-native-gesture-handler 1.6.1

diff --git a/node_modules/react-native-gesture-handler/android/lib/src/main/java/com/swmansion/gesturehandler/NativeViewGestureHandler.java b/node_modules/react-native-gesture-handler/android/lib/src/main/java/com/swmansion/gesturehandler/NativeViewGestureHandler.java
index ca69016..fdfa662 100644
--- a/node_modules/react-native-gesture-handler/android/lib/src/main/java/com/swmansion/gesturehandler/NativeViewGestureHandler.java
+++ b/node_modules/react-native-gesture-handler/android/lib/src/main/java/com/swmansion/gesturehandler/NativeViewGestureHandler.java
@@ -4,6 +4,7 @@ import android.os.SystemClock;
 import android.view.MotionEvent;
 import android.view.View;
 import android.view.ViewGroup;
+import com.swmansion.gesturehandler.react.RNGestureHandlerButtonViewManager;

 public class NativeViewGestureHandler extends GestureHandler<NativeViewGestureHandler> {

@@ -13,7 +14,17 @@ public class NativeViewGestureHandler extends GestureHandler<NativeViewGestureHa
   public NativeViewGestureHandler() {
     setShouldCancelWhenOutside(true);
   }
-
+  private boolean isPreventedFromBeginning() {
+    // RNGestureHandlerButtonViewManager is connected with logic
+    // related to handling exclusive touches which should prevent
+    // another buttons from beginning gesture recognition.
+    View view = getView();
+    if (view instanceof RNGestureHandlerButtonViewManager.ButtonViewGroup) {
+      // setting flag for an exclusive touch for buttons
+      return !((RNGestureHandlerButtonViewManager.ButtonViewGroup) view).setResponder();
+    }
+    return false;
+  }
   public NativeViewGestureHandler setShouldActivateOnStart(boolean shouldActivateOnStart) {
     mShouldActivateOnStart = shouldActivateOnStart;
     return this;
@@ -85,7 +96,7 @@ public class NativeViewGestureHandler extends GestureHandler<NativeViewGestureHa
       } else if (tryIntercept(view, event)) {
         view.onTouchEvent(event);
         activate();
-      } else if (state != STATE_BEGAN) {
+      } else if (state != STATE_BEGAN && !isPreventedFromBeginning()) {
         begin();
       }
     } else if (state == STATE_ACTIVE) {
diff --git a/node_modules/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerButtonViewManager.java b/node_modules/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerButtonViewManager.java
index fd625bd..250f1c3 100644
--- a/node_modules/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerButtonViewManager.java
+++ b/node_modules/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerButtonViewManager.java
@@ -21,11 +21,11 @@ import com.facebook.react.uimanager.annotations.ReactProp;
 public class RNGestureHandlerButtonViewManager extends
         ViewGroupManager<RNGestureHandlerButtonViewManager.ButtonViewGroup> {

-  static class ButtonViewGroup extends ViewGroup {
+  public static class ButtonViewGroup extends ViewGroup {

     static TypedValue sResolveOutValue = new TypedValue();
     static ButtonViewGroup sResponder;
-
+    private boolean mExclusive = true;
     int mBackgroundColor = Color.TRANSPARENT;
     // Using object because of handling null representing no value set.
     Integer mRippleColor;
@@ -44,6 +44,10 @@ public class RNGestureHandlerButtonViewManager extends
       mNeedBackgroundUpdate = true;
     }

+    public void setExclusive(boolean exclusive) {
+      mExclusive = exclusive;
+    }
+
     @Override
     public void setBackgroundColor(int color) {
       mBackgroundColor = color;
@@ -55,6 +59,16 @@ public class RNGestureHandlerButtonViewManager extends
       mNeedBackgroundUpdate = true;
     }

+    public boolean setResponder() {
+      if (sResponder == null) {
+        if (mExclusive) {
+          sResponder = this;
+        }
+        return true;
+      }
+      return false;
+    }
+
     public void setBorderRadius(float borderRadius) {
       mBorderRadius = borderRadius * (float)getResources().getDisplayMetrics().density;
       mNeedBackgroundUpdate = true;
@@ -173,7 +187,7 @@ public class RNGestureHandlerButtonViewManager extends
         // first button to be pressed grabs button responder
         sResponder = this;
       }
-      if (!pressed || sResponder == this) {
+      if (!pressed || sResponder == this || (sResponder == null && !mExclusive)) {
         // we set pressed state only for current responder
         super.setPressed(pressed);
       }
@@ -225,6 +239,11 @@ public class RNGestureHandlerButtonViewManager extends
     view.setRippleColor(rippleColor);
   }

+  @ReactProp(name = "exclusive", defaultBoolean = true)
+  public void setExclusive(ButtonViewGroup view, boolean exclusive) {
+    view.setExclusive(exclusive);
+  }
+
   @Override
   protected void onAfterUpdateTransaction(ButtonViewGroup view) {
     view.updateBackground();
akug17 commented 3 years ago

I tried TouchableOpacity from 'react-native' lib it works fine

PatricioBedregal commented 3 years ago

I think this issue also affects if you try to use createNativeStackNavigator from react-native-screens, because when I set that stack navigator all my Touchables from react-native-gesture-handler stops working

matt-dalton commented 3 years ago

@punisher97 Do you happen to know if the equivalent patch will work with 1.7.0? I just tried upgrading and added the same changes, and unfortunately it seems like presses aren't being registered. I can't really work out why as it doesn't look like this code has changed

pmutshipayi commented 3 years ago

I tried TouchableOpacity from 'react-native' lib it works fine

Thank you, works for me too

lightrow commented 3 years ago

1.8.0 same problem

huyhai commented 3 years ago

same

yuichi-saito-howtv commented 3 years ago

clicking on a nested View clicks on the View below

Sorry for referring to the above issue. The problem has not been resolved, but I will end it. I would appreciate it if you could ignore it.

cokepizza commented 3 years ago

same issue 1.9.0 in android touch events bubble inside multiple overlapping touch areas

a-eid commented 3 years ago

still an issue in ^1.9.0.

zachgibson commented 3 years ago

I switched to using the Button components which have basically all the feedback styles you'd want but more importantly they work when nested.

yfunk commented 3 years ago

I switched to using the Button components which have basically all the feedback styles you'd want but more importantly they work when nested.

That's actually pretty interesting, since from what I can tell the all the Touchables Gesture Handler exports are effectively just wrappers around BaseButton. Maybe the issue that causes the parents of nested Touchables to also be pressed is somewhere in the implementation of GenericTouchable.js?

dandre-hound commented 3 years ago

Yeah I've been experiencing this same issue in the latest 1.9.0, it's pretty frustrating and I haven't found a solution yet. I thought this is what waitFor was used for...?

florian-milky commented 3 years ago

I switched to using the Button components which have basically all the feedback styles you'd want but more importantly they work when nested.

I can confirm that using a BaseButton instead of a TouchableOpacity in the outer component works as expected!

congdoKnovva commented 3 years ago

for Android, import the outer component from 'react-native' import { TouchableOpacity as NativeTouchableOpacity, Platform } from 'react-native'; import { TouchableOpacity as GHTouchableOpacity } from 'react-native-gesture-handler' const TouchableOpacity = Platform.OS === 'ios' ? GHTouchableOpacity : NativeTouchableOpacity;

tankers746 commented 3 years ago

Passing disallowInterruption={true} to the touchable should fix this.

<TouchableOpacity {...props} disallowInterruption={true} />,

Should this be enabled by default in GenericTouchable?

yfunk commented 3 years ago

@tankers746 This is a nice solution for when you are only using onPress (just like switching to Button). Unfortunately it doesn't work for any of the other events (onPressIn, onPressOut and onLongPress), so I don't think it should be enabled by default.

jotahws commented 3 years ago

Passing disallowInterruption={true} to the touchable should fix this.

<TouchableOpacity {...props} disallowInterruption={true} />,

Should this be enabled by default in GenericTouchable?

Thanks @tankers746 that works like a charm.

This solution also work for TouchableWithoutFeedback. The prop may be put just on the nested Component that it will work anyway

ezhkov commented 3 years ago

Hello. Any news about this? I have another issue: tap on Switch inside TouchableOpacity not working. https://snack.expo.io/@ezhkov/rngh-nested-touchable-android-bug

Created an issue #1428

aksejs commented 2 years ago

Passing disallowInterruption={true} to the touchable should fix this.

<TouchableOpacity {...props} disallowInterruption={true} />,

Should this be enabled by default in GenericTouchable?

Still doesn't work in topic starter snack example, without alert: https://snack.expo.io/xRdctOC8r (added disallowInterruption prop and console.log)

is there any actual solutions at this moment for this issue?

Maybe there is new patch for latest version?

janicduplessis commented 2 years ago

Seems like the issue happens on web too. I made a snack repro that shows the different behaviour with Touchable from react-native and react-native-gesture-handler. https://snack.expo.dev/JEnwHDo1h