Reset numberOfPointers in LongPressGestureHandler - #4479
Conversation
`setConfig` rebuilds a handler's config as `resetConfig` followed by `updateConfig`, so every property the setter writes must be cleared by the reset. The long press handler wrote `numberOfPointers` but never reset it, so once a gesture was configured with more than one pointer, dropping the property on a later config update left the old value in place and single pointer long presses stopped activating. Sibling handlers already reset their pointer counts: `Tap` resets `minNumberOfPointers`, `Fling` resets `numberOfPointersRequired` and `Pan` resets `minPointers`/`maxPointers`.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughChangesLong-press pointer reset defaults
Suggested reviewers: Merge Risk: ⚪ Minimal · up to Long-press handlers now restore the default single-pointer requirement when configuration removes a previous multi-pointer setting, preserving explicitly configured multi-pointer behavior. No current merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped, aligns with the existing resetConfig/updateConfig contract, and includes a targeted web regression test that demonstrates the fix.
Pull request overview
Fixes a stale-config bug where LongPressGestureHandler’s numberOfPointers/touch-count requirement could persist after a config update that no longer specifies it, by resetting the pointer-count default during resetConfig() across web, Apple, and Android implementations.
Changes:
- Web: introduce a named default for
numberOfPointersand reset it inresetConfig(). - Apple + Android: reset the platform-specific touch/pointer requirement back to the single-pointer default during
resetConfig(). - Add a Jest regression test on web covering removal of
numberOfPointersfrom config updates.
File summaries
| File | Description |
|---|---|
| packages/react-native-gesture-handler/src/web/handlers/LongPressGestureHandler.ts | Resets numberOfPointers to the default during config reset to prevent stale requirements after updates. |
| packages/react-native-gesture-handler/src/web/handlers/tests/LongPressGestureHandler.test.ts | Adds a regression test verifying that removing numberOfPointers from config restores the single-pointer default, plus a control test. |
| packages/react-native-gesture-handler/apple/Handlers/RNLongPressHandler.m | Resets numberOfTouchesRequired to 1 (non-tvOS) in resetConfig() so stale touch requirements don’t survive config rebuilds. |
| packages/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/core/LongPressGestureHandler.kt | Resets numberOfPointersRequired to a named default constant during resetConfig(). |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Description
LongPressGestureHandleracceptsnumberOfPointersbut never resets it, so the value survives a config update that no longer sets it.setConfigrebuilds a handler's whole config asresetConfig()followed byupdateConfig():src/web/handlers/GestureHandler.ts:790apple/RNGestureHandler.mm:131android/src/main/java/com/swmansion/gesturehandler/core/GestureHandler.kt:911updateConfigonly assigns properties that are present in the incoming config, so every property it can write has to be cleared byresetConfig.numberOfPointerswas written but never cleared:src/web/handlers/LongPressGestureHandler.ts:127writes it,:76did not reset itandroid/.../core/LongPressGestureHandler.kt:197writes it,:34did not reset itapple/Handlers/RNLongPressHandler.m:255writes it,:232did not reset itEvery sibling handler already resets its pointer count, which is what made this stand out:
TapresetsminNumberOfPointers,FlingresetsnumberOfPointersRequired,PanresetsminPointers/maxPointers.What breaks
GestureDetectorre-sends the full config on every update (src/handlers/gestures/GestureDetector/updateHandlers.ts:68), and the config only containsnumberOfPointerswhen the gesture actually sets it. So a long press that stops requesting a multi pointer press keeps the stale requirement:After
twoFingerModeflips back tofalse, the handler still requires 2 pointers. On webtryActivatereturns early becausetrackedPointersCount !== numberOfPointers(src/web/handlers/LongPressGestureHandler.ts:163), so a normal one finger long press never activates again. The same holds for the v2Gesture.LongPress().numberOfPointers(2)builder and the v1<LongPressGestureHandler numberOfPointers={2}>prop.The fix resets the value on all three platforms, using a named default in the two places that had a bare literal.
Test plan
Added
src/web/handlers/__tests__/LongPressGestureHandler.test.ts, following the existingGestureHandler.test.ts/webNativeViewGestureHandler.test.tspattern. One test drives the regression (config withnumberOfPointers: 2, then a config without it, then a single pointer press), and one control test confirmsnumberOfPointersstill applies while it is in the config.Counterfactual, run in this checkout. With the fix:
Then reverting only
src/web/handlers/LongPressGestureHandler.tsand keeping the test:4isState.ACTIVE,2isState.BEGAN: the handler stayed inBEGANbecause it was still waiting for a second pointer. Restoring the file turns it green again. The control test passes in both directions, so the failure is specific to the reset.Checks in this checkout:
yarn workspace react-native-gesture-handler test-> 20 suites, 163 tests passing (162 before this change)yarn workspace react-native-gesture-handler ts-check-> cleanyarn eslint --ext '.js,.ts,.tsx' src/-> 0 errors, 0 warnings on the touched filesyarn prettier --check './src/**/*.{js,jsx,ts,tsx}'-> all matched files use Prettier code style./android/gradlew -p android spotlessCheck -q-> exit 0clang-format --style=fileonRNLongPressHandler.m-> no diff on the changed linesThe Android and Apple changes mirror the web one and are not covered by the Jest suite; I did not run them on a device.