[iOS] Fix enabled state diverging between the handler and its recognizer#4140
Open
j-piasecki wants to merge 1 commit intomainfrom
Open
[iOS] Fix enabled state diverging between the handler and its recognizer#4140j-piasecki wants to merge 1 commit intomainfrom
enabled state diverging between the handler and its recognizer#4140j-piasecki wants to merge 1 commit intomainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an iOS edge case where a gesture handler’s enabled value could get out of sync with its underlying UIGestureRecognizer, causing “disabled” handlers to still interfere with other gestures (e.g., blocking recognition / canceling RN responders unexpectedly).
Changes:
- Removes a root-level failure-requirement override that attempted to account for disabled handlers at the root recognizer level.
- Aligns tap recognizer
enabledonresetwith the backing handler’senabled. - Removes an unnecessary
enabled = YESin pan recognizerreset(since pan doesn’t mutateenableditself).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/react-native-gesture-handler/apple/RNRootViewGestureRecognizer.m | Removes root recognizer logic that imposed failure requirements based on handler enabled. |
| packages/react-native-gesture-handler/apple/Handlers/RNTapHandler.m | Ensures tap recognizer enabled is restored to match _gestureHandler.enabled during reset. |
| packages/react-native-gesture-handler/apple/Handlers/RNPanHandler.m | Removes redundant enabled = YES in reset to avoid unnecessary state churn. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Since some recognizers changed their own state in
reset, it was possible for the value to desync between the recognizer and the handler. This could lead to issues where a "disabled" handler would block other gestures from firing.This PR:
enabled = YESfrom pan gesture, since it never changes the value by itselfenabled = YESwithenabled = _gestureHandler.enabledfor tap, since it can set its ownenabledtoNOto cancel itselfshouldBeRequiredToFailByGestureRecognizerfrom the root recognizer - it was added in de0ffb4, before aligningenabledbetween recognizer and handler in a64b425. Disabled recognizers should never be considered for recognition, so this is effectively a no-op.Test plan
Tested on the provided repro: https://github.com/pawicao/rngh-v3-testing/blob/enabled-repro/src/app/index.tsx
Zoom in, pan around, zoom out, try to scroll horizontally