-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Updated report styles #2297
Updated report styles #2297
Conversation
I'm using a 12px baseline font for this design. For the standalone report, I'd prefer to use a 14px baseline (should only require tweaking a few CSS variables). Just let me know how we'll differentiate a DevTools- and CLI-generated report and I can make the adjustments. |
@chowse we have Also the |
Adding it to devtools is just a few line change though if we need it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks reaaaaaal nice 👌
|
||
const sparklineBarEl = this._dom.find('.lh-sparkline__bar', tmpl); | ||
sparklineBarEl.style.width = `${audit.result.rawValue / scale * 100}%`; | ||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leftover comment?
passedSummary.textContent = `View ${elements.length} passed items`; | ||
const passedElem = this._renderAuditGroup({ | ||
title: `${elements.length} Passed Audits`, | ||
description: '', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can probably just not set this now that we've made it optional
} | ||
const nonPassedElem = this._renderAuditGroup({ | ||
title: `${nonPassedAudits.length} failed audits`, | ||
description: '', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
padding-left: var(--expandable-indent); | ||
} | ||
|
||
.lh-audit-group[open] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this have any content?
@@ -639,24 +750,20 @@ span.lh-node:hover { | |||
} | |||
|
|||
.lh-category .lh-audit { | |||
margin-left: calc(var(--default-padding) * 2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are none of these rules needed anymore then?
--inset-size: calc(var(--circle-size) - var(--circle-border-width)); | ||
--circle-background: #ccc; | ||
--circle-border-width: 2px; | ||
--inset-size: calc(var(--circle-size) - 2* var(--circle-border-width)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: 2 *
I can submit a fix to #2385 once this gets merged |
@paulirish do you have a sense of remaining things to resolve? Can we make a list? |
Yah. I have a branch pushed with some more work. I'll prep a list of remaining work tonight. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the Lighthouse report's styling to better match the DevTools UI (with options to restyle it using CSS variables).