DataViews: Migrate modals from @wordpress/components Modal to @wordpress/ui Dialog#76837
Draft
DataViews: Migrate modals from @wordpress/components Modal to @wordpress/ui Dialog#76837
Conversation
github-actions
bot
added
[Package] Compose
ciampo
force-pushed
the
refactor/dataviews-ui-dialog
branch
from
50a77c2 to
a8b8a61
Compare
|
Size Change: +183 kB (+2.36%) Total Size: 7.93 MB 📦 View Changed
ℹ️ View Unchanged
|
|
Flaky tests detected in 6354875. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/24565156155
|
This was referenced Mar 26, 2026
ciampo
force-pushed
the
refactor/dataviews-ui-dialog
branch
from
a8b8a61 to
1aa3fc8
Compare
github-actions
bot
added
[Package] UI
ciampo
force-pushed
the
refactor/dataviews-ui-dialog
branch
from
641b86b to
cec69a8
Compare
ciampo
commented
Apr 8, 2026
|
|
||
| return ( | ||
| <_AlertDialog.Portal> | ||
| <_AlertDialog.Portal className={ portalClassName }> |
Contributor
Author
There was a problem hiding this comment.
@WordPress/gutenberg-components I realized that, for components rendering both a Popup and a Backdrop (ie Dialog, AlertDialog, Drawer, ...), the Backrdop element won't normally inherit the z-index variable when it's passed to the Popup element, since they're siblings. For now, I resorted to adding a portalClassName prop, but I'm open to ideas
Member
There was a problem hiding this comment.
the
Backrdopelement won't normally inherit thez-indexvariable when it's passed to thePopupelement
How does this relate to the current setup we have, as described in the "With Custom z-index" story? The story says you need to override the z-index value globally. Are you suggesting that we make it locally overrideable?
Contributor
Author
There was a problem hiding this comment.
The story says you need to override the z-index value globally
That assumes that we'll never need to give different z-indexes to two separate instances of, eg., Dialog or Popover — which, looking at packages/base-styles/_z-index.scss, we may need to? (at least during the migration phase?)
Member
There was a problem hiding this comment.
💀
At this point, I'm wondering if our Popover subcomponents should have a portal prop that takes an optional Portal subcomponent to capture all the portal props including container, className, and style.
<AlertDialog.Popup portal={ <AlertDialog.Portal className="foo" /> } />
Contributor
Author
There was a problem hiding this comment.
Interesting point. My question then would be: why a render prop and not a Portal subcomponent to mirror base ui? Better default behaviour?
Member
There was a problem hiding this comment.
Yes, just for ergonomics, because the vast majority of uses will not need a custom portal.
ciampo
force-pushed
the
refactor/dataviews-ui-dialog
branch
2 times, most recently
from
64dc918 to
d482b19
Compare
ciampo
force-pushed
the
refactor/dataviews-ui-dialog
branch
from
d410b6f to
a2c9043
Compare
ciampo
force-pushed
the
refactor/dataviews-ui-dialog
branch
from
cb17dc4 to
94800e6
Compare
- Remove unused `dataforms-layouts-panel__modal` className from Dialog.Popup in panel modal (no CSS rules target it). - Use VisuallyHidden render prop for Dialog.Title composition, producing one DOM node instead of two. Made-with: Cursor
Move breaking change entries from the already-released 14.0.0 section to Unreleased. Consolidate the two separate entries into one that also covers the removed dataforms-layouts-panel__modal CSS class. Made-with: Cursor
Allow consumers to set per-instance CSS custom properties on the portal container (e.g. --wp-ui-dialog-z-index) so they cascade to both the backdrop and popup without affecting other dialog instances. Made-with: Cursor
Replace the global :root z-index override with a scoped .dataviews-action-modal-portal class passed via portalClassName, ensuring the z-index only applies to action modal dialogs. Made-with: Cursor
Cover dialog/alertdialog role rendering, modalSize 'fill' deprecation mapping, and firstInputElement focus behavior. Made-with: Cursor
- Replace AlertDialog.Root + Dialog.Popup hybrid with Dialog.Root using disablePointerDismissal and role="alertdialog" via conditional spread, fixing the broken focus trap. - Add a dedicated legacy-compat CSS layer (wp-ui-legacy-compat.scss) following the Phase 2 overlay migration strategy. Sets generic --wp-ui-*-z-index defaults on :root matching @wordpress/components base values (Modal screen overlay for dialog, Tooltip for tooltip). - Use portalClassName to apply the higher .dataviews-action-modal z-index only to action modal dialogs that need to stack above legacy popovers. Made-with: Cursor
Base UI ≥1.4.0 switched to an internal tabbable implementation that uses checkVisibility() for visibility detection. Per spec, checkVisibility() returns false for display:contents elements (no generated box). Since ThemeProvider wraps popup elements and uses display:contents, every element inside the popup was classified as non-tabbable, completely breaking focus-trap cycling. Override display:contents to display:block on the ThemeProvider div when it directly wraps a popup. The popup is position:fixed, so an extra block wrapper is layout-inert. The rule is unlayered to win over ThemeProvider's unlayered display:contents. Affects: Dialog, AlertDialog, Popover, Tooltip, Select. Made-with: Cursor
Add a ThemeProvider hasLayoutBox escape hatch so overlay popups can opt out of display: contents without duplicating CSS overrides across each popup stylesheet. Made-with: Cursor
Revert the ThemeProvider hasLayoutBox follow-up and keep the temporary unlayered popup stylesheet override while we reassess the longer-term fix upstream. Made-with: Cursor
Reference mui/base-ui#4622 and keep note that rules stay outside @layer to override ThemeProvider’s unlayered display: contents. Made-with: Cursor
Move the visual menu chrome into an inner surface so the Ariakit popover element can stay static. This isolates our menu styling and animation work from the underlying disclosure root while we debug the modal handoff behavior. Made-with: Cursor
Add an Unreleased changelog note in the Internal section for the Menu popover surface refactor. Made-with: Cursor
ciampo
force-pushed
the
refactor/dataviews-ui-dialog
branch
from
8de6745 to
0ecf570
Compare
github-actions
bot
removed
[Package] Blocks
ciampo
added a commit
to ciampo/gutenberg
that referenced
this pull request
Apr 17, 2026
Extract the @wordpress/components Menu changes from WordPress#76837: wrap menu content in MenuSurface, move variant styling and open animations to the inner surface so they are decoupled from the Ariakit Menu root (immediate close, no leave transition blocking scroll lock).
ciampo
added a commit
to ciampo/gutenberg
that referenced
this pull request
Apr 17, 2026
Extract the @wordpress/components Menu changes from WordPress#76837: wrap menu content in MenuSurface, move variant styling and open animations to the inner surface so they are decoupled from the Ariakit Menu root (immediate close, no leave transition blocking scroll lock).
ciampo
added a commit
to ciampo/gutenberg
that referenced
this pull request
Apr 17, 2026
Inject scoped styles so layered portals (for example Dialog backdrops) behave consistently when viewing @wordpress/ui stories. Extracted from WordPress#76837. Made-with: Cursor
The .popup structural class pairs with the unlayered :has(> .popup) rule, not a removed hasLayoutBox prop. Made-with: Cursor
ciampo
force-pushed
the
refactor/dataviews-ui-dialog
branch
from
9e0a4f9 to
6354875
Compare
This was referenced Apr 17, 2026
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.
Dependencies
stopPropagation()to Escape handler #76861AlertDialog#76937What?
Migrate all modals in
@wordpress/dataviewsfrom@wordpress/componentsModalto@wordpress/uiDialogandAlertDialog.See #76487 for the original assessment.
Why?
Dialogis the recommended replacement forModal. Migrating@wordpress/dataviewsvalidates the new API in a real-world consumer and moves the ecosystem towards@wordpress/ui.How?
ActionModal→ Dialog / AlertDialogDialog.Root+Dialog.PopupwithDialog.Header,Dialog.Title,Dialog.CloseIcon.hideModalHeader: true— Delete, Trash, Reset):AlertDialog.Root+Dialog.Popup.AlertDialog.Rootprovidesrole="alertdialog"semantics and blocks backdrop-click dismissal through the shared Base UIDialogStorecontext.Dialog.Popupis used (instead ofAlertDialog.Popup) because the actions provide their own buttons viaRenderModal.VisuallyHiddenrender prop whenhideModalHeaderis set.ModalContent(DataForm panel) → DialogDialog.Root+Dialog.PopupwithDialog.Header,Dialog.Footerfor Cancel/Apply buttons.Supporting changes
:root { --wp-ui-dialog-z-index }via SCSSz-index()function.modalSizetype: Added'stretch'and'full'; deprecated'fill'.@wordpress/edit-siteoverrides for duplicate-template-part / duplicate-pattern modals → replaced withmodalSize: 'small'on the action definitions. Removed.dataforms-layouts-panel__modal-footermargin rule and staledataforms-layouts-panel__modalclassName.Focus management mapping
The old
focusOnMountprop is mapped to Base UI'sinitialFocus:focusOnMountinitialFocustrue/'firstElement'/'firstContentElement'true— Dialog's smart default (first tabbable content element, skipping close icon)falsefalse'firstInputElement'contentReffor the firstinput/select/textareaDialog.Popup's smart default focus deprioritizes the close icon, which covers the'firstContentElement'case without a custom callback.Why AlertDialog.Root + Dialog.Popup (hybrid pattern)?
Destructive actions use
AlertDialog.Rootfor alertdialog semantics, butDialog.Popupfor rendering. This works because Base UI'sAlertDialogRootcreates aDialogStorewithrole: 'alertdialog'anddisablePointerDismissal: true, andDialog.Popupreads both values from the shared store context.AlertDialog.Popupis not used because it renders built-in confirm/cancel buttons, title, and description — which would conflict with the action'sRenderModalthat already provides its own buttons and content.This hybrid is a transitional step. A follow-up PR will introduce a new action modal API that maps destructive actions cleanly to
AlertDialog.Popup.Files changed
@wordpress/dataviewsdataviews-item-actions/index.tsxActionModal@wordpress/dataviewsdataviews-item-actions/style.scss--wp-ui-dialog-z-index@wordpress/dataviewsdataform-layouts/panel/modal.tsxModalContent@wordpress/dataviewsdataform-layouts/panel/style.scss@wordpress/dataviewstypes/dataviews.tsmodalSizeunion@wordpress/dataviewsdataviews-picker/stories/index.story.tsx@wordpress/edit-sitepage-patterns/style.scss@wordpress/fieldsduplicate-template-part.tsx,duplicate-pattern.tsxmodalSize: 'small'Testing Instructions
Keyboard testing
TODO / Follow-ups
New action modal API (separate PR)
The current action modal props (
RenderModal,hideModalHeader,modalHeader,modalSize,modalFocusOnMount) conflate concerns and force every action — including simple confirm/cancel flows — to provide full UI. A follow-up PR will introduce a newmodalproperty using a discriminated union:Proposed API design
type: 'dialog'→Dialog.Root+Dialog.Popupwith header/close icontype: 'confirm'→AlertDialog.Root+AlertDialog.Popupwith built-in confirm/cancel buttons, loading spinner, error handlingRenderModal,hideModalHeader, etc.) continue working with a deprecation warninguseDispatch) are replaceable byregistry.dispatch()(same pattern asActionButton.callback)RenderExtraserves as an escape hatch for hooks or extra UI inside confirm dialogsOther follow-ups
routes/post-list/quick-edit-modal.tsxto aDrawerin a separate PR.type: 'custom'variant and what constraints DataViews should enforce vs. delegate.@base-ui/reactmay break nested overlay stacking. Known cross-bundle limitation to monitor.Use of AI Tools
Cursor + Claude Opus 4.6