[RPP Observations] Improve help links

- Reintroduce textual local/field help link but smaller
- Add (?) buttons for each metric only visible on hover
- Move the network cache tooltip to the checkbox and label

https://screenshot.googleplex.com/ALg4pwyLgCrss3s

Bug: 362252927
Change-Id: I5759c70dee25740d282323b9d1adfbcb1a6644e3
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5983127
Reviewed-by: Paul Irish <[email protected]>
Commit-Queue: Adam Raine <[email protected]>

Author: Adam Raine

front_end/core/common/SettingRegistration.ts

@@ -302,7 +302,7 @@ export interface SettingRegistration {
  */
 export interface LearnMore {
   tooltip: () => Platform.UIString.LocalizedString;
-  url: Platform.DevToolsPath.UrlString;
+  url?: Platform.DevToolsPath.UrlString;
 }
 
 interface LocalizedSettingExtensionOption {

front_end/core/sdk/sdk-meta.ts

@@ -315,6 +315,11 @@ const UIStrings = {
    */
   enableRemoteFileLoading:
       'Allow `DevTools` to load resources, such as source maps, from remote file paths. Disabled by default for security reasons.',
+  /**
+   * @description Tooltip text for a setting that controls the network cache. Disabling the network cache can simulate the network connections of users that are visiting a page for the first time.
+   */
+  networkCacheExplanation:
+      'Disabling the network cache will simulate a network experience similar to a first time visitor.',
 };
 const str_ = i18n.i18n.registerUIStrings('core/sdk/sdk-meta.ts', UIStrings);
 const i18nLazyString = i18n.i18n.getLazilyComputedLocalizedString.bind(undefined, str_);
@@ -1040,6 +1045,9 @@ Common.Settings.registerSettingExtension({
       title: i18nLazyString(UIStrings.enableCache),
     },
   ],
+  learnMore: {
+    tooltip: i18nLazyString(UIStrings.networkCacheExplanation),
+  },
 });
 
 Common.Settings.registerSettingExtension({

front_end/panels/timeline/components/LiveMetricsView.test.ts

@@ -729,7 +729,7 @@ describeWithMockConnection('LiveMetricsView', () => {
       assert.match(fieldMessage!.innerText, /See how your local metrics compare/);
 
       const title = getLiveMetricsTitle(view);
-      assert.strictEqual(title.innerText, 'Local metrics ');
+      assert.strictEqual(title.innerText, 'Local metrics');
     });
 
     it('should show when crux is enabled', async () => {
@@ -760,7 +760,7 @@ describeWithMockConnection('LiveMetricsView', () => {
       assert.match(fieldMessage!.innerText, /Jan.+2024/);
 
       const title = getLiveMetricsTitle(view);
-      assert.strictEqual(title.innerText, 'Local and field metrics ');
+      assert.strictEqual(title.innerText, 'Local and field metrics');
     });
 
     it('should show empty values when crux is enabled but there is no field data', async () => {
@@ -784,7 +784,7 @@ describeWithMockConnection('LiveMetricsView', () => {
       assert.match(fieldMessage!.textContent!, /Not enough data/);
 
       const title = getLiveMetricsTitle(view);
-      assert.strictEqual(title.innerText, 'Local and field metrics ');
+      assert.strictEqual(title.innerText, 'Local and field metrics');
     });
 
     it('should make initial request on render when crux is enabled', async () => {

front_end/panels/timeline/components/LiveMetricsView.ts

@@ -10,7 +10,6 @@ import '../../../ui/components/menus/menus.js';
 import './MetricCard.js';
 
 import * as Common from '../../../core/common/common.js';
-import * as Host from '../../../core/host/host.js';
 import * as i18n from '../../../core/i18n/i18n.js';
 import type * as Platform from '../../../core/platform/platform.js';
 import type * as SDK from '../../../core/sdk/sdk.js';
@@ -175,11 +174,6 @@ const UIStrings = {
    * @description Text for a link that is inserted inside a larger text block that explains how to simulate different mobile and desktop devices.
    */
   simulateDifferentDevices: 'simulate different devices',
-  /**
-   * @description Tooltip text that explains how disabling the network cache can simulate the network connections of users that are visiting a page for the first time.
-   */
-  networkCacheExplanation:
-      'Disabling the network cache will simulate a network experience similar to a first time visitor.',
   /**
    * @description Text label for a checkbox that controls if the network cache is disabled.
    */
@@ -220,11 +214,15 @@ const UIStrings = {
    * @example {Chrome UX Report} PH1
    */
   seeHowYourLocalMetricsCompare: 'See how your local metrics compare to real user data in the {PH1}.',
+  /**
+   * @description Text for a link that goes to more documentation about local and field data. "Local" refers to performance metrics measured in the developers local environment. "field data" is data measured by real users in the field.
+   */
+  localFieldLearnMoreLink: 'Learn more about local and field data',
   /**
    * @description Tooltip text for a link that goes to documentation explaining the difference between local and field metrics. "Local metrics" are performance metrics measured in the developers local environment. "field data" is data measured by real users in the field.
    */
-  learnMoreAboutMetrics:
-      'Local metrics are captured from the current page using your network connection and device. Field data is measured by real users using many different network connections and devices. Click to learn more about local and field metrics.',
+  localFieldLearnMoreTooltip:
+      'Local metrics are captured from the current page using your network connection and device. Field data is measured by real users using many different network connections and devices.',
   /**
    * @description Tooltip text explaining that this user interaction was ignored when calculating the Interaction to Next Paint (INP) metric because the interaction delay fell beyond the 98th percentile of interaction delays on this page. "INP" is an acronym and should not be translated.
    */
@@ -664,12 +662,7 @@ export class LiveMetricsView extends LegacyWrapper.LegacyWrapper.WrappableCompon
             textOverride: i18nString(UIStrings.disableNetworkCache),
           } as Settings.SettingCheckbox.SettingCheckboxData}
         ></setting-checkbox>
-        <devtools-icon
-          class="setting-hint"
-          name="help"
-          title=${i18nString(UIStrings.networkCacheExplanation)}
-        ></devtools-icon>
-        </div>
+      </div>
     `;
     // clang-format on
   }
@@ -1106,18 +1099,8 @@ export class LiveMetricsView extends LegacyWrapper.LegacyWrapper.WrappableCompon
     const output = html`
       <div class="container">
         <div class="live-metrics-view">
-          <main class="live-metrics"
-          >
-            <h2 class="section-title">
-              ${liveMetricsTitle}
-              <devtools-button
-                class="section-title-help"
-                title=${i18nString(UIStrings.learnMoreAboutMetrics)}
-                .iconName=${'help'}
-                .variant=${Buttons.Button.Variant.ICON}
-                @click=${() => Host.InspectorFrontendHost.InspectorFrontendHostInstance.openInNewTab(helpLink)}
-              ></devtools-button>
-            </h2>
+          <main class="live-metrics">
+            <h2 class="section-title">${liveMetricsTitle}</h2>
             <div class="metric-cards"
               on-render=${ComponentHelpers.Directives.nodeRenderedCallback(node => {
                 this.#tooltipContainerEl = node;
@@ -1133,6 +1116,11 @@ export class LiveMetricsView extends LegacyWrapper.LegacyWrapper.WrappableCompon
                 ${this.#renderInpCard()}
               </div>
             </div>
+            <x-link
+              href=${helpLink}
+              class="local-field-link"
+              title=${i18nString(UIStrings.localFieldLearnMoreTooltip)}
+            >${i18nString(UIStrings.localFieldLearnMoreLink)}</x-link>
             ${this.#renderLogSection()}
           </main>
           <aside class="next-steps" aria-labelledby="next-steps-section-title">

front_end/panels/timeline/components/MetricCard.ts

@@ -2,9 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import * as Host from '../../../core/host/host.js';
 import * as i18n from '../../../core/i18n/i18n.js';
 import * as Platform from '../../../core/platform/platform.js';
 import * as CrUXManager from '../../../models/crux-manager/crux-manager.js';
+import * as Buttons from '../../../ui/components/buttons/buttons.js';
 import * as ComponentHelpers from '../../../ui/components/helpers/helpers.js';
 import * as LitHtml from '../../../ui/lit-html/lit-html.js';
 
@@ -121,6 +123,20 @@ const UIStrings = {
    * @description Column header for table cell values representing a phase duration (in milliseconds) that was measured in the developers local environment.
    */
   duration: 'Local duration (ms)',
+  /**
+   * @description Tooltip text for a link that goes to documentation explaining the Largest Contentful Paint (LCP) metric. "LCP" is an acronym and should not be translated.
+   */
+  lcpHelpTooltip:
+      'LCP reports the render time of the largest image, text block, or video visible in the viewport. Click here to learn more about LCP.',
+  /**
+   * @description Tooltip text for a link that goes to documentation explaining the Cumulative Layout Shift (CLS) metric. "CLS" is an acronym and should not be translated.
+   */
+  clsHelpTooltip: 'CLS measures the amount of unexpected shifted content. Click here to learn more about CLS.',
+  /**
+   * @description Tooltip text for a link that goes to documentation explaining the Interaction to Next Paint (INP) metric. "INP" is an acronym and should not be translated.
+   */
+  inpHelpTooltip:
+      'INP measures the overall responsiveness to all click, tap, and keyboard interactions. Click here to learn more about INP.',
 };
 
 const str_ = i18n.i18n.registerUIStrings('panels/timeline/components/MetricCard.ts', UIStrings);
@@ -291,6 +307,28 @@ export class MetricCard extends HTMLElement {
     }
   }
 
+  #getHelpLink(): Platform.DevToolsPath.UrlString {
+    switch (this.#data.metric) {
+      case 'LCP':
+        return 'https://web.dev/articles/lcp' as Platform.DevToolsPath.UrlString;
+      case 'CLS':
+        return 'https://web.dev/articles/cls' as Platform.DevToolsPath.UrlString;
+      case 'INP':
+        return 'https://web.dev/articles/inp' as Platform.DevToolsPath.UrlString;
+    }
+  }
+
+  #getHelpTooltip(): string {
+    switch (this.#data.metric) {
+      case 'LCP':
+        return i18nString(UIStrings.lcpHelpTooltip);
+      case 'CLS':
+        return i18nString(UIStrings.clsHelpTooltip);
+      case 'INP':
+        return i18nString(UIStrings.inpHelpTooltip);
+    }
+  }
+
   #getLocalValue(): number|undefined {
     const {localValue} = this.#data;
     if (localValue === undefined) {
@@ -579,12 +617,20 @@ export class MetricCard extends HTMLElement {
 
   #render = (): void => {
     const fieldEnabled = CrUXManager.CrUXManager.instance().getConfigSetting().get().enabled;
+    const helpLink = this.#getHelpLink();
 
     // clang-format off
     const output = html`
       <div class="metric-card">
         <h3 class="title">
           ${this.#getTitle()}
+          <devtools-button
+            class="title-help"
+            title=${this.#getHelpTooltip()}
+            .iconName=${'help'}
+            .variant=${Buttons.Button.Variant.ICON}
+            @click=${() => Host.InspectorFrontendHost.InspectorFrontendHostInstance.openInNewTab(helpLink)}
+          ></devtools-button>
         </h3>
         <div tabindex="0" class="metric-values-section"
           @mouseenter=${() => this.#showTooltip(500)}

front_end/panels/timeline/components/liveMetricsView.css

@@ -73,11 +73,6 @@
   margin-bottom: 10px;
 }
 
-.section-title-help {
-  height: var(--sys-typescale-headline4-line-height);
-  vertical-align: top;
-}
-
 .settings-card {
   border-radius: var(--sys-shape-corner-small);
   padding: 14px 16px 16px;
@@ -135,6 +130,10 @@
   min-width: 0;
 }
 
+.local-field-link {
+  margin-top: 8px;
+}
+
 .logs-section {
   margin-top: 24px;
   display: flex;
@@ -293,13 +292,6 @@ x-link { /* stylelint-disable-line selector-type-no-unknown */
   margin-top: 8px;
 }
 
-.setting-hint {
-  margin-left: 6px;
-  vertical-align: bottom;
-  height: 18px;
-  width: 18px;
-}
-
 .environment-recs-title {
   margin-top: 12px;
   font-weight: var(--ref-typeface-weight-medium);
@@ -336,7 +328,7 @@ x-link { /* stylelint-disable-line selector-type-no-unknown */
 
 .phase-table {
   border-top: 1px solid var(--sys-color-divider);
-  padding: 7px 0;
+  padding: 7px 4px;
   margin-left: var(--phase-table-margin);
 }
 

front_end/panels/timeline/components/metricCard.css

@@ -10,16 +10,27 @@
   background-color: var(--sys-color-surface3);
   height: 100%;
   box-sizing: border-box;
+
+  &:not(:hover) .title-help {
+    visibility: hidden;
+  }
 }
 
 .title {
+  display: flex;
+  justify-content: space-between;
   font-size: var(--sys-typescale-headline5-size);
   line-height: var(--sys-typescale-headline5-line-height);
   font-weight: var(--ref-typeface-weight-medium);
   margin: 0;
   margin-bottom: 6px;
 }
 
+.title-help {
+  height: var(--sys-typescale-headline5-line-height);
+  margin-left: 4px;
+}
+
 .metric-values-section {
   position: relative;
   display: flex;

front_end/ui/components/settings/SettingCheckbox.ts

@@ -69,7 +69,8 @@ export class SettingCheckbox extends HTMLElement {
     }
 
     const learnMore = this.#setting.learnMore();
-    if (learnMore) {
+    if (learnMore && learnMore.url) {
+      const url = learnMore.url;
       const data: Buttons.Button.ButtonData = {
         iconName: 'help',
         variant: Buttons.Button.Variant.ICON,
@@ -78,7 +79,7 @@ export class SettingCheckbox extends HTMLElement {
         title: i18nString(UIStrings.learnMore),
       };
       const handleClick = (event: MouseEvent): void => {
-        Host.InspectorFrontendHost.InspectorFrontendHostInstance.openInNewTab(learnMore.url);
+        Host.InspectorFrontendHost.InspectorFrontendHostInstance.openInNewTab(url);
         event.consume();
       };
       return html`<devtools-button