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.

Sign up for GitHub

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

Jump to bottom

Add shortcut aria label for unreadable shortcuts #9582

Merged
merged 9 commits into from
Sep 13, 2018
Merged

Add shortcut aria label for unreadable shortcuts #9582

merged 9 commits into from
Sep 13, 2018

Conversation

talldan
Copy link
Contributor

@talldan talldan commented Sep 3, 2018

Description

Fixes #9362

Some keyboard shortcuts contain characters that screen readers don't typically read (e.g. the toggle sidebar shortcut Cmd+Shift+,). This PR adds an aria-label for those shortcuts. The changes:

  • Adds a new utility function in keycodes for building an aria-label string.
  • Adds a shortcut component that can receive the additional aria label prop.
  • Implements the shortcut component in MenuItem and Tooltip

How has this been tested?

  • Tested using voiceover

Screenshots

screen shot 2018-09-03 at 4 25 13 pm

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@talldan talldan added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Sep 3, 2018
@talldan talldan added this to the 3.8 milestone Sep 3, 2018
@talldan talldan self-assigned this Sep 3, 2018
@talldan talldan requested review from ellatrix and afercia September 3, 2018 15:31
@afercia
Copy link
Contributor

afercia commented Sep 3, 2018

@talldan thanks so much for tackling this. Unrelated but since we're here: I'd suggest to take the opportunity to fix the string
Navigate to a the next part of the editor.

where the a seems a typo.

@talldan
Copy link
Contributor Author

talldan commented Sep 5, 2018

Thanks for catching that @afercia - added a commit to fix it.

@talldan talldan force-pushed the add/shortcut-aria-label branch from b8fee7c to 0370f36 Compare September 5, 2018 16:07
@talldan talldan force-pushed the add/shortcut-aria-label branch from 0370f36 to 776c2ad Compare September 5, 2018 16:07
@youknowriad youknowriad modified the milestones: 3.8, 3.9 Sep 5, 2018
@talldan talldan requested a review from a team September 11, 2018 20:12
youknowriad
youknowriad approved these changes Sep 13, 2018
View reviewed changes
Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

Nice 👍

@youknowriad youknowriad merged commit 6712e28 into master Sep 13, 2018
@youknowriad youknowriad deleted the add/shortcut-aria-label branch September 13, 2018 09:43
@talldan talldan mentioned this pull request May 30, 2019
Open
@talldan talldan added the [Type] Enhancement A suggestion for improvement. label Mar 26, 2021
@t-hamano t-hamano mentioned this pull request Nov 27, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@youknowriad youknowriad youknowriad approved these changes

@ellatrix ellatrix Awaiting requested review from ellatrix

@afercia afercia Awaiting requested review from afercia

Assignees

@talldan talldan

Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Enhancement A suggestion for improvement.
Projects
None yet
Milestone
3.9
Development

Successfully merging this pull request may close these issues.

Some keyboard shortcuts are announced incorrectly by screen readers
3 participants
@talldan @afercia @youknowriad