-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Okaidia: Update comment text color to meet WCAG contrast recommendations to AA level #2292
Okaidia: Update comment text color to meet WCAG contrast recommendations to AA level #2292
Conversation
We usually don't accept stylistic changes to Prism's default themes. That being said, I think that enforcing good color contrasts is a good thing but I think that if we do it, we should do it for all themes (Just from looking, I guess that other themes could benefit from higher contrasts as well). |
Thanks for your quick response Michael. I understand your point of view with unexpected surprises for existing users. To provide more context, the contrast between So I think it comes down to how much breaking changes are we willing to make, and if the reduction of code:comment contrast by |
In general, I'd be in favor of passing WCAG for contrast, especially since the change doesn't seem huge. However, the proposed color has a different hue. Please use https://css.land/lch or any other Lab or LCH picker to produce a suitable rgb() color with the same hue and chroma, just higher lightness. Please note that HSL is not a suitable model for this adjustment. |
…endations to AA level
Thank you so much for your feedback @LeaVerou . I did not know about LCH colors, and it indeed makes sense. I learned this today and will be super helpful to personal projects too. The current color is I updated the issue summary with new color and attached a screenshot too. Same screenshot below: |
Should I make a test that checks the contrast of (almost) all token colors? (Maybe even with automated contrast correction based on LCH?) I mean, there are a few colors in other themes that have even lower contrasts than comments in Okaidia (e.g. |
Aw yes, tests throughout the appearance would be lovely! I think Google Chrome lighthouse CI can run tests on an example rendered page that we try to all possible tokens can help? I don't have expertise with it but I'm available to collaborate if necessary. Thank you. |
I was thinking of parsing the theme file, extraction all colors, and computing the contrast against a fixed background color. It's a simple approach and it allows us to exclude some tokens (e.g. it's fine if the contrast of punctuation was a little lower). |
I'm still mildly in favor of this but would also be ok with slotting this into a potential 2.0 along with #2313. |
I think it's ok to merge this. The change is relatively small and, IMO, can be seen as a fix. |
…endations to AA level (PrismJS#2292)
Hi everyone,
Text color for comments used in the Okaidia theme is slightly falling behind the minimum text contrast level required by WCAG guidelines.
This PR changes the text color slightly to meet the 4.5 requirement.
Please see the comparison below:
New4.5481909e129, 144, 158209, 13%, 56%58.928% 9.916 250.732Fingers crossed this can be merged and release, but I'd be happy to make any further changes if necessary.
Thank you, and stay safe 🙏🏽.
Edit: The first version of this PR suggested to use color
#81909e
, but it turns out to be a not good measurement to make lightness improvements. That commit was force-pushed to use the new color#8292a2
, which only makes lightness increments until it meets to the WCAG recommendation, and then converted to the nearest hex color.