-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Menu: more granular sub-components #67422
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. |
184ecb7
to
72c8324
Compare
Size Change: +235 B (+0.01%) Total Size: 1.84 MB
ℹ️ View Unchanged
|
Flaky tests detected in 3949e54. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12349870891
|
c14c9a6
to
fdb7b93
Compare
Update: I managed to fix the CI issue by increasing the memory used by @WordPress/gutenberg-components I think this is a good time to have a first round of review and collect some feedback:
You may want to give a quick smoke test to Storybook and the editor too. |
How about we split this into a stacked PR, with each integration change as a separate sub-PR? |
Sure, I can do that! I'd still like to gather any high-level feedback before breaking the PR into smaller one, if folks have any. |
The one high-level question I have is about having a submenu trigger component separate from the root-level trigger component. For example in the submenu example in Ariakit it's switching based on |
That's basically the logic that the component currently implements on
|
fdb7b93
to
8324bd3
Compare
Co-authored-by: ciampo <mciampini@git.wordpress.org>
…enu (#67639) Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: oandregal <oandregal@git.wordpress.org>
…enu (#67640) Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: oandregal <oandregal@git.wordpress.org>
Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gave this another round of testing, including with a custom story I built to test all components without a Menu
wrapper component.
All looks good and tests well. ✅
Let's go 👍 🚢
e78ae86
to
3949e54
Compare
Rebased to solve confilict — note that the global styles preview/style book menu has been removed in favour of a toggle |
* MenuItem: add render and store props * Extract sub-components: popover, trigger button, submenu trigger item * Unit tests * CHANGELOG * Add more memory to node on CI * Refactor block bindings panel menu (WordPress#67633) Co-authored-by: ciampo <mciampini@git.wordpress.org> * Storybook (WordPress#67632) * Refactor dataviews item actions menu (WordPress#67636) * Refactor dataviews view config menu (WordPress#67637) * Refactor global styles shadows edit panel menu (WordPress#67641) * Refactor global styles font size menus (WordPress#67642) * Refactor "Add filter" dataviews menu (WordPress#67634) * Menu granular subcomponents: Refactor dataviews list layout actions menu (WordPress#67639) Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: oandregal <oandregal@git.wordpress.org> * Menu granular subcomponents: Refactor dataviews table layout header menu (WordPress#67640) Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: oandregal <oandregal@git.wordpress.org> * Menu granular subcomponents: Refactor post actions menu (WordPress#67645) Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> * Better comments for submenu trigger store * Typo * Remove unnecessary MenuSubmenuTriggerItemProps type * Don't break the rules of hooks 🪝 * Move CHANGELOG entry to unreleased section * Add explicit MenuProps to improve TS performance * Remove node memory settings --------- Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: mirka <0mirka00@git.wordpress.org> Co-authored-by: oandregal <oandregal@git.wordpress.org>
* MenuItem: add render and store props * Extract sub-components: popover, trigger button, submenu trigger item * Unit tests * CHANGELOG * Add more memory to node on CI * Refactor block bindings panel menu (WordPress#67633) Co-authored-by: ciampo <mciampini@git.wordpress.org> * Storybook (WordPress#67632) * Refactor dataviews item actions menu (WordPress#67636) * Refactor dataviews view config menu (WordPress#67637) * Refactor global styles shadows edit panel menu (WordPress#67641) * Refactor global styles font size menus (WordPress#67642) * Refactor "Add filter" dataviews menu (WordPress#67634) * Menu granular subcomponents: Refactor dataviews list layout actions menu (WordPress#67639) Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: oandregal <oandregal@git.wordpress.org> * Menu granular subcomponents: Refactor dataviews table layout header menu (WordPress#67640) Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: oandregal <oandregal@git.wordpress.org> * Menu granular subcomponents: Refactor post actions menu (WordPress#67645) Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> * Better comments for submenu trigger store * Typo * Remove unnecessary MenuSubmenuTriggerItemProps type * Don't break the rules of hooks 🪝 * Move CHANGELOG entry to unreleased section * Add explicit MenuProps to improve TS performance * Remove node memory settings --------- Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: mirka <0mirka00@git.wordpress.org> Co-authored-by: oandregal <oandregal@git.wordpress.org>
What?
Superseeds #66383
Related to #63704
Change the APIs of the
Menu
component, adding more granular sub-components to specify menu trigger button, submenu trigger items, and menu popover individually.Why?
As discussed in #63704, we want to offer lower-level primitives to give consumers more flexibility in building menu-based UIs.
We believe this new configuration also offers better DX:
ref
s to both the button and the menu popover;How?
Menu
component;In short, this is how the APIs have changed:
trigger
prop is removed, in favor of using theMenu.TriggerButton
andMenu.SubmenuTriggerItem
components;Menu.Popover
components.Refactors across Gutenberg are carried out in separate PRs, to be reviewed individually before being merged back into this PR:
Follow-ups
Menu.Popover
component (see Why are popover-related props passed to store and store provider ariakit/ariakit#4295)Testing Instructions
Menu
Storybook examples and make sure that they behave like ontrunk
;