fix: support native-presented grab surfaces - #11
Merged
V3RON merged 5 commits intoAug 31, 2026
Conversation
Contributor
Author
|
Hey @V3RON ! When you have time, could you please review this solution? My project is currently blocked on supporting content hosted in native-presented sheets/modals (like TrueSheet), so I’d really appreciate your feedback on this approach and whether it looks correct / idiomatic for react-native-grab. |
Contributor
Author
|
Native TrueSheet verification is now complete on iOS Simulator using the repository Expo/Fabric example and Observed end to end:
The submitted SHA remains |
Adds a sheet playground screen exercising ReactNativeGrabSurface against a real native-presented sheet, which is the case ReactNativeGrabSurface exists for and which the existing modal route does not cover. Two sheets so surface ordering is testable: - an `auto` detent sheet, where the surface must stay content-measured - a fixed-detent sheet stacked on top, where the surface must fill instead Grab targets render `Text` directly rather than through `ThemedText` so the selection menu title names the surface the element came from (`Text (in AutoSheetTarget)`), making it possible to tell which owner resolved the selection. Each sheet drives `active` from onDidPresent/onDidDismiss rather than from the press handlers, so drag-to-dismiss also deactivates the surface. Claude-Session: https://claude.ai/code/session_01KDiabfZtYE2vxCeybZnA4D
Review follow-ups on the ReactNativeGrabSurface work: - Drop the `fill` prop from GrabSelectionOwnerView. Root and screen pass the fill style themselves, so a surface can now be made to fill its container with a plain `style` prop. Without this a surface wrapping full-screen content collapsed it in development and laid out correctly in production, where the component compiles away to a passthrough. - Make surface activation symmetric: deactivate on effect cleanup instead of relying on the owner being unregistered first. - Report a failed owner registration instead of returning silently. A surface whose ref never attached would otherwise ignore every later activation and stay permanently unselectable with no signal. - Render children from the web shims. `ReactNativeGrabRoot` and `ReactNativeGrabScreen` returned null, which drops the entire subtree on web; all three shims now preserve layout instead. - Replace the per-resolve filter and sort of active surfaces with a single scan. Resolution runs inside useSyncExternalStore for every mounted overlay. - Delete the unused selection owners store snapshot API. Claude-Session: https://claude.ai/code/session_01KDiabfZtYE2vxCeybZnA4D
findNodeAtPoint resolves its point against the shadow node it is given, but the overlay passed raw page coordinates. The two only coincide when the owner sits at the window origin, which is the case for ReactNativeGrabRoot — so the bug stayed invisible until an owner was offset. Verified on an iOS simulator: on a screen under a native stack header, tapping an element selected whatever sat one header-height below it. For a natively presented surface the offset is the sheet's own origin, several hundred points down, so ReactNativeGrabSurface could not have selected its content at all. The owner rect was already measured to make the highlight rect relative; it now also rebases the incoming point. Claude-Session: https://claude.ai/code/session_01KDiabfZtYE2vxCeybZnA4D
Contributor
Author
|
@V3RON When you have new version. Let's tag me Please. Thanks |
Contributor
|
@huytdps13400 Already out and you're tagged in the release. Thanks for contributing! |
Contributor
Author
|
Thank you! Great to see the fix released, and I appreciate the tag. Glad to contribute. |
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
2 participants
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.
Summary
ReactNativeGrabSurfacefor React content hosted in separately presented native sheets and modalsCloses #10.
Why
findNodeAtPointsearches from the resolved owner shadow node. Content presented by libraries such as TrueSheet can live outside the focused screen native subtree, so Grab cannot highlight or map it back to its React Fiber.ReactNativeGrabSurface active={isPresented}explicitly registers that native subtree only while it is presented. This avoids private global Fabric enumeration and prevents a dismissed sheet from stealing selection.Verification
npx vitest run src/react-native/__tests__/selection-owners.test.ts— passednpm test— 13/13 passednpm run format:check— passed on the submitted commitgit diff --check— passednpm run lint— passed with the optionalexpo-dev-menudeclaration availablenpm run build— ESM and CJS builds passed with the optional declaration available@lodev09/react-native-true-sheet3.10.1, and this PR branch: the active surface was selected, dragging highlighted the TrueSheet target, theView / Copy / Openmenu appeared, and Copy resolved the host element back to the example source file and lineonDidDismissOn a clean npm 11 install, lint/build otherwise stop at the existing type-only
expo-dev-menuoptional-peer resolution indev-menu.ts; no package metadata was changed in this PR.