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

Add more resilience to decorative SVGs #3892

Merged
merged 8 commits into from
Dec 15, 2020
Merged

Conversation

thisisdano
Copy link
Member

@thisisdano thisisdano commented Dec 14, 2020

Use the aria-hidden="true" property on decorative images (<img>) whose source is an SVG.

Adding role="img" fixes a webkit bug that causes images with an SVG source be be announced as a group, not an image. However, adding role="img" and alt="" result in an object that's selectable by AT but not announced (at least in our testing on VoiceOver and Chrome). Adding aria-hidden="true" results in a properly hidden element.

Adding aria-hidden="true" only would be equally effective (it renders role="img" moot) but we choose to add all three to be as consistent as possible with <img>, even if it is redundant. Thus, each of the following can always be true for <img>:

  • Use alt="" for decorative image
  • Use role="img" for images with an SVG source
  • Use aria-hidden="true" for decorative images with an SVG source

Add focusable="false" to <svg> elements meant to be images (ie not text/data).

This prevents an IE11 bug that could allow the Tab key to navigate into the SVG.

For decorative SVGs, use role="img" and aria-hidden="true" as above. Using both allows us the following consistent rules for the <svg> element:

  • Use role="img" and focusable="false" for SVGs meant to be images (ie don't contain text/data)
  • Use aria-hidden="true" for decorative SVGs
  • Use aria-label="[accessible name]" or aria-labelledby="[id list]" for SVGs that convey information and need an accessible description

Source: https://www.scottohara.me/blog/2019/05/22/contextual-images-svgs-and-a11y.html

- name: report
meta: "exlamation danger stop"
meta: "exclamation danger stop"
Copy link
Contributor

Choose a reason for hiding this comment

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

Doh!

Copy link
Contributor

@jaredcunha jaredcunha left a comment

Choose a reason for hiding this comment

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

This looks good to me. Do you want to update the SVGs in https://github.com/uswds/uswds/blob/develop/src/components/icons/icons--sizes.njk to include focusable="false"?

@thisisdano
Copy link
Member Author

Mmm, yes I can do that.

Copy link
Contributor

@mejiaj mejiaj left a comment

Choose a reason for hiding this comment

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

I think we should move the focusable attribute to <span class="icon-lock">.

Sidenote

I'm not sure if it's a blink/webkit thing, but I had trouble tabbing to the banner contents on the latest chromium version of Edge.

src/components/banner/banner.njk Show resolved Hide resolved
@jaredcunha
Copy link
Contributor

Oh, there's also the icon list too!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants