Skip to content

Commit

Permalink
report: add top-level section for passed audits that had warnings (#6989
Browse files Browse the repository at this point in the history
)
  • Loading branch information
connorjclark authored and brendankenny committed Jan 15, 2019
1 parent 120d769 commit 534621c
Show file tree
Hide file tree
Showing 8 changed files with 102 additions and 7 deletions.
4 changes: 4 additions & 0 deletions lighthouse-core/lib/i18n/en-US.json
Expand Up @@ -1079,6 +1079,10 @@
"message": "Values are estimated and may vary.",
"description": "Disclaimer shown to users below the metric values (First Contentful Paint, Time to Interactive, etc) to warn them that the numbers they see will likely change slightly the next time they run Lighthouse."
},
"lighthouse-core/report/html/renderer/util.js | warningAuditsGroupTitle": {
"message": "Passed audits but with warnings",
"description": "Section heading shown above a list of passed audits that contain warnings. Audits under this section do not negatively impact the score, but Lighthouse has generated some potentially actionable suggestions that should be reviewed. This section is expanded by default and displays after the failing audits."
},
"lighthouse-core/report/html/renderer/util.js | warningHeader": {
"message": "Warnings: ",
"description": "This label is shown above a bulleted list of warnings. It is shown directly below an audit that produced warnings. Warnings describe situations the user should be aware of, as Lighthouse was unable to complete all the work required on this audit. For example, The 'Unable to decode image (biglogo.jpg)' warning may show up below an image encoding audit."
Expand Down
26 changes: 22 additions & 4 deletions lighthouse-core/report/html/renderer/category-renderer.js
Expand Up @@ -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 {
/**
Expand All @@ -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,
Expand Down Expand Up @@ -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') {
clumpElement.setAttribute('open', '');
}

const summaryInnerEl = this.dom.find('.lh-audit-group__summary', clumpElement);
const chevronEl = summaryInnerEl.appendChild(this._createChevron());
chevronEl.title = Util.UIStrings.auditGroupExpandTooltip;
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -382,6 +399,7 @@ class CategoryRenderer {
/** @type {Map<TopLevelClumpId, Array<LH.ReportResult.AuditRef>>} */
const clumps = new Map();
clumps.set('failed', []);
clumps.set('warning', []);
clumps.set('manual', []);
clumps.set('passed', []);
clumps.set('notApplicable', []);
Expand Down
2 changes: 2 additions & 0 deletions lighthouse-core/report/html/renderer/util.js
Expand Up @@ -477,6 +477,8 @@ Util.UIStrings = {
warningHeader: 'Warnings: ',
/** The tooltip text on an expandable chevron icon. Clicking the icon expands a section to reveal a list of audit results that was hidden by default. */
auditGroupExpandTooltip: 'Show audits',
/** Section heading shown above a list of passed audits that contain warnings. Audits under this section do not negatively impact the score, but Lighthouse has generated some potentially actionable suggestions that should be reviewed. This section is expanded by default and displays after the failing audits. */
warningAuditsGroupTitle: 'Passed audits but with warnings',
/** Section heading shown above a list of audits that are passing. 'Passed' here refers to a passing grade. This section is collapsed by default, as the user should be focusing on the failed audits instead. Users can click this heading to reveal the list. */
passedAuditsGroupTitle: 'Passed audits',
/** Section heading shown above a list of audits that do not apply to the page. For example, if an audit is 'Are images optimized?', but the page has no images on it, the audit will be marked as not applicable. This is neither passing or failing. This section is collapsed by default, as the user should be focusing on the failed audits instead. Users can click this heading to reveal the list. */
Expand Down
5 changes: 5 additions & 0 deletions lighthouse-core/report/html/report-styles.css
Expand Up @@ -93,6 +93,7 @@
--check-circle-icon-url: url('data:image/svg+xml;utf8,<svg xmlns="http://www.w3.org/2000/svg" width="48" height="48"><path d="M0 0h48v48H0z" fill="none"/><path d="M24 4C12.95 4 4 12.95 4 24c0 11.04 8.95 20 20 20 11.04 0 20-8.96 20-20 0-11.05-8.96-20-20-20zm-4 30L10 24l2.83-2.83L20 28.34l15.17-15.17L38 16 20 34z" fill="%235E6268"/></svg>');
--check-icon-url: url('data:image/svg+xml;utf8,<svg xmlns="http://www.w3.org/2000/svg" width="48" height="48"><path d="M0 0h48v48H0z" fill="none"/><path d="M18 32.34L9.66 24l-2.83 2.83L18 38l24-24-2.83-2.83z"/></svg>');

--warning-icon-url: url('data:image/svg+xml;utf8,<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 48 48"><title>warn</title><path fill="%235E6268" d="M2 42h44L24 4 2 42zm24-6h-4v-4h4v4zm0-8h-4v-8h4v8z"/></svg>');
--search-icon-url: url('data:image/svg+xml;utf8,<svg xmlns="http://www.w3.org/2000/svg" width="48" height="48"><path d="M31 28h-1.59l-.55-.55C30.82 25.18 32 22.23 32 19c0-7.18-5.82-13-13-13S6 11.82 6 19s5.82 13 13 13c3.23 0 6.18-1.18 8.45-3.13l.55.55V31l10 9.98L40.98 38 31 28zm-12 0a9 9 0 1 1 .001-18.001A9 9 0 0 1 19 28z" fill="%235E6268"/><path d="M0 0h48v48H0z" fill="none" /></svg>');
--remove-circle-icon-url: url('data:image/svg+xml;utf8,<svg height="24" width="24" xmlns="http://www.w3.org/2000/svg"><path d="M0 0h24v24H0z" fill="none"/><path d="M7 11v2h10v-2H7zm5-9C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zm0 18c-4.41 0-8-3.59-8-8s3.59-8 8-8 8 3.59 8 8-3.59 8-8 8z" fill="%235E6268"/></svg>');

Expand Down Expand Up @@ -596,6 +597,10 @@
vertical-align: middle;
}

.lh-clump--warning > summary .lh-audit-group__header::before {
content: '';
background-image: var(--warning-icon-url);
}
.lh-clump--manual > summary .lh-audit-group__header::before {
content: '';
background-image: var(--search-icon-url);
Expand Down
Expand Up @@ -334,16 +334,24 @@ describe('CategoryRenderer', () => {
});
});

describe('clumping passed/failed/manual', () => {
describe('clumping passed/failed/warning/manual', () => {
it('separates audits in the DOM', () => {
const category = sampleResults.reportCategories.find(c => c.id === 'pwa');
const elem = renderer.render(category, sampleResults.categoryGroups);
const categoryClone = JSON.parse(JSON.stringify(category));
// Give the first two passing grades warnings
const passingRefs = categoryClone.auditRefs.filter(ref => ref.result.score === 1);
passingRefs[0].result.warnings = ['Some warning'];
passingRefs[1].result.warnings = ['Some warning'];

const elem = renderer.render(categoryClone, sampleResults.categoryGroups);
const passedAudits = elem.querySelectorAll('.lh-clump--passed .lh-audit');
const failedAudits = elem.querySelectorAll('.lh-clump--failed .lh-audit');
const warningAudits = elem.querySelectorAll('.lh-clump--warning .lh-audit');
const manualAudits = elem.querySelectorAll('.lh-clump--manual .lh-audit');

assert.equal(passedAudits.length, 4);
assert.equal(passedAudits.length, 2);
assert.equal(failedAudits.length, 8);
assert.equal(warningAudits.length, 2);
assert.equal(manualAudits.length, 3);
});

Expand All @@ -358,6 +366,59 @@ describe('CategoryRenderer', () => {
assert.equal(passedAudits.length, 0);
assert.equal(failedAudits.length, 12);
});

it('expands warning audit group', () => {
const category = sampleResults.reportCategories.find(c => c.id === 'pwa');
const categoryClone = JSON.parse(JSON.stringify(category));
categoryClone.auditRefs[0].result.warnings = ['Some warning'];

const auditDOM = renderer.render(categoryClone, sampleResults.categoryGroups);
const warningClumpEl = auditDOM.querySelector('.lh-clump--warning');
const isExpanded = warningClumpEl.hasAttribute('open');
assert.ok(isExpanded, 'Warning audit group should be expanded by default');
});

it('only passing audits with warnings show in warnings section', () => {
const failingWarning = 'Failed and warned';
const passingWarning = 'A passing warning';
const category = {
id: 'test',
title: 'Test',
score: 0,
auditRefs: [{
id: 'failing',
result: {
id: 'failing',
title: 'Failing with warning',
description: '',
scoreDisplayMode: 'numeric',
score: 0,
warnings: [failingWarning],
},
}, {
id: 'passing',
result: {
id: 'passing',
title: 'Passing with warning',
description: '',
scoreDisplayMode: 'numeric',
score: 1,
warnings: [passingWarning],
},
}],
};
const categoryDOM = renderer.render(category);

const shouldBeFailed = categoryDOM.querySelectorAll('.lh-clump--failed .lh-audit');
assert.strictEqual(shouldBeFailed.length, 1);
assert.strictEqual(shouldBeFailed[0].id, 'failing');
assert.ok(shouldBeFailed[0].textContent.includes(failingWarning));

const shouldBeWarning = categoryDOM.querySelectorAll('.lh-clump--warning .lh-audit');
assert.strictEqual(shouldBeWarning.length, 1);
assert.strictEqual(shouldBeWarning[0].id, 'passing');
assert.ok(shouldBeWarning[0].textContent.includes(passingWarning));
});
});

it('can set a custom templateContext', () => {
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/test/results/sample_v2.json
Expand Up @@ -4343,6 +4343,7 @@
"scorescaleLabel": "Score scale:",
"toplevelWarningsMessage": "There were issues affecting this run of Lighthouse:",
"varianceDisclaimer": "Values are estimated and may vary.",
"warningAuditsGroupTitle": "Passed audits but with warnings",
"warningHeader": "Warnings: "
},
"icuMessagePaths": {
Expand Down
3 changes: 3 additions & 0 deletions proto/lighthouse-result.proto
Expand Up @@ -331,6 +331,9 @@ message I18n {

// The title of the lab data performance category
string lab_data_title = 16;

// The heading that is shown above a list of audits that have warnings
string warning_audits_group_title = 17;
}

// The message holding all formatted strings
Expand Down
1 change: 1 addition & 0 deletions proto/sample_v2_round_trip.json
Expand Up @@ -3286,6 +3286,7 @@
"scorescaleLabel": "Score scale:",
"toplevelWarningsMessage": "There were issues affecting this run of Lighthouse:",
"varianceDisclaimer": "Values are estimated and may vary.",
"warningAuditsGroupTitle": "Passed audits but with warnings",
"warningHeader": "Warnings: "
}
},
Expand Down

0 comments on commit 534621c

Please sign in to comment.