-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
report: New audit clump for audits with warnings (#5327) #6989
Changes from all commits
bb6b1aa
d660748
fb8834b
4dcd6dc
3737a96
3c322af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,7 +22,7 @@ | |
/** @typedef {import('./report-renderer.js')} ReportRenderer */ | ||
/** @typedef {import('./details-renderer.js')} DetailsRenderer */ | ||
/** @typedef {import('./util.js')} Util */ | ||
/** @typedef {'failed'|'manual'|'passed'|'notApplicable'} TopLevelClumpId */ | ||
/** @typedef {'failed'|'warning'|'manual'|'passed'|'notApplicable'} TopLevelClumpId */ | ||
|
||
class CategoryRenderer { | ||
/** | ||
|
@@ -45,6 +45,7 @@ class CategoryRenderer { | |
*/ | ||
get _clumpTitles() { | ||
return { | ||
warning: Util.UIStrings.warningAuditsGroupTitle, | ||
manual: Util.UIStrings.manualAuditsGroupTitle, | ||
passed: Util.UIStrings.passedAuditsGroupTitle, | ||
notApplicable: Util.UIStrings.notApplicableAuditsGroupTitle, | ||
|
@@ -264,6 +265,10 @@ class CategoryRenderer { | |
const clumpTmpl = this.dom.cloneTemplate('#tmpl-lh-clump', this.templateContext); | ||
const clumpElement = this.dom.find('.lh-clump', clumpTmpl); | ||
|
||
if (clumpId === 'warning') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we want it to be an open There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. renderUnexpandableClump's don't have titles/descriptions so .... let's stick with this approach. |
||
clumpElement.setAttribute('open', ''); | ||
} | ||
|
||
const summaryInnerEl = this.dom.find('.lh-audit-group__summary', clumpElement); | ||
const chevronEl = summaryInnerEl.appendChild(this._createChevron()); | ||
chevronEl.title = Util.UIStrings.auditGroupExpandTooltip; | ||
|
@@ -333,6 +338,14 @@ class CategoryRenderer { | |
return tmpl; | ||
} | ||
|
||
/** | ||
* @param {LH.ReportResult.AuditRef} audit | ||
* @return {boolean} | ||
*/ | ||
_auditHasWarning(audit) { | ||
return Boolean(audit.result.warnings && audit.result.warnings.length); | ||
} | ||
|
||
/** | ||
* Returns the id of the top-level clump to put this audit in. | ||
* @param {LH.ReportResult.AuditRef} auditRef | ||
|
@@ -345,15 +358,19 @@ class CategoryRenderer { | |
} | ||
|
||
if (Util.showAsPassed(auditRef.result)) { | ||
return 'passed'; | ||
if (this._auditHasWarning(auditRef)) { | ||
return 'warning'; | ||
} else { | ||
return 'passed'; | ||
} | ||
} else { | ||
return 'failed'; | ||
} | ||
} | ||
|
||
/** | ||
* Renders a set of top level sections (clumps), under a status of failed, manual, | ||
* passed, or notApplicable. The result ends up something like: | ||
* Renders a set of top level sections (clumps), under a status of failed, warning, | ||
* manual, passed, or notApplicable. The result ends up something like: | ||
* | ||
* failed clump | ||
* ├── audit 1 (w/o group) | ||
|
@@ -382,6 +399,7 @@ class CategoryRenderer { | |
/** @type {Map<TopLevelClumpId, Array<LH.ReportResult.AuditRef>>} */ | ||
const clumps = new Map(); | ||
clumps.set('failed', []); | ||
clumps.set('warning', []); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should add 'warning' to the list of statuses in the method jsdoc for |
||
clumps.set('manual', []); | ||
clumps.set('passed', []); | ||
clumps.set('notApplicable', []); | ||
|
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.
it's unfortunate we used "group" for these :/