-
Notifications
You must be signed in to change notification settings - Fork 4.5k
List View: Fix focus outline style #69201
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
Conversation
Size Change: -103 B (-0.01%) Total Size: 1.89 MB
ℹ️ View Unchanged
|
16b84d8
to
56d6150
Compare
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 ced36d4. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/14348791290
|
56d6150
to
81ee833
Compare
81ee833
to
9f2d2c3
Compare
9f2d2c3
to
ced36d4
Compare
ced36d4
to
e1d0313
Compare
e1d0313
to
0c9790f
Compare
0c9790f
to
116821b
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 is great, thanks for fixing this up @t-hamano, and for explaining the reasoning behind the change (that very much helped me in getting back up to speed with where the list view is at). I tested a bunch of different configurations in the post and site editors, and didn't run into any issues. In particular it's very satisfying to see this resolved:
Before | After |
---|---|
Also, thank you for the patience on this one, I see this PR was originally opened over 5 months ago!
LGTM 🚀
@andrewserong Thanks for the review! |
Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: afercia <afercia@git.wordpress.org>
What?
Closes #69189
This PR will ensure that the correct focus style is applied regardless of where the ListView component is used.
Why?
A list view item consists of two cells, the second cell is expected to be 28px wide. This 28px is the combined value of the dropdown toggle button width (
24px
) + right padding (4px
):However, for blocks that don't allow dropdown toggles, only an empty cell will be rendered. The cell itself has a width of
24px
.In the post editor, the
box-sizing
value of this cell is the browser's defaultcontent-box
, so the cell width is24px + 4px right padding = 28px
, which means no outline shift.On the other hand, in the site editor, the
box-sizing
value of this cell isborder-box
, which means the cell has a width of24px
.The fact that the cell is
24px
wide instead of the expected28px
will cause the absolutely positioned focus to shift right.How?
24px
in size. To get around this, addsize:small
to thetoggleProps
. As a result, remove the CSS override, which is no longer needed.Testing Instructions
Go to the following places where list views are used:
Make sure all styles are applied correctly:
Screenshots or screencast
In the screenshot below, I've applied some pseudo styles.
Site Editor > Pages Menu > Page
Site Editor > Navigation Menu
The selected and focused item is resolved.
Site Editor > Navigation Block > Block Sitebar > Settings Tab
No visual changes.