[RPP Observations] Remove collapsible container for env recs

https://screenshot.googleplex.com/ACPKBbu5GhDdTK2

Bug: None
Change-Id: Ib44671217c74f8beb76a7565843ae029dcbb0701
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5941208
Reviewed-by: Connor Clark <[email protected]>
Commit-Queue: Adam Raine <[email protected]>

Author: Adam Raine

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

@@ -37,7 +37,7 @@ function getFieldMetricValue(view: Element, metric: string): HTMLElement|null {
 }
 
 function getEnvironmentRecs(view: Element): HTMLElement[] {
-  return Array.from(view.shadowRoot!.querySelectorAll<HTMLElement>('.environment-recs li'));
+  return Array.from(view.shadowRoot!.querySelectorAll<HTMLElement>('.environment-rec'));
 }
 
 function getInteractions(view: Element): HTMLElement[] {
@@ -726,10 +726,11 @@ describeWithMockConnection('LiveMetricsView', () => {
       await coordinator.done();
 
       const envRecs = getEnvironmentRecs(view);
-      assert.lengthOf(envRecs, 0);
+      assert.strictEqual(envRecs[0].textContent, 'Not enough data');
+      assert.strictEqual(envRecs[1].textContent, 'Not enough data');
 
       const fieldMessage = getFieldMessage(view);
-      assert.isNull(fieldMessage);
+      assert.match(fieldMessage!.textContent!, /Not enough data/);
 
       const dataDescriptions = getDataDescriptions(view);
       assert.match(dataDescriptions.innerText, /local metrics/);
@@ -883,7 +884,7 @@ describeWithMockConnection('LiveMetricsView', () => {
 
         const envRecs = getEnvironmentRecs(view);
         assert.lengthOf(envRecs, 2);
-        assert.match(envRecs[0].textContent!, /60%.*desktop/);
+        assert.strictEqual(envRecs[0].textContent, '30% mobile, 60% desktop');
         assert.match(envRecs[1].textContent!, /Slow 4G/);
       });
 
@@ -896,8 +897,8 @@ describeWithMockConnection('LiveMetricsView', () => {
         await coordinator.done();
 
         const envRecs = getEnvironmentRecs(view);
-        assert.lengthOf(envRecs, 1);
-        assert.match(envRecs[0].textContent!, /60%.*desktop/);
+        assert.strictEqual(envRecs[0].textContent, '30% mobile, 60% desktop');
+        assert.strictEqual(envRecs[1].textContent, 'Not enough data');
       });
 
       it('should suggest no throttling for very low latency', async () => {
@@ -912,9 +913,8 @@ describeWithMockConnection('LiveMetricsView', () => {
         await coordinator.done();
 
         const envRecs = getEnvironmentRecs(view);
-        assert.lengthOf(envRecs, 2);
-        assert.match(envRecs[0].textContent!, /60%.*desktop/);
-        assert.match(envRecs[1].textContent!, /no throttling/);
+        assert.strictEqual(envRecs[0].textContent, '30% mobile, 60% desktop');
+        assert.match(envRecs[1].textContent!, /too fast to simulate with throttling/);
       });
 
       it('should ignore presets that are generally too far off', async () => {
@@ -929,8 +929,8 @@ describeWithMockConnection('LiveMetricsView', () => {
         await coordinator.done();
 
         const envRecs = getEnvironmentRecs(view);
-        assert.lengthOf(envRecs, 1);
-        assert.match(envRecs[0].textContent!, /60%.*desktop/);
+        assert.strictEqual(envRecs[0].textContent, '30% mobile, 60% desktop');
+        assert.strictEqual(envRecs[1].textContent, 'Not enough data');
       });
     });
 
@@ -943,8 +943,7 @@ describeWithMockConnection('LiveMetricsView', () => {
         await coordinator.done();
 
         const envRecs = getEnvironmentRecs(view);
-        assert.lengthOf(envRecs, 2);
-        assert.match(envRecs[0].textContent!, /60%.*desktop/);
+        assert.strictEqual(envRecs[0].textContent, '30% mobile, 60% desktop');
         assert.match(envRecs[1].textContent!, /Slow 4G/);
       });
 
@@ -962,8 +961,7 @@ describeWithMockConnection('LiveMetricsView', () => {
         await coordinator.done();
 
         const envRecs = getEnvironmentRecs(view);
-        assert.lengthOf(envRecs, 2);
-        assert.match(envRecs[0].textContent!, /80%.*mobile/);
+        assert.strictEqual(envRecs[0].textContent, '80% mobile, 10% desktop');
         assert.match(envRecs[1].textContent!, /Slow 4G/);
       });
 
@@ -981,8 +979,8 @@ describeWithMockConnection('LiveMetricsView', () => {
         await coordinator.done();
 
         const envRecs = getEnvironmentRecs(view);
-        assert.lengthOf(envRecs, 1);
-        assert.match(envRecs[0].textContent!, /Slow 4G/);
+        assert.strictEqual(envRecs[0].textContent, '49% mobile, 49% desktop');
+        assert.match(envRecs[1].textContent!, /Slow 4G/);
       });
     });
   });

front_end/panels/timeline/components/LiveMetricsView.ts

@@ -81,6 +81,15 @@ const UIStrings = {
    * @example {Mobile} PH1
    */
   showFieldDataForDevice: 'Show field data for device type: {PH1}',
+  /**
+   * @description Text indicating that there is not enough data to report real user statistics.
+   */
+  notEnoughData: 'Not enough data',
+  /**
+   * @description Label for a text block that describes the network connections of real users.
+   * @example {75th percentile is similar to Slow 4G throttling} PH1
+   */
+  network: 'Network: {PH1}',
   /**
    * @description Label for an select box that selects which device type field data be shown for (e.g. desktop/mobile/all devices/etc).
    * @example {Mobile} PH1
@@ -141,30 +150,20 @@ const UIStrings = {
    */
   showFieldDataForPage: 'Show field data for {PH1}',
   /**
-   * @description Tooltip text explaining that real user connections are similar to a test environment with no throttling. "latencies" refers to the time it takes for a website server to respond. "throttling" is when the network is intentionally slowed down to simulate a slower connection.
+   * @description Tooltip text explaining that real user connections are similar to a test environment with no throttling. "throttling" is when the network is intentionally slowed down to simulate a slower connection.
    */
-  tryDisablingThrottling:
-      'The 75th percentile of real users experienced network latencies similar to a connection with no throttling.',
+  tryDisablingThrottling: '75th percentile is too fast to simulate with throttling',
   /**
-   * @description Tooltip text explaining that real user connections are similar to a specif network throttling setup. "latencies" refers to the time it takes for a website server to respond. "throttling" is when the network is intentionally slowed down to simulate a slower connection.
+   * @description Tooltip text explaining that real user connections are similar to a specif network throttling setup. "throttling" is when the network is intentionally slowed down to simulate a slower connection.
    * @example {Slow 4G} PH1
    */
-  tryUsingThrottling: 'The 75th percentile of real users experienced network latencies similar to {PH1} throttling.',
+  tryUsingThrottling: '75th percentile is similar to {PH1} throttling',
   /**
-   * @description Tooltip text explaining that a majority of users are using a mobile form factor with the specific percentage.
+   * @description Text block listing what percentage of real users are on different device form factors.
    * @example {60%} PH1
+   * @example {30%} PH2
    */
-  mostUsersMobile: '{PH1} of users are on mobile.',
-  /**
-   * @description Tooltip text explaining that a majority of users are using a desktop form factor with the specific percentage.
-   * @example {60%} PH1
-   */
-  mostUsersDesktop: '{PH1} of users are on desktop.',
-  /**
-   * @description Text for a percentage value.
-   * @example {60} PH1
-   */
-  percentage: '{PH1}%',
+  percentDevices: '{PH1}% mobile, {PH2}% desktop',
   /**
    * @description Text block explaining how to simulate different mobile and desktop devices. The placeholder at the end will be a link with the text "simulate different devices" translated separately.
    * @example {simulate different devices} PH1
@@ -205,8 +204,9 @@ const UIStrings = {
   }`,
   /**
    * @description Label for a a range of dates that represents the period of time a set of field data is collected from.
+   * @example {Oct 1, 2024 - Nov 1, 2024} PH1
    */
-  collectionPeriod: 'Collection period:',
+  collectionPeriod: 'Collection period: {PH1}',
   /**
    * @description Text showing a range of dates meant to represent a period of time.
    * @example {Oct 1, 2024} PH1
@@ -246,9 +246,9 @@ const UIStrings = {
    */
   clearCurrentLog: 'Clear the current log',
   /**
-   * @description Title for an expandable section that contains more information about real user environments. This message is meant to prompt the user to understand the conditions experienced by real users.
+   * @description Title for a section that contains more information about real user environments. This message is meant to prompt the user to understand the conditions experienced by real users.
    */
-  considerRealUser: 'Consider real user environments',
+  realUserEnvironments: 'Real user environments',
   /**
    * @description Title for a page load phase that measures the time between when the page load starts and the time when the first byte of the initial document is downloaded.
    */
@@ -610,47 +610,44 @@ export class LiveMetricsView extends LegacyWrapper.LegacyWrapper.WrappableCompon
     return i18nString(UIStrings.tryUsingThrottling, {PH1: title});
   }
 
-  #getDeviceRec(): Common.UIString.LocalizedString|null {
+  #getDeviceRec(): string|null {
     // `form_factors` metric is only populated if CrUX data is fetched for all devices.
     const fractions = this.#cruxPageResult?.[`${this.#fieldPageScope}-ALL`]?.record.metrics.form_factors?.fractions;
     if (!fractions) {
       return null;
     }
 
-    if (fractions.desktop > 0.5) {
-      const percentage = i18nString(UIStrings.percentage, {PH1: Math.round(fractions.desktop * 100)});
-      return i18nString(UIStrings.mostUsersDesktop, {PH1: percentage});
-    }
-
-    if (fractions.phone > 0.5) {
-      const percentage = i18nString(UIStrings.percentage, {PH1: Math.round(fractions.phone * 100)});
-      return i18nString(UIStrings.mostUsersMobile, {PH1: percentage});
-    }
-
-    return null;
+    return i18nString(UIStrings.percentDevices, {
+      PH1: Math.round(fractions.phone * 100),
+      PH2: Math.round(fractions.desktop * 100),
+    });
   }
 
   #renderRecordingSettings(): LitHtml.LitTemplate {
-    const envRecs = [
-      this.#getDeviceRec(),
-      this.#getNetworkRec(),
-    ].filter(rec => rec !== null);
+    const fieldEnabled = CrUXManager.CrUXManager.instance().getConfigSetting().get().enabled;
 
     const deviceLinkEl = UI.XLink.XLink.create(
         'https://developer.chrome.com/docs/devtools/device-mode', i18nString(UIStrings.simulateDifferentDevices));
     const deviceMessage = i18n.i18n.getFormatLocalizedString(str_, UIStrings.useDeviceToolbar, {PH1: deviceLinkEl});
 
+    const deviceRecEl = document.createElement('span');
+    deviceRecEl.classList.add('environment-rec');
+    deviceRecEl.textContent = this.#getDeviceRec() || i18nString(UIStrings.notEnoughData);
+
+    const networkRecEl = document.createElement('span');
+    networkRecEl.classList.add('environment-rec');
+    networkRecEl.textContent = this.#getNetworkRec() || i18nString(UIStrings.notEnoughData);
+
     // clang-format off
     return html`
       <h3 class="card-title">${i18nString(UIStrings.environmentSettings)}</h3>
       <div class="device-toolbar-description">${deviceMessage}</div>
-      ${envRecs.length > 0 ? html`
-        <details class="environment-recs">
-          <summary>${i18nString(UIStrings.considerRealUser)}</summary>
-          <ul class="environment-recs-list">
-            ${envRecs.map(rec => html`<li>${rec}</li>`)}
-          </ul>
-        </details>
+      ${fieldEnabled ? html`
+        <div class="environment-recs-title">${i18nString(UIStrings.realUserEnvironments)}</div>
+        <ul class="environment-recs-list">
+          <li>${i18n.i18n.getFormatLocalizedString(str_, UIStrings.device, {PH1: deviceRecEl})}</li>
+          <li>${i18n.i18n.getFormatLocalizedString(str_, UIStrings.network, {PH1: networkRecEl})}</li>
+        </ul>
       ` : nothing}
       <div class="environment-option">
         <devtools-cpu-throttling-selector></devtools-cpu-throttling-selector>
@@ -837,10 +834,10 @@ export class LiveMetricsView extends LegacyWrapper.LegacyWrapper.WrappableCompon
     // clang-format on
   }
 
-  #renderCollectionPeriod(): LitHtml.LitTemplate {
+  #getCollectionPeriodRange(): string|null {
     const selectedResponse = this.#getSelectedFieldResponse();
     if (!selectedResponse) {
-      return LitHtml.nothing;
+      return null;
     }
 
     const {firstDate, lastDate} = selectedResponse.record.collectionPeriod;
@@ -864,18 +861,25 @@ export class LiveMetricsView extends LegacyWrapper.LegacyWrapper.WrappableCompon
       day: 'numeric',
     };
 
-    const dateEl = document.createElement('span');
-    dateEl.classList.add('collection-period-range');
-    dateEl.textContent = i18nString(UIStrings.dateRange, {
+    return i18nString(UIStrings.dateRange, {
       PH1: formattedFirstDate.toLocaleDateString(undefined, options),
       PH2: formattedLastDate.toLocaleDateString(undefined, options),
     });
+  }
+
+  #renderCollectionPeriod(): LitHtml.LitTemplate {
+    const range = this.#getCollectionPeriodRange();
+
+    const dateEl = document.createElement('span');
+    dateEl.classList.add('collection-period-range');
+    dateEl.textContent = range || i18nString(UIStrings.notEnoughData);
+
+    const message = i18n.i18n.getFormatLocalizedString(str_, UIStrings.collectionPeriod, {
+      PH1: dateEl,
+    });
 
     return html`
-      <div class="field-data-message">
-        ${i18nString(UIStrings.collectionPeriod)}
-        ${dateEl}
-      </div>
+      <div class="field-data-message">${message}</div>
     `;
   }
 

front_end/panels/timeline/components/liveMetricsView.css

@@ -301,20 +301,20 @@ x-link { /* stylelint-disable-line selector-type-no-unknown */
   width: 18px;
 }
 
-.environment-recs {
-  margin: 12px 0;
-}
-
-.environment-recs > summary {
+.environment-recs-title {
+  margin-top: 12px;
   font-weight: var(--ref-typeface-weight-medium);
-  margin-bottom: 4px;
 }
 
 .environment-recs-list {
   margin: 0;
   padding-left: 20px;
 }
 
+.environment-rec {
+  font-weight: var(--ref-typeface-weight-medium);
+}
+
 .link-to-log {
   padding: unset;
   background: unset;