diff --git a/flow-report/assets/styles.css b/flow-report/assets/styles.css index bf628376a5bd..5eac8df8cf90 100644 --- a/flow-report/assets/styles.css +++ b/flow-report/assets/styles.css @@ -369,7 +369,7 @@ width: max-content; background-color: var(--report-background-color); border: 1px solid var(--color-gray-900); - border-radius: 5px; + border-radius: 3px; padding: var(--base-spacing); right: 0; box-shadow: 0px 4px 4px var(--summary-tooltip-box-shadow-color); diff --git a/flow-report/src/summary/category.tsx b/flow-report/src/summary/category.tsx index 99e7bf1b5563..27ffcc33c36f 100644 --- a/flow-report/src/summary/category.tsx +++ b/flow-report/src/summary/category.tsx @@ -35,11 +35,16 @@ export const SummaryTooltip: FunctionComponent<{ gatherMode: LH.Result.GatherMode }> = ({category, gatherMode}) => { const strings = useUIStrings(); - const {numPassed, numAudits, totalWeight} = Util.calculateCategoryFraction(category); + const { + numPassed, + numPassableAudits, + numInformative, + totalWeight, + } = Util.calculateCategoryFraction(category); const displayAsFraction = Util.shouldDisplayAsFraction(gatherMode); const rating = displayAsFraction ? - Util.calculateRating(numPassed / numAudits) : + Util.calculateRating(numPassed / numPassableAudits) : Util.calculateRating(category.score); return ( @@ -64,8 +69,19 @@ export const SummaryTooltip: FunctionComponent<{ }
- {`${numPassed} audits passed / ${numAudits} audits run`} + { + // TODO(FLOW-I18N): Placeholder format. + `${numPassed} audits passed / ${numPassableAudits} passable audits` + }
+ { + // TODO(FLOW-I18N): Placeholder format. + numInformative ? +
+ {`${numInformative} informative audits`} +
: + null + } ); }; diff --git a/flow-report/src/summary/summary.tsx b/flow-report/src/summary/summary.tsx index d890e723a041..82b783ff468b 100644 --- a/flow-report/src/summary/summary.tsx +++ b/flow-report/src/summary/summary.tsx @@ -127,6 +127,7 @@ export const SummaryHeader: FunctionComponent = () => { } } + // TODO(FLOW-I18N): Placeholder format. const subtitleCounts = []; if (numNavigation) subtitleCounts.push(`${numNavigation} navigation reports`); if (numTimespan) subtitleCounts.push(`${numTimespan} timespan reports`); diff --git a/flow-report/test/summary/category-test.tsx b/flow-report/test/summary/category-test.tsx index 6b8df3a42669..ffe3b4bb9570 100644 --- a/flow-report/test/summary/category-test.tsx +++ b/flow-report/test/summary/category-test.tsx @@ -28,11 +28,12 @@ beforeEach(() => { describe('SummaryTooltip', () => { it('renders tooltip with rating', async () => { const category: any = { + id: 'performance', score: 1, auditRefs: [ - {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1}, - {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1}, - {result: {score: 0, scoreDisplayMode: 'binary'}, weight: 1}, + {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1, group: 'diagnostics'}, + {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1, group: 'diagnostics'}, + {result: {score: 0, scoreDisplayMode: 'binary'}, weight: 1, group: 'diagnostics'}, ], }; @@ -43,16 +44,17 @@ describe('SummaryTooltip', () => { expect(root.getByText('Average')).toBeTruthy(); expect(() => root.getByText(/^[0-9]+$/)).toThrow(); - expect(root.getByText('2 audits passed / 3 audits run')).toBeTruthy(); + expect(root.getByText('2 audits passed / 3 passable audits')).toBeTruthy(); }); it('renders tooltip without rating', async () => { const category: any = { + id: 'performance', score: 1, auditRefs: [ - {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 0}, - {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 0}, - {result: {score: 0, scoreDisplayMode: 'binary'}, weight: 0}, + {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 0, group: 'diagnostics'}, + {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 0, group: 'diagnostics'}, + {result: {score: 0, scoreDisplayMode: 'binary'}, weight: 0, group: 'diagnostics'}, ], }; @@ -63,16 +65,17 @@ describe('SummaryTooltip', () => { expect(() => root.getByText(/^(Average|Good|Poor)$/)).toThrow(); expect(() => root.getByText(/^[0-9]+$/)).toThrow(); - expect(root.getByText('2 audits passed / 3 audits run')).toBeTruthy(); + expect(root.getByText('2 audits passed / 3 passable audits')).toBeTruthy(); }); it('renders scored category tooltip with score', async () => { const category: any = { + id: 'performance', score: 1, auditRefs: [ - {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1}, - {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1}, - {result: {score: 0, scoreDisplayMode: 'binary'}, weight: 1}, + {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1, group: 'diagnostics'}, + {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1, group: 'diagnostics'}, + {result: {score: 0, scoreDisplayMode: 'binary'}, weight: 1, group: 'diagnostics'}, ], }; @@ -83,6 +86,28 @@ describe('SummaryTooltip', () => { expect(root.getByText('Good')).toBeTruthy(); expect(root.getByText('100')).toBeTruthy(); - expect(root.getByText('2 audits passed / 3 audits run')).toBeTruthy(); + expect(root.getByText('2 audits passed / 3 passable audits')).toBeTruthy(); + }); + + it('renders informative audit count if any', async () => { + const category: any = { + id: 'performance', + score: 1, + auditRefs: [ + {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1, group: 'diagnostics'}, + {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1, group: 'diagnostics'}, + {result: {score: 0, scoreDisplayMode: 'informative'}, weight: 1, group: 'diagnostics'}, + ], + }; + + const root = render( + , + {wrapper} + ); + + expect(root.getByText('Good')).toBeTruthy(); + expect(root.getByText('100')).toBeTruthy(); + expect(root.getByText('2 audits passed / 2 passable audits')).toBeTruthy(); + expect(root.getByText('1 informative audits')).toBeTruthy(); }); }); diff --git a/lighthouse-core/util-commonjs.js b/lighthouse-core/util-commonjs.js index 4ce534006e24..1859136ba834 100644 --- a/lighthouse-core/util-commonjs.js +++ b/lighthouse-core/util-commonjs.js @@ -521,15 +521,31 @@ class Util { * @param {LH.ReportResult.Category} category */ static calculateCategoryFraction(category) { - const numAudits = category.auditRefs.length; - + let numPassableAudits = 0; let numPassed = 0; + let numInformative = 0; let totalWeight = 0; for (const auditRef of category.auditRefs) { + const auditPassed = Util.showAsPassed(auditRef.result); + const notDisplayed = !auditRef.group && category.id === 'performance'; + + // Don't count the audit if it's manual, N/A, or isn't displayed. + if (notDisplayed || + auditRef.result.scoreDisplayMode === 'manual' || + auditRef.result.scoreDisplayMode === 'notApplicable') { + continue; + } else if (auditRef.result.scoreDisplayMode === 'informative') { + if (!auditPassed) { + ++numInformative; + } + continue; + } + + ++numPassableAudits; totalWeight += auditRef.weight; - if (Util.showAsPassed(auditRef.result)) numPassed++; + if (auditPassed) numPassed++; } - return {numPassed, numAudits, totalWeight}; + return {numPassed, numPassableAudits, numInformative, totalWeight}; } } diff --git a/report/renderer/category-renderer.js b/report/renderer/category-renderer.js index c3ec8e3f0e83..32ec1226fc6c 100644 --- a/report/renderer/category-renderer.js +++ b/report/renderer/category-renderer.js @@ -375,12 +375,12 @@ export class CategoryRenderer { const wrapper = this.dom.find('a.lh-fraction__wrapper', tmpl); this.dom.safelySetHref(wrapper, `#${category.id}`); - const {numPassed, numAudits, totalWeight} = Util.calculateCategoryFraction(category); + const {numPassed, numPassableAudits, totalWeight} = Util.calculateCategoryFraction(category); - const fraction = numPassed / numAudits; + const fraction = numPassed / numPassableAudits; const content = this.dom.find('.lh-fraction__content', tmpl); const text = this.dom.createElement('span'); - text.textContent = `${numPassed}/${numAudits}`; + text.textContent = `${numPassed}/${numPassableAudits}`; content.appendChild(text); let rating = Util.calculateRating(fraction); diff --git a/report/renderer/performance-category-renderer.js b/report/renderer/performance-category-renderer.js index c0f1235aec3b..a0df6cb19b6e 100644 --- a/report/renderer/performance-category-renderer.js +++ b/report/renderer/performance-category-renderer.js @@ -172,33 +172,35 @@ export class PerformanceCategoryRenderer extends CategoryRenderer { } // Metrics. - const metricAuditsEl = this.renderAuditGroup(groups.metrics); + const metricAudits = category.auditRefs.filter(audit => audit.group === 'metrics'); + if (metricAudits.length) { + const metricAuditsEl = this.renderAuditGroup(groups.metrics); - // Metric descriptions toggle. - const toggleTmpl = this.dom.createComponent('metricsToggle'); - const _toggleEl = this.dom.find('.lh-metrics-toggle', toggleTmpl); - metricAuditsEl.append(..._toggleEl.childNodes); + // Metric descriptions toggle. + const toggleTmpl = this.dom.createComponent('metricsToggle'); + const _toggleEl = this.dom.find('.lh-metrics-toggle', toggleTmpl); + metricAuditsEl.append(..._toggleEl.childNodes); - const metricAudits = category.auditRefs.filter(audit => audit.group === 'metrics'); - const metricsBoxesEl = this.dom.createChildOf(metricAuditsEl, 'div', 'lh-metrics-container'); + const metricsBoxesEl = this.dom.createChildOf(metricAuditsEl, 'div', 'lh-metrics-container'); - metricAudits.forEach(item => { - metricsBoxesEl.appendChild(this._renderMetric(item)); - }); + metricAudits.forEach(item => { + metricsBoxesEl.appendChild(this._renderMetric(item)); + }); - const estValuesEl = this.dom.createChildOf(metricAuditsEl, 'div', 'lh-metrics__disclaimer'); - const disclaimerEl = this.dom.convertMarkdownLinkSnippets(strings.varianceDisclaimer); - estValuesEl.appendChild(disclaimerEl); + const estValuesEl = this.dom.createChildOf(metricAuditsEl, 'div', 'lh-metrics__disclaimer'); + const disclaimerEl = this.dom.convertMarkdownLinkSnippets(strings.varianceDisclaimer); + estValuesEl.appendChild(disclaimerEl); - // Add link to score calculator. - const calculatorLink = this.dom.createChildOf(estValuesEl, 'a', 'lh-calclink'); - calculatorLink.target = '_blank'; - calculatorLink.textContent = strings.calculatorLink; - this.dom.safelySetHref(calculatorLink, this._getScoringCalculatorHref(category.auditRefs)); + // Add link to score calculator. + const calculatorLink = this.dom.createChildOf(estValuesEl, 'a', 'lh-calclink'); + calculatorLink.target = '_blank'; + calculatorLink.textContent = strings.calculatorLink; + this.dom.safelySetHref(calculatorLink, this._getScoringCalculatorHref(category.auditRefs)); - metricAuditsEl.classList.add('lh-audit-group--metrics'); - element.appendChild(metricAuditsEl); + metricAuditsEl.classList.add('lh-audit-group--metrics'); + element.appendChild(metricAuditsEl); + } // Filmstrip const timelineEl = this.dom.createChildOf(element, 'div', 'lh-filmstrip-container'); diff --git a/report/renderer/util.js b/report/renderer/util.js index f4e09046db93..488911b3d283 100644 --- a/report/renderer/util.js +++ b/report/renderer/util.js @@ -518,15 +518,31 @@ export class Util { * @param {LH.ReportResult.Category} category */ static calculateCategoryFraction(category) { - const numAudits = category.auditRefs.length; - + let numPassableAudits = 0; let numPassed = 0; + let numInformative = 0; let totalWeight = 0; for (const auditRef of category.auditRefs) { + const auditPassed = Util.showAsPassed(auditRef.result); + const notDisplayed = !auditRef.group && category.id === 'performance'; + + // Don't count the audit if it's manual, N/A, or isn't displayed. + if (notDisplayed || + auditRef.result.scoreDisplayMode === 'manual' || + auditRef.result.scoreDisplayMode === 'notApplicable') { + continue; + } else if (auditRef.result.scoreDisplayMode === 'informative') { + if (!auditPassed) { + ++numInformative; + } + continue; + } + + ++numPassableAudits; totalWeight += auditRef.weight; - if (Util.showAsPassed(auditRef.result)) numPassed++; + if (auditPassed) numPassed++; } - return {numPassed, numAudits, totalWeight}; + return {numPassed, numPassableAudits, numInformative, totalWeight}; } } diff --git a/report/test/renderer/category-renderer-test.js b/report/test/renderer/category-renderer-test.js index f655d570cf21..4b1a70975b11 100644 --- a/report/test/renderer/category-renderer-test.js +++ b/report/test/renderer/category-renderer-test.js @@ -297,7 +297,7 @@ describe('CategoryRenderer', () => { ); const gauge = categoryDOM.querySelector('.lh-fraction__content'); - assert.equal(gauge.textContent.trim(), '49/54', 'fraction is included'); + assert.equal(gauge.textContent.trim(), '13/18', 'fraction is included'); const score = categoryDOM.querySelector('.lh-category-header'); const title = score.querySelector('.lh-fraction__label'); diff --git a/report/test/renderer/performance-category-renderer-test.js b/report/test/renderer/performance-category-renderer-test.js index d356a6dfadfa..e04ad360f309 100644 --- a/report/test/renderer/performance-category-renderer-test.js +++ b/report/test/renderer/performance-category-renderer-test.js @@ -69,6 +69,18 @@ describe('PerfCategoryRenderer', () => { assert.equal(timelineElements.length + nontimelineElements.length, metricAudits.length); }); + it('does not render metrics section if no metric group audits', () => { + // Remove metrics from category + const newCategory = JSON.parse(JSON.stringify(category)); + newCategory.auditRefs = category.auditRefs.filter(audit => audit.group !== 'metrics'); + + const categoryDOM = renderer.render(newCategory, sampleResults.categoryGroups); + const sections = categoryDOM.querySelectorAll('.lh-category > .lh-audit-group'); + const metricSection = categoryDOM.querySelector('.lh-audit-group--metrics'); + assert.ok(!metricSection); + assert.equal(sections.length, 4); + }); + it('renders the metrics variance disclaimer as markdown', () => { const categoryDOM = renderer.render(category, sampleResults.categoryGroups); const disclaimerEl = diff --git a/report/test/renderer/util-test.js b/report/test/renderer/util-test.js index b55a4d99143d..63640dce6b20 100644 --- a/report/test/renderer/util-test.js +++ b/report/test/renderer/util-test.js @@ -399,17 +399,77 @@ describe('util helpers', () => { describe('#calculateCategoryFraction', () => { it('returns passed audits and total audits', () => { const category = { + id: 'performance', auditRefs: [ - {weight: 3, result: {score: 1, scoreDisplayMode: 'binary'}}, - {weight: 2, result: {score: 1, scoreDisplayMode: 'binary'}}, - {weight: 0, result: {score: 1, scoreDisplayMode: 'binary'}}, - {weight: 1, result: {score: 0, scoreDisplayMode: 'binary'}}, + {weight: 3, result: {score: 1, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, + {weight: 2, result: {score: 1, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, + {weight: 0, result: {score: 1, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, + {weight: 1, result: {score: 0, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, ], }; - const {numPassed, numAudits, totalWeight} = Util.calculateCategoryFraction(category); - expect(numPassed).toEqual(3); - expect(numAudits).toEqual(4); - expect(totalWeight).toEqual(6); + const fraction = Util.calculateCategoryFraction(category); + expect(fraction).toEqual({ + numPassableAudits: 4, + numPassed: 3, + numInformative: 0, + totalWeight: 6, + }); + }); + + it('ignores manual audits, N/A audits, and performance audits with no group', () => { + const category = { + id: 'performance', + auditRefs: [ + {weight: 1, result: {score: 1, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, + {weight: 1, result: {score: 1, scoreDisplayMode: 'binary'}}, + {weight: 1, result: {score: 0, scoreDisplayMode: 'manual'}, group: 'diagnostics'}, + {weight: 1, result: {score: 0, scoreDisplayMode: 'notApplicable'}, group: 'diagnostics'}, + ], + }; + const fraction = Util.calculateCategoryFraction(category); + expect(fraction).toEqual({ + numPassableAudits: 1, + numPassed: 1, + numInformative: 0, + totalWeight: 1, + }); + }); + + it('does not ignore audits with no group in non-performance category', () => { + const category = { + id: 'seo', + auditRefs: [ + {weight: 1, result: {score: 1, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, + {weight: 1, result: {score: 1, scoreDisplayMode: 'binary'}}, + {weight: 1, result: {score: 0, scoreDisplayMode: 'manual'}, group: 'diagnostics'}, + ], + }; + const fraction = Util.calculateCategoryFraction(category); + expect(fraction).toEqual({ + numPassableAudits: 2, + numPassed: 2, + numInformative: 0, + totalWeight: 2, + }); + }); + + it('tracks informative audits separately', () => { + const category = { + id: 'performance', + auditRefs: [ + {weight: 1, result: {score: 1, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, + {weight: 1, result: {score: 1, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, + {weight: 0, result: {score: 1, scoreDisplayMode: 'informative'}, group: 'diagnostics'}, + {weight: 1, result: {score: 0, scoreDisplayMode: 'informative'}, group: 'diagnostics'}, + ], + }; + const fraction = Util.calculateCategoryFraction(category); + expect(fraction).toEqual({ + numPassableAudits: 2, + numPassed: 2, + numInformative: 2, + totalWeight: 2, + }); }); }); });