-
Notifications
You must be signed in to change notification settings - Fork 574
Improve sidebar menu contrast to fix accessibility issues #749
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
Thank you! Makes sense to me; what do you think, @morrisonlevi? |
Hi @javiereguiluz, Levi and I aren't design experts, so this kind of detail isn't something we'd ever pick up on, so thanks for this knowledge and PR. Your change is good with me. As for the consistency of this change, our "stylesheet" is spread out across a number of different areas, so I think this change needs to be replicated across more .css files. Our php.net web stylesheet is/was separated from the docs.php.net, so I recommend looking around different parts of the site to see if you can find more stylesheets that have the "old value" in them.. You can also What do you think @javiereguiluz and are you able to find more places to make this same change? :) |
Thanks for the quick review! I've looked for and there are no other occurrences of this in In the https://github.com/php/web-wiki repository, there's a sidebar that is similar to the one in |
There are some more places with links that are magenta-on-hover on dark backgrounds, including but not limited to:
Also, it may make more sense to refactor the approach. In the medium theme, there is the concept of "background" (i.e. on the dark background, which is also the default) and "content" (i.e. on the light background) colours: maybe instead make changes for those broader areas rather than on individual components. |
@salathe thanks for reviewing this and I'm sorry I missed all that. In the latest commit I fixed the contrast of links in footer, breadcrumbs and other sidebar occurrences. Thanks. |
I think that the grey background should be less darker to improve accessibility without changing the tint of the magenta accent. |
I wouldn't change the background color because that would be a significant change to the design of the site. But of course I will do any change that the maintainers of this project ask me to do in order to make this mergeable. Thanks. |
In order to be accessible, text must have a contrast ratio of at least 4.5:1
See https://www.w3.org/WAI/WCAG22/Understanding/contrast-minimum.html
This increases the contrast of the selected item in the sidebar menu to reach that ratio: