report(flow): category tooltip highest impact (#13230)

2021-10-26 13:41:32 -07:00 · 2021-10-26 13:41:32 -07:00 · d22d73f5a2
--- a/flow-report/assets/styles.css
+++ b/flow-report/assets/styles.css
@ -31,6 +31,7 @@
  --summary-subtitle-font-size: 12px;
  --summary-title-font-size: 20px;
  --summary-tooltip-box-shadow-color: rgba(0, 0, 0, 0.25);
+  --summary-tooltip-line-height: 16px;
  --topbar-button-font-size: 14px;
  --topbar-button-size: 45px;
  --topbar-title-font-size: 14px;
@ -394,22 +395,33 @@
  border-radius: 3px;
  padding: var(--base-spacing);
  right: 0;
+  line-height: var(--summary-tooltip-line-height);
  box-shadow: 0px 4px 4px var(--summary-tooltip-box-shadow-color);
  z-index: 1;
 }

 .SummaryTooltip__title {
  font-weight: var(--bold-weight);
-  margin-bottom: var(--half-base-spacing);
+}
+
+.SummaryTooltip__url {
+  overflow-x: hidden;
+  text-overflow: ellipsis;
+  white-space: nowrap;
+  margin-top: calc(0.25 * var(--base-spacing));
+  margin-bottom: calc(0.75 * var(--base-spacing));
+  color: var(--color-gray-700);
 }

 .SummaryTooltip__category {
  font-weight: var(--bold-weight);
  display: flex;
+  align-items: center;
  margin-top: var(--half-base-spacing);
 }

 .SummaryTooltip__category-title {
+  line-height: 24px;
  flex-grow: 1;
 }

@ -424,6 +436,50 @@
  color: var(--color-fail);
 }

+.SummaryTooltip__fraction {
+  color: var(--color-gray-700);
+}
+
+.SummaryTooltip__informative {
+  margin-top: calc(0.25 * var(--base-spacing));
+  color: var(--color-gray-700);
+}
+
+.SummaryTooltipAudits__title {
+  line-height: 24px;
+  margin-top: calc(0.75 * var(--base-spacing));
+  font-weight: var(--bold-weight);
+}
+
+.SummaryTooltipAudit {
+  display: flex;
+  margin: calc(0.25 * var(--base-spacing)) 0px;
+  color: var(--color-gray-700);
+  --score-icon-size: 10px;
+}
+.SummaryTooltipAudit code {
+  color: var(--snippet-color);
+}
+.SummaryTooltipAudit::before {
+  content: '';
+  height: var(--score-icon-size);
+  width: var(--score-icon-size);
+  margin-top: calc((var(--summary-tooltip-line-height) - var(--score-icon-size)) / 2);
+  margin-right: var(--half-base-spacing);
+}
+.SummaryTooltipAudit--pass::before {
+  background-color: var(--color-pass);
+  border-radius: 50%;
+}
+.SummaryTooltipAudit--average::before {
+  background-color: var(--color-average);
+}
+.SummaryTooltipAudit--fail::before {
+  border-left: calc(var(--score-icon-size) / 2) solid transparent;
+  border-right: calc(var(--score-icon-size) / 2) solid transparent;
+  border-bottom: var(--score-icon-size) solid var(--color-fail);
+}
+
 .SummaryNavigationHeader {
  font-size: var(--summary-navigation-header-font-size);
  line-height: 16px;
--- a/flow-report/src/i18n/ui-strings.js
+++ b/flow-report/src/i18n/ui-strings.js
@ -131,4 +131,6 @@ export const UIStrings = {
    =1 {{numInformative} informative audit}
    other {{numInformative} informative audits}
  }`,
+  /** Label for a list of Lighthouse audits that are the most impactful. */
+  highestImpact: 'Highest impact',
 };
--- a/flow-report/src/summary/category.tsx
+++ b/flow-report/src/summary/category.tsx
@ -10,9 +10,14 @@ import {Util} from '../../../report/renderer/util';
 import {Separator} from '../common';
 import {CategoryScore} from '../wrappers/category-score';
 import {useI18n, useStringFormatter, useLocalizedStrings} from '../i18n/i18n';
+import {Markdown} from '../wrappers/markdown';

 import type {UIStringsType} from '../i18n/ui-strings';

+const MAX_TOOLTIP_AUDITS = 2;
+
+type ScoredAuditRef = LH.ReportResult.AuditRef & {result: {score: number}};
+
 function getGatherModeLabel(gatherMode: LH.Result.GatherMode, strings: UIStringsType) {
  switch (gatherMode) {
    case 'navigation': return strings.navigationReport;
@ -30,10 +35,71 @@ function getCategoryRating(rating: string, strings: UIStringsType) {
  }
 }

+function getScoreToBeGained(audit: ScoredAuditRef): number {
+  return audit.weight * (1 - audit.result.score);
+}
+
+function getOverallSavings(audit: LH.ReportResult.AuditRef): number {
+  return (
+    audit.result.details &&
+    audit.result.details.type === 'opportunity' &&
+    audit.result.details.overallSavingsMs
+  ) || 0;
+}
+
+const SummaryTooltipAudit: FunctionComponent<{audit: LH.ReportResult.AuditRef}> = ({audit}) => {
+  const rating = Util.calculateRating(audit.result.score, audit.result.scoreDisplayMode);
+  return (
+    <div className={`SummaryTooltipAudit SummaryTooltipAudit--${rating}`}>
+      <Markdown text={audit.result.title}/>
+    </div>
+  );
+};
+
+const SummaryTooltipAudits: FunctionComponent<{category: LH.ReportResult.Category}> =
+({category}) => {
+  const strings = useLocalizedStrings();
+
+  function isRelevantAudit(audit: LH.ReportResult.AuditRef): audit is ScoredAuditRef {
+    return audit.result.score !== null &&
+      // Metrics should not be displayed in this group.
+      audit.group !== 'metrics' &&
+      // Audits in performance without a group are hidden.
+      (audit.group !== undefined || category.id !== 'performance') &&
+      // We don't want unweighted audits except for opportunities with potential savings.
+      (audit.weight > 0 || getOverallSavings(audit) > 0) &&
+      // Passing audits should never be high impact.
+      !Util.showAsPassed(audit.result);
+  }
+
+  const audits = category.auditRefs
+    .filter(isRelevantAudit)
+    .sort((a, b) => {
+      // Remaining score should always be 0 for perf opportunities because weight is 0.
+      // In that case, we want to sort by `overallSavingsMs`.
+      const remainingScoreA = getScoreToBeGained(a);
+      const remainingScoreB = getScoreToBeGained(b);
+      if (remainingScoreA !== remainingScoreB) return remainingScoreB - remainingScoreA;
+      return getOverallSavings(b) - getOverallSavings(a);
+    })
+    .splice(0, MAX_TOOLTIP_AUDITS);
+  if (!audits.length) return null;
+
+  return (
+    <div className="SummaryTooltipAudits">
+      <div className="SummaryTooltipAudits__title">{strings.highestImpact}</div>
+      {
+        audits.map(audit => <SummaryTooltipAudit key={audit.id} audit={audit}/>)
+      }
+    </div>
+  );
+};
+
 export const SummaryTooltip: FunctionComponent<{
  category: LH.ReportResult.Category,
-  gatherMode: LH.Result.GatherMode
-}> = ({category, gatherMode}) => {
+  gatherMode: LH.Result.GatherMode,
+  url: string,
+}> = ({category, gatherMode, url}) => {
  const strings = useLocalizedStrings();
  const str_ = useStringFormatter();
  const {
@ -53,6 +119,7 @@ export const SummaryTooltip: FunctionComponent<{
  return (
    <div className="SummaryTooltip">
      <div className="SummaryTooltip__title">{getGatherModeLabel(gatherMode, strings)}</div>
+      <div className="SummaryTooltip__url">{url}</div>
      <Separator/>
      <div className="SummaryTooltip__category">
        <div className="SummaryTooltip__category-title">
@ -81,6 +148,7 @@ export const SummaryTooltip: FunctionComponent<{
          {str_(strings.informativeAuditCount, {numInformative})}
        </div>
      }
+      <SummaryTooltipAudits category={category}/>
    </div>
  );
 };
@ -89,7 +157,8 @@ export const SummaryCategory: FunctionComponent<{
  category: LH.ReportResult.Category|undefined,
  href: string,
  gatherMode: LH.Result.GatherMode,
-}> = ({category, href, gatherMode}) => {
+  finalUrl: string,
+}> = ({category, href, gatherMode, finalUrl}) => {
  return (
    <div className="SummaryCategory">
      {
@ -100,7 +169,7 @@ export const SummaryCategory: FunctionComponent<{
              href={href}
              gatherMode={gatherMode}
            />
-            <SummaryTooltip category={category} gatherMode={gatherMode}/>
+            <SummaryTooltip category={category} gatherMode={gatherMode} url={finalUrl}/>
          </div> :
          <div className="SummaryCategory__null" data-testid="SummaryCategory__null"/>
      }
--- a/flow-report/src/summary/summary.tsx
+++ b/flow-report/src/summary/summary.tsx
@ -76,6 +76,7 @@ export const SummaryFlowStep: FunctionComponent<{
            category={reportResult.categories[c]}
            href={`#index=${hashIndex}&anchor=${c}`}
            gatherMode={lhr.gatherMode}
+            finalUrl={lhr.finalUrl}
          />
        ))
      }
--- a/flow-report/src/wrappers/markdown.tsx
+++ b/flow-report/src/wrappers/markdown.tsx
@ -0,0 +1,27 @@
+/**
+ * @license Copyright 2021 The Lighthouse Authors. All Rights Reserved.
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
+ */
+
+import {FunctionComponent} from 'preact';
+import {useLayoutEffect, useRef} from 'preact/hooks';
+
+import {useReportRenderer} from '../wrappers/report-renderer';
+
+export const Markdown: FunctionComponent<{text: string}> = ({text}) => {
+  const {dom} = useReportRenderer();
+  const ref = useRef<HTMLSpanElement>(null);
+
+  useLayoutEffect(() => {
+    if (ref.current) {
+      const md = dom.convertMarkdownCodeSnippets(text);
+      ref.current.appendChild(md);
+    }
+    return () => {
+      if (ref.current) ref.current.innerHTML = '';
+    };
+  }, [text]);
+
+  return <span ref={ref}/>;
+};
--- a/flow-report/test/summary/category-test.tsx
+++ b/flow-report/test/summary/category-test.tsx
@ -11,6 +11,7 @@ import {SummaryTooltip} from '../../src/summary/category';
 import {flowResult} from '../sample-flow';
 import {I18nProvider} from '../../src/i18n/i18n';
 import {FlowResultContext} from '../../src/util';
+import {ReportRendererProvider} from '../../src/wrappers/report-renderer';

 let wrapper: FunctionComponent;

@ -18,9 +19,11 @@ beforeEach(() => {
  // Include sample flowResult for locale in I18nProvider.
  wrapper = ({children}) => (
    <FlowResultContext.Provider value={flowResult}>
+      <ReportRendererProvider>
        <I18nProvider>
          {children}
        </I18nProvider>
+      </ReportRendererProvider>
    </FlowResultContext.Provider>
  );
 });
@ -31,14 +34,16 @@ describe('SummaryTooltip', () => {
      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: 'binary'}, weight: 1, group: 'diagnostics'},
+        /* eslint-disable max-len */
+        {result: {score: 1, scoreDisplayMode: 'binary', title: 'Audit 1'}, weight: 1, group: 'diagnostics'},
+        {result: {score: 1, scoreDisplayMode: 'binary', title: 'Audit 2'}, weight: 1, group: 'diagnostics'},
+        {result: {score: 0, scoreDisplayMode: 'binary', title: 'Audit 3'}, weight: 1, group: 'diagnostics'},
+        /* eslint-enable max-len */
      ],
    };

    const root = render(
-      <SummaryTooltip category={category} gatherMode="snapshot"/>,
+      <SummaryTooltip category={category} gatherMode="snapshot" url="https://example.com"/>,
      {wrapper}
    );

@ -46,6 +51,7 @@ describe('SummaryTooltip', () => {
    expect(() => root.getByText(/^[0-9]+$/)).toThrow();
    expect(root.getByText('2 audits passed')).toBeTruthy();
    expect(root.getByText('3 passable audits')).toBeTruthy();
+    expect(root.getByText('https://example.com'));
  });

  it('renders tooltip without rating', async () => {
@ -53,14 +59,16 @@ describe('SummaryTooltip', () => {
      id: 'performance',
      score: 1,
      auditRefs: [
-        {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'},
+        /* eslint-disable max-len */
+        {result: {score: 1, scoreDisplayMode: 'binary', title: 'Audit 1'}, weight: 0, group: 'diagnostics'},
+        {result: {score: 1, scoreDisplayMode: 'binary', title: 'Audit 2'}, weight: 0, group: 'diagnostics'},
+        {result: {score: 0, scoreDisplayMode: 'binary', title: 'Audit 3'}, weight: 0, group: 'diagnostics'},
+        /* eslint-enable max-len */
      ],
    };

    const root = render(
-      <SummaryTooltip category={category} gatherMode="snapshot"/>,
+      <SummaryTooltip category={category} gatherMode="snapshot" url="https://example.com"/>,
      {wrapper}
    );

@ -68,6 +76,7 @@ describe('SummaryTooltip', () => {
    expect(() => root.getByText(/^[0-9]+$/)).toThrow();
    expect(root.getByText('2 audits passed')).toBeTruthy();
    expect(root.getByText('3 passable audits')).toBeTruthy();
+    expect(root.getByText('https://example.com'));
  });

  it('renders scored category tooltip with score', async () => {
@ -75,14 +84,16 @@ describe('SummaryTooltip', () => {
      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: 'binary'}, weight: 1, group: 'diagnostics'},
+        /* eslint-disable max-len */
+        {result: {score: 1, scoreDisplayMode: 'binary', title: 'Audit 1'}, weight: 1, group: 'diagnostics'},
+        {result: {score: 1, scoreDisplayMode: 'binary', title: 'Audit 2'}, weight: 1, group: 'diagnostics'},
+        {result: {score: 0, scoreDisplayMode: 'binary', title: 'Audit 3'}, weight: 1, group: 'diagnostics'},
+        /* eslint-enable max-len */
      ],
    };

    const root = render(
-      <SummaryTooltip category={category} gatherMode="navigation"/>,
+      <SummaryTooltip category={category} gatherMode="navigation" url="https://example.com"/>,
      {wrapper}
    );

@ -90,6 +101,7 @@ describe('SummaryTooltip', () => {
    expect(root.getByText('100')).toBeTruthy();
    expect(root.getByText('2 audits passed')).toBeTruthy();
    expect(root.getByText('3 passable audits')).toBeTruthy();
+    expect(root.getByText('https://example.com'));
  });

  it('renders informative audit count if any', async () => {
@ -97,14 +109,16 @@ describe('SummaryTooltip', () => {
      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'},
+        /* eslint-disable max-len */
+        {result: {score: 1, scoreDisplayMode: 'binary', title: 'Audit 1'}, weight: 1, group: 'diagnostics'},
+        {result: {score: 1, scoreDisplayMode: 'binary', title: 'Audit 2'}, weight: 1, group: 'diagnostics'},
+        {result: {score: null, scoreDisplayMode: 'informative', title: 'Audit 3'}, weight: 1, group: 'diagnostics'},
+        /* eslint-enable max-len */
      ],
    };

    const root = render(
-      <SummaryTooltip category={category} gatherMode="navigation"/>,
+      <SummaryTooltip category={category} gatherMode="navigation" url="https://example.com"/>,
      {wrapper}
    );

@ -113,5 +127,83 @@ describe('SummaryTooltip', () => {
    expect(root.getByText('2 audits passed')).toBeTruthy();
    expect(root.getByText('2 passable audits')).toBeTruthy();
    expect(root.getByText('1 informative audit')).toBeTruthy();
+    expect(root.getByText('https://example.com'));
+  });
+
+  it('renders highest impact audits', async () => {
+    const category: any = {
+      id: 'seo',
+      score: 1,
+      auditRefs: [
+        /* eslint-disable max-len */
+        {result: {score: 0, scoreDisplayMode: 'binary', title: 'Audit 1'}, weight: 1, group: 'group'},
+        {result: {score: 0, scoreDisplayMode: 'binary', title: 'Audit 2'}, weight: 2, group: 'group'},
+        {result: {score: 0, scoreDisplayMode: 'binary', title: 'Audit 3'}, weight: 3, group: 'group'},
+        /* eslint-enable max-len */
+      ],
+    };
+
+    const root = render(
+      <SummaryTooltip category={category} gatherMode="navigation" url="https://example.com"/>,
+      {wrapper}
+    );
+
+    const audits = root.getAllByText(/^Audit [0-9]$/);
+
+    expect(root.getByText('Highest impact')).toBeTruthy();
+    expect(audits.map(a => a.textContent)).toEqual([
+      'Audit 3',
+      'Audit 2',
+    ]);
+  });
+
+  it('renders highest impact audits in performance', async () => {
+    const category: any = {
+      id: 'performance',
+      score: 0.75,
+      auditRefs: [
+        /* eslint-disable max-len */
+        {result: {score: 0.75, scoreDisplayMode: 'numeric', title: 'Metric 1'}, weight: 1, group: 'metrics'},
+        {result: {score: 0, scoreDisplayMode: 'numeric', title: 'Audit 1', details: {type: 'opportunity', overallSavingsMs: 500}}, weight: 0, group: 'opportunities'},
+        {result: {score: 0, scoreDisplayMode: 'numeric', title: 'Audit 2', details: {type: 'opportunity', overallSavingsMs: 1000}}, weight: 0, group: 'opportunities'},
+        {result: {score: 0, scoreDisplayMode: 'numeric', title: 'Audit 3', details: {type: 'opportunity', overallSavingsMs: 100}}, weight: 0, group: 'opportunities'},
+        /* eslint-enable max-len */
+      ],
+    };
+
+    const root = render(
+      <SummaryTooltip category={category} gatherMode="navigation" url="https://example.com"/>,
+      {wrapper}
+    );
+
+    const audits = root.getAllByText(/^(Audit|Metric) [0-9]$/);
+
+    expect(root.getByText('Highest impact')).toBeTruthy();
+    expect(audits.map(a => a.textContent)).toEqual([
+      'Audit 2',
+      'Audit 1',
+    ]);
+  });
+
+  it('hides highest impact if nothing to show', async () => {
+    const category: any = {
+      id: 'performance',
+      score: 1,
+      auditRefs: [
+        /* eslint-disable max-len */
+        {result: {score: 1, scoreDisplayMode: 'binary', title: 'Audit 1'}, weight: 1, group: 'diagnostics'},
+        {result: {score: 0, scoreDisplayMode: 'binary', title: 'Audit 2'}, weight: 1},
+        {result: {score: null, scoreDisplayMode: 'informative', title: 'Audit 3'}, weight: 1, group: 'diagnostics'},
+        /* eslint-enable max-len */
+      ],
+    };
+
+    const root = render(
+      <SummaryTooltip category={category} gatherMode="navigation" url="https://example.com"/>,
+      {wrapper}
+    );
+
+    expect(() => root.getByText('Highest impact')).toThrow();
+    expect(() => root.getByText(/^Audit [0-9]$/)).toThrow();
  });
 });
--- a/flow-report/test/wrappers/markdown-test.tsx
+++ b/flow-report/test/wrappers/markdown-test.tsx
@ -0,0 +1,29 @@
+/**
+ * @license Copyright 2021 The Lighthouse Authors. All Rights Reserved.
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
+ */
+
+import {render} from '@testing-library/preact';
+import {FunctionComponent} from 'preact';
+
+import {Markdown} from '../../src/wrappers/markdown';
+import {ReportRendererProvider} from '../../src/wrappers/report-renderer';
+
+let wrapper: FunctionComponent;
+
+beforeEach(() => {
+  wrapper = ({children}) => (
+    <ReportRendererProvider>
+      {children}
+    </ReportRendererProvider>
+  );
+});
+
+describe('Markdown', () => {
+  it('renders markdown text', () => {
+    const root = render(<Markdown text="Some `fancy` text"/>, {wrapper});
+    const text = root.getByText(/^Some.*text$/);
+    expect(text.innerHTML).toEqual('Some <code>fancy</code> text');
+  });
+});
--- a/shared/localization/locales/en-US.json
+++ b/shared/localization/locales/en-US.json
@ -59,6 +59,9 @@
  "flow-report/src/i18n/ui-strings.js | helpUseCaseTimespan2": {
    "message": "Discover performance opportunities to improve the experience for long-lived pages and single-page applications."
  },
+  "flow-report/src/i18n/ui-strings.js | highestImpact": {
+    "message": "Highest impact"
+  },
  "flow-report/src/i18n/ui-strings.js | informativeAuditCount": {
    "message": "{numInformative, plural,\n    =1 {{numInformative} informative audit}\n    other {{numInformative} informative audits}\n  }"
  },
--- a/shared/localization/locales/en-XL.json
+++ b/shared/localization/locales/en-XL.json
@ -59,6 +59,9 @@
  "flow-report/src/i18n/ui-strings.js | helpUseCaseTimespan2": {
    "message": "D̂íŝćôv́êŕ p̂ér̂f́ôŕm̂án̂ćê óp̂ṕôŕt̂ún̂ít̂íêś t̂ó îḿp̂ŕôv́ê t́ĥé êx́p̂ér̂íêńĉé f̂ór̂ ĺôńĝ-ĺîv́êd́ p̂áĝéŝ án̂d́ ŝín̂ǵl̂é-p̂áĝé âṕp̂ĺîćât́îón̂ś."
  },
+  "flow-report/src/i18n/ui-strings.js | highestImpact": {
+    "message": "Ĥíĝh́êśt̂ ím̂ṕâćt̂"
+  },
  "flow-report/src/i18n/ui-strings.js | informativeAuditCount": {
    "message": "{numInformative, plural,\n    =1 {{numInformative} îńf̂ór̂ḿât́îv́ê áûd́ît́}\n    other {{numInformative} îńf̂ór̂ḿât́îv́ê áûd́ît́ŝ}\n  }"
  },