-
Notifications
You must be signed in to change notification settings - Fork 333
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
Make the site higher contrast for accessibility #970
Conversation
A side note for myself; should probably do a cleanup of the scss at some point. Remove duplication, centralize more universal variables in |
@TimJentzsch if you have time and interest I'd love your opinion on this: you did a great job helping me out with website a11y in the past. |
This reverts commit aff7c3d. It was out of scope for this PR. Will be dealt with in bevyengine#971 .
The color used (forest green) is arbitrary and just works; I'm not sure what would be the best universal color.
1122f25
to
ec59445
Compare
(I do support this general idea though) |
Okay, I think I've got all the contrast issues based on manual checking. Should note that until #973 is merged the anchor links will still show as having issues. |
Agreed. I think it would also be better to use semantic variable names like primary and secondary instead of stuff like blue and pink, that would also make it easier to introduce themes (e.g. a light theme). |
I made a website for exactly this issue: Finding a higher contrast color while staying as close as possible to the original: https://color-id.timjen.net/?color=%23bb799c&secondary=%23ececec#accessibility Looks like it suggests |
This reverts commit 2558476.
Uses the color suggestions provided by `color-id.timjen.net` to minimize the difference whilst ensuring the colors are contrasting.
In the original the button borders are darker than the primary color. I'd changed it to be lighter thinking it'd fit more; but since stuff is wanted similar to original I'm making the border darker again.
Also reverted the white color change since I remembered off-white is preferable. |
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.
Personally, I like the changes.
Accessibility by default is always good to pursuit and this would also make it better e.g. when viewing the page on mobile in bright sunlight.
Cart will have to approve this before merging though.
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.
Alrighty I think this is in a reasonable spot. I opted to lighten the border instead of darken it because I think it makes the buttons pop / feel less drab. Thanks for putting this together!
Actually one sec lets not merge yet. I've just noticed the forestgreen image-comparison color 😆 |
Feel free to change it to whichever color—that is generically contrasting enough—that suits your fancy. That was just the one I found that seemed to work; not attached to it. |
I strongly suspect that it is somehow confused due to how we are rendering that text. The text is white with a black outline, which should make it pop in pretty much every scenario. The "best case" scenario (black background, white text) fails the firefox contrast test, despite being significantly more contrast-ey than the rest of the site: I'm going to revert this change for now in the interest of moving forward quickly. If you want to make your case for changing this one, lets do it in a separate PR. |
Alright. I suspect it will be forced to be dealt with once accessibility linting is added; in that case CI will complain about it until a solution is found. |
Was supposed to be fixed in bevyengine#970, but it got lost in a confusing merge conflict.
Makes the site higher contrast to address accessibility issues with the site.
Going thru this I think #396 is a good idea to catch any accessibility issues since at the moment I'm just checking the Firefox accessibility tools manually.
Fixes #3