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(material/divider): non-text color contrast issues #28995

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

DBowen33
Copy link
Contributor

@DBowen33 DBowen33 commented May 2, 2024

Fixes color contrast issues with non-text elements, including divider. Changed from "outline-variant" to "outline" colors to pass 3:1 ratio between light/dark mode backgrounds. Also changing for expansion, stepper, and table so they can also pass not-text color contrast.

Before (Light mode):
FG- #c4c6d0 (neutral-variant80)
BG- #fff
Ratio: 1.70:1
Screenshot:
image

After (Light mode):
FG- #747775 (neutral-variant50)
BG- #fff
Ratio: 4.52:1
Screenshot:
image

Before (Dark mode):
FG- #44474E (neutral-variant30)
BG- #1f1f1f
Ratio: 1.77:1
Screenshot:
image

After (Dark mode):
FG- #8e9099 (neutral-variant60)
BG- #1f1f1f
Ratio: 5.17:1
Screenshot:
image

Fixes b/291964002

@DBowen33 DBowen33 marked this pull request as ready for review May 2, 2024 19:49
@DBowen33 DBowen33 requested a review from mmalerba as a code owner May 2, 2024 19:49
Fixes color contrast issues with non-text elements. Changed from outline-variant to outline colors to pass 3:1 ratio between light/dark mode backgrounds.

Fixes b/291964002
@DBowen33 DBowen33 force-pushed the a11y-divider-contrast-fix branch from 8c001cf to 2a77bbf Compare May 2, 2024 20:01
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

It might just be me, but the new color seems a bit too dark. Also worth noting that the outline-variant is according to the spec here: https://m3.material.io/components/divider/specs

@andrewseguin
Copy link
Contributor

Unfortunately the spec explicitly states that it does not account for contrast ratios for the divider. Whether we go with outline or some slightly less-contrasty color, I think it's going to look strange at a the minimum 3:1 ratio. This one is a bit out of our hands unless we want to revise this to be a different palette color than outline.

To make this a little more acceptable, can we update the list demo's outline color to match this value? It does look weird that the list outline is so much lighter than the divider

fixed border list demo color to match divider color

fixes b/291964002
@DBowen33
Copy link
Contributor Author

DBowen33 commented May 8, 2024

Unfortunately the spec explicitly states that it does not account for contrast ratios for the divider. Whether we go with outline or some slightly less-contrasty color, I think it's going to look strange at a the minimum 3:1 ratio. This one is a bit out of our hands unless we want to revise this to be a different palette color than outline.

To make this a little more acceptable, can we update the list demo's outline color to match this value? It does look weird that the list outline is so much lighter than the divider

Changed border color in demo list to match mat-divider color so it looks visually better

@andrewseguin andrewseguin added the dev-app preview When applied, previews of the dev-app are deployed to Firebase label May 9, 2024
Copy link

github-actions bot commented May 9, 2024

Deployed dev-app for e65e9d9 to: https://ng-dev-previews-comp--pr-angular-components-28995-dev-nng8pkz4.web.app

Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev-app preview When applied, previews of the dev-app are deployed to Firebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants