Skip to content

Fix color contrast failures in default and colorblind/tritanopia themes#1405

Merged
jonrohan merged 8 commits into
merge-34-56-testfrom
sneharao3/fix-dark-on-emphasis-contrast
Jul 7, 2026
Merged

Fix color contrast failures in default and colorblind/tritanopia themes#1405
jonrohan merged 8 commits into
merge-34-56-testfrom
sneharao3/fix-dark-on-emphasis-contrast

Conversation

@sneharao3

@sneharao3 sneharao3 commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

Refs github/core-ux#2821

Fixes failing color-contrast checks across the default light/dark themes (now fully passing) and reduces failures in the colorblind/tritanopia themes. All changes are accessibility-only token repoints — no hue changes or brand-affecting shifts. Targets merge-34-56-test, since the fixes depend on its neutral-scale refactor.

List of notable changes

  • updated fgColor.onEmphasis (dark) to white — was a light gray on emphasis backgrounds, failing ~3.2–4.5:1
  • updated fgColor.muted / fgColor.neutral to neutral.9 (the text step) — were on neutral.7, a background step; placeholder, closed, and invisible-icon tokens inherit this
  • updated control.borderColor.emphasis (default themes) to the softest neutral step clearing the 3:1 border threshold
  • updated button.danger and button.primary text tokens to white to match onEmphasis
  • updated bgColor.neutral.emphasis (light) so white text passes

Contrast check results

Failing pairs per theme, before → after:

Theme Before After
light 15 0
dark 44 0
light_protanopia_deuteranopia 21 2
dark_protanopia_deuteranopia 48 2
light_tritanopia 21 2
dark_tritanopia 48 2
light_high_contrast 16 15
dark_high_contrast 37 10
light_protanopia_deuteranopia_high_contrast 21 19
dark_protanopia_deuteranopia_high_contrast 39 15
light_tritanopia_high_contrast 21 19
dark_tritanopia_high_contrast 39 15
dark_dimmed_high_contrast 11 11

What should reviewers focus on?

  • The step choices on the gray tokens — neutral.9 is the lightest/darkest step that clears 4.5:1 and is the designated text step.
  • Two known gaps, intentionally left for follow-up:
    • The colorblind/tritanopia themes still have 2 failing control-borderColor-emphasis rows each — those variants have their own border override the default-scoped fix didn't touch.
    • The high-contrast themes are not addressed here so the HC contrast checks will still show failures in CI by design.

@sneharao3
sneharao3 requested a review from a team as a code owner June 25, 2026 19:35
@sneharao3
sneharao3 requested a review from llastflowers June 25, 2026 19:35
@changeset-bot

changeset-bot Bot commented Jun 25, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 80cca74

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@sneharao3
sneharao3 requested review from lukasoppermann and removed request for llastflowers June 25, 2026 19:35
@github-actions
github-actions Bot temporarily deployed to Preview (Storybook) June 25, 2026 19:37 Inactive
@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Design Token Diff (CSS)

The message is too long to be displayed here. For more details, please check the job summary.

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Design Token Diff (StyleLint)

The message is too long to be displayed here. For more details, please check the job summary.

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Design Token Diff (Figma)

The message is too long to be displayed here. For more details, please check the job summary.

@sneharao3
sneharao3 requested a review from jesussandreas June 26, 2026 17:03
@@ -22,54 +22,31 @@ const Color = ({color: colorName, length = 12, border}: {color: string; length?:
export const AllScales = () => {
return (
<div className="ColorScale--grid">

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@sneharao3 why did you delete the colors from the story here?

@sneharao3 sneharao3 Jun 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think it's showing because this branch is based on merge-34-56-test and is picking up some of those commits. I don't think this one is from my changes since my changes are only the token repoints in the color JSON files. How do you think I should handle this?

Comment thread src/tokens/component/button.json5 Outdated
'org.primer.overrides': {
dark: {
'$value': '{fgColor.white}'
'$value': '{base.color.white}'

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@sneharao3 why is this not using fgColor.white, does that make sense?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch! You're right, this should be fgColor.white. That was an unintended change

@jonrohan
jonrohan merged commit 6570bbf into merge-34-56-test Jul 7, 2026
18 of 26 checks passed
@jonrohan
jonrohan deleted the sneharao3/fix-dark-on-emphasis-contrast branch July 7, 2026 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants