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.

Sign up for GitHub

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: show fail icon for errors #9272

Closed
wants to merge 2 commits into from
Closed

report: show fail icon for errors #9272

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8984f34
report: show fail icon for errors
connorjclark Jun 24, 2019
7ed1598
use md icon
connorjclark Jun 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 4 additions & 11 deletions lighthouse-core/report/html/report-styles.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,8 @@
.lh-audit--fail .lh-audit__display-text {
color: var(--color-fail-secondary);
}
.lh-audit--fail .lh-audit__score-icon {
.lh-audit--fail .lh-audit__score-icon,
.lh-audit--error .lh-audit__score-icon {
border-left: calc(var(--score-icon-size) / 2) solid transparent;
border-right: calc(var(--score-icon-size) / 2) solid transparent;
border-bottom: var(--score-icon-size) solid var(--color-fail);
Expand All @@ -444,10 +445,6 @@
background: var(--color-gray-400);
}

.lh-audit--error .lh-audit__score-icon {
display: none;
}

.lh-audit--informative .lh-audit__display-text {
color: var(--color-gray-600);
}
Expand Down Expand Up @@ -694,7 +691,8 @@
.lh-metric--fail .lh-metric__value {
color: var(--color-fail-secondary);
}
.lh-metric--fail .lh-metric__innerwrap::before {
.lh-metric--fail .lh-metric__innerwrap::before,
.lh-metric--error .lh-metric__innerwrap::before {
border-left: calc(var(--score-icon-size) / 2) solid transparent;
border-right: calc(var(--score-icon-size) / 2) solid transparent;
border-bottom: var(--score-icon-size) solid var(--color-fail);
Expand All @@ -705,11 +703,6 @@
color: var(--color-fail-secondary);
}

/* Hide icon if there was an error */
Copy link
Collaborator

Choose a reason for hiding this comment

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

old selves seemed fairly convinced we should intentionally, but why 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

#5077 seems to be the last time it was touched, so it's fair to say a lot has changed since then :)

Copy link
Member

Choose a reason for hiding this comment

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

seems to be the last time it was touched, so it's fair to say a lot has changed since then :)

It's a shame we didn't preserve much context. You were very excited @patrickhulce :)

One issue is conflation of lighthouse errors with audit failures, which is something some people already get confused about (thinking "Errror!" means their page had an error). I don't know what a better icon would be, though.

Copy link
Collaborator

Choose a reason for hiding this comment

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

lol, yeah it's hard to tell which subchange specifically I was most excited about there, but I remember being super jazzed about the error tooltip and no more informative scores that people have no idea what they mean, so I'm guessing it's somewhat tangential to our change here?

agreed more context would've been helpful now. I also seems worth it to try to move these two states to be less similar in the UI given how frequently people confuse the Error for their page doing something wrong.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

black hexagon (stop sign)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@exterkamp "upside down red triangle. or an X"

.lh-metric--error .lh-metric__innerwrap::before {
display: none;
}

/* Perf load opportunity */

.lh-load-opportunity__cols {
Expand Down