report: improve color contrast of top-level errors #10636

baseeee · 2020-04-25T17:47:15Z

Summary

Added higher contrast colors and made slight layout adjustments to the runtime error warning box on the report webpage.

Related Issues/PRs

…corners

googlebot · 2020-04-25T17:47:21Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

baseeee · 2020-04-25T17:55:47Z

@googlebot I signed it!

googlebot · 2020-04-25T17:55:51Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

connorjclark

thanks for capturing the details in the issue! there's a small contrast issue, but most of my comments have to due with maintaining the consistency in this file.

connorjclark · 2020-04-27T23:35:07Z

lighthouse-core/report/html/report-styles.css

@@ -66,7 +67,7 @@
  --color-informative: var(--color-blue-900);
  --color-pass-secondary: var(--color-green-700);
  --color-pass: var(--color-green);
-  --color-hover: var(--color-gray-50);
+  --color-hover: var(--color-gray-50);  


could you alphabetize this? you can just select all the lines and "sort" in your IDE–we keep everything here sorted.

also I think there is some whitespace at the end of this line (and a couple others)

Corrected these

connorjclark · 2020-04-27T23:42:46Z

lighthouse-core/report/html/report-styles.css

@@ -1010,16 +1011,21 @@

 .lh-warnings--toplevel {
  --item-margin: calc(var(--header-line-height) / 4);
-  color: var(--report-text-color-secondary);
+  color: var(--report-text-color);  


can you make a --toplevel-warning-text-color variable, initialized to var(--report-text-color), and vary it to var(--color-orange-700) in dark mode? Trying to solve these things:

color contrast in light mode for this text fails the axe audit. Let's ignore the color given in the design for that and just use black.

we try not to use colors directly in the css, but instead alias to a component variable. see the "naming" comment at the top of the file

I may have misunderstood this so apologies if this needs corrected.

I believe it was the text "There were issues affecting this run of Lighthouse:" that was causing the contrast issue, the text color for this is controlled by the .lh-warnings__msg class so I have created another component variable for this text color along with the --toplevel-warning-text-color requested.

I think I've followed the correct conventions for this but I'm unsure if the result is exactly what you were looking for.

connorjclark · 2020-04-27T23:59:22Z

lighthouse-core/report/html/report-styles.css

-  max-width: calc(var(--report-width) - var(--category-padding) * 2);
-  background-color: var(--color-amber-50);
+  max-width: calc(var(--report-width) - var(--category-padding) * 2);  
+  background-color: rgba(var(--color-orange-rgb), .1);


introduce a new variable --toplevel-warning-background-color

Instead of using alpha, I think we want it set to orange 100 (#FFCCBC) in light mode and #544B40 for dark mode (I don't see a clean fit from the material ui colors, oh well)

initial value: var(--color-orange-100)
dark mode: #544B40 (unless you can find a good material ui color)

Corrected this, although I set the light mode background color to orange-50 instead of orange-100 as I felt this was a closer match to the original color

connorjclark · 2020-05-04T21:47:00Z

@baseeee thanks for your contributions so far! I think this is important for our 6.0 release in ~3 weeks. Are you able to incorporate the feedback by then? If not, I'm happy to take over.

baseeee · 2020-05-05T18:25:38Z

@connorjclark Hello, sorry for my periods of inactivity, life's been pretty manic recently but I still really want to help out. I can sort out the issues this weekend if that suits your timescale.

Thank you for the comprehensive notes and feedback too, it's a massive help.

connorjclark · 2020-05-05T19:55:49Z

@connorjclark Hello, sorry for my periods of inactivity, life's been pretty manic recently but I still really want to help out. I can sort out the issues this weekend if that suits your timescale.

Thank you for the comprehensive notes and feedback too, it's a massive help.

No worries! There's still plenty of time :)

paulirish · 2020-05-07T20:28:47Z

No worries! There's still plenty of time :)

A quick note. We're cutting our release next week, actually. We'll very happily finish off this PR monday if you don't get around to it, @baseeee. No big deal either way. :D

baseeee · 2020-05-09T15:23:21Z

@connorjclark I've finished making these changes, sorry if you still need to tweek anything before the release. Thanks for letting me help out :)

connorjclark

thanks for sticking to it! css and vars look good. only thing left are trivial:

using spaces instead of tabs (sry! I wish we had a linter for this)
asking @hwikyounglee if we meant to drop the bold text

lighthouse-core/report/html/report-styles.css

connorjclark · 2020-05-09T20:33:27Z

lighthouse-core/report/html/templates.html

@@ -18,7 +18,7 @@
 <!-- Lighthouse run warnings -->
 <template id="tmpl-lh-warnings--toplevel">
  <div class="lh-warnings lh-warnings--toplevel">
-    <strong class="lh-warnings__msg"></strong>
+    <p class="lh-warnings__msg"></p>


@hwikyounglee just wanna double check we want to make this be not bold.

this:

or that:

Per the design

No bold

On the light mode: the title 'There were...' should be in orange #d04900

Could we try the color for the light mode?
I think the colors would do it without bold weight.

Side note: the orange in dark mode is #ffa400

https://yuinchien.com/sandbox/lighthouse/ (click on the (i) at the top right, and also 'd' for light/dark toggle)

the color contrast for those light colors #d04900 (text) and rgb(255, 246, 234) (background) has too low contrast according to axe :(

Wondering if we're in a place to add or replace the color with #BD4200

It's 4.97
https://contrast-ratio.com/#%23BD4200-on-%20rgb%28255%2C%20246%2C%20234%29

seems good to me.

lighthouse-core/report/html/report-styles.css

connorjclark · 2020-05-12T21:58:08Z

thanks @baseeee and @hwikyounglee ! lookin good https://lighthouse-fxbzonyd8.now.sh/error/

baseeee added 2 commits March 22, 2020 15:02

Changed runtime error message color themes

7177fd0

Reduced padding on runtime error message and rounded the message box …

15827c0

…corners

baseeee requested a review from a team as a code owner April 25, 2020 17:47

baseeee requested review from connorjclark and removed request for a team April 25, 2020 17:47

vercel bot deployed to Preview April 25, 2020 17:47 View deployment

googlebot added the cla: no label Apr 25, 2020

googlebot added cla: yes and removed cla: no labels Apr 25, 2020

devtools-bot assigned connorjclark Apr 26, 2020

devtools-bot added the waiting4reviewer label Apr 26, 2020

connorjclark requested changes Apr 28, 2020

View reviewed changes

connorjclark added waiting4committer 6.0 and removed waiting4reviewer labels May 1, 2020

GoogleChrome deleted a comment May 5, 2020

Fix contrast issue and correct file inconsistencies

33bb1aa

vercel bot deployed to Preview May 9, 2020 14:38 View deployment

Merge branch 'master' into master

8f402b9

vercel bot deployed to Preview May 9, 2020 14:57 View deployment

connorjclark requested changes May 9, 2020

View reviewed changes

Replace tabs with spaces and sort color variables

b7a38cb

vercel bot deployed to Preview May 10, 2020 14:51 View deployment

connorjclark reviewed May 12, 2020

View reviewed changes

lighthouse-core/report/html/report-styles.css Outdated Show resolved Hide resolved

Update lighthouse-core/report/html/report-styles.css

9d33ee2

vercel bot deployed to Preview May 12, 2020 21:51 View deployment

connorjclark changed the title ~~report(webpage): update the style of runtime error message~~ report: improve color contrast of top-level errors May 12, 2020

connorjclark added the land-when-ci-is-green label May 12, 2020

connorjclark approved these changes May 12, 2020

View reviewed changes

devtools-bot merged commit a462d31 into GoogleChrome:master May 12, 2020

devtools-bot removed the land-when-ci-is-green label May 12, 2020

paulirish mentioned this pull request May 13, 2020

misc: drop trailing whitespace in css #10765

Merged

connorjclark mentioned this pull request Jun 9, 2020

runtime errors in dark mode looks bad #9559

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

report: improve color contrast of top-level errors #10636

report: improve color contrast of top-level errors #10636

baseeee commented Apr 25, 2020

googlebot commented Apr 25, 2020

baseeee commented Apr 25, 2020

googlebot commented Apr 25, 2020

connorjclark left a comment

connorjclark Apr 27, 2020

baseeee May 9, 2020

connorjclark Apr 27, 2020

baseeee May 9, 2020

connorjclark Apr 27, 2020

baseeee May 9, 2020

connorjclark commented May 4, 2020

baseeee commented May 5, 2020

connorjclark commented May 5, 2020

paulirish commented May 7, 2020 •

edited

Loading

baseeee commented May 9, 2020

connorjclark left a comment

connorjclark May 9, 2020

hwikyounglee May 10, 2020 •

edited

Loading

connorjclark May 11, 2020

hwikyounglee May 12, 2020

connorjclark May 12, 2020

connorjclark commented May 12, 2020

report: improve color contrast of top-level errors #10636

report: improve color contrast of top-level errors #10636

Conversation

baseeee commented Apr 25, 2020

googlebot commented Apr 25, 2020

What to do if you already signed the CLA

Individual signers

Corporate signers

baseeee commented Apr 25, 2020

googlebot commented Apr 25, 2020

connorjclark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

connorjclark commented May 4, 2020

baseeee commented May 5, 2020

connorjclark commented May 5, 2020

paulirish commented May 7, 2020 • edited Loading

baseeee commented May 9, 2020

connorjclark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hwikyounglee May 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

connorjclark commented May 12, 2020

paulirish commented May 7, 2020 •

edited

Loading

hwikyounglee May 10, 2020 •

edited

Loading