Menu: more granular sub-components #67422
Open
Conversation
This file contains 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
|
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. |
ciampo
force-pushed
the
feat/menu-granular-trigger-components
branch
from
184ecb7 to
72c8324
Compare
ciampo
changed the title
|
Size Change: +252 B (+0.01%) Total Size: 1.84 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in e538597. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12299000281
|
ciampo
commented
Dec 2, 2024
ciampo
force-pushed
the
feat/menu-granular-trigger-components
branch
2 times, most recently
from
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. |
ciampo
added
[Type] Enhancement
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
|
ciampo
force-pushed
the
feat/menu-granular-trigger-components
branch
from
fdb7b93 to
8324bd3
Compare
This was referenced Dec 5, 2024
|
Update:
@WordPress/gutenberg-components This PR is ready for a final code review and a general smoke test. @ntsekouras @oandregal note that this is a breaking change to |
|
I gave this a round of testing in Storybook and the editor, everything smoke tested well 👍 Code also looks good. 👍 Unless anyone else has any feedback, I think once we address the TS complexity issue (see my proposal) and remove the config change, this PR should be good to go! |
|
I smoke tested a few places (dataviews & styles) and didn't run into any issue. |
Co-authored-by: ciampo <mciampini@git.wordpress.org>
…review menu (#67644) * Refactor global styles sidebar wrapper preview menu * Update packages/edit-site/src/components/sidebar-global-styles-wrapper/index.js Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com> --------- Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com>
…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>
ciampo
force-pushed
the
feat/menu-granular-trigger-components
branch
from
e538597 to
e78ae86
Compare
|
Update the PR with the changes suggested from @tyxla . I believe we can merge as soon as the PR gets an approval! |
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.
What?
Superseeds #66383
Related to #63704
Change the APIs of the
Menucomponent, 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:
refs to both the button and the menu popover;How?
Menucomponent;In short, this is how the APIs have changed:
triggerprop is removed, in favor of using theMenu.TriggerButtonandMenu.SubmenuTriggerItemcomponents;Menu.Popovercomponents.Refactors across Gutenberg are carried out in separate PRs, to be reviewed individually before being merged back into this PR:
Follow-ups
Menu.Popovercomponent (see Why are popover-related props passed to store and store provider ariakit/ariakit#4295)Testing Instructions
MenuStorybook examples and make sure that they behave like ontrunk;