Skip to content

fix: gesture not activating due to outdated handler tag#3578

Merged
m-bert merged 5 commits into
software-mansion:mainfrom
hannojg:fix/gesture-not-activating
Oct 14, 2025
Merged

fix: gesture not activating due to outdated handler tag#3578
m-bert merged 5 commits into
software-mansion:mainfrom
hannojg:fix/gesture-not-activating

Conversation

@hannojg

@hannojg hannojg commented Jun 26, 2025

Copy link
Copy Markdown
Contributor

Description

This fixes an issue where the manual activation of a handler wouldn't work.
I unfortunately didn't had time to make a full reproduction. The scenario roughly looks like this:

const gesture = useMemo(() -> Gesture.Pan() // Note happens also without useMemo
	.manualActivation(true)
	.onTouchesMove((event, manager) => {
			// Some criteria to met:
			manager.activate()
	})
	.onUpdate(() => {
		// Wouldn't get called on a remount
	})

return <GestureDetector gesture={gesture}> // ...

This is being used in a component on a screen. When we navigate from the screen the component gets unmounted and I can see that his logic is being triggered:

useLayoutEffect(() => {
const viewTag = findNodeHandle(state.viewRef) as number;
preparedGesture.isMounted = true;
attachHandlers({
preparedGesture,
gestureConfig,
gesturesToAttach,
webEventHandlersRef,
viewTag,
});
return () => {
preparedGesture.isMounted = false;
dropHandlers(preparedGesture);
};
}, []);

attachHandler 1
dropHandler 1

I think the screen might get frozen instead of unmounted, but not entirely sure, but the cleanup function gets called and the handler gets dropped.
Now, when reopening the screen the handlers will reattach, but have a different handler tag now.

attachHandler 1
dropHandler 1
attachHandler 2 

When I now run the gesture I can see that the manager still has the previous handler tag, and thus on the native side it can't find the handler to activate. Its calling setGestureHandlerState(1 /* previous id */) here (when I call manager.activate):

I found that attaching the handler tag to the GestureStateMangaer and recreating it if it has changed fixes the issue.
If you need a full reproduction to proceed with this change I can understand, just let me know then I can try to find time to create one 👍

Test plan

Well, shamefully I didn't create a reproduction so I guess the test plan is "trust me bro" 😅 not the best I know - sorry.

@hannojg

hannojg commented Jun 26, 2025

Copy link
Copy Markdown
Contributor Author

note: we only started observing this change after switching to new arch

@m-bert

m-bert commented Jun 27, 2025

Copy link
Copy Markdown
Collaborator

Hi @hannojg! Thanks for spotting this issue and submitting a PR! It would be great if you could prepare a small reproduction 😅
Also, could you mark handlerTag inside manager as internal?

@m-bert

m-bert commented Jun 27, 2025

Copy link
Copy Markdown
Collaborator

I can see that CI fails, could you also update gestureStateManager.web.ts file?

image

@m-bert

m-bert commented Jul 7, 2025

Copy link
Copy Markdown
Collaborator

Hi @hannojg! What's the status of this PR? 😅

@m-bert m-bert requested a review from j-piasecki October 8, 2025 16:52
@m-bert

m-bert commented Oct 8, 2025

Copy link
Copy Markdown
Collaborator

Since there's no activity on this PR, I took the liberty of editing this PR. Hope you don't mind 😅

@j-piasecki j-piasecki left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please format code before merging.

@m-bert m-bert merged commit ae25b49 into software-mansion:main Oct 14, 2025
1 check passed
j-piasecki pushed a commit that referenced this pull request Oct 27, 2025
## Description

<!--
Description and motivation for this PR.

Include 'Fixes #<number>' if this is fixing some issue.
-->

This fixes an issue where the manual activation of a handler wouldn't
work.
I unfortunately didn't had time to make a full reproduction. The
scenario roughly looks like this:

```ts
const gesture = useMemo(() -> Gesture.Pan() // Note happens also without useMemo
	.manualActivation(true)
	.onTouchesMove((event, manager) => {
			// Some criteria to met:
			manager.activate()
	})
	.onUpdate(() => {
		// Wouldn't get called on a remount
	})

return <GestureDetector gesture={gesture}> // ...
```

This is being used in a component on a screen. When we navigate from the
screen the component gets unmounted and I can see that his logic is
being triggered:


https://github.com/software-mansion/react-native-gesture-handler/blob/1e1f4e035ea6d4c38684c4abc43d91dbf5b46dc6/packages/react-native-gesture-handler/src/handlers/gestures/GestureDetector/index.tsx#L152-L168

```
attachHandler 1
dropHandler 1
```

I think the screen might get frozen instead of unmounted, but not
entirely sure, but the cleanup function gets called and the handler gets
dropped.
Now, when reopening the screen the handlers will reattach, but have a
different handler tag now.

```
attachHandler 1
dropHandler 1
attachHandler 2 
```

When I now run the gesture I can see that the `manager` still has the
**previous handler tag**, and thus on the native side it can't find the
handler to activate. Its calling `setGestureHandlerState(1 /* previous
id */)` here (when I call `manager.activate`):


https://github.com/software-mansion/react-native-gesture-handler/blob/1e1f4e035ea6d4c38684c4abc43d91dbf5b46dc6/packages/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerModule.kt#L108

I found that attaching the handler tag to the GestureStateMangaer and
recreating it if it has changed fixes the issue.
If you need a full reproduction to proceed with this change I can
understand, just let me know then I can try to find time to create one 👍


## Test plan

<!--
Describe how did you test this change here.
-->

Well, shamefully I didn't create a reproduction so I guess the test plan
is "trust me bro" 😅 not the best I know - sorry.

---------

Co-authored-by: Michał Bert <63123542+m-bert@users.noreply.github.com>
Co-authored-by: Michał <michal.bert@swmansion.com>
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.

3 participants