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

Simplify the Inserter accessibility #7058

Merged
merged 2 commits into from
Jun 4, 2018

Conversation

afercia
Copy link
Contributor

@afercia afercia commented May 31, 2018

Description

This PR aims to simplify the Inserter accessibility. The Inserter is now based on collapsible accordions and many a11y things were implemented with the previous version in mind. Please refer to the related issue #7052 for more details

  • removes the ARIA roles
  • removes arrows navigation
  • removes the active item tracking
  • uses a list for the items list

Using a list adds an useful information for users: assistive technologies will announce the total number of items in a list and some screen readers will also announce the position of each item. See screenshot below:

screen shot 2018-05-31 at 15 24 30

Worth noting Safari has a bug (or "feature"?) and doesn't expose styled lists as lists. It needs an explicitly set ARIA role list which would trigger a jsx-a11y validation error (while the W3C validator throws just a warning). It's a known Safari bug you can test on this pen: https://codepen.io/afercia/full/WxmJWx/

Check the Safari dev tools > "node" tab, and see the styled ul has "no matching ARIA roles". Now that Chrome has a nice accessibility tab, you can double check on Chrome and see it correctly exposes the styled list as list. So, it seems Safari-specific.

screen shot 2018-05-31 at 16 00 52

How has this been tested?

Fixes #7052

@afercia
Copy link
Contributor Author

afercia commented May 31, 2018

Advantages:

@afercia afercia requested a review from mtias May 31, 2018 16:08
@afercia afercia mentioned this pull request May 31, 2018
@mtias
Copy link
Member

mtias commented Jun 1, 2018

there's a lot of red in the diff

Always good!

@mtias mtias added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Feature] Inserter The main way to insert blocks using the + button in the editing interface labels Jun 1, 2018
@mtias mtias added this to the 3.0 milestone Jun 1, 2018
@youknowriad youknowriad force-pushed the update/inserter-simplify-a11y branch from 7be7f40 to ad79082 Compare June 4, 2018 10:09
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.

LGTM 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Inserter The main way to insert blocks using the + button in the editing interface [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants