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

fix(a11y): add a11y to modal close button svg #13049

Merged
merged 1 commit into from
May 31, 2022
Merged

fix(a11y): add a11y to modal close button svg #13049

merged 1 commit into from
May 31, 2022

Conversation

sardesam
Copy link
Contributor

Because

  • We want ensure all user interactions meet accessibility standards

This pull request

  • Adds a11y attributes to modal close button svg

Issue that this pull request solves

Closes: #12921

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).

Screenshots (Optional)

N/A

Other information (Optional)

The <svg> can be hidden from screen readers since the parent <button> already has an aria-label associated with it, and <svg> is being used strictly for visual purposes.

Because:

* We want ensure all user interactions meet accessibility standards

This commit:

* Adds a11y attributes to modal close button svg

Closes #12921
@sardesam sardesam requested a review from a team as a code owner May 25, 2022 22:01
Copy link
Contributor

@StaberindeZA StaberindeZA left a comment

Choose a reason for hiding this comment

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

r+

Just out of curiosity, is there a reason you went with aria-hidden over adding aria-labelledby.

@sardesam
Copy link
Contributor Author

In this particular situation, aria-labelledby wasn't needed (I did also test out of curiosity, and to verify). There are a few different ways one can code a (single) button without text. A few examples (not all) include:

  1. no aria attribute on the button, but text inside the button that is automatically read by the screen reader, and optional graphic (usually hidden - unless it really adds to the context and purpose of the button in some way)
  2. an aria-labelledby attribute on the button, attached to a label inside the button or another labelled element explaining the purpose of the button
  3. an aria-label on the button, no text or label, and non-text graphic inside

Although option 1 is always preferred, it's fairly common to see option 3 in this scenario, particularly the top right-hand corner of a modal (option 2 happens but there's a lot more factors to consider; I usually see it for information 'tooltips'/'bubbles' or non-button elements like tables with visually-hidden headers or charts, etc.).

For option 3, the 'X' is strictly a visual aid for sighted users. Whether they use a screen reader or not, they know it's a button and can infer its purpose. But non-sighted users will only see what they hear and because the button includes an aria-label describing its exact purpose, it's not necessary to even mention the existence of or focus on the 'X' graphic.

@sardesam sardesam merged commit ccb45c5 into main May 31, 2022
@sardesam sardesam deleted the FXA-5115 branch May 31, 2022 18:19
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.

[a11y] <svg> elements with an img role must have an alternative text (subplat)
2 participants