Skip to content
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

Site Icon Focus fix #66952

Merged
merged 2 commits into from
Nov 13, 2024
Merged

Conversation

karthick-murugan
Copy link
Contributor

@karthick-murugan karthick-murugan commented Nov 13, 2024

What?

Fixes #61339
Fixes #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

  1. Open the dashboard and navigate to Appearance > Editor.
  2. Click anywhere in the container to close the navigation.
  3. Use the keyboard to focus on the site icon in the top left corner.
  4. Ensure the blue border outline appears around the site icon when focused using keyboard navigation.

Testing Instructions for Keyboard

Ensure the blue border outline appears around the site icon when focused using keyboard navigation.

Screenshots or screencast

Screenshot:
site_icon_focus

Video:
https://github.com/user-attachments/assets/4a5609a0-0827-4e43-b64d-c8b27f714aaf

Copy link

github-actions bot commented Nov 13, 2024

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.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @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.

Unlinked contributors: kNoCoding.

Co-authored-by: karthick-murugan <[email protected]>
Co-authored-by: jasmussen <[email protected]>
Co-authored-by: vipul0425 <[email protected]>
Co-authored-by: jeryj <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Nov 13, 2024
Copy link

👋 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.

@karthick-murugan
Copy link
Contributor Author

@jeryj - Closed the previous PR #65936 and created this new one, since resolving some conflict issue brought in many commits to that PR. We can continue our discussion here.

Regarding your latest message, I tried with the before , after pseudo element but did not get the desired result. Adding a 1px padding was doing the job, but not sure if that will be a accepted fix. Please provide your inputs.

@Mamaduka Mamaduka added the [Type] Bug An existing feature does not function as intended label Nov 13, 2024
@Mamaduka Mamaduka requested a review from jasmussen November 13, 2024 09:07
@jasmussen
Copy link
Contributor

Nice work, thanks for the PR.

It's not clear that this fully solves the original issue, though:

Screenshot 2024-11-13 at 10 10 35

The 1px padding also makes the icon itself jump a little bit inwards, which is jarring.

I think what we need to do is create a pseudo element. Someting like this might work:

.edit-site-editor__view-mode-toggle button:focus{
	position: relative;

	&::before {
		content:"";
		display: block;
		position: absolute;
		z-index: 1;
		top: 0;
		right: 0;
		bottom: 0;
		left: 0;
		box-shadow: inset 0 0 0 var(--wp-admin-border-width-focus) var(--wp-admin-theme-color), inset 0 0 0 calc(#{ $border-width } + var(--wp-admin-border-width-focus)) $white;
	}
}

Screenshot 2024-11-13 at 10 20 42

@karthick-murugan
Copy link
Contributor Author

jasmussen Updated the proposed changes. Can you please review it?

Copy link
Contributor

@jasmussen jasmussen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WFM!

@Mamaduka Mamaduka added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Nov 13, 2024
Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes test well for me as well ✅

@Mamaduka Mamaduka merged commit 65c45a3 into WordPress:trunk Nov 13, 2024
66 checks passed
@github-actions github-actions bot added this to the Gutenberg 19.7 milestone Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No focus style for Open Navigation button Focus ring doesn't match button Site Hub Icon size
3 participants