diff --git a/flow-report/test/setup/env-setup.ts b/flow-report/test/setup/env-setup.ts index 21d406d9d81c..49e173896b0a 100644 --- a/flow-report/test/setup/env-setup.ts +++ b/flow-report/test/setup/env-setup.ts @@ -37,6 +37,7 @@ const rootHooks = { // Use JSDOM types as necessary. global.Blob = window.Blob; + global.HTMLElement = window.HTMLElement; global.HTMLInputElement = window.HTMLInputElement; // Functions not implemented in JSDOM. diff --git a/report/renderer/category-renderer.js b/report/renderer/category-renderer.js index 01bb77e298dd..b6e94d29e729 100644 --- a/report/renderer/category-renderer.js +++ b/report/renderer/category-renderer.js @@ -38,11 +38,11 @@ export class CategoryRenderer { /** * @param {LH.ReportResult.AuditRef} audit - * @return {Element} + * @return {HTMLElement} */ renderAudit(audit) { const component = this.dom.createComponent('audit'); - return this.populateAuditValues(audit, component); + return /** @type {HTMLElement} */ (this.populateAuditValues(audit, component)); } /** @@ -298,10 +298,10 @@ export class CategoryRenderer { * Take a set of audits and render in a top-level, expandable clump that starts * in a collapsed state. * @param {Exclude} clumpId - * @param {{auditRefs: Array, description?: string, openByDefault?: boolean}} clumpOpts + * @param {{auditRefsOrEls: Array, description?: string, openByDefault?: boolean}} clumpOpts * @return {!Element} */ - renderClump(clumpId, {auditRefs, description, openByDefault}) { + renderClump(clumpId, {auditRefsOrEls, description, openByDefault}) { const clumpComponent = this.dom.createComponent('clump'); const clumpElement = this.dom.find('.lh-clump', clumpComponent); @@ -314,10 +314,16 @@ export class CategoryRenderer { this.dom.find('.lh-audit-group__title', headerEl).textContent = title; const itemCountEl = this.dom.find('.lh-audit-group__itemcount', clumpElement); - itemCountEl.textContent = `(${auditRefs.length})`; + itemCountEl.textContent = `(${auditRefsOrEls.length})`; // Add all audit results to the clump. - const auditElements = auditRefs.map(this.renderAudit.bind(this)); + const auditElements = auditRefsOrEls.map(audit => { + if (audit instanceof HTMLElement) { + return audit; + } else { + return this.renderAudit(audit); + } + }); clumpElement.append(...auditElements); const el = this.dom.find('.lh-audit-group', clumpComponent); @@ -564,7 +570,11 @@ export class CategoryRenderer { // Expand on warning, or manual audits when there are no failing audits. const openByDefault = clumpId === 'warning' || (clumpId === 'manual' && numFailingAudits === 0); - const clumpElem = this.renderClump(clumpId, {auditRefs, description, openByDefault}); + const clumpElem = this.renderClump(clumpId, { + auditRefsOrEls: auditRefs, + description, + openByDefault, + }); element.append(clumpElem); } diff --git a/report/renderer/performance-category-renderer.js b/report/renderer/performance-category-renderer.js index 01bf180b5b32..2ff17325fccc 100644 --- a/report/renderer/performance-category-renderer.js +++ b/report/renderer/performance-category-renderer.js @@ -5,6 +5,7 @@ */ /** @typedef {import('./dom.js').DOM} DOM */ +/** @typedef {LH.Result.MetricAcronym | 'All'} FilterType */ import {CategoryRenderer} from './category-renderer.js'; import {ReportUtils} from './report-utils.js'; @@ -211,73 +212,106 @@ export class PerformanceCategoryRenderer extends CategoryRenderer { filmstripEl && timelineEl.append(filmstripEl); } - const filterableMetrics = metricAudits.filter(a => !!a.relevantAudits); - // TODO: only add if there are opportunities & diagnostics rendered. - if (filterableMetrics.length) { - this.renderMetricAuditFilter(filterableMetrics, element); - } + const allInsights = category.auditRefs + .filter(audit => this._isPerformanceInsight(audit)) + .map(auditRef => { + const {overallImpact, overallLinearImpact} = this.overallImpact(auditRef, metricAudits); + const guidanceLevel = auditRef.result.guidanceLevel || 1; + const auditEl = this.renderAudit(auditRef); + + return {auditRef, auditEl, overallImpact, overallLinearImpact, guidanceLevel}; + }); // Diagnostics - const diagnosticAudits = category.auditRefs - .filter(audit => this._isPerformanceInsight(audit)) - .filter(audit => !ReportUtils.showAsPassed(audit.result)); - - /** @type {Array<{auditRef:LH.ReportResult.AuditRef, overallImpact: number, overallLinearImpact: number, guidanceLevel: number}>} */ - const auditImpacts = []; - diagnosticAudits.forEach(audit => { - const { - overallImpact: overallImpact, - overallLinearImpact: overallLinearImpact, - } = this.overallImpact(audit, metricAudits); - const guidanceLevel = audit.result.guidanceLevel ?? 1; - auditImpacts.push({auditRef: audit, overallImpact, overallLinearImpact, guidanceLevel}); - }); + const diagnosticAudits = allInsights + .filter(audit => !ReportUtils.showAsPassed(audit.auditRef.result)); + + const passedAudits = allInsights + .filter(audit => ReportUtils.showAsPassed(audit.auditRef.result)); + + const [groupEl, footerEl] = this.renderAuditGroup(groups['diagnostics']); + groupEl.classList.add('lh-audit-group--diagnostics'); + + /** + * @param {FilterType} acronym + */ + function refreshFilteredAudits(acronym) { + for (const audit of allInsights) { + if (acronym === 'All') { + audit.auditEl.hidden = false; + } else { + const shouldHide = audit.auditRef.result.metricSavings?.[acronym] === undefined; + audit.auditEl.hidden = shouldHide; + } + } + + diagnosticAudits.sort((a, b) => { + // If the score display mode is "metricSavings", the `score` will be a coarse indicator of the overall impact. + // Therefore, it makes sense to sort audits by score first to ensure visual clarity with the score icons. + const scoreA = a.auditRef.result.scoreDisplayMode === 'informative' + ? 100 + : Number(a.auditRef.result.score); + const scoreB = b.auditRef.result.scoreDisplayMode === 'informative' + ? 100 + : Number(b.auditRef.result.score); + if (scoreA !== scoreB) return scoreA - scoreB; + + // If there is a metric filter applied, we should sort by the impact to that specific metric. + if (acronym !== 'All') { + const aSavings = a.auditRef.result.metricSavings?.[acronym] ?? -1; + const bSavings = b.auditRef.result.metricSavings?.[acronym] ?? -1; + if (aSavings !== bSavings) return bSavings - aSavings; + } + + // Overall impact is the estimated improvement to the performance score + if (a.overallImpact !== b.overallImpact) { + return b.overallImpact * b.guidanceLevel - a.overallImpact * a.guidanceLevel; + } + + // Fall back to the linear impact if the normal impact is rounded down to 0 + if ( + a.overallImpact === 0 && b.overallImpact === 0 && + a.overallLinearImpact !== b.overallLinearImpact + ) { + return b.overallLinearImpact * b.guidanceLevel - a.overallLinearImpact * a.guidanceLevel; + } + + // Audits that have no estimated savings should be prioritized by the guidance level + return b.guidanceLevel - a.guidanceLevel; + }); - auditImpacts.sort((a, b) => { - // Performance diagnostics should only have score display modes of "informative" and "metricSavings" - // If the score display mode is "metricSavings", the `score` will be a coarse approximation of the overall impact. - // Therefore, it makes sense to sort audits by score first to ensure visual clarity with the score icons. - const scoreA = a.auditRef.result.scoreDisplayMode === 'informative' - ? 100 - : Number(a.auditRef.result.score); - const scoreB = b.auditRef.result.scoreDisplayMode === 'informative' - ? 100 - : Number(b.auditRef.result.score); - if (scoreA !== scoreB) return scoreA - scoreB; - - // Overall impact is the estimated improvement to the performance score - if (a.overallImpact !== b.overallImpact) { - return b.overallImpact * b.guidanceLevel - a.overallImpact * a.guidanceLevel; + for (const audit of diagnosticAudits) { + groupEl.insertBefore(audit.auditEl, footerEl); } + } - // Fall back to the linear impact if the normal impact is rounded down to 0 - if ( - a.overallImpact === 0 && b.overallImpact === 0 && - a.overallLinearImpact !== b.overallLinearImpact - ) { - return b.overallLinearImpact * b.guidanceLevel - a.overallLinearImpact * a.guidanceLevel; + /** @type {Set} */ + const filterableMetricAcronyms = new Set(); + for (const audit of diagnosticAudits) { + const metricSavings = audit.auditRef.result.metricSavings || {}; + for (const [key, value] of Object.entries(metricSavings)) { + if (typeof value === 'number') filterableMetricAcronyms.add(key); } + } - // Audits that have no estimated savings should be prioritized by the guidance level - return b.guidanceLevel - a.guidanceLevel; - }); + const filterableMetrics = + metricAudits.filter(a => a.acronym && filterableMetricAcronyms.has(a.acronym)); - if (auditImpacts.length) { - const [groupEl, footerEl] = this.renderAuditGroup(groups['diagnostics']); - auditImpacts.forEach(item => groupEl.insertBefore(this.renderAudit(item.auditRef), footerEl)); - groupEl.classList.add('lh-audit-group--diagnostics'); - element.append(groupEl); + // TODO: only add if there are opportunities & diagnostics rendered. + if (filterableMetrics.length) { + this.renderMetricAuditFilter(filterableMetrics, element, refreshFilteredAudits); } - // Passed audits - const passedAudits = category.auditRefs - .filter(audit => - this._isPerformanceInsight(audit) && ReportUtils.showAsPassed(audit.result)); + refreshFilteredAudits('All'); + + if (diagnosticAudits.length) { + element.append(groupEl); + } if (!passedAudits.length) return element; const clumpOpts = { - auditRefs: passedAudits, + auditRefsOrEls: passedAudits.map(audit => audit.auditEl), groupDefinitions: groups, }; const passedElem = this.renderClump('passed', clumpOpts); @@ -318,16 +352,17 @@ export class PerformanceCategoryRenderer extends CategoryRenderer { * Render the control to filter the audits by metric. The filtering is done at runtime by CSS only * @param {LH.ReportResult.AuditRef[]} filterableMetrics * @param {HTMLDivElement} categoryEl + * @param {(acronym: FilterType) => void} onFilterChange */ - renderMetricAuditFilter(filterableMetrics, categoryEl) { + renderMetricAuditFilter(filterableMetrics, categoryEl, onFilterChange) { const metricFilterEl = this.dom.createElement('div', 'lh-metricfilter'); const textEl = this.dom.createChildOf(metricFilterEl, 'span', 'lh-metricfilter__text'); textEl.textContent = Globals.strings.showRelevantAudits; - const filterChoices = /** @type {LH.ReportResult.AuditRef[]} */ ([ - ({acronym: 'All'}), + const filterChoices = [ + /** @type {const} */ ({acronym: 'All'}), ...filterableMetrics, - ]); + ]; // Form labels need to reference unique IDs, but multiple reports rendered in the same DOM (eg PSI) // would mean ID conflict. To address this, we 'scope' these radio inputs with a unique suffix. @@ -341,7 +376,7 @@ export class PerformanceCategoryRenderer extends CategoryRenderer { const labelEl = this.dom.createChildOf(metricFilterEl, 'label', 'lh-metricfilter__label'); labelEl.htmlFor = elemId; - labelEl.title = metric.result?.title; + labelEl.title = 'result' in metric ? metric.result.title : ''; labelEl.textContent = metric.acronym || metric.id; if (metric.acronym === 'All') { @@ -357,17 +392,7 @@ export class PerformanceCategoryRenderer extends CategoryRenderer { } categoryEl.classList.toggle('lh-category--filtered', metric.acronym !== 'All'); - for (const perfAuditEl of categoryEl.querySelectorAll('div.lh-audit')) { - if (metric.acronym === 'All') { - perfAuditEl.hidden = false; - continue; - } - - perfAuditEl.hidden = true; - if (metric.relevantAudits && metric.relevantAudits.includes(perfAuditEl.id)) { - perfAuditEl.hidden = false; - } - } + onFilterChange(metric.acronym || 'All'); // Hide groups/clumps if all child audits are also hidden. const groupEls = categoryEl.querySelectorAll('div.lh-audit-group, details.lh-audit-group'); diff --git a/report/renderer/pwa-category-renderer.js b/report/renderer/pwa-category-renderer.js index 983112694f37..2ca457b942f1 100644 --- a/report/renderer/pwa-category-renderer.js +++ b/report/renderer/pwa-category-renderer.js @@ -29,8 +29,11 @@ export class PwaCategoryRenderer extends CategoryRenderer { // Manual audits are still in a manual clump. const manualAuditRefs = auditRefs.filter(ref => ref.result.scoreDisplayMode === 'manual'); - const manualElem = this.renderClump('manual', - {auditRefs: manualAuditRefs, description: category.manualDescription, openByDefault: true}); + const manualElem = this.renderClump('manual', { + auditRefsOrEls: manualAuditRefs, + description: category.manualDescription, + openByDefault: true, + }); categoryElem.append(manualElem); return categoryElem; diff --git a/report/test/clients/bundle-test.js b/report/test/clients/bundle-test.js index 8d7a391e258b..6ff6013bdfc9 100644 --- a/report/test/clients/bundle-test.js +++ b/report/test/clients/bundle-test.js @@ -25,6 +25,7 @@ describe('lighthouseRenderer bundle', () => { global.window = global.self = window; global.window.requestAnimationFrame = fn => fn(); + global.HTMLElement = window.HTMLElement; global.HTMLInputElement = window.HTMLInputElement; // Stub out matchMedia for Node. global.self.matchMedia = function() { diff --git a/report/test/renderer/category-renderer-test.js b/report/test/renderer/category-renderer-test.js index 001065a55b0a..39e67fe256c1 100644 --- a/report/test/renderer/category-renderer-test.js +++ b/report/test/renderer/category-renderer-test.js @@ -29,7 +29,10 @@ describe('CategoryRenderer', () => { reportJson: null, }); - const {document} = new jsdom.JSDOM().window; + const window = new jsdom.JSDOM().window; + const document = window.document; + global.HTMLElement = window.HTMLElement; + const dom = new DOM(document); const detailsRenderer = new DetailsRenderer(dom); renderer = new CategoryRenderer(dom, detailsRenderer); diff --git a/report/test/renderer/performance-category-renderer-test.js b/report/test/renderer/performance-category-renderer-test.js index dfde4f13c97a..073cd727e22f 100644 --- a/report/test/renderer/performance-category-renderer-test.js +++ b/report/test/renderer/performance-category-renderer-test.js @@ -30,7 +30,10 @@ describe('PerfCategoryRenderer', () => { reportJson: null, }); - const {document} = new jsdom.JSDOM().window; + const window = new jsdom.JSDOM().window; + const document = window.document; + global.HTMLElement = window.HTMLElement; + const dom = new DOM(document); const detailsRenderer = new DetailsRenderer(dom); renderer = new PerformanceCategoryRenderer(dom, detailsRenderer); @@ -398,6 +401,69 @@ Array [ expect(diagnosticElementIds.map(el => el.id)).toEqual(['audit-3', 'audit-1', 'audit-4', 'audit-2']); // eslint-disable-line max-len }); + it('audits in order of single metric savings when filter active', () => { + fakeCategory = { + id: 'performance', + title: 'Performance', + score: 0.5, + supportedModes: category.supportedModes, + }; + + fakeCategory.auditRefs = [{ + id: 'audit-1', + result: { + id: 'audit-1', + metricSavings: {'LCP': 50, 'FCP': 5}, + score: 0, + ...defaultAuditRef, + }, + }, { + id: 'audit-2', + result: { + id: 'audit-2', + score: 0.5, + ...defaultAuditRef, + }, + }, { + id: 'audit-3', + result: { + id: 'audit-3', + score: 0, + metricSavings: {'LCP': 50, 'FCP': 15}, + ...defaultAuditRef, + }, + }, { + id: 'audit-4', + result: { + id: 'audit-4', + score: 0, + metricSavings: {'FCP': 15}, + ...defaultAuditRef, + }, + }, + ...metricAudits]; + + const categoryDOM = renderer.render(fakeCategory, sampleResults.categoryGroups); + + const diagnosticSection = categoryDOM.querySelector( + '.lh-category > .lh-audit-group.lh-audit-group--diagnostics'); + let diagnosticElements = [...diagnosticSection.querySelectorAll('.lh-audit')]; + expect(diagnosticElements.map(el => el.id)).toEqual(['audit-3', 'audit-1', 'audit-4', 'audit-2']); // eslint-disable-line max-len + + let hiddenElements = [...diagnosticSection.querySelectorAll('.lh-audit[hidden]')]; + expect(hiddenElements).toHaveLength(0); + + const fcpFilterButton = + categoryDOM.querySelector('.lh-metricfilter__label[title="First Contentful Paint"]'); + fcpFilterButton.click(); + + diagnosticElements = [...diagnosticSection.querySelectorAll('.lh-audit')]; + expect(diagnosticElements.map(el => el.id)).toEqual(['audit-3', 'audit-4', 'audit-1', 'audit-2']); // eslint-disable-line max-len + + hiddenElements = [...diagnosticSection.querySelectorAll('.lh-audit[hidden]')]; + expect(hiddenElements.map(el => el.id)).toEqual(['audit-2']); + }); + it('audits sorted with guidance level', () => { fakeCategory.auditRefs = [{ id: 'audit-1', diff --git a/report/test/renderer/pwa-category-renderer-test.js b/report/test/renderer/pwa-category-renderer-test.js index 98d92f55da74..356d3bd95646 100644 --- a/report/test/renderer/pwa-category-renderer-test.js +++ b/report/test/renderer/pwa-category-renderer-test.js @@ -30,7 +30,10 @@ describe('PwaCategoryRenderer', () => { reportJson: null, }); - const {document} = new jsdom.JSDOM().window; + const window = new jsdom.JSDOM().window; + const document = window.document; + global.HTMLElement = window.HTMLElement; + const dom = new DOM(document); const detailsRenderer = new DetailsRenderer(dom); pwaRenderer = new PwaCategoryRenderer(dom, detailsRenderer); diff --git a/report/test/renderer/report-renderer-test.js b/report/test/renderer/report-renderer-test.js index a6db5a5126ee..0348c27ec663 100644 --- a/report/test/renderer/report-renderer-test.js +++ b/report/test/renderer/report-renderer-test.js @@ -36,6 +36,7 @@ describe('ReportRenderer', () => { const {window} = new jsdom.JSDOM(); global.self = window; + global.HTMLElement = window.HTMLElement; const dom = new DOM(window.document); const detailsRenderer = new DetailsRenderer(dom); diff --git a/types/config.d.ts b/types/config.d.ts index 1b3de1779c34..6fbd7213b18e 100644 --- a/types/config.d.ts +++ b/types/config.d.ts @@ -9,6 +9,7 @@ import {Audit} from '../core/audits/audit.js'; import {SharedFlagsSettings, ConfigSettings} from './lhr/settings.js'; import Gatherer from './gatherer.js'; import {IcuMessage} from './lhr/i18n.js'; +import Result from './lhr/lhr.js'; interface ClassOf { new (): T; @@ -81,7 +82,7 @@ declare module Config { id: string; weight: number; group?: string; - acronym?: string; + acronym?: Result.MetricAcronym; relevantAudits?: string[]; } diff --git a/types/lhr/audit-result.d.ts b/types/lhr/audit-result.d.ts index ce84f6cb2c2e..ef84db955995 100644 --- a/types/lhr/audit-result.d.ts +++ b/types/lhr/audit-result.d.ts @@ -6,6 +6,7 @@ import {FormattedIcu} from './i18n.js'; import AuditDetails from './audit-details.js'; +import LHResult from './lhr.js'; interface ScoreDisplayModes { /** Scores of 0-1 (map to displayed scores of 0-100). */ @@ -31,13 +32,9 @@ interface ScoreDisplayModes { type ScoreDisplayMode = ScoreDisplayModes[keyof ScoreDisplayModes]; -interface MetricSavings { - LCP?: number; - FCP?: number; - CLS?: number; - TBT?: number; - INP?: number; -} +export type MetricSavings = { + [M in LHResult.MetricAcronym]?: number; +}; /** Audit result returned in Lighthouse report. All audits offer a description and score of 0-1. */ export interface Result { diff --git a/types/lhr/lhr.d.ts b/types/lhr/lhr.d.ts index df4edf201ab1..8fcd907ea4fa 100644 --- a/types/lhr/lhr.d.ts +++ b/types/lhr/lhr.d.ts @@ -115,7 +115,7 @@ declare module Result { /** Optional grouping within the category. Matches the key of a Result.Group. */ group?: string; /** The conventional acronym for the audit/metric. */ - acronym?: string; + acronym?: MetricAcronym; /** Any audit IDs closely relevant to this one. */ relevantAudits?: string[]; } @@ -197,6 +197,8 @@ declare module Result { /** Gather mode used to collect artifacts. */ type GatherMode = 'navigation'|'timespan'|'snapshot'; + + type MetricAcronym = 'FCP' | 'LCP' | 'TBT' | 'CLS' | 'INP' | 'SI' | 'TTI' | 'FMP'; } export default Result;