-
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
Update custom data view button #56002
Conversation
@@ -6,3 +6,7 @@ | |||
text-transform: uppercase; | |||
} | |||
} | |||
|
|||
.edit-site-sidebar-navigation-screen-dataviews__ellipsis-button { | |||
display: flex; |
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.
cc @ciampo If you place a small
button inside a DropDownMenu
then .components-dropdown-menu
renders at a peculiar size. Here is was 26.5px tall, in Storybook it's 27.5px. Seems like a bug to fix separately, then we can remove this style.
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.
Opened #56005
to fix this at the component level, you should be able to remove the custom styles once that gets merged
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.
New attempt in #56136
Size Change: +20 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
Flaky tests detected in 9e45ebe. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6813706061
|
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 know it's not the scope of this PR, but while testing I noticed that the dropdown menu's toggle is rendered as a child of another button
. Having a button
nested inside another button
is not valid HTML, since interactive elements shouldn't have other interactive elements as their content.
I suggest we refactor slightly this part of the UI so that the dropdown menu toggle sits on the side of the custom view button
@@ -6,3 +6,7 @@ | |||
text-transform: uppercase; | |||
} | |||
} | |||
|
|||
.edit-site-sidebar-navigation-screen-dataviews__ellipsis-button { | |||
display: flex; |
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.
Opened #56005
to fix this at the component level, you should be able to remove the custom styles once that gets merged
Closing in favor of #56005 |
Just noting that, if you wanted to update the dropdown menu button to have |
True, but it seems the design has already changed a bit so I'll probably open a new PR if needed. |
What?
Update the custom data view button to ensure the footprint matches regular data view buttons.
How?
Use the
small
Button variant.Testing Instructions
Before
Note the custom data view button is much taller due to the regular size ellipsis button.
After