-
-
Notifications
You must be signed in to change notification settings - Fork 78.9k
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
Docs: missing aria-hidden on some decorative SVGs #40756
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -66,35 +66,35 @@ <h2 class="pb-2 border-bottom">Columns with icons</h2> | |||
<div class="row g-4 py-5 row-cols-1 row-cols-lg-3"> | |||
<div class="feature col"> | |||
<div class="feature-icon d-inline-flex align-items-center justify-content-center text-bg-primary bg-gradient fs-2 mb-3"> | |||
<svg class="bi" width="1em" height="1em"><use xlink:href="#collection"/></svg> | |||
<svg class="bi" width="1em" height="1em" aria-hidden="true"><use xlink:href="#collection"/></svg> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note that aria-hidden="true"
is unnecessary if the svg doesn't contain any text elements. SRs will ignore them the same way they ignore a series of purely empty presentational <div>
s and <span>
s
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(just as a heads-up before you laboriously go and add it to every, single, svg)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your answer :-) Our accessibility experts asked us to add aria-hidden on decorative SVGs in our Boosted doc, so I did it on Bootstrap when relevant. There aren't so much in the doc, but a lot more in the examples...! So if you're ok with this I can stop there and submit the PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wonder what the rationale provided by the accessibility experts there was.
personally, I have no problem with having all the aria-hidden
s added, it just felt a bit wasteful/pointless. but if we're going to add them, then I'd say it should cover all instances, as it would be more confusing if we had a mix of "some <svg>
s do have it, others don't". so i'd say either remove the ones you did from this PR, or...go through all the rest
See also #40686 |
Just for info, after some discussions with the accessibility team, they confirmed that aria-hidden is needed on SVGs until now, even if most of assistive technologies will ignore SVGs that do not contain texts. They rely on criterion WCAG 1.1.1 saying:
The last paragraph says we must do what is needed to ensure that decorative elements are ignored, and the only way to do it, is to use I'm going to make another PR about the tooltip icon's label (#40808 ) so that we can at least fix this label issue... |
…ole="img"` to informative ones in examples until dropdowns
…ole="img"` to informative ones in examples until modals
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine having this in Bootstrap.
Description
Add aria-hidden on decorative SVGs to make sure they are ignored by assistive technologies.
Motivation & Context
Type of changes
Checklist
npm run lint
)Live previews
Related issues