-
Notifications
You must be signed in to change notification settings - Fork 180
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
Accessibility fixes #435
Accessibility fixes #435
Conversation
Could you add before/after screenshots for relevant places, please? |
Also CC @isabela-pf and @tonyfast as this one tackles accessibility :) |
@krassowski ☝️ |
I would approve
without text shadow: with And example on buttons: none:
|
@krassowski I did that for you in #441 |
Just letting you know I'm looking at this now. Thanks for your patience! |
First off, thanks again for exploring this work. It’s always easier to jump on board when someone else gets this started! Second, I do think everything proposed above works in terms of compliance. Both changing text color and/or adding text shadows will increase contrast, I just think they don’t fit quite as well with the Jupyter branding guidelines so I think they risk not being merged or getting overwritten in the future. But I’m not here to complain and not provide another option, so here are some ideas of what we could do (based on the examples @palewire so kindly showed above). My most common recommendation is to adjust the background colors instead of the text. ProposalsGreyMost of the greys in the brand guide do pass AA regular text contrast with white text
so I think the easiest change for some of these may be to replace BlueAs for the blue used in the cards, I genuinely don’t know where that color is coming from (the closest options I found were in this secondary brand color issue. Since I don’t see it in the brand though, that likely means we can adjust the shade of blue if we want it to look closer to the current styling to keep the white text. Muddling around gave me ButtonsThe buttons are trickier because we definitely can’t change the orange without larger approval and that orange does not play nice with light or dark colors. The dark text passes, but I still don’t find it easy to read. The text shadow also technically works, but since we don’t use it anywhere else in the interface I think there’s a chance it could get overwritten in the future as inconsistent styling. They’re probably the best short-term options though. Another approach (possibly outside the scope of this PR) would be to have a mostly grey button with orange decorative detail so that it does not need to have sufficient contrast. I'm thinking something along these lines: As a quick listSuggestions include:
Let me know if you have any thoughts or questions on this! |
Thanks for this feedback, @isabela-pf. I will withdraw this pull request and submit a future patch that complies with your advice. |
I ran a Lighthouse test on the site and found a few small accessibility bugs. Things like this:
I used an accessibility site to tune things up a bit, like so
The results are in this pull request. Every page except widgets should get a 100 Lighthouse score on accessibility after this is merged. The bug on widgets is in the underlying bootstrap library's gray code snippet background color. I figured got into bootstrap was beyond the scope of what we should do here.