-
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
Accessibility: Fixed reading order of keyboard shortcuts. #15631
Accessibility: Fixed reading order of keyboard shortcuts. #15631
Conversation
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.
Thanks a lot @nicolad for working on this. The native order improves things a lot.
Noticed a couple things that should be addressed:
1
Safari doesn't expose styled lists as lists to assistive technologies. This is a known Safari "feature", which requires to add a redundant role="list"
to the <ul>
element. As a consequence, the related ESLint rule needs to be disabled with an explanatory comment. See for example:
/* | |
* Disable reason: The `list` ARIA role is redundant but | |
* Safari+VoiceOver won't announce the list otherwise. | |
*/ | |
/* eslint-disable jsx-a11y/no-redundant-roles */ | |
<ul className="editor-block-navigation__list block-editor-block-navigation__list" role="list"> |
2
<li>
elements inherit margin-bottom: 6px;
from WordPress, this should be reset to not introduce visual changes
Lastly, I've noticed some shortcuts miss the aria-label
, which is meant to make the various shortcuts be announced in a more "human friendly" way. This is a pre-existing problem though, will create a separate issue.
Thank you @afercia, you have 👀 of the 🐯 |
Looks good to me, thanks! 👍 |
Description
Fixes: #15317
How has this been tested?
This was tested locally.
Types of changes
Replaced description list with unordered list.
Removed
order
prop and changed the order of elements.Checklist: