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

Improve a11y of settings sidebar tabs #10927

Merged
merged 1 commit into from Oct 25, 2018

Conversation

Projects
None yet
3 participants
@brandonpayton
Member

brandonpayton commented Oct 23, 2018

Description

The purpose of this PR is to improve the accessibility of settings sidebar tabs by:

  • Providing the tabs as a list so screen readers will announce the tab count.
  • Updating the ARIA label of the selected tab to include "(selected)".

Contributes to #8079.

How has this been tested?

  • Ran automated tests.
  • Opened Gutenberg in Chrome, Firefox, macOS Safari, iOS Safari, IE11, and Edge and observed the tabs behaved normally.
  • Opened Gutenberg in Chrome with Voice Over enabled in macOS and observed the tab count being announced and the ARIA label changing on buttons after they were selected.

Checklist:

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

@brandonpayton brandonpayton added this to the 4.2 milestone Oct 23, 2018

@brandonpayton brandonpayton self-assigned this Oct 23, 2018

@brandonpayton brandonpayton requested review from gziolo and afercia Oct 23, 2018

@gziolo

gziolo approved these changes Oct 23, 2018

Code wise looks good, I will defer the final 👍 to someone from a11y team.

@afercia

This comment has been minimized.

Contributor

afercia commented Oct 23, 2018

@brandonpayton thanks!

When testing the list, worth noting Safari has a bug (or a "feature"?) with styled lists and doesn't expose them as "list". VoiceOver won't announce the list / number of items. That's unfortunate and the only work around I'm aware of is adding a redundant role="list" to the... list. Sometimes I do that, on the other hand that goes against the spec just to "fix" a browser's bug 😞 Test page: https://codepen.io/afercia/full/WxmJWx/

screen shot 2018-10-23 at 09 01 59

screen shot 2018-10-23 at 09 02 03

@gziolo

This comment has been minimized.

Member

gziolo commented Oct 25, 2018

Let’s merge it as is and open another PR when we decide that we want to add the redundant role to the list. If this is a case here it might be a general issue with Safari. If this is a case I would create a general purpose List component which would handle it for us, similtto SVG component which adds all a11y features out of the box.

@gziolo gziolo merged commit 2be63c0 into master Oct 25, 2018

2 checks passed

codecov/project No report found to compare against
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@gziolo gziolo deleted the fix/sidebar-selection-a11y branch Oct 25, 2018

@gziolo

This comment has been minimized.

Member

gziolo commented Oct 25, 2018

@tofumatt, can you check two previous comments and decide what to do next?

@afercia

This comment has been minimized.

Contributor

afercia commented Oct 25, 2018

👍 Yep, it is a general issue with Safari. Any friends at Apple? 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment