UI: Portal prop and Portal subcomponents for overlay Popups#77452
Merged
Conversation
de785b8 to
0c6b612
Compare
|
Flaky tests detected in 27a552d. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/24713148780
|
726184b to
53c7517
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a composable portal API to @wordpress/ui overlay popups by introducing Portal subcomponents and replacing Popup’s direct portal-target props with a single portal element prop.
Changes:
- Add
X.Portalsubcomponents (Dialog, AlertDialog, Popover, Tooltip, Select) and export theirPortalPropstypes. - Replace
Popup’scontainer-style portal targeting withportal={ <X.Portal … /> }and update popup implementations to render children via the provided portal element (or default portal). - Update unit tests, stories, and
packages/uichangelog to reflect the breaking portal API change.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ui/src/tooltip/types.ts | Replace container prop with portal element prop in Tooltip popup types. |
| packages/ui/src/tooltip/test/index.test.tsx | Update Tooltip tests to use portal={ <Tooltip.Portal … /> }. |
| packages/ui/src/tooltip/portal.tsx | Add Tooltip.Portal wrapper + PortalProps. |
| packages/ui/src/tooltip/popup.tsx | Render Tooltip popup content via provided/default portal element. |
| packages/ui/src/tooltip/index.ts | Export Portal and PortalProps. |
| packages/ui/src/popover/types.ts | Replace container prop with portal element prop in Popover popup types. |
| packages/ui/src/popover/test/index.test.tsx | Update Popover tests to use portal={ <Popover.Portal … /> }. |
| packages/ui/src/popover/stories/index.story.tsx | Update Popover stories to demonstrate Popover.Portal usage. |
| packages/ui/src/popover/root.tsx | Update Popover root docs to mention portal-by-default / customizable portal. |
| packages/ui/src/popover/portal.tsx | Add Popover.Portal wrapper + PortalProps. |
| packages/ui/src/popover/popup.tsx | Render Popover popup/backdrop via provided/default portal element. |
| packages/ui/src/popover/index.ts | Export Portal and PortalProps. |
| packages/ui/src/form/primitives/select/types.ts | Replace container prop with portal element prop in Select popup types. |
| packages/ui/src/form/primitives/select/test/index.test.tsx | Update Select tests to use portal={ <Select.Portal … /> }. |
| packages/ui/src/form/primitives/select/portal.tsx | Add Select.Portal wrapper + PortalProps. |
| packages/ui/src/form/primitives/select/popup.tsx | Render Select popup via provided/default portal element. |
| packages/ui/src/form/primitives/select/index.ts | Export Portal and PortalProps. |
| packages/ui/src/dialog/types.ts | Replace container prop with portal element prop in Dialog popup types. |
| packages/ui/src/dialog/test/index.test.tsx | Update Dialog tests to use portal={ <Dialog.Portal … /> }. |
| packages/ui/src/dialog/portal.tsx | Add Dialog.Portal wrapper + PortalProps. |
| packages/ui/src/dialog/popup.tsx | Render Dialog popup/backdrop via provided/default portal element. |
| packages/ui/src/dialog/index.ts | Export Portal and PortalProps. |
| packages/ui/src/alert-dialog/types.ts | Replace container prop with portal element prop in AlertDialog popup types. |
| packages/ui/src/alert-dialog/test/index.test.tsx | Update AlertDialog tests to use portal={ <AlertDialog.Portal … /> }. |
| packages/ui/src/alert-dialog/portal.tsx | Add AlertDialog.Portal wrapper + PortalProps. |
| packages/ui/src/alert-dialog/popup.tsx | Render AlertDialog popup/backdrop via provided/default portal element. |
| packages/ui/src/alert-dialog/index.ts | Export Portal and PortalProps. |
| packages/ui/CHANGELOG.md | Document breaking portal API change across overlay popups. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
53c7517 to
b48b625
Compare
|
Size Change: 0 B Total Size: 7.75 MB ℹ️ View Unchanged
|
f102e21 to
5e4ec2c
Compare
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
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.
Follow-up to #76837 (comment).
What?
Adds an optional
portalprop on overlayPopupcomponents in@wordpress/ui, plus exportedPortalsubcomponents, so callers can pass portal options (container,className,style, …) onPortalinstead of growing ad hoc props onPopup. Whenportalis omitted, the default Base UI portal is used.Covers
Dialog,AlertDialog,Popover,Tooltip, andSelect.Why?
Aligns with Mirka's suggestion: compose
portal={ <Dialog.Portal className="…" /> }rather than forwarding portal-only props throughPopup.How?
Details
Popupresolvesconst rootPortal = portal ?? <Portal />, buildsportalChildren, then usescloneElement( rootPortal, { children: portalChildren } )whenrootPortalis a valid element (otherwise falls back to<Portal>{ portalChildren }</Portal>). The pattern is intentionally mirrored per component family rather than a shared cross-package helper.forwardRefportal.tsxfiles wrap Base UIPortalper family;index.tsexportsPortalalongside existing subcomponents.Migration
PopupDialog/AlertDialogcontainer,portalClassNameportal={ <Dialog.Portal … /> }Popover/Tooltip/Selectcontainerportal={ <X.Portal … /> }Testing Instructions
npm installif needed, thennpm run buildso generated packages (e.g. icons) exist.npm run test:unit -- packages/ui/src/dialog/test packages/ui/src/popover/test packages/ui/src/tooltip/test packages/ui/src/form/primitives/select/testTesting Instructions for Keyboard
No intentional behavior change beyond DOM mount target; run the same unit suites above and spot-check a
Dialog/Popoverstory in Storybook if desired.Screenshots or screencast
Not applicable (API / composition only).
Use of AI Tools
Developed with assistance from Cursor; author reviewed the final diff.