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

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

Open
gaearon opened this issue Oct 3, 2023 · 13 comments
Open

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

gaearon opened this issue Oct 3, 2023 · 13 comments
Assignees
Labels
Missing repro Platform: Android This issue is specific to Android

Comments

@gaearon
Copy link

gaearon commented Oct 3, 2023

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 #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 #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 #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, #2332 (comment). 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 don't understand why parent ScrollView steals item's pinch
  • I couldn't figure it out from the docs — they barely mention coordination with ScrollView
  • The docs that do mention coordination with ScrollView seem to be focused on old APIs
  • GH issues appear to reference fixes that either have no docs for them, or were not reviewed

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 github-actions bot added the Platform: Android This issue is specific to Android label Oct 3, 2023
@github-actions
Copy link

github-actions bot commented Oct 3, 2023

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
Copy link
Author

gaearon commented Oct 3, 2023

Here's the flow:

  • Search for waitFor. Brings me here which — alas — is a page for old API.
  • It suggests creating refs using createRef but it's unclear what to do when I have a dynamic array of items, and I need the scroll view to not activate when any gesture inside (like pinch) is active.
  • "Read more" link there brings me to another outdated API page.

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
Copy link
Author

gaearon commented Oct 3, 2023

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
Copy link
Author

gaearon commented Oct 3, 2023

My current workaround looks like this: bluesky-social/social-app@115d60d. 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:

  • In a neutral state, the user may either pinch into a picture (pinch gesture inside an item) or swipe between pictures (native scroll view behavior).
  • If the user has started pinching, I want to disable the outer scroll view. The scroll view should only be enabled when we're fully zoomed out.
  • The user should be able to pan within a zoomed in picture. This should not trigger the scroll view.
  • The user should be able to start pinching and then remove one finger and have that be recognized as a pan. This doesn't happen with my current workaround. Basically, a pan should be able to "join" the pinch as soon as we're zoomed in a bit.
  • If I'm swiping the scroll view, all gestures inside the item should be disabled. There can't be any pinching or panning.

@gaearon
Copy link
Author

gaearon commented Oct 3, 2023

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
Copy link
Member

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
Copy link
Author

gaearon commented Oct 3, 2023

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 #2138 (comment).

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:

ezgif.com-resize.mov

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

  • ✅ I'm able to pinch to zoom in
  • ✅ I'm able to pan a zoomed in picture
  • ✅ I'm able to switch from a pinch to a pan midway
  • ✅ I'm able to switch from a pan to a pinch midway
  • 🥲 I'm not able to switch two times (e.g. pinch -> pan -> pinch) without lifting all fingers (it does nothing)
  • 🫥 Swiping between images in the gallery is not implemented yet

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:

bluesky-social/social-app@d9489c3

Here's the video of the new behavior:

ezgif.com-resize.mov

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

  • ❌ I'm able to pinch to zoom in but not reliably (horizontal pinch may scroll the gallery)
  • ✅ I'm able to pan a zoomed in picture
  • ✅ I'm able to switch from a pinch to a pan midway
  • ❌ I'm able to switch from a pan to a pinch midway but not reliably (horizontal pinch may scroll the gallery)
  • ❌ I'm not able to switch two times (e.g. pinch -> pan -> pinch) without lifting all fingers (horizontal pinch may scroll the gallery)
  • ❌ I'm not able to reliably swipe between gallery items (it almost always doesn't do anything)

Here the main showstopper issues are:

  • The scroll view starts scrolling while we're in pinched state, but shouldn't ever allow this.
  • The scroll view almost never starts scrolling when we're in pinched out state, but it always should!

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:

bluesky-social/social-app@b945c64

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:

  • ❌ I'm able to pinch to zoom in but not reliably (horizontal pinch may scroll the gallery)
  • ✅ I'm able to pan a zoomed in picture
  • ✅ I'm able to switch from a pinch to a pan midway
  • ✅ I'm able to switch from a pan to a pinch midway
  • 🥲 I'm not able to switch two times (e.g. pinch -> pan -> pinch) without lifting all fingers (it does nothing)
  • ❌ I'm not able to reliably swipe between gallery items (it almost always doesn't do anything)

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:

bluesky-social/social-app@99a22a0

Have a look at what happens:

ezgif.com-resize.1.mov

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

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

  • ❌ I'm not able to pinch to zoom in (horizontal pinch may scroll the gallery)
  • ✅ I'm able to pan a zoomed in picture
  • 🥲 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'm not able to switch from a pan to a pinch midway (it does nothing)
  • 🥲 I'm not able to switch two times (e.g. pinch -> pan -> pinch) without lifting all fingers (it does nothing)
  • ✅ I'm able to reliably swipe between gallery items

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

  • Huge problem: pinch no longer works reliably
  • Smaller problem: pan no longer works during first pinch until you lift all fingers

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.

bluesky-social/social-app@b3d0597

Here's the video:

ezgif.com-resize.2.mov

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

  • ✅ I'm able to pinch to zoom in
  • ✅ I'm able to pan a zoomed in picture
  • 🥲 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'm able to switch from a pan to a pinch midway
  • 🥲 I'm not able to switch two times (e.g. pinch -> pan -> pinch) without lifting all fingers (it does nothing)
  • ✅ I'm able to reliably swipe between gallery items

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:

  • Vertical swipe always reliably dismisses the item
  • Whether we start with pinch or pan, as soon as we're even a little zoomed in, we should be able to keep switching between panning and pinching (by lifting or placing fingers) as a continuous gesture
  • If we're not zoomed in, the scroll view must ignore any two finger gestures and let pinch handle it
  • If we're not zoomed in, the scroll view must handle one finger horizontal gestures
  • If the scroll view started reacting to a gesture, nothing should be able to take it away, it's too late

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

@gaearon
Copy link
Author

gaearon commented Oct 3, 2023

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
Copy link
Author

gaearon commented Oct 4, 2023

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
Copy link
Author

gaearon commented Oct 4, 2023

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
Copy link
Author

gaearon commented Oct 4, 2023

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
Copy link
Author

gaearon commented Oct 4, 2023

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 (facebook/react-native#33474, facebook/react-native#32696). I also don't think relying on an event would be "fast enough".

I ended up going with this slightly weird approach: bluesky-social/social-app@3c2654a. 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
Copy link
Author

gaearon commented Oct 6, 2023

Here's what I ended up with: bluesky-social/social-app#1624

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Missing repro Platform: Android This issue is specific to Android
Projects
None yet
Development

No branches or pull requests

2 participants