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

Swipes incorrectly trigger taps on Android if JS thread is busy #960

Open
gaearon opened this issue Jan 14, 2025 · 7 comments
Open

Swipes incorrectly trigger taps on Android if JS thread is busy #960

gaearon opened this issue Jan 14, 2025 · 7 comments

Comments

@gaearon
Copy link

gaearon commented Jan 14, 2025

Environment

  • React Native 0.76.3 (but previous versions too)
  • Reanimated 3.17 alpha (but previous versions too)
  • React Native Pager View 6.6.1 (but previous versions too)
  • Paper (have not checked Fabric yet)

Description

If I nest Pressable into PagerView, and if I have a Reanimated event listener on it that makes the JS thread busy right after the swipe begins, the RN Pressable mistakingly recognizes a pager swipe as a press. I don't know exactly where the issue is but I suspect it should be up to this library to somehow prevent this from happening since it's the one emitting the scroll state change event (and thus should be able to disable/cancel touches inside in time).

This might be related to #954 or might even be the same issue.

Reproducible Demo

Here you go:

bugg.mov
import {Alert, View, Pressable} from 'react-native'
import Animated, {useEvent, useHandler, runOnJS} from 'react-native-reanimated'
import PagerView from 'react-native-pager-view'

const AnimatedPagerView = Animated.createAnimatedComponent(PagerView)

function doNothingSlowly() {
  let now = performance.now()
  while (performance.now() - now < 200) {
    // do nothing
  }
}

export default function App() {
  const handlePageScroll = useEvent(
    () => {
      'worklet'
      runOnJS(doNothingSlowly)()
    },
    ['onPageScrollStateChanged'],
    true,
  )
  return (
    <AnimatedPagerView style={{flex: 1}} onPageScroll={handlePageScroll}>
      <Page />
      <Page />
    </AnimatedPagerView>
  )
}

function Page() {
  return (
    <Pressable
      style={{
        width: 300,
        height: 200,
        backgroundColor: 'yellow',
        margin: 50,
      }}
      onPress={() => {
        Alert.alert('hello')
      }}
    />
  )
}

This reliably reproduces for me both on the simulator and on the device. If it doesn't for you, try tweaking the sleep ms number.

@gaearon
Copy link
Author

gaearon commented Jan 14, 2025

Application-level hackfix to work around this: bluesky-social/social-app#7448

I'm not happy with it ofc but it may point to some way to solve it.

@chrfalch
Copy link

After spending some time investigating this issue I think I have an idea of what's going on in your example.

When using the Animated PagerView the handlePageScroll event will be called immediately and not be dispatched through the regular channels - caused by a hook REA has installed on the RN event emitter that captures worklet-based events in animated components and dispatches them directly on the main thread.

This causes your js code to be executed and block the js-thread before the RN renderer will be able to process further touch events on the child views inside the pager view.

Pressable (or rather components using the responder system and the Pressable implementation) receives touch events even while the parent's gestures are running - this is common for containers like Views, ScrollView, FastList etc - not only for the PagerView. The responder system uses these events to dispatch termination events (cancelling the touch operation) or release events (comitting a touch and calling onPress as a side effect).

More details on how the responder system and Pressable uses this can be seen here:

I believe that what you're experiencing here is not a problem with the react-native-pager-view component, but rather the fact that you're blocking the js thread very early in the touch responder chain - causing the js responder system to fail to detect cancellation of touches correctly and deterministically.

If I understand your use-case correctly from your example (using a callback on the JS thread while scrolling) you want to be able to perform some kind of lengthy operation without ruining smooth scrolling. A solution to this might be to use a separate worklet runtime that runs neither on the js nor the ui thread.

This can be done by using the createWorkletRuntime function in REA like this:

// Outside your component
const runtime = createWorkletRuntime("Background processing");

// Component
const Component = () => {
  const handlePageScrollStateChanged = () => {
    runOnRuntime(runtime, doNothingSlowly);
  };

  return (
    <AnimatedPagerView
      style={{ flex: 1 }}
      onPageScrollStateChanged={handlePageScrollStateChanged}
    >
      <Page />
      <Page />
      <Page />
    </AnimatedPagerView>
  );
}

@gaearon
Copy link
Author

gaearon commented Jan 15, 2025

We discussed privately but the tldr is — although the problem is likely due to JS thread being busy close to the touch move, the issue still is in the purview of this library.

You can’t control when the JS thread is going to be busy (can happen at any moment) so there has to be some way to prevent the tap/swipe confusion other than just not scheduling JS work. In my case that’s not possible anyway because the JS work I’m doing (outside of this simplified example) is a React setState.

We know this issue doesn’t occur with regular ScrollView or FlatList. So the way they interact with the JS responder system somehow avoids this. So regardless of how busy the JS thread is, there has to be some way for the pager to coordinate with the responder system in a similar way to prevent the underlying page from receiving remaining events once the swipe activates.

@MrRefactor
Copy link
Collaborator

Looking into this!

@gaearon
Copy link
Author

gaearon commented Jan 15, 2025

This is how I’m working around it in userland for now: bluesky-social/social-app#7448. But obviously not very happy with it. It also only works on Android but breaks things badly in iOS (which is fine because the fix is conditional but seems to point to the fix not being quite right).

@gaearon
Copy link
Author

gaearon commented Jan 15, 2025

Here's another approach, haven't tested yet bluesky-social/social-app#7459

@gaearon
Copy link
Author

gaearon commented Jan 17, 2025

#961

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants