[iOS] Keep button hit testing within its own subtree - #4495
Conversation
Stop the upward hit-test search at the button when no eligible target has been found. This prevents returning an ancestor outside the button’s subtree while keeping eligible descendants reachable.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughThe iOS button hit-test loop now stops at the button boundary when the button rejects a touch. This prevents traversal into ancestor views and avoids recursive hit testing in SwiftUI-backed view hierarchies. ChangesiOS hit testing
Suggested reviewers: Priority: ➖ Normal — Impact reflects medium issue severity. Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Disabled button areas now stop hit testing at the button boundary, preventing ancestor traversal and the reported crash while retaining eligible nested controls. 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 small, well-scoped to disabled-button hit testing, and the new guard prevents returning out-of-subtree ancestors without affecting eligible descendant selection.
Pull request overview
Fixes an iOS hit-testing edge case in RNGestureHandlerButton where, when the button is disabled and no eligible descendant view is found, the hit-test search could continue past the button and incorrectly return an ancestor outside the button’s subtree (which can contribute to recursive hit testing / SwiftUI-hosting-related crashes as described in #4494).
Changes:
- Add a boundary guard in
-hitTest:withEvent:to stop the “walk up superviews” loop at the button itself and returnnilinstead of returning an ancestor outside the subtree.
File summaries
| File | Description |
|---|---|
| packages/react-native-gesture-handler/apple/RNGestureHandlerButton.mm | Stops the hit-test eligibility search from walking past a disabled RNGestureHandlerButton into ancestor views by returning nil when the search reaches the button itself. |
Review details
- Files reviewed: 1/1 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.
m-bert
left a comment
m-bert
left a comment
There was a problem hiding this comment.
Hi @rileysay! Thank you for this PR!
The change seems ok, but there's one problem - it now allows siblings of disabled buttons to take over the touch, even though they shouldn't.
Here is the example of the failing structure
<View style={{ height: 120 }}>
<Touchable
onPress={() => console.log('outer')}
style={{
position: 'absolute',
inset: 0,
backgroundColor: '#9fd8a8',
}}
/>
<Touchable
disabled
style={{
position: 'absolute',
left: 40,
top: 20,
width: 200,
height: 80,
backgroundColor: '#9a9a9a',
}}
/>
</View>To fix this, we can slightly change the approach:
- Return
selfinstead ofnil, so that button still can become a touch target:
- while (inner && ![self shouldHandleTouch:inner atPoint:point]) {
+ while (inner && inner != self && ![self shouldHandleTouch:inner atPoint:point]) {- Early return from
beginTrackingWithTouch:withEvent:when button is disabled:
- (BOOL)beginTrackingWithTouch:(UITouch *)touch withEvent:(UIEvent *)event
{
if (!_userEnabled) {
return NO;
}
...- Early return from point 2 should also be applied to
mouseDown,mouseUpandmouseDraggedmethods to keepmacOSbehavior correct.
After that everything should work fine 😅
Description
Fixes #4494.
When a disabled
RNGestureHandlerButtonhas no eligible touch target, the hit-test loop can continue past the button and return an ancestor outside its subtree.This change returns
nilwhen the search reaches the button itself without finding an eligible target. Eligible descendants can still be selected before reaching that boundary.The guard prevents the SwiftUI-backed blur crash in my device testing. The suspected connection between returning a hosting ancestor and recursive hit testing is described in the linked issue.
Test plan
Tested the same boundary guard in my app’s test screen on an iPhone 16 Pro running iOS 26.5.2, using an Expo SDK 57 development build with React Native 0.86.3 and Gesture Handler 3.2.1.
React Native’s experimental
enableSwiftUIBasedFiltersflag was enabled.Reproduction project and build instructions:
https://github.com/rileysay/rngh-swiftui-hit-test-repro
The standalone project has built successfully on EAS. The device observations above were recorded using the test screen in my original app.
AI assistance: I used Codex to help investigate the native code, prepare the reproduction project, and propose this patch. I personally verified the device observations above.