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: +188 kB (+2.43%) Total Size: 7.93 MB 📦 View Changed
ℹ️ View Unchanged
|
|
Flaky tests detected in 6e8b515. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/24150004486
|
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
…ess/ui Dialog Replace all usages of `@wordpress/components` `Modal` in `@wordpress/dataviews` with `@wordpress/ui` `Dialog` compound components. Key changes: - Migrate `ActionModal` to use `Dialog.Root`, `Dialog.Popup`, `Dialog.Header`, `Dialog.Title`, `Dialog.CloseIcon`, and `VisuallyHidden` for hidden headers. - Migrate `ModalContent` (DataForm panel) to use Dialog with `Dialog.Footer` for Cancel/Apply buttons. - Expose `initialFocus` and `finalFocus` props on `@wordpress/ui` `Dialog.Popup`. - Add `event.stopPropagation()` to Escape key handler in `@wordpress/compose` `useDialog` to prevent bubbling to parent Base UI overlays. - Set `--wp-ui-dialog-z-index` via SCSS `z-index()` function for backwards compatibility with existing overlay z-index hierarchy. - Add `'stretch'` and `'full'` to `modalSize` type; deprecate `'fill'`. - Add `modalSize: 'small'` to duplicate-template-part and duplicate-pattern actions, replacing CSS-based width overrides. - Remove CSS overrides targeting `.components-modal__frame` and `[role="document"]` in `@wordpress/edit-site`. - Remove `.dataforms-layouts-panel__modal-footer` margin-top rule. - Update `WithModal` story to use Dialog. Made-with: Cursor
Remove custom initialFocus workarounds that are no longer needed now that Dialog.Popup deprioritizes the close icon by default: - panel/modal.tsx: Remove focusFirstInput callback — Dialog's default focuses the first input (first non-close-icon tabbable in content). - dataviews-item-actions/index.tsx: Simplify useMapFocusOnMount — the 'firstContentElement' case now maps to Dialog's default. Only 'firstInputElement' still needs a custom callback. Made-with: Cursor
ciampo
force-pushed
the
refactor/dataviews-ui-dialog
branch
from
641b86b to
cec69a8
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
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?)
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