report(performance): use metric savings for metric filter

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.

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<TopLevelClumpId, 'failed'>} clumpId
-   * @param {{auditRefs: Array<LH.ReportResult.AuditRef>, description?: string, openByDefault?: boolean}} clumpOpts
+   * @param {{auditRefsOrEls: Array<LH.ReportResult.AuditRef | HTMLElement>, 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,8 @@ 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 clumpOpts = {auditRefsOrEls: auditRefs, description, openByDefault};
+      const clumpElem = this.renderClump(clumpId, clumpOpts);
       element.append(clumpElem);
     }
 

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,71 +212,103 @@
       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');
 
-    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 - a.overallImpact;
-
-      // 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 - a.overallLinearImpact;
+    /**
+     * @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;
+        }
       }
 
-      // Audits that have no estimated savings should be prioritized by the guidance level
-      return b.guidanceLevel - a.guidanceLevel;
-    });
+      diagnosticAudits.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;
+
+        // 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;
+        }
 
-    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);
+        // Overall impact is the estimated improvement to the performance score
+        if (a.overallImpact !== b.overallImpact) return b.overallImpact - a.overallImpact;
+
+        // 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 - a.overallLinearImpact;
+        }
+
+        // Audits that have no estimated savings should be prioritized by the guidance level
+        return b.guidanceLevel - a.guidanceLevel;
+      }).forEach(item => {
+        groupEl.insertBefore(item.auditEl, footerEl);
+      });
+    }
+
+    /** @type {Set<string>} */
+    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);
+      }
     }
 
-    // Passed audits
-    const passedAudits = category.auditRefs
-        .filter(audit =>
-          this._isPerformanceInsight(audit) && ReportUtils.showAsPassed(audit.result));
+    const filterableMetrics =
+      metricAudits.filter(a => a.acronym && filterableMetricAcronyms.has(a.acronym));
+
+    // TODO: only add if there are opportunities & diagnostics rendered.
+    if (filterableMetrics.length) {
+      this.renderMetricAuditFilter(filterableMetrics, element, refreshFilteredAudits);
+    }
+
+    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);
@@ -315,16 +348,17 @@
    * 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.
@@ -338,7 +372,7 @@
 
       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') {
@@ -354,17 +388,7 @@
         }
         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');

report/renderer/pwa-category-renderer.js

@@ -29,8 +29,12 @@ 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 clumpOpts = {
+      auditRefsOrEls: manualAuditRefs,
+      description: category.manualDescription,
+      openByDefault: true,
+    };
+    const manualElem = this.renderClump('manual', clumpOpts);
     categoryElem.append(manualElem);
 
     return categoryElem;

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() {

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);

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);

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);

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);

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<T> {
   new (): T;
@@ -115,7 +116,7 @@ declare module Config {
     id: string;
     weight: number;
     group?: string;
-    acronym?: string;
+    acronym?: Result.MetricAcronym;
     relevantAudits?: string[];
   }
 

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 {