Skip to content

Remove role attributes on SVGs meant for "decoration" to improve a11y#38301

Merged
ndiego merged 3 commits into
WordPress:trunkfrom
ndiego:fix/improve-a11y-on-svgs
Jan 31, 2022
Merged

Remove role attributes on SVGs meant for "decoration" to improve a11y#38301
ndiego merged 3 commits into
WordPress:trunkfrom
ndiego:fix/improve-a11y-on-svgs

Conversation

@ndiego

@ndiego ndiego commented Jan 27, 2022

Copy link
Copy Markdown
Member

Description

Fixes #38286

In many places throughout Gutenberg, we render SVGs with role="img" but we do not include an aria-label. At first glance this may seem ok because in all instances we also have something like this:

<svg role="img" aria-hidden="true" focusable="false" width="24" height="24" viewBox="0 0 24 24" version="1.1" xmlns="http://www.w3.org/2000/svg">
      ...
</svg>

However, aria-hidden="true" focusable="false" is sufficient here. Since we are including role="img", there should also be an aria-label as described here in the MDN Wed Docs. The solution is to simply remove the role attribute. Refer to #38286 for further discussion on why this is the correct approach.

Testing Instructions

  1. Using the latest version of Gutenberg, add a Social Icons block to the page.
  2. Add a couple social links
  3. Preview the page on the frontend and notice role="img" is no longer added to all SVG icons. 🙌

Screenshots

Types of changes

Bug fix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).
  • I've updated related schemas if appropriate.

@ndiego ndiego requested review from Mamaduka and joedolson January 27, 2022 20:21
@ndiego ndiego added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Jan 27, 2022
@ndiego ndiego self-assigned this Jan 27, 2022
@ndiego

ndiego commented Jan 28, 2022

Copy link
Copy Markdown
Member Author

I discovered that correcting this issue actually touches virtually every part of the Gutenberg interface. The SVG primitive was also adding the role="img" attribute. I have made the update, but given how much this impacts, very thorough testing is needed. As of yet, I have not been able to identify any issues with this update. I will keep testing tomorrow...

@Mamaduka Mamaduka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It looks like there are a few more snapshots that need an update.

@ndiego

ndiego commented Jan 28, 2022

Copy link
Copy Markdown
Member Author

Yeah that last change to the SVG seems to have touched a lot of tests. I will remedy today.

@aristath aristath left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code here looks good and is a small - but important - improvement.
I'll go ahead and pre-approve this PR so it can be merged as soon as all tests are fixed 👍

@ndiego ndiego requested review from nerrad and ntwb as code owners January 28, 2022 16:40
@ndiego

ndiego commented Jan 28, 2022

Copy link
Copy Markdown
Member Author

@Mamaduka and @aristath I believe I successfully fixed all the tests, but I am still getting one failure related to the gallery block, which is a bit confusing as my edits didn't touch that. I am seeing the same failure in many of the latest PRs. 🤔

@aristath

Copy link
Copy Markdown
Member

One of the tests is still failing here... @ndiego could you please rebase the PR? That should fix the error here. If it doesn't, then we should take another look and see if there's something wrong 👍

@ndiego

ndiego commented Jan 31, 2022

Copy link
Copy Markdown
Member Author

Looks like everything has passed after the rebase. Merging 🙌

@ndiego ndiego merged commit 55ca64f into WordPress:trunk Jan 31, 2022
@ndiego ndiego deleted the fix/improve-a11y-on-svgs branch January 31, 2022 15:29
@github-actions github-actions Bot added this to the Gutenberg 12.6 milestone Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Improve a11y on SVGs throughout Gutenberg

3 participants