Fix/site icon focus #65936
Fix/site icon focus #65936
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @gutenbergplugin, @kNoCoding. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. 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. |
github-actions
bot
added
the
First-time Contributor
|
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @karthick-murugan! In case you missed it, we'd love to have you join us in our Slack community. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
akasunil
added
[Type] Bug
| @@ -19,4 +24,9 @@ | |||
| .edit-site-layout.is-full-canvas & { | |||
| border-radius: 0; | |||
| } | |||
|
|
|||
| // Add focus-visible styles | |||
| button[data-focus-visible="true"] & { | |||
There was a problem hiding this comment.
Is there a reason to use [data-focus-visible="true"] instead of :focus here?
There was a problem hiding this comment.
Yes I will try to update it with :focus.
|
|
||
| // Add focus-visible styles | ||
| button[data-focus-visible="true"] & { | ||
| border: 2px solid #1e90ff; |
There was a problem hiding this comment.
I believe we have some focus style mixins or variables we can use instead of hardcoding these.
There was a problem hiding this comment.
Yes will use variables.
| // Add focus-visible styles | ||
| button[data-focus-visible="true"] & { | ||
| border: 2px solid #1e90ff; | ||
| } |
There was a problem hiding this comment.
Do we need to repeat this code block within each section, or is there something higher up we can hook into so we only need to have this focus style one time?
There was a problem hiding this comment.
Yes we can update that style to one time.
There was a problem hiding this comment.
@jeryj - Thank you for your feedback and I have updated all the feedback in latest commit. Please have a look.
| button:focus { | ||
| .edit-site-site-icon__icon, | ||
| .edit-site-site-icon__image { | ||
| border: var(--wp-admin-border-width-focus) solid var(--wp-admin-theme-color); |
There was a problem hiding this comment.
A border will change the sizing of the image. I believe we have a mixin or variable that uses the box-shadow for the focus style instead. Also, I think we should apply the focus style to the button, not the inner part of it.
There was a problem hiding this comment.
Thank you for your feedback @jeryj When I try to add box-shadow I can see the outline on the right side and bottom only and cannot see it in top and left. This is because the Site Icon is on the Top Left. Please have a look at this screenshot
Please advice how to proceed in this case. Thanks.
There was a problem hiding this comment.
@jeryj if you have some time, please advice on this. Thanks in advance.
There was a problem hiding this comment.
You may need to apply the focus ring to a :before or :after pseudo element in order to have it be visible and inset slightly from the edge.
Co-authored-by: AhmarZaidi <[email protected]> Co-authored-by: afercia <[email protected]>
Co-authored-by: youknowriad <[email protected]> Co-authored-by: ellatrix <[email protected]> Co-authored-by: draganescu <[email protected]> Co-authored-by: getdave <[email protected]>
Co-authored-by: youknowriad <[email protected]> Co-authored-by: oandregal <[email protected]>
…age (WordPress#66335) Co-authored-by: youknowriad <[email protected]> Co-authored-by: ellatrix <[email protected]>
Update package management to use npm workspaces instead of lerna. Lerna is obsolete for many repository management operations, such as dependency installation or script running. All of these are supported by npm now. Lerna continues to be used for publishing flows to ensure the appropriate dependencies are updated and published with the correct versions. The hope is that this simplfies dependendy management and maintains a more sane dependency tree. Dependency management has become increasingly difficult as Gutenberg has grown. --- This was originally merged in 4693c0f. It was reverted in order to merge it at a more time. --- Co-authored-by: t-hamano [email protected] Co-authored-by: sirreal [email protected] Co-authored-by: gziolo [email protected] Co-authored-by: ciampo [email protected] Co-authored-by: jsnajdr [email protected] Co-authored-by: yusuke-omae [email protected] Co-authored-by: kevin940726 [email protected]
Co-authored-by: Mamaduka <[email protected]> Co-authored-by: youknowriad <[email protected]> Co-authored-by: tyxla <[email protected]>
) Co-authored-by: ellatrix <[email protected]> Co-authored-by: youknowriad <[email protected]>
Co-authored-by: Mamaduka <[email protected]> Co-authored-by: youknowriad <[email protected]> Co-authored-by: gziolo <[email protected]>
Co-authored-by: gigitux <[email protected]> Co-authored-by: jameskoster <[email protected]> Co-authored-by: ntsekouras <[email protected]> Co-authored-by: oandregal <[email protected]> Co-authored-by: youknowriad <[email protected]>
Co-authored-by: tyxla <[email protected]> Co-authored-by: Mamaduka <[email protected]> Co-authored-by: mcsf <[email protected]>
Co-authored-by: gigitux <[email protected]> Co-authored-by: oandregal <[email protected]> Co-authored-by: jameskoster <[email protected]>
Co-authored-by: MD-sunilprajapati <[email protected]> Co-authored-by: poojabhimani12 <[email protected]> Co-authored-by: rishishah-multidots <[email protected]> Co-authored-by: ingeniumed <[email protected]> Co-authored-by: ellatrix <[email protected]> Co-authored-by: tyxla <[email protected]> Co-authored-by: ciampo <[email protected]> Co-authored-by: mtias <[email protected]> Co-authored-by: jasmussen <[email protected]> Co-authored-by: youknowriad <[email protected]> Co-authored-by: annezazu <[email protected]> Co-authored-by: jameskoster <[email protected]>
Co-authored-by: jeryj <[email protected]> Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: aaronrobertshaw <[email protected]> Co-authored-by: ramonjd <[email protected]>
) Co-authored-by: aaronrobertshaw <[email protected]> Co-authored-by: ramonjd <[email protected]>
Co-authored-by: afercia <[email protected]> Co-authored-by: gigitux <[email protected]>
…66250) Co-authored-by: oandregal <[email protected]> Co-authored-by: ntsekouras <[email protected]> Co-authored-by: mcsf <[email protected]>
Co-authored-by: jameskoster <[email protected]> Co-authored-by: ciampo <[email protected]> Co-authored-by: tyxla <[email protected]> Co-authored-by: jasmussen <[email protected]>
* Add a transform from the Separator block to the Spacer block * Add a transform from the Separator block to the Spacer block Feat WordPress#65492 * Add transformation from spacer to separator - Remove the default height of 50px after transformation to default 100px that of Spacer - Update the transform of blocks so that it retains anchor attribute * Add new transformation block titles in Test * Add new transformation block titles in Test * Add new transformation block titles in Test and resolve linting problem * Add new transformation block titles in Test and resolve linting problem * Update test suite snapshot for test [mobile] to pass successfully --------- Co-authored-by: talldan <[email protected]> Co-authored-by: rudrakshi-gupta <[email protected]>
…65340) Co-authored-by: Mamaduka <[email protected]> Co-authored-by: ciampo <[email protected]> Co-authored-by: tyxla <[email protected]> Co-authored-by: t-hamano <[email protected]> Co-authored-by: mirka <[email protected]>
karthick-murugan
requested review from
kevin940726,
desrosj,
ellatrix,
gziolo,
adamsilverstein,
talldan,
swissspidy,
ajitbohra,
peterwilsoncc,
ntwb,
nerrad,
ryanwelcher,
ndiego,
luisherranz,
DAreRodz,
draganescu,
adamziel,
getdave,
dmsnell,
atimmer,
artemiomorales,
ZebulanStanphill,
michalczaplinski,
geriux,
mmtr,
juanmaguitar and
fabiankaegy
as code owners
What?
Fixes 61339 and #66375
This PR updates the SCSS file for the site icon to improve focus visibility.
Why?
The focus ring on the site hub icon was not visible or visually appealing when using keyboard navigation, impacting accessibility.
How?
The SCSS file gutenberg/packages/edit-site/src/components/site-icon/style.scss has been modified to add a blue border outline around the site icon when it is focused.
Testing Instructions
Testing Instructions for Keyboard
Ensure the blue border outline appears around the site icon when focused using keyboard navigation.
Screenshots or screencast
Screenshot:
Video:
https://github.com/user-attachments/assets/4a5609a0-0827-4e43-b64d-c8b27f714aaf