Conversation
added 3 commits
September 15, 2026 10:07
Re-provide `ReduceMotionContext` in `Portal`, alongside the settings, locale and theme contexts already forwarded across the portal boundary, so portal content stops falling back to the context default of `false`.
Compare the key when looking up the queued `mount` to replace, so an update that arrives before the `PortalManager` ref is attached no longer overwrites an unrelated queued portal.
Add an opt-in `overlay` prop to `Portal` that hides every layer below it -- the app content and any portal mounted earlier -- from assistive technology and from the web focus order, while portals mounted on top stay reachable.
This was referenced Sep 16, 2026
satya164
requested changes
Sep 17, 2026
| import OverlayLayer from './OverlayLayer'; | ||
|
|
||
| type Props = { | ||
| pageContent?: React.ReactNode; |
Member
There was a problem hiding this comment.
pageContent is misleading name as a portal provider doesn't render a page. it should just be content or children.
it should also not be optional.
Suggested change
| pageContent?: React.ReactNode; | |
| children: React.ReactNode; |
Comment on lines
+19
to
+28
| /** | ||
| * Whether this portal hides everything below it -- the app content and any | ||
| * portal mounted before it -- from screen readers and the focus order. | ||
| * Portals mounted after it stay reachable. | ||
| * | ||
| * Tie it to whether the overlay is open rather than to how long it stays | ||
| * painted: a layer gives the screen back the moment it starts closing, so | ||
| * what is underneath is reachable again while the overlay fades out. | ||
| */ | ||
| overlay?: boolean; |
Member
There was a problem hiding this comment.
modal maybe a better name instead overlay
|
|
||
| return ( | ||
| <> | ||
| {/* Need collapsable=false here to clip the elevations, otherwise they appear above Portal components */} |
Member
There was a problem hiding this comment.
move the comment to collapsable like the other one
Comment on lines
+19
to
+27
| /** | ||
| * Whether this portal hides everything below it -- the app content and any | ||
| * portal mounted before it -- from screen readers and the focus order. | ||
| * Portals mounted after it stay reachable. | ||
| * | ||
| * Tie it to whether the overlay is open rather than to how long it stays | ||
| * painted: a layer gives the screen back the moment it starts closing, so | ||
| * what is underneath is reachable again while the overlay fades out. | ||
| */ |
Member
There was a problem hiding this comment.
please write the comment by hand. don't use claude for documentation.
Suggested change
| /** | |
| * Whether this portal hides everything below it -- the app content and any | |
| * portal mounted before it -- from screen readers and the focus order. | |
| * Portals mounted after it stay reachable. | |
| * | |
| * Tie it to whether the overlay is open rather than to how long it stays | |
| * painted: a layer gives the screen back the moment it starts closing, so | |
| * what is underneath is reachable again while the overlay fades out. | |
| */ | |
| /** | |
| * Whether the portal hides items below it from screen readers and focus order. | |
| * | |
| * Ensure it's set to true only when the modal is open. | |
| */ |
Comment on lines
+1
to
+31
| import { Button, Portal, Dialog, Palette } from 'react-native-paper'; | ||
|
|
||
| import { TextComponent } from './DialogTextComponent'; | ||
|
|
||
| const DialogWithOverlay = ({ | ||
| visible, | ||
| close, | ||
| }: { | ||
| visible: boolean; | ||
| close: () => void; | ||
| }) => ( | ||
| <Portal overlay={visible}> | ||
| <Dialog onDismiss={close} visible={visible}> | ||
| <Dialog.Title>Alert</Dialog.Title> | ||
| <Dialog.Content> | ||
| <TextComponent> | ||
| While this dialog is open, everything behind it is hidden from screen | ||
| readers and skipped by the focus order! | ||
| </TextComponent> | ||
| </Dialog.Content> | ||
| <Dialog.Actions> | ||
| <Button textColor={Palette.tertiary50} disabled> | ||
| Disagree | ||
| </Button> | ||
| <Button onPress={close}>Agree</Button> | ||
| </Dialog.Actions> | ||
| </Dialog> | ||
| </Portal> | ||
| ); | ||
|
|
||
| export default DialogWithOverlay; |
Member
There was a problem hiding this comment.
I don't think a separate example is needed. all dialogs need to be used this way. update all the existing examples
Contributor
Author
There was a problem hiding this comment.
Updated. Now Modals render the Portal inside them
added 4 commits
September 17, 2026 15:17
Address review feedback on callstack#5126: - rename the `overlay` prop to `modal` - rename `PortalManager`'s `pageContent` prop to `children` and make it required, since a portal host doesn't render a page - move the `collapsable` comment onto the prop it explains - rewrite the `modal` prop documentation
A `Modal` is an overlay, so it always needs a `Portal` with `modal` set to hide the content behind it. Render one itself instead of asking every call site to wrap the modal and pass the prop. BREAKING CHANGE: `Modal` and `Dialog` no longer need to be wrapped in a `Portal`.
Every dialog now hides the content behind it, so the dedicated "Inert background" example no longer has anything of its own to show.
`Dialog` renders itself in a `Portal`, so the examples no longer need to wrap it in one.
|
Found potential problems with the pull request:
|
konstmar
changed the title
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.
Motivation
With a dialog open, a screen reader can still wander into the app content behind it.
Adds an opt-in
modalprop toPortal. A portal marked as a modal hides every layer below it - the app content and any portal mounted earlier - from assistive technology and from the web focus order. Portals mounted on top stay reachable, so a menu or snackbar above a dialog still works.Modalnow renders itself in a Portal withmodalprop tied to visible. Other overlay components will adopt it separately.Breaking:
ModalandDialogrender themselves in aPortal, so they now need aPortal.Hostabove them in the tree.Related issue
Notion
Screenshots / Videos
inert-background-android-after.mp4
inert-background-ios-after.mp4
Test plan
yarn test,yarn lint,yarn typecheckpass. Six newPortaltests.On a device, open any dialog in the example app and keep swiping past its last button with a screen reader, or hold Tab on web: focus stays inside the dialog, and the screen behind it is reachable again once it closes.