fix: add content modal wrapper for expo router issues#549
fix: add content modal wrapper for expo router issues#549ignaciosantise merged 6 commits intodevelopfrom
Conversation
🦋 Changeset detectedLatest commit: f6b43cc The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Claude finished @ignaciosantise's task —— View job PR Review: fix/expo-router-modal-wrapperTasks:
Issues Found1.
const handleModalClose = useCallback(async () => {
await close();
}, [close]);
const handleBackPress = useCallback(() => {
if (RouterController.state.history.length > 1) {
return RouterController.goBack();
}
return handleModalClose();
}, [handleModalClose]);2. Mismatched changeset scope — The changeset only bumps 3. Inline style in example — The workaround uses an inline style object: <View style={{ position: 'absolute', height: '100%', width: '100%' }}>Per project conventions, inline styles should be avoided. This should be extracted to a 4. The Modal: ({ children, testID }: any) => <MockView testID={testID}>{children}</MockView>,This means tests can't catch regressions where the modal renders its contents when it should be hidden (e.g., 5. Unused The mock state hardcodes Minor Notes
|
There was a problem hiding this comment.
Pull request overview
Adds support for optionally wrapping AppKit’s internal modal (to work around Expo Router modal layering issues) and updates Jest config so workspace packages resolve internal @reown/appkit-* imports from source during tests.
Changes:
- Add optional
modalWrapperprop toAppKitand export the related public types. - Add regression tests for wrapped vs unwrapped AppKit modal rendering and adjust an existing hook test mock.
- Update root + package Jest configs to map
@reown/appkit-*modules topackages/*/srcduring tests.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ui/jest.config.ts | Reuses root Jest config and preserves moduleNameMapper while overriding shared setup path. |
| packages/core/jest.config.ts | Same Jest config consolidation to inherit root mappers. |
| packages/common/jest.config.ts | Same Jest config consolidation to inherit root mappers. |
| packages/appkit/jest.config.ts | Same Jest config consolidation to inherit root mappers. |
| jest.config.ts | Adds moduleNameMapper entries mapping @reown/appkit-* to workspace src entrypoints for tests. |
| packages/appkit/src/modal/w3m-modal/index.tsx | Introduces modalWrapper prop to optionally wrap the internal modal tree. |
| packages/appkit/src/index.ts | Re-exports AppKitProps and wrapper-related types as part of the public API. |
| packages/appkit/src/tests/modal/w3m-modal.test.tsx | Adds regression coverage for wrapped/unwrapped modal rendering. |
| packages/appkit/src/tests/hooks/useAppKitTheme.test.tsx | Updates core module mock to be virtual (aligning with new mapper behavior). |
| examples/expo-multichain/app/_layout.tsx | Updates guidance comment to mention modalWrapper for Expo Router layering issues. |
| .changeset/sharp-cars-approve.md | Changeset for patch release including the new modalWrapper prop. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@claude review |
|
Claude finished @ignaciosantise's task in 2m 2s —— View job PR Review: fix/expo-router-modal-wrapperTasks:
Found 5 issue(s)Issue 1:
|
|
|
@claude review |
|
Claude finished @ignaciosantise's task in 2s —— View job PR Review: fix/expo-router-modal-wrapperTasks:
🔄 Incremental Review — checking state of previously flagged issues and scanning for new ones. Resolved since last review:
Dismissed by author (not re-flagging):
Found 3 issue(s)Issue 1: Changeset bumps all 9 packages despite only 2 having source changesID: changeset-scope-all-packages-b2e4 Context:
Recommendation: Scope to only the packages with source changes: ---
'@reown/appkit-react-native': patch
'@reown/appkit-ui-react-native': patch
---Issue 2: Changeset description says
|
|



Summary
contentModalWrapperprop toAppKitso apps can wrap the internal RN modal content with a custom window overlay such asFullWindowOverlay@reown/appkit-*imports fromsrcduring testsTesting