Report: don't display score gauge header if there's only one. #2261
Conversation
| for (const category of report.reportCategories) { | ||
| scoreHeader.appendChild(this._categoryRenderer.renderScoreGauge(category)); | ||
| if (!isSoloCategory) |
There was a problem hiding this comment.
I thought we didn't allow no braces multiline if statement
There was a problem hiding this comment.
au contraire!
(it was a surprise to me, too)
|
|
||
| container.appendChild(this._renderReportHeader(report)); // sticky header goes at the top. | ||
| container.appendChild(this._renderReportNav(report)); | ||
|
|
||
| const reportSection = container.appendChild(this._dom.createElement('div', 'lh-report')); | ||
|
|
||
| const scoreHeader = reportSection.appendChild( | ||
| this._dom.createElement('div', 'lh-scores-header')); | ||
| const scoreHeaderClass = `lh-scores-header ${isSoloCategory ? 'lh-scores-header__solo' : ''}`; |
There was a problem hiding this comment.
Can we just not have a score header if < 2 categories?
There was a problem hiding this comment.
well i was thinking that too, but our styling uses that header's margin to get out from under the pos:fixed header.
There was a problem hiding this comment.
ok so the better fix would be to put that margin on something else.
i guess that's the action necessary here.
|
@brendankenny ptal |
| @@ -139,18 +139,22 @@ class ReportRenderer { | |||
| */ | |||
| _renderReport(report) { | |||
| const container = this._dom.createElement('div', 'lh-container'); | |||
| const isSoloCategory = report.reportCategories.length === 1; | |||
| let scoreHeader; | |||
There was a problem hiding this comment.
move these two lines down to above if (!isSoloCategory) {?
|
style nit still applies but other than that, still looks good :P |
Got tired of looking at tops of my perf/a11y reports that look like this:
This PR doesn't show the overview gauges if there's only one.