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
6.11k stars 980 forks source link

It's hard to understand how to compose gestures with ScrollView #2616

Open gaearon opened 1 year ago

gaearon commented 1 year ago

Description

I've been scratching my head at this for the last few days so I figured I'd report.

Basically, my problem looks like https://github.com/software-mansion/react-native-gesture-handler/issues/1082. I have a horizontal ScrollView, and it "steals" horizontal pinches inside of it. The Pinch gesture is inside individual items, but for some reason the outer ScrollView's scroll wins even though I'm definitely pinching (with two fingers).

I'm at a loss about how to debug this or even think about this.

There's something that maybe looks like a fix in https://github.com/software-mansion/react-native-gesture-handler/pull/1034, but I'll join the chorus of people who didn't understand what the fix is and how to use it. The comment says to use simultaneousHandlers. But isn't that the opposite of what the OP wanted? The OP wanted to have the pinch gesture "win" and prevent the scrolling. The documentation for simultaneousHandlers seems to imply it's for allowing two gestures to be active at the same time β€” which is not what the OP wanted. So it's unclear how that fix is related to the issue.

There's also https://github.com/software-mansion/react-native-gesture-handler/pull/2370 which seems related but was not reviewed. The problem in the video there seems to be exactly the same as what I'm experiencing (except it's vertical rather than horizontal). I have no idea if that fix makes sense though.

There are some scattered examples in the Issues of using Gesture.Native(), waitFor, simultaneousWithExternalGesture, ScrollView (undocumented?), and so on. For example, https://github.com/software-mansion/react-native-gesture-handler/issues/2332#issuecomment-1342733651. I found it very difficult to understand how these APIs are supposed to be used based on the docs. Do I want the built-in RN scroll view to "wait for" my pinch gesture inside it? Shouldn't the innermost gesture always win anyway? If I have many items in the list, do I need a ref to a gesture inside every item, and then somehow coordinate those refs with the ScrollView above? Do I need to be using ScrollView from react-native-gesture-handler for gestures to work at all? If I search for waitFor in the docs, I seem to only be getting results with outdated APIs (like this) which makes it further confusing.

Anyway, tldr is:

I hope this can be improved somehow! I'll try to see if I can find some way that works.

Steps to reproduce

irrelevant

Snack or a link to a repository

irrelevant

Gesture Handler version

2.12.1

React Native version

0.72.4

Platforms

Android

JavaScript runtime

Hermes

Workflow

None

Architecture

None

Build type

None

Device

None

Device model

No response

Acknowledgements

Yes

github-actions[bot] commented 1 year ago

Hey! πŸ‘‹

The issue doesn't seem to contain a minimal reproduction.

Could you provide a snack or a link to a GitHub repository under your username that reproduces the problem?

gaearon commented 1 year ago

Here's the flow:

I try search again:

Screenshot 2023-10-03 at 03 57 39

This "replacing waitFor" link seems promising (although it implies that maybe I'm not supposed to use waitFor API?) However, that link is 404.

This page has some mentions of waitFor and simultaneousHandlers but they're clearly written for someone who already knows how to use them. Searching for these APIs also brings me back to old API pages.

gaearon commented 1 year ago

I'm not sure I understand the intended design for waitFor or requireExternalGestureToFail when there's an unknown number of items. These APIs expect to receive refs, but I can't have a dynamic number of useRef calls in React β€” and I don't know how many child items I'll have in the scroll view. For now I'm planning to eagerly preallocate an array of items.length size with createRef values inside, keep it in state, and extend this array during rendering if there are ever more items. But it seems weird that I have no way to clean it up. Normally in React we'd use callback refs for this, but .withRef() doesn't seem to support callback refs.

gaearon commented 1 year ago

My current workaround looks like this: https://github.com/bluesky-social/social-app/pull/1563/commits/115d60d616e8fc044ccd8b63693edeea818de8f9. This makes it so that I can pinch into individual images in the gallery even though there's a scroll view around them.

This still doesn't work like I want though.

The behavior I want is:

gaearon commented 1 year ago

I've seen Gestures.Native supposedly being related, but I don't understand how to use it from documentation at all. The documentation says it lets other views "participate" in the gesture system but it's unclear if this would work for composing my behaviors with Android's scroll view default behavior. Does it let me disable and enable Android's scroll view?

I tried wrapping it around my VirtualizedList's renderScrollComponent implementation like:

const native = Gestures.Native()
  .enabled(!isZoomed)
// ...
renderScrollComponent={props =>
  <GestureDetector gesture={native}>
    <ScrollView {...props} />
  </GestureDetector>
}

This didn't work β€” it told me that I'm only allowed to have one native child inside. But this does look like a single child to me β€” is it not? I tried adding collapsible={false} to no avail. Then I tried wrapping VirtualizedList itself into GestureDetector. That didn't error but also seemingly didn't do anything.

j-piasecki commented 1 year ago

Hi! As @kacperkapusciak already mentioned the Gesture Handler's docs aren't in the best state at the moment and are due for a full rewrite in the future. We will look into addressing the problems you mentioned.

I'm not sure I understand the intended design for waitFor or requireExternalGestureToFail when there's an unknown number of items.

The approach you described is the best considering what the current API allows you to do (although I don't quite see how callback refs would help clean up in this case). That said we want to add an option to specify reverse waitFor relations. That should make creating many-to-one relations simpler.

I've seen Gestures.Native supposedly being related, but I don't understand how to use it from documentation at all. The documentation says it lets other views "participate" in the gesture system but it's unclear if this would work for composing my behaviors with Android's scroll view default behavior.

Gesture.Native is a new-api equivalent of NativeViewGestureHandler, it adds a middle-man when it comes to handling native touches, allowing Gesture Handler to intercept, and possibly deny, events to the view it wraps. Components exported by Gesture Handler are already wrapped with it, so if you were using ScrollView from GH, it wouldn't make a difference.

Does it let me disable and enable Android's scroll view?

That's a bit tricky. That would be the case assuming there is an active gesture at the time, since in that case Gesture Handler is the only thing handling touch events from the OS, and it would'n deliver them to the ScrollView. However, if there's no gesture active, Gesture Handler doesn't have the exclusivity for handling touch, so it falls back on system behavior - even if native gesture is disabled, falling back on the default system behavior would deliver the events to the scroll view.

Although, why not use scrollEnabled prop of ScrollView for this?

I tried wrapping it around my VirtualizedList's renderScrollComponent implementation

I'm not sure why it errored out in this case, will look into it.

And as for the question from the original post:

I don't understand why parent ScrollView steals item's pinch

It's a race condition between the two recognizers - Scroll and pinch. One activates based on the translation of the finger, the other based on the distance between two fingers, whichever threshold is met first decides which will activate. Native views have mechanisms for handling that, but Gesture Handler itself doesn't have a concept of hierarchy - it doesn't know that the pinch is attached to a child of the ScrollView. It simply maps handlers to the views they are attached to.

gaearon commented 1 year ago

Alright, so I'm going to document my entire journey here in case it helps guide the API design and/or examples.

To clarify, I was not able to find a fully satisfactory solution with the provided primitives.

Part 1: No ScrollView

The code we're starting off will have no scroll view. Essentially it's just an Expo Image with a single composed gesture recognizer. This example is very much based on @j-piasecki's code in https://github.com/software-mansion/react-native-gesture-handler/issues/2138#issuecomment-1231634779.

Here is the source code for the gesture configuration setup:

  const pinch = Gesture.Pinch()
    .onStart((e) => {
      // ...
    })
    .onChange((e) => {
      // ...
    })
    .onEnd(() => {
      // ...
    });

  const pan = Gesture.Pan()
    .averageTouches(true)
    .onChange((e) => {
      // ...
    })
    .onEnd(() => {
      // ...
    });

  const doubleTap = Gesture.Tap()
    .numberOfTaps(2)
    .onEnd((e) => {
      // ...
    });

  const dismissSwipePan = Gesture.Pan()
    .enabled(!isScaled)
    .activeOffsetY([-10, 10])
    .failOffsetX([-10, 10])
    .maxPointers(1)
    .onUpdate(e => {
      // ...
    })
    .onEnd((e) => {
      // ...
    });

  return (
    <View>
      <GestureDetector gesture={Gesture.Exclusive(dismissSwipePan, Gesture.Simultaneous(pinch, pan), doubleTap)}>

(If you'd like to run the app, I'm happy to send an invite code.)

Here's how it works:

https://github.com/software-mansion/react-native-gesture-handler/assets/810438/61ab5127-c947-4ba3-a243-76be3b69d7a2

Turn on the sound for commentary. Here's a summary:

This is not a showstopper but it's the first thing that kinda sucks. There's going to be a bit more trouble later on though.

Part 2: Adding a ScrollView above

Now let's add a <ScrollView> around the image so that we can show multiple that the user can page through:

https://github.com/bluesky-social/social-app/commit/d9489c39aef8642bef7a23598a1b68044f086f87

Here's the video of the new behavior:

https://github.com/software-mansion/react-native-gesture-handler/assets/810438/d3551486-968b-4a7f-9cfb-fbfbaf0ea627

Turn on the sound for commentary. Here's a summary:

Here the main showstopper issues are:

So it's kind of the opposite of the behavior we want. Let's see if we can find some fix.

Part 3: Trying scrollEnabled

It seems like we want to disable the scroll view as soon you pinch in (even a little bit).

We already have an animated reaction tracking the transform and calling props.onZoom(isScaled) on JS thread whenever "are we currently pinched in" changes. So let's add a state variable to the parent component as well, and determine scrollEnabled based on that:

https://github.com/bluesky-social/social-app/commit/b945c64af424b3a92e08c80c5422df43b7b19a85

Note: I don't actually know if this is a reliable way to do that! Could there be an async gap between useAnimatedReaction callback runs and the state update is flushed? What if the native view receives some touches in between? Could the scroll view start panning before the JS side realizes we've "pinched in" and shouldn't allow the scroll view to pan? I don't know.

I didn't record a video for this one because it hasn't changed that much. With this change, we get:

In other words, we prevented the most egregious case (flipping items while already pinched in), but the gallery still doesn't work β€” and we also still have the problem of the initial pinch not always working.

Part 4: Conditionally disabling pan

Okay, so the reason swiping is unreliable is that the individual item's pan "steals" the gesture from the scroll view β€” but we really only ever want to pan when we are zoomed in. Let's add .enabled(isScaled) to the Pan gesture:

https://github.com/bluesky-social/social-app/commit/99a22a0508f5b043369b5b526da8671e6f1e5f73

Have a look at what happens:

https://github.com/software-mansion/react-native-gesture-handler/assets/810438/559b3132-dd00-47e7-91dc-03778c0ab0e7

Now swiping works, but we've regressed on some aspects of other issues.

Turn on the sound for commentary. Here's a summary:

So, we fixed the item pan "stealing" scroll, but we created two new problems:

Part 5: Forcing the scroll view to "wait for" pinch

This is the workaround I posted earlier about. Since scroll view now "steals" pinches, let's force it to "wait for" pinches.

https://github.com/bluesky-social/social-app/commit/b3d05972e4b2b1b07ef2fb01ec056575fa4ab322

Here's the video:

https://github.com/software-mansion/react-native-gesture-handler/assets/810438/3fb411ae-1a4c-41b8-862b-f3590a0ff3e6

Like before, turn on the sound for commentary. This so far is the nicest outcome:

In summary, this isn't too bad. However, it seems like a significant regression that I can no longer pan immediately during the first pinch. I'd like to recover that somehow but I don't know how to do it without breaking all the other things.

The ideal behavior

Ideally, here's what I want:

I don't know how to express this but hope it's helpful!

gaearon commented 1 year ago

I suspect I can maybe knock out the two remaining issues if I reimplement pinch/pan manually as a single continuous gesture instead of a composition of Pan and Pinch. That kind of undermines the promise of this library though. Not sure if I'm missing something.

gaearon commented 1 year ago

While thinking about this problem:

πŸ₯² I'm not always able to switch from a pinch to a pan midway (it does nothing the first time, but works other times)

I accidentally stumbled upon the "manual activation" pattern. I wouldn't have guessed it's possible because manualActivation and StateManager docs are a bit sparse, and the API documentation for Gesture.Manual does not explain how to use it. However, this guide with a section on it was extremely helpful as it gave me some ideas.

My idea is to try something like this:

   const pan = Gesture.Pan()
     .averageTouches(true)
-    .enabled(isScaled)
+    .manualActivation(true)
+    .onTouchesDown((e, manager) => {
+      if (e.numberOfTouches > 1 || isScaled) {
+        manager.activate()
+      }
+    })
     .onChange((e) => {

Essentially, I want to always enable the pan gesture if we start with two fingers β€” even if we're not zoomed in yet.

I don't think this exact solution works though β€” it breaks the "double tap" gesture. I assume it's because now we're activating the pan "too eagerly" and we should in fact check whether the fingers have panned enough distance. That feels like reimplementing the pan recognizer so not super fun but maybe I'll take a crack at it later. I hope there's some easier way.

I also realized there's another problem I haven't yet described. If I start swiping the scroll view (to switch between items) and then start pinching midway, the image scales. However, pinching (and frankly, any of my own gestures) should be completely disabled once the scroll view is moving. Haven't found an easy way to do this. I guess I could listen to scroll view events and set some state variable so that the gestures are disable while scroll view isn't at rest. It would be nice if there was an easier way to disable all of my gestures while the native gesture is ongoing.

gaearon commented 1 year ago

It would be nice if there was an API like .enabled() but that took a lazily run function. E.g. enabled(e => e.numberOfTouches > 1 || isScaled). I don't want the manualActivation(true) thing because it seems to place too much of reimplementation burden on me. I just want some condition that's evaluated at decision time. Another thing I'd want it for is enabled(() => !isScrollViewScrolling.current).

gaearon commented 1 year ago

I think I may have found another way to solve the "pan during first pinch" issue:

  const pan = Gesture.Pan()
    .averageTouches(true)
-   .enabled(isScaled)
+   .minPointers(isScaled ? 1 : 2)

This gives me the "allow panning, but only when pinched in" behavior.

gaearon commented 1 year ago

I also realized there's another problem I haven't yet described. If I start swiping the scroll view (to switch between items) and then start pinching midway, the image scales. However, pinching (and frankly, any of my own gestures) should be completely disabled once the scroll view is moving. Haven't found an easy way to do this. I guess I could listen to scroll view events and set some state variable so that the gestures are disable while scroll view isn't at rest. It would be nice if there was an easier way to disable all of my gestures while the native gesture is ongoing.

I was hoping that maybe disallowInterruption={true} would do the trick but it doesn't seem to do anything. Then I tried setting a state boolean in onScrollBeginDrag and resetting it in onMomentumScrollEnd. However, these events appear somewhat buggy in RN and don't fire in a reliable order or quantity (https://github.com/facebook/react-native/issues/33474, https://github.com/facebook/react-native/issues/32696). I also don't think relying on an event would be "fast enough".

I ended up going with this slightly weird approach: https://github.com/bluesky-social/social-app/pull/1563/commits/3c2654ac3d6d151430bd56976e3b084c8c152e33. The idea is to define a gesture that "eats" any gestures when the container isn't positioned exactly at the viewport (i.e. while it's being scrolled or has momentum):

  const consumeHScroll = Gesture.Manual()
    .onTouchesDown((e, manager) => {
      const measurement = measure(containerRef);
      if (!measurement || measurement.pageX !== 0) {
        manager.activate();
      } else {
        manager.fail();
      }
    })

Then this gesture is fed to my Exclusive() call like Exclusive(consumeHScroll, dismissSwipePan, Gesture.Simultaneous(pinch, pan), doubleTap) to make sure it prevents any other gestures in the chain if activated. My scroll has no bounce so this might be good enough for almost all cases (unless, I guess, you start dragging it, and then drag it back precisely into the viewport).

Open to other solutions.

gaearon commented 1 year ago

Here's what I ended up with: https://github.com/bluesky-social/social-app/pull/1624