diff --git a/package-lock.json b/package-lock.json index 161a005bd7fa37..80c51b1964aa05 100644 --- a/package-lock.json +++ b/package-lock.json @@ -59594,6 +59594,7 @@ "@storybook/react-vite": "^10.2.8", "@testing-library/jest-dom": "^6.9.1", "@types/jest": "^29.5.14", + "@wordpress/jest-console": "file:../jest-console", "esbuild": "^0.27.2", "storybook": "^10.2.8" }, diff --git a/packages/dataviews/CHANGELOG.md b/packages/dataviews/CHANGELOG.md index 3122729d43562a..f6de5690bb42cd 100644 --- a/packages/dataviews/CHANGELOG.md +++ b/packages/dataviews/CHANGELOG.md @@ -2,11 +2,19 @@ ## Unreleased -## 14.2.0 (2026-04-29) - ### Enhancements -- DataForm: Render field `description` as help text in the `array` control.[#77554](https://github.com/WordPress/gutenberg/pull/77554) +- DataForm: `PanelModal`'s Apply button is now disabled while the in-progress form has validation errors, preventing invalid edits from being committed silently. Cancel remains enabled so users can always discard the draft. ([#78028](https://github.com/WordPress/gutenberg/pull/78028)) + +### Code Quality + +- DataViews: Item actions now share a single `ItemActionsMenu` host that renders the kebab `` and the per-action `Dialog.Root` as siblings. The dialog is hoisted out of `Menu.Popover`'s `unmountOnHide` subtree so a modal action stays mounted while the menu closes. `ButtonTrigger` (used in the inline-button path) gained `forwardRef` and spreads unknown props so it composes under `Dialog.Trigger`. A new `genericForwardRef` helper centralises the `forwardRef`-with-generics TypeScript workaround. ([#78028](https://github.com/WordPress/gutenberg/pull/78028)) + +### Breaking Changes + +- DataViews: Migrate action modals and DataForm panel modals from `@wordpress/components` `Modal` to `@wordpress/ui` `Dialog`. Action modals with `hideModalHeader: true` now render a `Dialog.Popup` with `role="alertdialog"` and `disablePointerDismissal`, requiring an explicit user action to dismiss. Custom CSS targeting `.components-modal__*` classes inside action modals will no longer work. The `dataforms-layouts-panel__modal` CSS class on the panel modal and the `dataforms-layouts-panel__modal-footer` CSS class have been removed. The `modalFocusOnMount` value `'firstElement'` now behaves like `'firstContentElement'` (the new Dialog primitive's smart default already skips the close icon and focuses the first content tabbable, so the legacy distinction between the two is no longer meaningful). ([#78028](https://github.com/WordPress/gutenberg/pull/78028)) + +## 14.2.0 (2026-04-29) ## 14.1.0 (2026-04-15) diff --git a/packages/dataviews/global.d.ts b/packages/dataviews/global.d.ts index 07d96c55dc3554..707804b8732422 100644 --- a/packages/dataviews/global.d.ts +++ b/packages/dataviews/global.d.ts @@ -3,3 +3,4 @@ // To ensure that global types are included, we need to // explicitly reference them here. import '@testing-library/jest-dom'; +import '@wordpress/jest-console'; diff --git a/packages/dataviews/package.json b/packages/dataviews/package.json index 87e7a6e5b2ea01..94c539bfa2b45d 100644 --- a/packages/dataviews/package.json +++ b/packages/dataviews/package.json @@ -79,6 +79,7 @@ "@storybook/react-vite": "^10.2.8", "@testing-library/jest-dom": "^6.9.1", "@types/jest": "^29.5.14", + "@wordpress/jest-console": "file:../jest-console", "esbuild": "^0.27.2", "storybook": "^10.2.8" }, diff --git a/packages/dataviews/src/components/dataform-layouts/panel/dropdown.tsx b/packages/dataviews/src/components/dataform-layouts/panel/dropdown.tsx index 74c1f7c193b796..6f6d7354677eb0 100644 --- a/packages/dataviews/src/components/dataform-layouts/panel/dropdown.tsx +++ b/packages/dataviews/src/components/dataform-layouts/panel/dropdown.tsx @@ -154,6 +154,7 @@ function PanelDropdown< Item >( { disabled={ fieldDefinition.readOnly === true } onClick={ onToggle } aria-expanded={ isOpen } + aria-haspopup="true" /> ) } renderContent={ ( { onClose } ) => ( diff --git a/packages/dataviews/src/components/dataform-layouts/panel/modal.tsx b/packages/dataviews/src/components/dataform-layouts/panel/modal.tsx index cd7caee2d7150d..9265a445ffe6a8 100644 --- a/packages/dataviews/src/components/dataform-layouts/panel/modal.tsx +++ b/packages/dataviews/src/components/dataform-layouts/panel/modal.tsx @@ -6,15 +6,9 @@ import deepMerge from 'deepmerge'; /** * WordPress dependencies */ -import { - __experimentalSpacer as Spacer, - Button, - Modal, -} from '@wordpress/components'; - import { useContext, useMemo, useRef, useState } from '@wordpress/element'; -import { useFocusOnMount, useMergeRefs } from '@wordpress/compose'; -import { Stack } from '@wordpress/ui'; +// eslint-disable-next-line @wordpress/use-recommended-components +import { Dialog } from '@wordpress/ui'; /** * Internal dependencies @@ -31,22 +25,21 @@ import { DataFormLayout } from '../data-form-layout'; import { DEFAULT_LAYOUT } from '../normalize-form'; import SummaryButton from './summary-button'; import useFormValidity from '../../../hooks/use-form-validity'; +import useMapFocusOnMount from '../../../hooks/use-map-focus-on-mount'; import useReportValidity from '../../../hooks/use-report-validity'; import DataFormContext from '../../dataform-context'; import useFieldFromFormField from './utils/use-field-from-form-field'; -function ModalContent< Item >( { +function PanelDialogContent< Item >( { data, field, onChange, fieldLabel, - onClose, touched, }: { data: Item; field: NormalizedFormField; onChange: ( data: Partial< Item > ) => void; - onClose: () => void; fieldLabel: string; touched: boolean; } ) { @@ -54,6 +47,7 @@ function ModalContent< Item >( { const { applyLabel, cancelLabel } = openAs as PanelOpenAsModal; const { fields } = useContext( DataFormContext ); const [ changes, setChanges ] = useState< Partial< Item > >( {} ); + const modalData = useMemo( () => { return deepMerge( data, changes, { arrayMerge: ( target, source ) => source, @@ -84,12 +78,11 @@ function ModalContent< Item >( { maxLength: f.isValid.maxLength?.constraint, }, } ) ); - const { validity } = useFormValidity( modalData, fieldsAsFieldType, form ); - - const onApply = () => { - onChange( changes ); - onClose(); - }; + const { validity, isValid } = useFormValidity( + modalData, + fieldsAsFieldType, + form + ); const handleOnChange = ( newValue: Partial< Item > ) => { setChanges( ( prev ) => @@ -99,23 +92,21 @@ function ModalContent< Item >( { ); }; - const focusOnMountRef = useFocusOnMount( 'firstInputElement' ); const contentRef = useRef< HTMLDivElement >( null ); - const mergedRef = useMergeRefs( [ focusOnMountRef, contentRef ] ); // When the modal is opened after being previously closed (touched), // trigger reportValidity to show field-level errors. useReportValidity( contentRef, touched ); + const initialFocus = useMapFocusOnMount( 'firstInputElement', contentRef ); + return ( - -
+ + + { fieldLabel } + + + ( { /> ) } -
- - - - - - + + + ); } @@ -172,8 +151,14 @@ function PanelModal< Item >( { validity, }: FieldLayoutProps< Item > ) { const [ touched, setTouched ] = useState( false ); - - const [ isOpen, setIsOpen ] = useState( false ); + // `Dialog.Root` stays mounted across opens, so the session component + // holding the in-progress `changes` state would also persist by default. + // Bump `sessionKey` on `onOpenChangeComplete` (when the exit animation + // finishes) to force-remount `` between sessions — + // this preserves the existing "Cancel/close always wipes the draft" + // semantic without disturbing the form contents during the exit + // animation itself. + const [ sessionKey, setSessionKey ] = useState( 0 ); const { fieldDefinition, fieldLabel, summaryFields } = useFieldFromFormField( field ); @@ -181,35 +166,45 @@ function PanelModal< Item >( { return null; } - const handleClose = () => { - setIsOpen( false ); - setTouched( true ); - }; - return ( - <> - { + if ( ! open ) { + // Mark the field as "touched" once the dialog has been + // opened and dismissed at least once, so validation + // messages on the summary trigger the next time the + // user opens it. + setTouched( true ); + } + } } + onOpenChangeComplete={ ( open ) => { + if ( ! open ) { + setSessionKey( ( k ) => k + 1 ); + } + } } + > + + } + /> + setIsOpen( true ) } - aria-expanded={ isOpen } /> - { isOpen && ( - - ) } - + ); } diff --git a/packages/dataviews/src/components/dataform-layouts/panel/style.scss b/packages/dataviews/src/components/dataform-layouts/panel/style.scss index 86725adcae452b..50932940a0bc60 100644 --- a/packages/dataviews/src/components/dataform-layouts/panel/style.scss +++ b/packages/dataviews/src/components/dataform-layouts/panel/style.scss @@ -159,10 +159,6 @@ margin-bottom: $grid-unit-20; } -.dataforms-layouts-panel__modal-footer { - margin-top: $grid-unit-20; -} - .components-popover.components-dropdown__content.dataforms-layouts-panel__field-dropdown { z-index: z-index(".components-popover.components-dropdown__content.dataforms-layouts-panel__field-dropdown"); } diff --git a/packages/dataviews/src/components/dataform-layouts/panel/summary-button.tsx b/packages/dataviews/src/components/dataform-layouts/panel/summary-button.tsx index ad748f0b010804..f13ac7e315e1fd 100644 --- a/packages/dataviews/src/components/dataform-layouts/panel/summary-button.tsx +++ b/packages/dataviews/src/components/dataform-layouts/panel/summary-button.tsx @@ -2,6 +2,13 @@ * External dependencies */ import clsx from 'clsx'; +import type { + AriaAttributes, + MouseEventHandler, + ReactElement, + Ref, + SyntheticEvent, +} from 'react'; /** * WordPress dependencies @@ -10,7 +17,7 @@ import { Button, Icon, Tooltip } from '@wordpress/components'; import { sprintf, _x } from '@wordpress/i18n'; import { error as errorIcon, pencil } from '@wordpress/icons'; import { useInstanceId } from '@wordpress/compose'; -import { useRef } from '@wordpress/element'; +import { forwardRef, useRef } from '@wordpress/element'; /** * Internal dependencies @@ -25,17 +32,7 @@ import getLabelClassName from './utils/get-label-classname'; import getLabelContent from './utils/get-label-content'; import getFirstValidationError from './utils/get-first-validation-error'; -export default function SummaryButton< Item >( { - data, - field, - fieldLabel, - summaryFields, - validity, - touched, - disabled, - onClick, - 'aria-expanded': ariaExpanded, -}: { +interface SummaryButtonProps< Item > { data: Item; field: NormalizedFormField; fieldLabel?: string; @@ -43,9 +40,64 @@ export default function SummaryButton< Item >( { validity?: FieldValidity; touched: boolean; disabled?: boolean; - onClick: () => void; + /* + * Click handler invoked from both the row's pointer interaction + * (`handleRowClick`) and the keyboard handler (`handleKeyDown`). + * Typed as `SyntheticEvent` because keyboard activation forwards the + * `KeyboardEvent` here without synthesising a `MouseEvent`. When + * `SummaryButton` is composed with `Dialog.Trigger` via the render-prop + * pattern, the trigger primitive injects its open-toggle handler here + * at runtime; otherwise the parent supplies the click handler + * directly (e.g. the `Dropdown` `renderToggle` callback). + */ + onClick?: ( event: SyntheticEvent ) => void; + /* + * The three `aria-*` props below are routed onto the inner pencil + * ` ); -} +} ); const EMPTY_ARRAY: [] = []; @@ -235,7 +241,6 @@ function ActionButton< Item >( { key={ action.id } action={ action } items={ selectedEligibleItems } - ActionTriggerComponent={ ActionTrigger } /> ); } diff --git a/packages/dataviews/src/components/dataviews-item-actions/index.tsx b/packages/dataviews/src/components/dataviews-item-actions/index.tsx index 26098143baf356..c56aada982d2e8 100644 --- a/packages/dataviews/src/components/dataviews-item-actions/index.tsx +++ b/packages/dataviews/src/components/dataviews-item-actions/index.tsx @@ -1,42 +1,67 @@ /** * External dependencies */ -import type { MouseEventHandler } from 'react'; +import type { MouseEventHandler, ReactElement } from 'react'; /** * WordPress dependencies */ import { Button, - Modal, privateApis as componentsPrivateApis, } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; -import { useMemo, useState } from '@wordpress/element'; +import { useCallback, useMemo, useRef, useState } from '@wordpress/element'; import { moreVertical } from '@wordpress/icons'; import { useRegistry } from '@wordpress/data'; import { useViewportMatch } from '@wordpress/compose'; -import { Stack } from '@wordpress/ui'; +// eslint-disable-next-line @wordpress/use-recommended-components +import { Dialog, Stack, VisuallyHidden } from '@wordpress/ui'; /** * Internal dependencies */ import { unlock } from '../../lock-unlock'; import type { Action, ActionModal as ActionModalType } from '../../types'; +import useMapFocusOnMount from '../../hooks/use-map-focus-on-mount'; +import genericForwardRef from '../../utils/generic-forward-ref'; +import getActionLabel from '../../utils/get-action-label'; const { Menu, kebabCase } = unlock( componentsPrivateApis ); export interface ActionTriggerProps< Item > { action: Action< Item >; - onClick: MouseEventHandler; + /** + * Click handler for direct usage. When the trigger is wrapped in a + * primitive that injects its own `onClick` via the render-prop pattern + * (e.g. `Dialog.Trigger render={ }`), the wrapper + * supplies the click handler and this prop should be omitted. + */ + onClick?: MouseEventHandler; isBusy?: boolean; items: Item[]; variant?: 'primary' | 'secondary' | 'tertiary' | 'link'; } export interface ActionModalProps< Item > { + /** + * The action whose modal should be rendered. Stable for the lifetime of + * this `ActionModal` instance — the parent renders one `ActionModal` + * per modal action; opening/closing happens through the surrounding + * `` (controlled or uncontrolled) rather than props on + * this component. + */ action: ActionModalType< Item >; items: Item[]; + /** + * Imperative close callback exposed to the action's `RenderModal` + * implementation as `closeModal`. The wrapping component (e.g. + * `ModalActionMenuItem` / `ModalActionInlineButton`) owns the + * dialog's open state and supplies a setter that toggles it back to + * `false`. Public `RenderModalProps` consumers may call this from + * async code (e.g. after a network request) — that's why a callback + * is required even though the dialog's primitives can also close it. + */ closeModal: () => void; } @@ -44,7 +69,14 @@ interface ActionsMenuGroupProps< Item > { actions: Action< Item >[]; item: Item; registry: ReturnType< typeof useRegistry >; - setActiveModalAction: ( action: ActionModalType< Item > | null ) => void; + /** + * Invoked when the user selects a modal action from the menu. The + * caller is expected to render a sibling `` outside this + * menu that hosts the action's popup body — keeping the dialog out of + * the `Menu.Popover`'s `unmountOnHide` subtree so it survives the + * menu's exit transition. + */ + onModalAction: ( action: ActionModalType< Item > ) => void; } interface ItemActionsProps< Item > { @@ -67,34 +99,44 @@ interface PrimaryActionsProps< Item > { buttonVariant?: 'primary' | 'secondary' | 'tertiary' | 'link'; } -function ButtonTrigger< Item >( { - action, - onClick, - items, - variant, -}: ActionTriggerProps< Item > ) { - const label = - typeof action.label === 'string' ? action.label : action.label( items ); +// `ButtonTrigger` forwards refs and unknown props onto its underlying +// `Button`, so it can be used directly (parent supplies `onClick`) or +// composed via render props +// (e.g. ` } />`). +const ButtonTrigger = genericForwardRef( function ButtonTrigger< Item >( + { action, items, variant, ...rest }: ActionTriggerProps< Item >, + ref: React.Ref< HTMLButtonElement > +) { + const label = getActionLabel( action, items ); return ( ); -} +} ); +// `MenuItemTrigger` is always rendered as a child of `` +// and never composed under `` — modal actions hoist +// their `Dialog.Root` outside the menu (see `ItemActionsMenu` below) so +// the `Menu.Item` only needs to fire its own `onClick`. No `forwardRef`, +// no `render` prop forwarding, no generic cast. function MenuItemTrigger< Item >( { action, - onClick, items, -}: ActionTriggerProps< Item > ) { - const label = - typeof action.label === 'string' ? action.label : action.label( items ); + onClick, +}: { + action: Action< Item >; + items: Item[]; + onClick: () => void; +} ) { + const label = getActionLabel( action, items ); return ( { label } @@ -102,31 +144,105 @@ function MenuItemTrigger< Item >( { ); } +function mapModalSize( + size: ActionModalType< unknown >[ 'modalSize' ] +): 'small' | 'medium' | 'large' | 'full' { + if ( size === 'fill' ) { + return 'full'; + } + return size ?? 'medium'; +} + +// Renders the popup half of a dataviews action modal. Must be wrapped +// in a `` that owns the open lifecycle (paired with +// `` at the call site, e.g. `ModalActionMenuItem` or +// `ModalActionInlineButton`). export function ActionModal< Item >( { action, items, closeModal, }: ActionModalProps< Item > ) { - const label = - typeof action.label === 'string' ? action.label : action.label( items ); + const contentRef = useRef< HTMLDivElement >( null ); + const initialFocus = useMapFocusOnMount( + action.modalFocusOnMount ?? true, + contentRef + ); + const label = getActionLabel( action, items ); const modalHeader = typeof action.modalHeader === 'function' ? action.modalHeader( items ) : action.modalHeader; + const title = modalHeader || label; + return ( - + } + initialFocus={ initialFocus } + { ...( action.hideModalHeader && { + role: 'alertdialog' as const, + } ) } > - - + { action.hideModalHeader ? ( + { title } } + /> + ) : ( + + { title } + + + ) } + + + + + ); +} + +// Wraps a single modal action as an inline button that opens the +// action's dialog. The `Dialog.Root` lives at this call site (outside +// any host that unmounts on click — `Menu.Popover` is the relevant one +// in the menu path, see `ItemActionsMenu`) so its lifecycle is not +// affected by surrounding popover transitions. +function ModalActionInlineButton< Item >( { + action, + items, + variant, +}: { + action: ActionModalType< Item >; + items: Item[]; + variant?: 'primary' | 'secondary' | 'tertiary' | 'link'; +} ) { + const [ open, setOpen ] = useState( false ); + const closeModal = useCallback( () => setOpen( false ), [] ); + return ( + + + } + /> + + ); } @@ -134,7 +250,7 @@ export function ActionsMenuGroup< Item >( { actions, item, registry, - setActiveModalAction, + onModalAction, }: ActionsMenuGroupProps< Item > ) { const { primaryActions, regularActions } = useMemo( () => { return actions.reduce( @@ -157,13 +273,11 @@ export function ActionsMenuGroup< Item >( { { - if ( 'RenderModal' in action ) { - setActiveModalAction( action ); - return; - } - action.callback( [ item ], { registry } ); - } } + onClick={ + 'RenderModal' in action + ? () => onModalAction( action ) + : () => action.callback( [ item ], { registry } ) + } items={ [ item ] } /> ) ); @@ -176,6 +290,84 @@ export function ActionsMenuGroup< Item >( { ); } +// Hosts a kebab-style menu of item actions plus the `Dialog.Root` that +// owns the active modal action's popup body. The dialog is rendered as +// a sibling of `` (not as a descendant of `Menu.Popover`) so it +// survives the menu's `unmountOnHide` exit transition. Both +// `CompactItemActions` and the list-layout's per-row menu use this +// component; they only differ in the trigger button passed in via +// `renderTrigger`. +export function ItemActionsMenu< Item >( { + item, + actions, + registry, + renderTrigger, +}: { + item: Item; + actions: Action< Item >[]; + registry: ReturnType< typeof useRegistry >; + renderTrigger: ReactElement; +} ) { + const [ activeModalAction, setActiveModalAction ] = + useState< ActionModalType< Item > | null >( null ); + // Snapshot of the most recently active action — kept around so the + // popup body stays mounted through the exit animation and is then + // cleared from `onOpenChangeComplete` once the close transition + // finishes (mirrors the `sessionKey` / defensive-setter patterns in + // `PanelModal` and the page-templates duplicate dialog). + const [ lastActiveModalAction, setLastActiveModalAction ] = + useState< ActionModalType< Item > | null >( null ); + const renderedAction = activeModalAction ?? lastActiveModalAction; + const closeModal = useCallback( () => setActiveModalAction( null ), [] ); + const onModalAction = useCallback( ( action: ActionModalType< Item > ) => { + setActiveModalAction( action ); + setLastActiveModalAction( action ); + }, [] ); + + return ( + <> + + + + + + + { + if ( ! open ) { + setActiveModalAction( null ); + } + } } + onOpenChangeComplete={ ( open ) => { + if ( ! open ) { + setLastActiveModalAction( null ); + } + } } + disablePointerDismissal={ renderedAction?.hideModalHeader } + > + { renderedAction && ( + // `key` per action.id force-remounts the popup body when + // the user closes one action and opens a different one, + // preventing the new action's `RenderModal` from + // inheriting the previous session's state. + + ) } + + + ); +} + export default function ItemActions< Item >( { item, actions, @@ -245,41 +437,22 @@ function CompactItemActions< Item >( { isSmall, registry, }: CompactItemActionsProps< Item > ) { - const [ activeModalAction, setActiveModalAction ] = useState( - null as ActionModalType< Item > | null - ); return ( - <> - - - } - /> - - - - - { !! activeModalAction && ( - setActiveModalAction( null ) } + - ) } - + } + /> ); } @@ -289,7 +462,6 @@ export function PrimaryActions< Item >( { registry, buttonVariant, }: PrimaryActionsProps< Item > ) { - const [ activeModalAction, setActiveModalAction ] = useState( null as any ); const isMobileViewport = useViewportMatch( 'medium', '<' ); if ( isMobileViewport ) { @@ -301,27 +473,25 @@ export function PrimaryActions< Item >( { } return ( <> - { actions.map( ( action ) => ( - { - if ( 'RenderModal' in action ) { - setActiveModalAction( action ); - return; + { actions.map( ( action ) => + 'RenderModal' in action ? ( + + ) : ( + + action.callback( [ item ], { registry } ) } - action.callback( [ item ], { registry } ); - } } - items={ [ item ] } - variant={ buttonVariant } - /> - ) ) } - { !! activeModalAction && ( - setActiveModalAction( null ) } - /> + items={ [ item ] } + variant={ buttonVariant } + /> + ) ) } ); diff --git a/packages/dataviews/src/components/dataviews-item-actions/style.scss b/packages/dataviews/src/components/dataviews-item-actions/style.scss index dbd4d7944e57a5..0338df97a72e9c 100644 --- a/packages/dataviews/src/components/dataviews-item-actions/style.scss +++ b/packages/dataviews/src/components/dataviews-item-actions/style.scss @@ -1,10 +1,4 @@ @use "@wordpress/base-styles/variables" as *; -@use "@wordpress/base-styles/colors" as *; -@use "@wordpress/base-styles/z-index" as *; - -.dataviews-action-modal { - z-index: z-index(".dataviews-action-modal"); -} // TODO: the way forward here would be to use the new unstyle button coming // from wordpress/ui package, but we're not ready to use it yet. diff --git a/packages/dataviews/src/components/dataviews-item-actions/test/index.tsx b/packages/dataviews/src/components/dataviews-item-actions/test/index.tsx new file mode 100644 index 00000000000000..7df61b1092b2af --- /dev/null +++ b/packages/dataviews/src/components/dataviews-item-actions/test/index.tsx @@ -0,0 +1,368 @@ +/** + * External dependencies + */ +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +// `Dialog.Popup` only reflects `size` through CSS-module class names, which +// are stripped in unit tests. Wrap the real `Dialog.Popup` so we can assert +// what `ActionModal` forwards across the boundary while still rendering the +// real popup behaviour (focus, role, backdrop, portal). Must be defined +// before the module is imported below — `jest.mock` is hoisted, so the +// captured spy needs a `mock`-prefixed name to satisfy the babel hoist +// guard. +const mockDialogPopupSpy = jest.fn(); +jest.mock( '@wordpress/ui', () => { + const actual = jest.requireActual( '@wordpress/ui' ); + const { forwardRef, createElement } = + jest.requireActual( '@wordpress/element' ); + const PopupSpy = forwardRef( + ( props: Record< string, unknown >, ref: unknown ) => { + mockDialogPopupSpy( props ); + return createElement( actual.Dialog.Popup, { ...props, ref } ); + } + ); + return { + ...actual, + Dialog: { ...actual.Dialog, Popup: PopupSpy }, + }; +} ); + +/** + * WordPress dependencies + */ +// eslint-disable-next-line @wordpress/use-recommended-components +import { Dialog } from '@wordpress/ui'; + +/** + * Internal dependencies + */ +import ItemActions, { ActionModal } from '../index'; +import type { ActionModal as ActionModalType } from '../../../types'; + +type TestItem = { id: number; title: string }; + +function createAction( + overrides: Partial< ActionModalType< TestItem > > = {} +): ActionModalType< TestItem > { + return { + id: 'test-action', + label: 'Test Action', + RenderModal: ( { closeModal } ) => ( +
+

Modal content

+ +
+ ), + ...overrides, + }; +} + +// Renders an `` inside a controlled `` so each +// test can drive the open state through `onOpenChange` (mirroring how +// the dataviews call sites — `ModalActionMenuItem`, +// `ModalActionInlineButton`, `PrimaryActionGridCell`, `ActionWithModal` +// — own the dialog state). +function renderActionModal( { + action, + items, + open = true, + onOpenChange = jest.fn(), +}: { + action: ActionModalType< TestItem >; + items: TestItem[]; + open?: boolean; + onOpenChange?: ( open: boolean ) => void; +} ) { + const closeModal = () => onOpenChange( false ); + return render( + onOpenChange( isOpen ) } + disablePointerDismissal={ action.hideModalHeader } + > + + + ); +} + +describe( 'ActionModal', () => { + it( 'renders with a dialog role by default', async () => { + const action = createAction(); + + renderActionModal( { + action, + items: [ { id: 1, title: 'Item' } ], + } ); + + await waitFor( () => { + expect( screen.getByRole( 'dialog' ) ).toBeVisible(); + } ); + } ); + + it( 'renders with an alertdialog role when hideModalHeader is true', async () => { + const action = createAction( { hideModalHeader: true } ); + + renderActionModal( { + action, + items: [ { id: 1, title: 'Item' } ], + } ); + + await waitFor( () => { + expect( screen.getByRole( 'alertdialog' ) ).toBeVisible(); + } ); + } ); + + it( 'focuses the first input when modalFocusOnMount is "firstInputElement"', async () => { + const action = createAction( { + modalFocusOnMount: 'firstInputElement', + RenderModal: () => ( +
+

Some text

+ + +
+ ), + } ); + + renderActionModal( { + action, + items: [ { id: 1, title: 'Item' } ], + } ); + + await waitFor( () => { + expect( screen.getByTestId( 'first-input' ) ).toHaveFocus(); + } ); + } ); + + it( 'falls back to the popup smart default when modalFocusOnMount is unset', async () => { + // `Dialog.Popup`'s default focus-on-mount lands on the first + // content tabbable rather than the close icon. + const action = createAction( { + RenderModal: () => ( +
+

Some text

+ +
+ ), + } ); + + renderActionModal( { + action, + items: [ { id: 1, title: 'Item' } ], + } ); + + await waitFor( () => { + expect( screen.getByTestId( 'content-button' ) ).toHaveFocus(); + } ); + } ); + + it.each( [ 'small', 'medium', 'large' ] as const )( + 'forwards modalSize %p to Dialog.Popup without emitting a deprecation warning', + async ( modalSize ) => { + const action = createAction( { modalSize } ); + + renderActionModal( { + action, + items: [ { id: 1, title: 'Item' } ], + } ); + + await screen.findByRole( 'dialog' ); + expect( console ).not.toHaveWarned(); + } + ); + + it( "translates modalSize 'fill' to Dialog.Popup size 'full' silently", async () => { + mockDialogPopupSpy.mockClear(); + const action = createAction( { modalSize: 'fill' } ); + + renderActionModal( { + action, + items: [ { id: 1, title: 'Item' } ], + } ); + + await screen.findByRole( 'dialog' ); + expect( mockDialogPopupSpy ).toHaveBeenCalledWith( + expect.objectContaining( { size: 'full' } ) + ); + expect( console ).not.toHaveWarned(); + } ); + + it( 'renders the popup inside the dataviews-action-modal__portal element', async () => { + const action = createAction(); + + renderActionModal( { + action, + items: [ { id: 1, title: 'Item' } ], + } ); + + const dialog = await screen.findByRole( 'dialog' ); + // The portal is a structural CSS wrapper with no semantic role, so + // there's no Testing Library query that can reach it directly. + // Walking up the tree is the most precise way to assert the popup + // renders inside the scoped portal that owns the per-instance + // `--wp-ui-dialog-z-index` override. + expect( + // eslint-disable-next-line testing-library/no-node-access + dialog.closest( '.dataviews-action-modal__portal' ) + ).not.toBeNull(); + } ); + + it( 'closes when the user presses Escape', async () => { + const user = userEvent.setup(); + const onOpenChange = jest.fn(); + const action = createAction(); + + renderActionModal( { + action, + items: [ { id: 1, title: 'Item' } ], + onOpenChange, + } ); + + await screen.findByRole( 'dialog' ); + await user.keyboard( '{Escape}' ); + + expect( onOpenChange ).toHaveBeenCalledWith( false ); + } ); + + it( 'closes when the user clicks the backdrop by default', async () => { + const user = userEvent.setup(); + const onOpenChange = jest.fn(); + const action = createAction(); + + renderActionModal( { + action, + items: [ { id: 1, title: 'Item' } ], + onOpenChange, + } ); + + await screen.findByRole( 'dialog' ); + const backdrop = screen.getByTestId( 'dialog-backdrop' ); + await user.click( backdrop ); + + expect( onOpenChange ).toHaveBeenCalledWith( false ); + } ); + + it( 'does not close on backdrop click for alert dialogs (hideModalHeader)', async () => { + const user = userEvent.setup(); + const onOpenChange = jest.fn(); + const action = createAction( { hideModalHeader: true } ); + + renderActionModal( { + action, + items: [ { id: 1, title: 'Item' } ], + onOpenChange, + } ); + + await screen.findByRole( 'alertdialog' ); + const backdrop = screen.getByTestId( 'dialog-backdrop' ); + await user.click( backdrop ); + + expect( onOpenChange ).not.toHaveBeenCalled(); + } ); + + it( 'invokes onOpenChange(false) when the RenderModal calls closeModal', async () => { + const user = userEvent.setup(); + const onOpenChange = jest.fn(); + const action = createAction(); + + renderActionModal( { + action, + items: [ { id: 1, title: 'Item' } ], + onOpenChange, + } ); + + await screen.findByRole( 'dialog' ); + await user.click( screen.getByRole( 'button', { name: /done/i } ) ); + + expect( onOpenChange ).toHaveBeenCalledWith( false ); + } ); +} ); + +describe( 'ItemActions — kebab menu → modal action', () => { + // Regression coverage for the composition between `Menu.Popover` + // (`unmountOnHide` + `Menu.Item.hideOnClick`) and the per-action + // `Dialog.Root` that owns each modal action's open state. The bug: + // hosting `Dialog.Root` inside the compact menu's popover means the + // dialog mounts and immediately unmounts when the menu hides on + // `Menu.Item` click, so consumers can't reach the modal body. These + // tests assert that selecting a modal action from the kebab menu + // opens its dialog and that the dialog body remains interactive long + // enough to dispatch its own actions. + const item = { id: 1, title: 'Item' }; + + function createMenuModalAction( + overrides: Partial< ActionModalType< TestItem > > = {} + ): ActionModalType< TestItem > { + return { + id: 'menu-modal-action', + label: 'Menu modal action', + RenderModal: ( { closeModal } ) => ( +
+

Menu modal content

+ +
+ ), + ...overrides, + }; + } + + it( 'opens the dialog when a modal action is selected from the kebab menu', async () => { + const user = userEvent.setup(); + const action = createMenuModalAction(); + + render( + + ); + + await user.click( screen.getByRole( 'button', { name: /actions/i } ) ); + await user.click( + screen.getByRole( 'menuitem', { name: /menu modal action/i } ) + ); + + // The dialog body must be mounted and reachable after the menu + // closes; on the buggy host it unmounts together with the menu + // popover and the assertion times out. + await waitFor( () => { + expect( + screen.getByTestId( 'menu-modal-content' ) + ).toBeInTheDocument(); + } ); + expect( + screen.getByRole( 'button', { name: /done/i } ) + ).toBeInTheDocument(); + } ); + + it( 'keeps the dialog interactive — closeModal from the body still dismisses it', async () => { + const user = userEvent.setup(); + const action = createMenuModalAction(); + + render( + + ); + + await user.click( screen.getByRole( 'button', { name: /actions/i } ) ); + await user.click( + screen.getByRole( 'menuitem', { name: /menu modal action/i } ) + ); + + const doneButton = await screen.findByRole( 'button', { + name: /done/i, + } ); + await user.click( doneButton ); + + await waitFor( () => { + expect( + screen.queryByTestId( 'menu-modal-content' ) + ).not.toBeInTheDocument(); + } ); + } ); +} ); diff --git a/packages/dataviews/src/components/dataviews-layouts/list/index.tsx b/packages/dataviews/src/components/dataviews-layouts/list/index.tsx index 573f026c832edf..4860c23783fbc1 100644 --- a/packages/dataviews/src/components/dataviews-layouts/list/index.tsx +++ b/packages/dataviews/src/components/dataviews-layouts/list/index.tsx @@ -7,12 +7,7 @@ import clsx from 'clsx'; * WordPress dependencies */ import { useInstanceId, usePrevious } from '@wordpress/compose'; -import { - Button, - privateApis as componentsPrivateApis, - Spinner, - Composite, -} from '@wordpress/components'; +import { Button, Spinner, Composite } from '@wordpress/components'; import { useCallback, useEffect, @@ -24,13 +19,13 @@ import { import { __, sprintf } from '@wordpress/i18n'; import { moreVertical } from '@wordpress/icons'; import { useRegistry } from '@wordpress/data'; -import { Stack, VisuallyHidden } from '@wordpress/ui'; +// eslint-disable-next-line @wordpress/use-recommended-components +import { Dialog, Stack, VisuallyHidden } from '@wordpress/ui'; /** * Internal dependencies */ -import { unlock } from '../../../lock-unlock'; -import { ActionsMenuGroup, ActionModal } from '../../dataviews-item-actions'; +import { ActionModal, ItemActionsMenu } from '../../dataviews-item-actions'; import DataViewsContext from '../../dataviews-context'; import { useDelayedLoading } from '../../../hooks/use-delayed-loading'; import type { @@ -38,9 +33,9 @@ import type { NormalizedField, ViewList as ViewListType, ViewListProps, - ActionModal as ActionModalType, } from '../../../types'; import getDataByGroup from '../utils/get-data-by-group'; +import getActionLabel from '../../../utils/get-action-label'; interface ListViewItemProps< Item > { view: ViewListType; @@ -57,8 +52,6 @@ interface ListViewItemProps< Item > { posinset?: number; } -const { Menu } = unlock( componentsPrivateApis ); - function generateItemWrapperCompositeId( idPrefix: string ) { return `${ idPrefix }-item-wrapper`; } @@ -83,41 +76,52 @@ function PrimaryActionGridCell< Item >( { } ) { const registry = useRegistry(); const [ isModalOpen, setIsModalOpen ] = useState( false ); + const closeModal = useCallback( () => setIsModalOpen( false ), [] ); const compositeItemId = generatePrimaryActionCompositeId( idPrefix, primaryAction.id ); - const label = - typeof primaryAction.label === 'string' - ? primaryAction.label - : primaryAction.label( [ item ] ); + const label = getActionLabel( primaryAction, [ item ] ); - return 'RenderModal' in primaryAction ? ( -
- setIsModalOpen( true ) } + if ( 'RenderModal' in primaryAction ) { + // Compose `Composite.Item` → `Dialog.Trigger` → `Button` so all + // three layers' props merge onto the same DOM button: + // composite-item navigation, dialog-trigger ARIA, and the + // visual `Button` styling. + return ( +
+ + + } + /> + } /> - } - > - { isModalOpen && ( action={ primaryAction } items={ [ item ] } - closeModal={ () => setIsModalOpen( false ) } + closeModal={ closeModal } /> - ) } - -
- ) : ( + +
+ ); + } + return (
( { const registry = useRegistry(); const [ isHovered, setIsHovered ] = useState( false ); - const [ activeModalAction, setActiveModalAction ] = useState( - null as ActionModalType< Item > | null - ); const handleHover: React.MouseEventHandler = ( { type } ) => { const isHover = type === 'mouseenter'; setIsHovered( isHover ); @@ -235,44 +236,28 @@ function ListItem< Item >( { ) } { ! hasOnlyOnePrimaryAction && (
- - - } - /> - } - /> - - + } /> - - - { !! activeModalAction && ( - setActiveModalAction( null ) } - /> - ) } + } + />
) } diff --git a/packages/dataviews/src/dataform/test/dataform.tsx b/packages/dataviews/src/dataform/test/dataform.tsx index 972c613d588267..b26849b21c650a 100644 --- a/packages/dataviews/src/dataform/test/dataform.tsx +++ b/packages/dataviews/src/dataform/test/dataform.tsx @@ -1,7 +1,7 @@ /** * External dependencies */ -import { render, screen } from '@testing-library/react'; +import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; /** @@ -354,8 +354,14 @@ describe( 'DataForm component', () => { } ); await user.click( cancelButton ); - // Modal should be closed - expect( screen.queryByRole( 'dialog' ) ).not.toBeInTheDocument(); + // Modal should be closed once the exit transition completes. + // `Dialog.Root` stays mounted in the React tree and the popup + // unmounts asynchronously after the close animation resolves. + await waitFor( () => { + expect( + screen.queryByRole( 'dialog' ) + ).not.toBeInTheDocument(); + } ); } ); it( 'should apply changes and close modal when apply button is clicked', async () => { @@ -398,11 +404,90 @@ describe( 'DataForm component', () => { } ); await user.click( applyButton ); - // Modal should be closed and onChange should be called - expect( screen.queryByRole( 'dialog' ) ).not.toBeInTheDocument(); + // Modal should be closed once the exit transition completes. + // `Dialog.Root` stays mounted in the React tree and the popup + // unmounts asynchronously after the close animation resolves. + await waitFor( () => { + expect( + screen.queryByRole( 'dialog' ) + ).not.toBeInTheDocument(); + } ); expect( onChange ).toHaveBeenCalledWith( { title: 'New Title' } ); } ); + it( 'should disable Apply when the form is invalid (Cancel stays enabled)', async () => { + const onChange = jest.fn(); + const requiredTitleFields = fields.map( ( field ) => + field.id === 'title' + ? { ...field, isValid: { required: true } } + : field + ); + const formWithRequiredTitle = { + fields: [ 'title' ], + layout: { + type: 'panel', + labelPosition: 'side', + openAs: 'modal', + } as const, + }; + + render( + + ); + + const user = await userEvent.setup(); + await user.click( fieldsSelector.title.view() ); + + expect( screen.getByRole( 'dialog' ) ).toBeInTheDocument(); + + // Apply is enabled while the title is non-empty. + const applyButton = screen.getByRole( 'button', { + name: /apply/i, + } ); + const cancelButton = screen.getByRole( 'button', { + name: /cancel/i, + } ); + expect( applyButton ).not.toHaveAttribute( + 'aria-disabled', + 'true' + ); + expect( cancelButton ).not.toHaveAttribute( + 'aria-disabled', + 'true' + ); + + // Clear the title to violate the `required` constraint. + await user.clear( fieldsSelector.title.edit() ); + + // Apply is disabled, Cancel stays enabled — users must always + // be able to discard the draft. `Dialog.Action` uses the + // focusable-when-disabled pattern (`aria-disabled="true"`) + // rather than the native `disabled` attribute, so assert on + // the ARIA state and on the functional consequence (click does + // not dispatch the change). + expect( applyButton ).toHaveAttribute( 'aria-disabled', 'true' ); + expect( cancelButton ).not.toHaveAttribute( + 'aria-disabled', + 'true' + ); + await user.click( applyButton ); + expect( onChange ).not.toHaveBeenCalled(); + + // Cancel still closes the modal. + await user.click( cancelButton ); + await waitFor( () => { + expect( + screen.queryByRole( 'dialog' ) + ).not.toBeInTheDocument(); + } ); + expect( onChange ).not.toHaveBeenCalled(); + } ); + it( 'should call onChange with the correct value for each typed character', async () => { const onChange = jest.fn(); render( diff --git a/packages/dataviews/src/dataviews-picker/stories/index.story.tsx b/packages/dataviews/src/dataviews-picker/stories/index.story.tsx index 3d97873cf0085a..061d2995da853e 100644 --- a/packages/dataviews/src/dataviews-picker/stories/index.story.tsx +++ b/packages/dataviews/src/dataviews-picker/stories/index.story.tsx @@ -7,8 +7,9 @@ import type { Meta } from '@storybook/react-vite'; * WordPress dependencies */ import { useState, useMemo, useEffect } from '@wordpress/element'; -import { Modal, Button } from '@wordpress/components'; -import { Stack } from '@wordpress/ui'; +import { Button } from '@wordpress/components'; +// eslint-disable-next-line @wordpress/use-recommended-components +import { Dialog, Stack } from '@wordpress/ui'; /** * Internal dependencies @@ -275,21 +276,19 @@ export const WithModal = ( {

) } { isModalOpen && ( - <> - - setIsModalOpen( false ) } - isFullScreen={ false } - size="fill" - > + } } + > + + + Select Items + + - - + + ) } ); diff --git a/packages/dataviews/src/hooks/use-map-focus-on-mount.ts b/packages/dataviews/src/hooks/use-map-focus-on-mount.ts new file mode 100644 index 00000000000000..376e75f0eb32f8 --- /dev/null +++ b/packages/dataviews/src/hooks/use-map-focus-on-mount.ts @@ -0,0 +1,67 @@ +/** + * WordPress dependencies + */ +import { useCallback } from '@wordpress/element'; +import type { RefObject } from 'react'; + +/** + * Internal dependencies + */ +import type { ActionModal } from '../types'; + +const FIRST_INPUT_SELECTOR = + 'input:not([type="hidden"]):not([disabled]), select:not([disabled]), textarea:not([disabled])'; + +/** + * Maps the legacy `Modal.focusOnMount` semantics onto the + * `Dialog.Popup.initialFocus` prop accepted by `@wordpress/ui`. + * + * Mapping for each of the five legacy values that `ActionModal.modalFocusOnMount` + * accepts (`Parameters< typeof useFocusOnMount >[ 0 ] | 'firstContentElement'`): + * + * - `false` — forwarded as-is to skip focus-on-mount entirely. + * - `'firstInputElement'` — returns a callback that resolves to the first + * focusable `` / `