WordPress
fix: wp icon focus issue
Fixes #62672
What?
- fix the focus issue with the wp icon in the editor.
Why?
#62672
How?
- moved the focus from before to the
atag itself.
Screenshots or screencast
https://github.com/WordPress/gutenberg/assets/75293077/3e2c9f4e-43f3-4dda-921e-d85be2120c0a
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 props-bot label.
If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
Co-authored-by: up1512001 <[email protected]>
Co-authored-by: youknowriad <[email protected]>
Co-authored-by: bgardner <[email protected]>
Co-authored-by: talldan <[email protected]>
Co-authored-by: afercia <[email protected]>
Co-authored-by: jasmussen <[email protected]>
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
@up1512001 Thanks for another contribution! Something that's helpful when creating PRs is to type 'Fixes #62672' in the PR description, that links the PR to the issue it solves, sets it in progress and will automatically close the issue when the PR's merged: https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/using-keywords-in-issues-and-pull-requests
I've gone ahead and added it to this PR's description.
It does look better than trunk:
But the focus style in this PR still looks too thin compared to a normal focus style:
I think because of the css scale that's applied to the anchor element as mentioned in the issue.
Maybe the scale implemented here:
https://github.com/WordPress/gutenberg/blob/58932edaccdf7148f1fae42dddfec4099e007e87/packages/edit-site/src/components/site-hub/index.js#L66
Could be implemented on a div around the <SiteIcon> instead? Or applied tothe SiteIcon itself. I imagine there's also a transition style somewhere that would have to be updated too.
I've added @youknowriad as a reviewer, as the change seems to have come from #61579.
It does look better than trunk:
Yes it deoes look better but i'm not sure it fully solves the issue. Ideally, the focus style should be the same blue outline used for all buttons.
I believe this issue comes from https://github.com/WordPress/gutenberg/pull/61579 Cc @youknowriad
While I understand the need for simplification on that PR, the CSS 'scale' should not be applied to the button itself because it impacts also the focus style. That said, I would eveb be in favor of entirely removing the animation because I'm not sure it adds great value. Also, as mentioned in that PR the view-transition CSS API is only fully supported in Chrome and I'm not sure WordPress should use unsupported features not even as progressive ehnancement.
Can we try to check that this doesn't impact the focus styles ... on the full screen editor. These styles might be shared between both and the expectations different. So I just want to make sure we don't regress elsewhere.
I have updated the shadow and it's looking like this please let me know if it's ok. cc: @youknowriad @afercia
https://github.com/WordPress/gutenberg/assets/75293077/4140417c-ccb7-454b-ad3f-fdf3570734fd
It is worth reminding that the whole mechanism of the WP logo / site icon is being discussed in https://github.com/WordPress/gutenberg/issues/57813.
To me, the current implementation especially in the Site Editor is unnecessarily complicated and it's mainly done this way only to add an animation that doesn't add great value. Also, as mentioned in https://github.com/WordPress/gutenberg/issues/57813, when a site icon is set, this link is used to 'preview' the site icon which, in my opinion, is just confusing. Given the complexity of this implementation, even a simple thing like making sure there is correct focus style becomes complicated. I'd rather see a drastic simplification:
- remove the animaiton
- remove the site icon preview
The focus styles in the full editor looks broken to me. (See top left)
@youknowriad I have added Fix for this can you please verify it?
Maybe I'm not testing properly but I think the focus style is meant to be like that in the editor
In my testing, the blue outline is not visible at the moment in editor mode.
@youknowriad This is how it's looking now.
https://github.com/user-attachments/assets/797624e1-568c-482f-a655-fc58868b3d22
@ellatrix I agree it is minor, but also seems low risk and affects quite a prominent part of the site editor.
I don't really mind, feel free to remove the label.