Skip to content
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

fix(a11y): update code highlight colors to meet contrast requirement #1724

Merged
merged 5 commits into from
May 2, 2024

Conversation

dancormier
Copy link
Contributor

@dancormier dancormier commented Apr 30, 2024

Addresses:


This PR resolves outstanding contrast issues with code highlight colors. The changes should be subtle enough as to barely be noticeable.

Note

We could attempt to change many of these colors to use core Stacks colors, but I figure it's best to keep it simple for now in order to resolve A11Y issues and decide later if we want to make more sweeping changes here.

Color variable changes

variable name mode old color old contrast new color new contrast
--highlight-literal dark hsl(27, 85%, 61.5%) Lc 55 hsl(27, 95%, 65%) Lc 60
--highlight-namespace dark hsl(27, 85%, 61.5%) Lc 55 hsl(27, 95%, 65%) Lc 60
--highlight-symbol dark hsl(306, 43%, 69%) Lc 51 hsl(306, 50%, 75%) Lc 60
--highlight-variable light HC hsl(88, 100%, 19%) 6.8:1 hsl(88, 100%, 18%) 7.3:1
Screenshot comparison (dark mode)

Note: The difference is in the orange (e.g.: 0.6em 0.7em) and purple (e.g.: border-radius) text.

Before

image

After

image

Screenshot comparison (light HC mode)

Note: The difference is in the green (e.g.: apples) text.

Before

image

After

image

To test

Copy link

netlify bot commented Apr 30, 2024

Deploy Preview for stacks ready!

Name Link
🔨 Latest commit 90e2854
🔍 Latest deploy log https://app.netlify.com/sites/stacks/deploys/66326cac46317e00084fcec7
😎 Deploy Preview https://deploy-preview-1724--stacks.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@dancormier dancormier changed the title fix(codeblock): resovle contrast requirement violations fix(code-block): resovle contrast requirement violations Apr 30, 2024
@dancormier dancormier changed the title fix(code-block): resovle contrast requirement violations fix(a11y): update code highlight colors to meet contrast requirement Apr 30, 2024
@dancormier dancormier marked this pull request as ready for review April 30, 2024 22:20
Copy link
Collaborator

@CGuindon CGuindon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done! I think this is subtle enough that even meta won't notice (the best kind of change) 🥇

For what it's worth, I vaguely remember discussing using Stacks colors for code way back when and the reason we didn't go that route was because it was:
A. really hard because we have less colors to work with
B. felt/looked odd to most developers because the colors were different than the tones they are used to seeing in code

I support the decision to NOT go down that rabbit hole — I think that would need CM support and lots of feedback rounds.

Copy link
Contributor

@giamir giamir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a fantastic subtle change that will make us compliant with our own contrast targets. Awesome work @dancormier ...and I love we have tests helping us to catch future regressions.

@dancormier dancormier merged commit 706c4b4 into develop May 2, 2024
11 checks passed
@dancormier dancormier deleted the dcormier/a11y-codeblock-dark-mode-contrast branch May 2, 2024 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants