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

Report: use accessible material colors #1758

Merged
merged 1 commit into from
Feb 21, 2017
Merged

Report: use accessible material colors #1758

merged 1 commit into from
Feb 21, 2017

Conversation

ebidel
Copy link
Contributor

@ebidel ebidel commented Feb 21, 2017

R: all

The report colors were recently adjusted to meet accessibility contrast for WCAG 2 AA, but the new average color is mustard and the red/green are quite deep. This PR dials them back a bit, returns the average color to an orange, but still keeps WCAG 2 AA at 18pt. I used this tool to verify the colors are still compliant.
The colors are from the MD color palette.

cc @robdodson

Before:

screen shot 2017-02-21 at 1 04 08 pm

After:

screen shot 2017-02-21 at 1 01 43 pm

Copy link
Member

@paulirish paulirish left a comment

Choose a reason for hiding this comment

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

Thanks for tweaking average-color. The brown was also bugging me too.

--poor-color: #e53935; /* md red 600 */
--good-color: #43a047; /* md green 600 */
--average-color: #ef6c00; /* md orange 800 */
--warning-color: #757575; /* md grey 600 */
Copy link
Member

Choose a reason for hiding this comment

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

In isolation this looks a bit odd, that a warning icon is done in gray.
I'm presuming you're okay with that because any warning debugString is displayed in red?

Personally I prefer a yellow/orange/mustard color for these error states, but I can live with this just fine. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. My thinking was that colors should signify level of passing. Since we moved the warning icon to orange, it's felt like mixed messaging. e.g. orange for an audit error and orange for a 50/100 audit is kinda confusing. If anything, it might make more sense to make the failures red so users know they contribute 0 to the score.

Copy link
Member

Choose a reason for hiding this comment

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

okay. i'm feeling like we'll soon graduate to sorting all results into three buckets: exceptions/failures/passing.
Once we do that, we can be very clear about how things bubble up to a score.
Also it generally seems weird to provide a score if there were exceptions in that section.

Copy link
Member

Choose a reason for hiding this comment

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

In the meantime.. I'm ambivalent. :)

Copy link
Contributor Author

@ebidel ebidel Feb 21, 2017

Choose a reason for hiding this comment

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

seems weird to provide a score if there were exceptions in that section.

Definitely need to shake that out. @brendankenny and I had similar questions when redoing the error stuff.

@ebidel ebidel merged commit 7a5448e into master Feb 21, 2017
@ebidel ebidel deleted the colors branch February 21, 2017 23:58
@robdodson
Copy link
Contributor

image

@XhmikosR
Copy link
Contributor

Personally I see this doesn't pass WCAG2AA for at least --good-color which I can test. My recommendation would be #2b882f.

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.

None yet

4 participants