report: use bang for errors #11670

rahulpatel596 · 2020-11-15T06:46:40Z

Summary
What kind of change does this PR introduce?
Report error design update to exclamation mark

Describe the need for this change
Runtime design update

Related Issues/PRs
Issue #9820
Please let me know if there is any issue or need any changes :)

google-cla · 2020-11-15T06:46:44Z

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.

rahulpatel596 · 2020-11-15T07:13:11Z

@googlebot I signed it!

…

On Sun, Nov 15, 2020 at 12:46 AM google-cla[bot] ***@***.***> wrote: 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/ <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 <https://cla.developers.google.com/clas> and verify that your email is set on your git commits <https://help.github.com/articles/setting-your-email-in-git/>. 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 <https://opensource.google/docs/cla/#troubleshoot>). - The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data <https://cla.developers.google.com/clas> and verify that your email is set on your git commits <https://help.github.com/articles/setting-your-email-in-git/>. - The email used to register you as an authorized contributor must also be attached to your GitHub account <https://github.com/settings/emails>. ℹ️ *Googlers: Go here <https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2FGoogleChrome%2Flighthouse%2Fpull%2F11670> for more info*. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#11670 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIAPUBOKR4BJOWJAILEQX43SP52N7ANCNFSM4TWAPNDA> .

google-cla · 2020-11-15T07:13:15Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

rahulpatel596 · 2020-11-15T07:19:31Z

@googlebot I fixed it!

google-cla · 2020-11-15T07:19:34Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

google-cla · 2020-11-15T07:20:24Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

updated category-renderer.js file to fix ubuntu test

google-cla · 2020-11-15T07:41:41Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

patrickhulce

Thanks so much @rahulpatel596! 🎉

If you take a look at https://lighthouse-19vbwclqz.vercel.app/error/ I think we're still missing the replacement of the ? with the exclamation for the category scores.

Looks good though! :)

patrickhulce · 2020-11-16T16:57:59Z

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

 .lh-scorescale-range--fail::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);
 }

+.lh-audit--error .lh-audit__score-icon{
+  height:28px;


Suggested change

height:28px;

height: 28px;

min-width: 10px;

without this the icons got squished

would also be nice to pull these values out into CSS variables like --score-icon-size is :)

patrickhulce · 2020-11-16T16:59:20Z

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

 .lh-scorescale-range--fail::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);
 }

+.lh-audit--error .lh-audit__score-icon{


Suggested change

.lh-audit--error .lh-audit__score-icon{

.lh-audit--error .lh-audit__score-icon {

Just curious, Is there a specific reason why is space required here or is it just code standard for project?

Just readability and consistency in style :)

patrickhulce · 2020-11-16T16:59:41Z

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

+.lh-metric--error .lh-metric__innerwrap::before{
+  height:28px;


Suggested change

.lh-metric--error .lh-metric__innerwrap::before{

height:28px;

.lh-metric--error .lh-metric__innerwrap::before {

height: 28px;

min-width: 10px;

rahulpatel596 · 2020-11-17T00:02:42Z

Hey @patrickhulce
Thank you so much for reviewing. I'll make changes and create a PR

patrickhulce · 2020-11-17T01:47:25Z

You're very welcome @rahulpatel596 thanks for putting this together and working on it!

I'll make changes and create a PR

No need to create another new PR, you can keep pushing to your fork to update this existing PR :)

google-cla · 2020-11-17T06:44:15Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

rahulpatel596 · 2020-11-17T06:55:02Z

Hey @patrickhulce I made changes for above reviews and also added exclamation for category scores. I had a quick question regarding header exclamation icon size. It seems like there were no details regarding this header icon size in issue design. would it be 120px * 120px or am I missing any details here.

patrickhulce

thanks @rahulpatel596!

looks like we're missing the top header overrides too

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

patrickhulce · 2020-11-17T16:57:24Z

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

@@ -128,6 +128,8 @@
  --score-icon-margin: 0 var(--score-icon-margin-right) 0 var(--score-icon-margin-left);
  --score-icon-size: 12px;
  --scores-container-padding: 20px 0 20px 0;
+  --error-icon-size: 28px;


there's a lot of padding here that inflates this number above how large the icon actually is, WDYT about using a version with the padding stripped off like the below?

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 12 18" width="12" height="18" fill="none"> <path d="M 4.5 16.5 L 7.5 16.5 L 7.5 13.5 L 4.5 13.5 L 4.5 16.5 Z" fill="#FF4E42"/> <path d="M 4.5 10.5 L 7.5 10.5 L 7.5 1.5 L 4.5 1.5 L 4.5 10.5 Z" fill="#FF4E42"/> </svg>

patrickhulce · 2020-11-17T17:01:36Z

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

@@ -1105,6 +1123,16 @@
  --gauge-circle-size: var(--gauge-circle-size-big);
 }

+.lh-sticky-header--visible > .lh-gauge__wrapper--fail > .lh-gauge__svg-wrapper {
+  background-image: url("data:image/svg+xml,%3Csvg width='36' height='36' viewBox='0 0 36 36' fill='none' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath d='M17 26H20V23H17V26Z' fill='%23FF4E42'/%3E%3Cpath d='M17 20H20V11H17V20Z' fill='%23FF4E42'/%3E%3C/svg%3E");


I don't think we need to modify the SVG here, just position it

background-position: center; background-repeat: no-repeat;

What you mean by not modifying svg in this comment. Do you mean using same sized svg for both these cases, and just scaling it up or down.
.lh-sticky-header--visible > .lh-gauge__wrapper--fail > .lh-gauge__svg-wrapper
&
.lh-category-header .lh-gauge__wrapper--fail .lh-gauge__svg-wrapper

I mean that we should reuse the same SVG data URL for all of these (kept in a CSS variable) and not mess with its width/height/viewBox.

In this case we don't even need to scale it up (though we will need to for the large gauge)

patrickhulce · 2020-11-17T17:03:00Z

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

+}
+
+.lh-category-header .lh-gauge__wrapper--fail  .lh-gauge__svg-wrapper{
+  background-image: url("data:image/svg+xml,%3Csvg width='120' height='120' viewBox='0 0 120 120' fill='none' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath d='M56 80H64V72.0001H56V80Z' fill='%23FF4E42'/%3E%3Cpath d='M56 64H64V41H56V64Z' fill='%23FF4E42'/%3E%3C/svg%3E%0A");


rather than modifying the SVG itself, let's try to use background-size to scale it up

maybe we can start with...

background-size: calc(var(--gauge-circle-size-big) * 0.6);

patrickhulce · 2020-11-17T17:04:04Z

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

+  width:36px;
+}
+
+.lh-category-header .lh-gauge__wrapper--fail  .lh-gauge__svg-wrapper{


we don't want this to appear with all failing scores, just errored ones

(what it looks like right now)

For guage, I was only able to find these classes for guage .lh-gauge__wrapper--pass .lh-gauge__wrapper--average .lh-gauge__wrapper--fail or .lh-gauge__wrapper--not-applicable
There doesn't seem to any class for error, Do I need to add it for not-applicable one?
Thank you so much for answering my noob questions :)

Looks like you'll need to add one in the same place where you updated percentageEl.textContent :)

Thank you so much for answering my noob questions :)

Of course that's what we're here for 😃

google-cla · 2020-11-18T01:58:09Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

patrickhulce · 2020-12-11T02:22:26Z

@rahulpatel596 are you still interested in taking this on? :) We have a release coming up if you'd like to include it, but just let us know if you've gotten busy with other work to hand this one off.

rahulpatel596 · 2020-12-13T01:25:53Z

are you still interested in taking this on? :) We have a release coming up if you'd like to include it, but just let us know if you've gotten busy with other work to hand this one off.

@patrickhulce Hey I am really sorry, I've gotten really busy with other stuff. You can hand this off, Thank you so much for all your help, Appreciate it :)

patrickhulce · 2020-12-14T18:16:26Z

Thanks @rahulpatel596 no worries we appreciate the progress you made :)

report-error-design-update

9f3fa32

rahulpatel596 requested a review from a team as a code owner November 15, 2020 06:46

rahulpatel596 requested review from patrickhulce and removed request for a team November 15, 2020 06:46

google-cla bot added the cla: no label Nov 15, 2020

vercel bot deployed to Preview November 15, 2020 06:46 View deployment

devtools-bot assigned patrickhulce Nov 15, 2020

devtools-bot added the waiting4reviewer label Nov 15, 2020

rahulpatel596 changed the title ~~report-error-design-update~~ report(runtime-error): design-update Nov 15, 2020

report(runtime-error): design-update

9d894d8

updated category-renderer.js file to fix ubuntu test

vercel bot deployed to Preview November 15, 2020 07:41 View deployment

patrickhulce suggested changes Nov 16, 2020

View reviewed changes

patrickhulce added waiting4committer and removed waiting4reviewer labels Nov 16, 2020

connorjclark changed the title ~~report(runtime-error): design-update~~ report: use bang for errors Nov 16, 2020

Rahul Patel added 2 commits November 17, 2020 00:41

report: use bang for error (updated category-icon)

15183b8

report: use bang for error (updated category-icon)

23891e4

vercel bot deployed to Preview November 17, 2020 06:44 View deployment

patrickhulce suggested changes Nov 17, 2020

View reviewed changes

report: use bang for errors(updated svg variables)

feefb09

vercel bot deployed to Preview November 18, 2020 01:58 View deployment

patrickhulce closed this Dec 14, 2020

patrickhulce mentioned this pull request Dec 14, 2020

Runtime Error Design Update #9820

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

report: use bang for errors #11670

report: use bang for errors #11670

rahulpatel596 commented Nov 15, 2020

google-cla bot commented Nov 15, 2020

rahulpatel596 commented Nov 15, 2020 via email

google-cla bot commented Nov 15, 2020

rahulpatel596 commented Nov 15, 2020 •

edited

Loading

google-cla bot commented Nov 15, 2020

google-cla bot commented Nov 15, 2020

google-cla bot commented Nov 15, 2020

patrickhulce left a comment

patrickhulce Nov 16, 2020

patrickhulce Nov 16, 2020

patrickhulce Nov 16, 2020

patrickhulce Nov 16, 2020

rahulpatel596 Nov 17, 2020 •

edited

Loading

patrickhulce Nov 17, 2020

patrickhulce Nov 16, 2020

rahulpatel596 commented Nov 17, 2020

patrickhulce commented Nov 17, 2020

google-cla bot commented Nov 17, 2020

rahulpatel596 commented Nov 17, 2020

patrickhulce left a comment

patrickhulce Nov 17, 2020

patrickhulce Nov 17, 2020

rahulpatel596 Nov 18, 2020

patrickhulce Nov 18, 2020

patrickhulce Nov 17, 2020

patrickhulce Nov 17, 2020

rahulpatel596 Nov 18, 2020

patrickhulce Nov 18, 2020 •

edited

Loading

google-cla bot commented Nov 18, 2020

patrickhulce commented Dec 11, 2020

rahulpatel596 commented Dec 13, 2020

patrickhulce commented Dec 14, 2020

	.lh-audit--error .lh-audit__score-icon{
	.lh-audit--error .lh-audit__score-icon {

		.lh-metric--error .lh-metric__innerwrap::before{
		height:28px;

-.lh-metric--error .lh-metric__innerwrap::before{
-  height:28px;
+.lh-metric--error .lh-metric__innerwrap::before {
+  height: 28px;
+  min-width: 10px;

report: use bang for errors #11670

report: use bang for errors #11670

Conversation

rahulpatel596 commented Nov 15, 2020

google-cla bot commented Nov 15, 2020

What to do if you already signed the CLA

Individual signers

Corporate signers

rahulpatel596 commented Nov 15, 2020 via email

google-cla bot commented Nov 15, 2020

rahulpatel596 commented Nov 15, 2020 • edited Loading

google-cla bot commented Nov 15, 2020

google-cla bot commented Nov 15, 2020

google-cla bot commented Nov 15, 2020

patrickhulce 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

rahulpatel596 Nov 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rahulpatel596 commented Nov 17, 2020

patrickhulce commented Nov 17, 2020

google-cla bot commented Nov 17, 2020

rahulpatel596 commented Nov 17, 2020

patrickhulce 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

Choose a reason for hiding this comment

patrickhulce Nov 18, 2020 • edited Loading

Choose a reason for hiding this comment

google-cla bot commented Nov 18, 2020

patrickhulce commented Dec 11, 2020

rahulpatel596 commented Dec 13, 2020

patrickhulce commented Dec 14, 2020

rahulpatel596 commented Nov 15, 2020 •

edited

Loading

rahulpatel596 Nov 17, 2020 •

edited

Loading

patrickhulce Nov 18, 2020 •

edited

Loading