-
-
Notifications
You must be signed in to change notification settings - Fork 79.1k
Improve accessibility (Section 508, WCAG) #9137
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
Conversation
<div class="container"> | ||
|
||
<!-- .navbar-toggle is used as the toggle for collapsed navbar content --> | ||
<button type="button" class="navbar-toggle" data-toggle="collapse" data-target=".navbar-responsive-collapse"> | ||
<span class="sr-only">Expand Navigation</span> |
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.
What about changing these to Toggle navigation
? This is the trigger to expand and collapse, and I push for sentence case just about everywhere :).
Thanks so much for the quick review. I think I've touched on all your comments with these added commits. |
We've done a lot to the docs today—any chance you could peep this tonight or tomorrow and get it up to date so we can do an easy merge without having to deal with conflicts? |
Definitely, will do so tonight. Should just a matter of merging the upstream repo in and fixing conflicts, right? |
Thanks, and yup, that should be it. |
Done, but not really sure what's up with the BrowserStack key and why tests are failing. |
That's expected; ignore it. |
Awesome. Let me know if there's anything else! |
I tried my hand at it, but failed and can't devote more time to it right now. Can you by chance rebase this @adamjacobbecker so we get it a single commit? |
I think you might have put too much faith in a guy who committed a merge conflict earlier. Having some trouble with this... let me try again, and then I'll call in the cavalry... |
@adamjacobbecker Hah, I know that feel :). I've never really tried before and but I'm so swamped I can't spend more time giving it another go. @cvrebert might have some guidance—he did one earlier. I was working from one of several guides I found whilst Googling. |
… explaining the function in the 'basic navbar' section
it's 1/2 as many characters and just as semantic* *although i guess you could argue that we shouldn't be abbreviating "screen reader" to "sr". oh well.
Might have it done. Just gotta verify that I didn't lose anything. |
Let me know how that looks. |
@adamjacobbecker Er, nope, still a kajillion commits. |
Result of my attempt: #9186 |
So are these changes in 3.0.2? |
@mgifford They're in v3.0.0+. Read the successor pull request's merge commit. |
Thanks! |
This PR significantly improves Bootstrap's accessibility for users of assistive technology, such as screen readers. Some of the these changes add additional markup to the source examples, but we believe that the sacrifice in readability is worth achieving more widespread usage of accessibility best-practices.
What was done
.sr-only
helper class, that is only readable by screen readers (and invisible for all other users). This lets us - make progress bars and paginations accessible to screen reading users..sr-only
What wasn't done
Major props to all that contributed: @bensheldon, @jasonlally, @criscristina, and @louh. Feel free to chime in, guys, if I've left anything out.