fix(Android): cancel button handlers when a native view takes the touch lock - #4433
Conversation
…ch lock `Pressable` (and every component built on the native button) keeps its `NativeViewGestureHandler` running when a native view grabs the touch lock, because that handler is attached with `ACTION_TYPE_NONE` and `cancelAllLegacyHandlers` only cancelled the action-driven ones. The visible symptom is a phantom press: a finger put down on a list row to stop a fling makes the enclosing `ScrollView` claim the touch (Android's `ScrollView` intercepts the `ACTION_DOWN` while the scroller is running and calls `requestDisallowInterceptTouchEvent`), yet the button handler survives, ends on the finger lift and dispatches `onPress` for a row the user never meant to tap. Cancel `NativeViewGestureHandler`s attached with `ACTION_TYPE_NONE` too. The root view's own handler shares that action type but is not a `NativeViewGestureHandler`, so it keeps running and interception is unaffected. Renamed the method since it no longer cancels only the legacy handlers.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR refines Android gesture-handler cancellation when a native view takes the touch lock by replacing “cancel all legacy handlers” behavior with a more targeted cancellation API and logic.
Changes:
- Renamed the orchestrator cancellation method and updated the root helper to call the new method.
- Expanded cancellation logic to also cancel
NativeViewGestureHandlerinstances withACTION_TYPE_NONE(e.g., button-managed handlers) when they would otherwise continue running.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerRootHelper.kt | Updates root helper to call the renamed/more specific orchestrator cancellation method. |
| packages/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/core/GestureHandlerOrchestrator.kt | Renames and documents cancellation method; adjusts predicate to also cancel certain NativeViewGestureHandlers. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| * the button handler and fires a press. The root view's own handler shares that action type and | ||
| * must keep running, hence the type check rather than a plain [GestureHandler.ACTION_TYPE_NONE] | ||
| * one. | ||
| */ |
| * the button handler and fires a press. The root view's own handler shares that action type and | ||
| * must keep running, hence the type check rather than a plain [GestureHandler.ACTION_TYPE_NONE] | ||
| * one. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan includes up to 8 reviews per rolling hour; 7 remain after this review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesNative gesture cancellation
Possibly related PRs
Suggested reviewers: Merge Risk: ⚪ Minimal · up to This localized Android change prevents unintended presses when a native view takes the touch lock; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 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 |
| it.actionType == GestureHandler.ACTION_TYPE_NATIVE_ANIMATED_EVENT || | ||
| (it is NativeViewGestureHandler && it.actionType == GestureHandler.ACTION_TYPE_NONE) |
…e touch (#4441) ## Description `Pressable` without relation props presses natively through `ButtonViewGroup`, whose managed `NativeViewGestureHandler` is attached with `ACTION_TYPE_NONE`. RNGH delivers touches through the orchestrator regardless of what happens in the native dispatch, so when a native `ScrollView` takes the gesture over, nothing stops the handler - it reaches `STATE_END` on lift and fires a press. This shows up in three ways: - fling catch: the `ScrollView` intercepts `DOWN` while decelerating, the button never sees any native event, yet `onPress` fires on lift (#4432) - drag: the `ScrollView` intercepts on `MOVE` when the finger starts scrolling from a row, and `onPress` still fires on lift ([comment](#4432 (comment))) - long press while scrolling: the content moves with the finger, so the pointer never leaves the row and the long-press timer posted on `BEGAN` fires mid-scroll (same comment) In all three the `ScrollView` calls `requestDisallowInterceptTouchEvent(true)`, but the existing sweep (`cancelAllLegacyHandlers`) only cancels action-driven handlers, and the `ButtonViewGroup` override from #4367 never runs since the request only bubbles up from the `ScrollView`. Cancelling button handlers directly at request time (the #4433 approach) is not valid either: an eager disallow-intercept (`react-native-pager-view`'s `NestedScrollableHost` requests it on `DOWN` whenever it's nested inside another `ViewPager`, without intercepting anything) is indistinguishable from a real interception at that moment, so every `Pressable` inside nested pagers (e.g. material top tabs in a pager) would go dead - the regression class #4367 fixed. The two can be told apart by when the grab happened and whether the native dispatch still reached the button: - `ButtonViewGroup` tracks `receivedNativeDown` - set in `dispatchTouchEvent` (handler delivery bypasses it), reset on `BEGAN`, which the orchestrator dispatches before the native dispatch of the same `DOWN`. - `RNGestureHandlerRootHelper` records the disallow request, and once the root view finishes `super.dispatchTouchEvent` runs `cancelHandlersOnNativeTouchGrab`, cancelling handlers whose hook opts in. - The hook decides via `shouldCancelOnNativeTouchGrab(grabbedMidGesture) = grabbedMidGesture || !receivedNativeDown`: a grab on any pass after `DOWN` means actual dragging (cancel, matching what the legacy `Pressable` and RN's `Pressable` do), while a grab during the `DOWN` pass spares a button that received that `DOWN` (a defensive disallow lets the event through). Only `ButtonViewGroup` opts into the hook, so handlers attached to detectors, scrollables and text inputs are unaffected. The cost on passes without a disallow request is a single boolean check. Fixes #4432 Supersedes #4433 ## Test plan Repro below: a `SectionList` with `Pressable` rows (`onPress` + `onLongPress`), a `Pressable` and a long-press `GestureDetector` inside nested `PagerView`s (the eager-disallow setup from #2383), and an engine toggle (v3 / `LegacyPressable` / RN `Pressable`). All runs on the same emulator, main vs this PR: | scenario | main | this PR | | --- | --- | --- | | fling the list, touch a row to stop it, lift | phantom `onPress` | nothing | | put a finger on a row and drag-scroll, lift | phantom `onPress` | nothing | | hold a row while drag-scrolling past 500 ms | phantom `onLongPress` | nothing | | tap a row on a settled list | `onPress` | `onPress` | | stationary long press on a row | `onLongPress` | `onLongPress` | | tap the `Pressable` inside nested pagers | `onPress` | `onPress` | | long press the detector box inside nested pagers (#2383) | activates | activates | `LegacyPressable` behaves the same in the list scenarios; inside nested pagers it doesn't fire on main either - its handlers are cancelled on any disallow-intercept request, which is the pre-existing legacy behavior this PR doesn't change. RN's `Pressable` doesn't go through RNGH and is clean everywhere. <details> <summary>Repro</summary> ```tsx import React, { useState } from 'react'; import { Pressable as RNPressable, SectionList, StyleSheet, Text, View, } from 'react-native'; import PagerView from 'react-native-pager-view'; import { GestureDetector, LegacyPressable, Pressable, useLongPressGesture, } from 'react-native-gesture-handler'; const SECTIONS = Array.from({ length: 8 }, (_, section) => ({ title: `Section ${section}`, data: Array.from({ length: 10 }, (_, index) => `Item ${section}-${index}`), })); const ENGINES = ['Pressable (v3)', 'LegacyPressable', 'RN Pressable'] as const; const COMPONENTS = [Pressable, LegacyPressable, RNPressable] as const; function LongPressBox({ onLongPress }: { onLongPress: () => void }) { const longPress = useLongPressGesture({ runOnJS: true, onActivate: onLongPress, }); return ( <GestureDetector gesture={longPress}> <View style={styles.gestureBox} /> </GestureDetector> ); } export default function EmptyExample() { const [engine, setEngine] = useState(0); const [lastEvent, setLastEvent] = useState('none'); const [eventCount, setEventCount] = useState(0); const Row = COMPONENTS[engine] as typeof Pressable; const report = (kind: string, item: string) => { setLastEvent(`${kind} ${item}`); setEventCount((count) => count + 1); }; return ( <View style={styles.root}> <View style={styles.banner}> <Text style={styles.bannerText}>engine: {ENGINES[engine]}</Text> <Text style={styles.bannerText}> last: {lastEvent} (count: {eventCount}) </Text> <Pressable style={styles.toggle} onPress={() => { setEngine((current) => (current + 1) % ENGINES.length); setLastEvent('none'); setEventCount(0); }}> <Text style={styles.toggleText}>Toggle engine</Text> </Pressable> </View> {/* Nested pagers: the inner pager's NestedScrollableHost calls requestDisallowInterceptTouchEvent(true) on ACTION_DOWN only when it sits inside another ViewPager2 — the eager-disallow case from #4367. */} <PagerView style={styles.pager} initialPage={0}> <View key="outer-a" style={styles.page}> <PagerView style={styles.innerPager} initialPage={0}> <View key="a" style={[styles.page, styles.pageRow]}> <Row style={styles.pagerButton} onPress={() => report('press', 'pager-button')}> <Text style={styles.toggleText}>Pager button</Text> </Row> {/* The #2383 setup: a long-press gesture inside nested pagers (material top tabs are pager-view underneath). */} <LongPressBox onLongPress={() => report('gesture', 'pager-box')} /> </View> <View key="b" style={styles.page}> <Text>Page B</Text> </View> </PagerView> </View> <View key="outer-b" style={styles.page}> <Text>Outer page B</Text> </View> </PagerView> <SectionList sections={SECTIONS} keyExtractor={(item) => item} renderSectionHeader={({ section }) => ( <Text style={styles.sectionHeader}>{section.title}</Text> )} renderItem={({ item }) => ( <Row style={styles.row} onPress={() => report('press', item)} onLongPress={() => report('longPress', item)}> <Text>{item}</Text> </Row> )} /> </View> ); } const styles = StyleSheet.create({ root: { flex: 1, }, banner: { padding: 16, gap: 8, backgroundColor: '#eee', }, bannerText: { fontWeight: 'bold', }, toggle: { alignSelf: 'flex-start', paddingVertical: 8, paddingHorizontal: 16, borderRadius: 8, backgroundColor: 'steelblue', }, toggleText: { color: 'white', }, sectionHeader: { paddingHorizontal: 24, paddingVertical: 8, fontWeight: 'bold', backgroundColor: '#ddd', }, row: { padding: 24, borderBottomWidth: 1, borderBottomColor: '#ddd', }, pager: { height: 110, borderBottomWidth: 2, borderBottomColor: '#bbb', }, page: { alignItems: 'center', justifyContent: 'center', }, pageRow: { flexDirection: 'row', gap: 16, }, gestureBox: { width: 64, height: 44, borderRadius: 8, backgroundColor: 'crimson', }, innerPager: { alignSelf: 'stretch', flex: 1, }, pagerButton: { paddingVertical: 12, paddingHorizontal: 24, borderRadius: 8, backgroundColor: 'darkorange', }, }); ``` </details>
Description
Fixes #4432
On Android, a
PressablefiresonPresswhen the touch was only meant to stop a fling: put a finger down on a list row while the list decelerates, lift it without moving, and the row under the finger gets pressed.Pressablewithout relation props rendersPressableWithTouchable, which presses natively throughButtonViewGroup. TheNativeViewGestureHandlerthe button manages for itself is attached withACTION_TYPE_NONE, so it isn't cancelled when a native view takes the touch lock:RNGestureHandlerRootHelper.requestDisallowInterceptTouchEvent()callscancelAllLegacyHandlers(), which only cancels the action-driven handlers (JS_FUNCTION_OLD_API,JS_FUNCTION_NEW_API,REANIMATED_WORKLET,NATIVE_ANIMATED_EVENT).Android's
ScrollViewintercepts theACTION_DOWNwhile its scroller is still running and callsrequestDisallowInterceptTouchEvent(true)up the tree, so that path does run — the button handler just isn't part of what it cancels. It then reachesSTATE_ENDon the finger lift and dispatches the press. The framework's ownACTION_CANCELdoesn't reach the button either, since RNGH delivers touches itself and ignoresonInterceptTouchEvent.This cancels
NativeViewGestureHandlers attached withACTION_TYPE_NONEas well. The root view's handler shares that action type but is not aNativeViewGestureHandler, so it keeps running and interception is unaffected.The v2
Pressablewent throughGestureDetector, so its handlers carried a JS action type and were cancelled by this very path —LegacyPressableandStatefulPressable(any relation prop) are unaffected today, which is a decent A/B when checking the fix.I also renamed
cancelAllLegacyHandlerstocancelHandlersLosingToNativeGesture, since it no longer cancels only the legacy handlers — happy to drop the rename if you'd rather keep the diff minimal.Test plan
Repro app: a
FlatList(fromreact-native) whose rows are wrapped inPressablefromreact-native-gesture-handler, each row logging itsonPress.Before:
onPressfires for the row under the finger.After: nothing fires, and a deliberate tap on a settled list still presses normally.