Skip to content

Conversation

@akwasniewski
Copy link
Contributor

@akwasniewski akwasniewski commented Jan 9, 2026

Description

The new StateManager is global and given handlerTag it can manually set the states of an arbitrary gesture. This causes errors, when the gesture, which state is being set, has not been yet recorded in the orchestrator. Recording gestures in the orchestrator on android is done lazily, thus if it never received touches it is not recorded.

Test plan

Tested on the following example

Details
import React from 'react';
import { View, Text, StyleSheet } from 'react-native';
import { GestureHandlerRootView, GestureDetector, useLongPressGesture, GestureStateManager, usePanGesture, useSimultaneousGestures, useTapGesture } from 'react-native-gesture-handler';

export default function TwoPressables() {
  const longPress = useLongPressGesture({
    onTouchesDown: (e) => {
      'worklet';
      console.log("touches down")
    },
    onActivate: () => {
      'worklet';
      console.log("long pressed")
    },
    minDuration: 100000000,
    disableReanimated: true
  })
  const pan = useTapGesture({
    onTouchesDown: () => {
      'worklet';
      console.log("tap")

      GestureStateManager.activate(longPress.tag)
    },
    disableReanimated: true
  });
  return (
    <GestureHandlerRootView>
      <View style={styles.root}>
        <GestureDetector gesture={longPress}>
          <View style={styles.outer}>
            <Text style={styles.label}>Long Press</Text>
          </View>
        </GestureDetector>
        <GestureDetector gesture={pan}>
          <View style={styles.outer}>
            <Text style={styles.label}>Pan</Text>
          </View>
        </GestureDetector>

      </View>
    </GestureHandlerRootView>
  );
}

const styles = StyleSheet.create({
  root: {
    flex: 1,
    justifyContent: 'center',
    alignItems: 'center',
    backgroundColor: '#f7f7f7',
  },
  outer: {
    padding: 20,
    backgroundColor: '#ddd',
    borderRadius: 12,
    marginBottom: 50
  },
  label: {
    fontSize: 18,
    marginBottom: 10,
  },
})

@akwasniewski akwasniewski requested a review from m-bert January 9, 2026 13:32
@akwasniewski akwasniewski changed the base branch from main to next January 9, 2026 13:32
Copy link
Contributor

@m-bert m-bert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As said in the first comment, I'm not a big fan of ForManual suffix. I think we could either use overloading (where possible) or add more descriptive suffix.

Comment on lines 617 to 620
if (orchestrator == null) {
// If the state is set manually, the handler may not have been fully recorded by the orchestrator.
hostDetectorView?.recordHandlerIfNotPresentForManual(this)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should throw error when both, orchestrator and hostDetectorView are null. This may happen if gesture is defined, but is not assigned to any of the detectors. In that case I believe it will still crash, but with other, less descriptive error (probably NullPointerException).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, f3c1e93, I will check whether this works on other platforms

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On web the error is more fitting, Expected delegate view to be HTMLElement and on iOS it fails silently

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On web the error is more fitting, Expected delegate view to be HTMLElement

I don't think it is more fitting in that case. It doesn't tell you what you've done wrong, so it won't be obvious for someone who tries to change state of the gesture.

on iOS it fails silently

I don't think it is good either. Such behaviour should be unified between platforms.

Comment on lines 210 to 222
private fun findGestureHandlerRootView(): RNGestureHandlerRootView? {
var parent: ViewParent? = this.parent
var gestureHandlerRootView: RNGestureHandlerRootView? = null

while (parent != null) {
if (parent is RNGestureHandlerRootView) {
gestureHandlerRootView = parent
}
parent = parent.parent
}

return gestureHandlerRootView
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we have this exact method already defined somewhere? Maybe it's worth moving it to some utils, or making it an extension on View?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I moved it to be companion object for the rootView in 8a4ffc9. Let me know what you think.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm not sure whether this.findGestureHandlerRootView()?... wouldn't have been cleaner, but I'll leave the final decision to you. Both work for me.

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

Successfully merging this pull request may close these issues.

4 participants