-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
DropdownMenuV2: add GroupLabel subcomponent #64854
Conversation
@WordPress/gutenberg-design I've started with some generic label styles, but this definitely needs some tweaks around:
|
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. |
Flaky tests detected in 10ab118. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10592844457
|
cc @cbravobernal @artemiomorales specifically to the block bindings refactor — could you please share a link to instructions on how to test the change in the block bindings UI ? The instructions in #62880 assume a level of knowledge that I don't have, and the sample code in https://make.wordpress.org/core/2024/03/06/new-feature-the-block-bindings-api/ doesn't produce an interactive UI in the block inspector sidebar. Thank you 🙏 |
Size Change: +2 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
In order to enable the UI, you need to enable it in Gutenberg experiments page. Then I recommend a PHP snippet to register a meta field. With something like this:
After that, create a post, add a paragraph, and it should appear on the right sidebar. This PR #64570 will eventually move the Gutenberg experiment to a settings field, I'll let you know when is updated. Happy to do a screen recording if needed. We can use it afterwards for documentation for the future release too. |
Thank you @cbravobernal , with your instructions I managed to test the UI 🎉 |
Using #64340 (comment) as a guide I suggest a couple of small tweaks;
Let's try
Yup, let's align with the separator. I suppose we should avoid truncation for now, as there's no way to read the full label. Guidelines can recommend keeping group labels as short as possible. |
10ab118
to
a50e36c
Compare
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.
This tests well and the code looks good to me 👍
Totally shippable, given all design feedback is addressed.
I've also left some minor suggestions, but nothing is blocking 🚀
render={ | ||
// @ts-expect-error The `children` prop is passed | ||
<Text | ||
upperCase |
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.
Is there any chance we might want the group label not to be uppercase? Or is this an intentional design decision?
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.
I assume it's intentional, but i'll let @jameskoster confirm
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.
The type should align to a specific style from the outcome of the work in #64340 (comment).
I don't know how that would be built into the component config file though, or consumed by the Text
component. I plan to open a PR soon to add tokens for the individual properties (font, size, line-height, weight), perhaps we can continue the discussion there?
Nothing further from my end - should be good to go once we confirm the group label should always be uppercase by design. 🚀 |
* DropdownMenuV2: add GroupLabel subcomponent * Use in Storybook examples * Use Text component * Use the first-party group label in the block bindings dropdown menu * Apply design feedback * Remove unneeded block-editor-bindings__popover classname * Add dedicated DropdownMenuGroupLabelProps type * Fix README * CHANGELOG --- Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: jameskoster <jameskoster@git.wordpress.org> Co-authored-by: cbravobernal <cbravobernal@git.wordpress.org>
What?
Part of #50459
Add
GroupLabel
sub-component toDropdownMenuV2
Why?
As discussed in #62880 (comment), there is a need for group labels in
DropdownMenuV2
.How?
DropdownMenuV2.GroupLabel
component based onAriakit.MenuGroupLabel
+<Text variant="muted" />
Testing Instructions
Check Storybook examples
Testing Instructions for Keyboard
Screenshots or screencast
Click to see screenshots: