Repository navigation
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: 0 B Total Size: 7.78 MB |
the Menu guidelines reserve the ellipsis for items needing more input
the items drop the nested Button; the new Menu styles its own
link actions mount as LinkItem, which renders the anchor natively
the current width becomes a radio selection instead of a disabled item
e1eae9c to
31aa785
Compare
a radio group belongs inside a command menu, not alone behind a trigger
| /> | ||
| } | ||
| /> | ||
| <Menu.Root> |
There was a problem hiding this comment.
This now conflicts with the policy gates added in #81967. When rebasing, could we keep one menu but render the width group only when canResize and the Remove item only when canRemove? Taking this side wholesale would let a policy-denied resize change the staged layout.
| <Menu.Popup> | ||
| { actions.map( ( action ) => ( | ||
| <Menu.LinkItem | ||
| key={ action.id } | ||
| href={ action.href } | ||
| download={ action.download } | ||
| openInNewTab={ action.openInNewTab } | ||
| closeOnClick | ||
| prefix={ | ||
| action.icon ? ( | ||
| <Icon icon={ action.icon } /> | ||
| ) : undefined | ||
| } | ||
| > | ||
| <Menu.ItemLabel>{ action.label }</Menu.ItemLabel> | ||
| </Menu.LinkItem> | ||
| ) ) } |
There was a problem hiding this comment.
#80991 now routes DOM suites only through the *.jsdom.test.* suffix. Could you rename this to menus.jsdom.test.tsx during the rebase so these rendered menu tests keep a JSDOM environment?
| it( 'surfaces the dashboard actions in the overflow menu', async () => { | ||
| const user = userEvent.setup(); | ||
| render( <Harness /> ); | ||
|
|
||
| await user.click( | ||
| screen.getByRole( 'button', { name: 'More options' } ) | ||
| ); | ||
|
|
||
| expect( | ||
| await screen.findByRole( 'menuitem', { name: 'Reset to default…' } ) | ||
| ).toBeInTheDocument(); | ||
| } ); |
There was a problem hiding this comment.
Could this click "Reset to default…" and assert that the "Reset dashboard to default?" alert dialog opens? That gives the migrated command a behavioral regression test, rather than only checking that its label renders.
There was a problem hiding this comment.
After rebasing, these entries auto-merge into the already released 0.6.0 section, and none links #81970. Could you move them to the current ## Unreleased section and add [#81970](https://github.com/WordPress/gutenberg/pull/81970)? And potentially merge them into one entry, if it makes sense
|
Closing as superseded by #81929 |
2 participants
What?
Part of #81230.
Migrates every menu in
@wordpress/widget-dashboardfrom the privateMenuin@wordpress/componentsto theMenuin@wordpress/ui:ActionsMenu).WidgetActions).WidgetLayoutControls).No usages of the old component remain in the package, and
routes/dashboardnever had any: it mountsWidgetDashboard.Actionsinto thePageheader, so the menu lives inside the package.Also applies two Menu usage guidelines the migration surfaced: the ellipsis convention on "Reset to default…", and folding widget removal into the customize-mode menu so its radio group sits inside a command menu.
Follows #81783, which did the same for
@wordpress/dataviews.Why?
See #81230.
Dogfoods the design-system
Menu(#79560) in another product surface, and removes this package's last consumers of the@wordpress/componentsprivate APIs.How?
Mostly a mechanical API mapping:
MenutoMenu.Root,Menu.TriggerButtontoMenu.Trigger,Menu.PopovertoMenu.Popup. Three places where the new component changes what gets rendered.Widget actions mount as
Menu.LinkItem. They were aMenu.Itemrendering aLink.Menu.LinkItemis the part meant for navigation destinations: it renders the<a role="menuitem">natively and handlesdownload,target="_blank"and the "(opens in a new tab)" indicator. That leaves the wrappingMenu.Groupand the two CSS rules that only existed to forcedisplay: inline-flexon the link with nothing to do.Overflow menu items no longer render a
Button. The newMenustyles its own items, so the nestedButtonwas redundant.The width menu uses
Menu.RadioGroup. The two widths are mutually exclusive states, and the active one was communicated by disabling it. The Menu usage guidelines put that in a radio group, which is also what #81783 did for the sorting directions and the layout switcher."Reset to default" becomes "Reset to default…". The Menu usage guidelines reserve the ellipsis for items that open another interface requiring more input before the command can finish, naming a confirmation dialog as the example. That item opens an
AlertDialogwithintent="irreversible"and a "Reset" confirm button, so the command does not complete on activation. The command palette entry keeps its own wording; the convention covers menu items.Widget removal moves into the customize-mode menu. The width options were the menu's only content, with removal sitting beside the trigger as a separate trash button. The guidelines reserve radio items for state that "belongs inside a broader command menu", and warn against using them to recreate a Select when choosing a value is the control's whole purpose. Removal now renders as a
Menu.Itemunder aMenu.Separator, keeping the trash icon as its prefix. It sits below the width group rather than above it, so neither the pointer nor the first ArrowDown lands on the destructive item.One behaviour detail the mapping could have dropped silently: the old
Menu.Itemclosed the menu on click (hideOnClickdefaults totrue), whileMenu.RadioItemandMenu.LinkItemdefaultcloseOnClicktofalse. Both now pass it explicitly.Adds
packages/widget-dashboard/src/test/menus.test.tsx. Nothing in the suite opened these menus, and the migration changes the rendered DOM: real anchors carryinghref/download/target, andmenuitemradioreporting the current width.Testing Instructions
'low'relevance action, so it lands in the menu. Open its "More" menu and verify "WordPress.org plugin page" navigates, opens in a new tab, and that middle-click and "Copy link address" work on it. Site Health has two more (Status,Info) as plain links.?wp_lang=ar) for popup alignment.No bundled widget declares a
downloadaction, so that path is covered by the unit test rather than manually.Testing Instructions for Keyboard
Screenshots or screencast
Two visual changes in the customize-mode widget chrome: the active width carries a selection indicator instead of appearing disabled, and the trash button beside the trigger is now a "Remove" command inside the menu.
Follow-ups
@wordpress/uiimports in these files carry a bareeslint-disablefor@wordpress/use-recommended-components. The Menu isuse-with-cautionpending Prepare@wordpress/uifor use in Gutenberg #76135, so the disables should name that reason, as DataViews: useMenufrom@wordpress/ui#81783 does.ActionsMenuItem.disabledTooltipis public API with no in-tree caller. Now thatMenu.ItemDescriptionexists for supplementary content announced to assistive technology, it is a better fit than a tooltip on a disabled item.