RPP: add aria-labels for estimated savings

This CL adds information about estimated savings to the aria-label for
the insight for screen readers to hear when they tab onto the insight.

I also took the chance to update the "Ask AI" label as "Ask AI" to a
screen reader is unintuitive given you might not have remembered which
insight you have expanded.

Fixed: 408299691
Change-Id: I362eebd4b12c8accbdaf2b7d940f2fc57779106f
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6433947
Reviewed-by: Alina Varkki <alinavarkki@chromium.org>
Commit-Queue: Alina Varkki <alinavarkki@chromium.org>
Auto-Submit: Jack Franklin <jacktfranklin@chromium.org>

Author: jackfranklin

front_end/panels/timeline/components/insights/BaseInsightComponent.test.ts

@@ -107,6 +107,110 @@ describeWithEnvironment('BaseInsightComponent', () => {
     });
   });
 
+  describe('estimated savings output', () => {
+    let testComponentIndex = 0;  // used for defining the custom element and making it unique
+    function makeTestComponent(opts: {byteSavings?: number, timeSavings?: number}) {
+      class TestInsight extends BaseInsightComponent<Trace.Insights.Types.InsightModel> {
+        override internalName = 'test-insight';
+        override createOverlays(): TimelineOverlay[] {
+          return [];
+        }
+
+        override getEstimatedSavingsTime(): Trace.Types.Timing.Milli|null {
+          return opts.timeSavings ? Trace.Types.Timing.Milli(opts.timeSavings) : null;
+        }
+
+        override getEstimatedSavingsBytes(): number|null {
+          return opts.byteSavings ?? null;
+        }
+
+        override renderContent(): Lit.LitTemplate {
+          return html`<div>test content</div>`;
+        }
+      }
+      customElements.define(`test-insight-est-savings-${testComponentIndex++}`, TestInsight);
+      return new TestInsight();
+    }
+
+    it('outputs the correct estimated savings for both bytes and time', async () => {
+      const component = makeTestComponent({byteSavings: 5_000, timeSavings: 50});
+      component.model = {
+        insightKey: 'LCPPhases',
+        strings: {},
+        title: 'LCP by Phase' as Common.UIString.LocalizedString,
+        description: 'some description' as Common.UIString.LocalizedString,
+        category: Trace.Insights.Types.InsightCategory.ALL,
+        state: 'fail',
+        frameId: '123',
+      };
+      renderElementIntoDOM(component);
+
+      await RenderCoordinator.done();
+      const estSavings = component.shadowRoot?.querySelector<HTMLElement>('slot[name=insight-savings]');
+      assert.isOk(estSavings);
+      assert.strictEqual(estSavings.innerText, 'Est savings: 50 ms & 5.0 kB');
+    });
+
+    it('outputs the correct estimated savings for bytes only', async () => {
+      const component = makeTestComponent({byteSavings: 5_000});
+      component.model = {
+        insightKey: 'LCPPhases',
+        strings: {},
+        title: 'LCP by Phase' as Common.UIString.LocalizedString,
+        description: 'some description' as Common.UIString.LocalizedString,
+        category: Trace.Insights.Types.InsightCategory.ALL,
+        state: 'fail',
+        frameId: '123',
+      };
+      renderElementIntoDOM(component);
+
+      await RenderCoordinator.done();
+      const estSavings = component.shadowRoot?.querySelector<HTMLElement>('slot[name=insight-savings]');
+      assert.isOk(estSavings);
+      assert.strictEqual(estSavings.innerText, 'Est savings: 5.0 kB');
+    });
+
+    it('outputs the correct estimated savings for time only', async () => {
+      const component = makeTestComponent({timeSavings: 50});
+      component.model = {
+        insightKey: 'LCPPhases',
+        strings: {},
+        title: 'LCP by Phase' as Common.UIString.LocalizedString,
+        description: 'some description' as Common.UIString.LocalizedString,
+        category: Trace.Insights.Types.InsightCategory.ALL,
+        state: 'fail',
+        frameId: '123',
+      };
+      renderElementIntoDOM(component);
+
+      await RenderCoordinator.done();
+      const estSavings = component.shadowRoot?.querySelector<HTMLElement>('slot[name=insight-savings]');
+      assert.isOk(estSavings);
+      assert.strictEqual(estSavings.innerText, 'Est savings: 50 ms');
+    });
+
+    it('includes the output in the insight aria label', async () => {
+      const component = makeTestComponent({byteSavings: 5_000, timeSavings: 50});
+      component.model = {
+        insightKey: 'LCPPhases',
+        strings: {},
+        title: 'LCP by Phase' as Common.UIString.LocalizedString,
+        description: 'some description' as Common.UIString.LocalizedString,
+        category: Trace.Insights.Types.InsightCategory.ALL,
+        state: 'fail',
+        frameId: '123',
+      };
+      renderElementIntoDOM(component);
+
+      await RenderCoordinator.done();
+      const label = component.shadowRoot?.querySelector('header')?.getAttribute('aria-label');
+      assert.isOk(label);
+
+      assert.strictEqual(
+          label, 'View details for LCP by Phase insight. Estimated savings for this insight: 50 ms and 5.0 kB');
+    });
+  });
+
   describe('Ask AI Insights', () => {
     const FAKE_PARSED_TRACE = {} as unknown as Trace.Handlers.Types.ParsedTrace;
     const FAKE_LCP_MODEL = {
@@ -145,6 +249,20 @@ describeWithEnvironment('BaseInsightComponent', () => {
       assert.isOk(button);
     });
 
+    it('adds a descriptive aria label to the button', async () => {
+      updateHostConfig({
+        devToolsAiAssistancePerformanceAgent: {
+          enabled: true,
+          insightsEnabled: true,
+        }
+      });
+      const component = await renderComponent({insightHasAISupport: true});
+      assert.isOk(component.shadowRoot);
+      const button = component.shadowRoot.querySelector('devtools-button[data-insights-ask-ai]');
+      assert.isOk(button);
+      assert.strictEqual(button.getAttribute('aria-label'), 'Ask AI about LCP by Phase insight');
+    });
+
     it('does not render the "Ask AI" button if disabled by enterprise policy', async () => {
       updateHostConfig({
         devToolsAiAssistancePerformanceAgent: {

front_end/panels/timeline/components/insights/BaseInsightComponent.ts

@@ -43,11 +43,23 @@ const UIStrings = {
    * @example {112 kB} PH2
    */
   estimatedSavingsTimingAndBytes: 'Est savings: {PH1} & {PH2}',
+  /**
+   * @description Text to tell the user the estimated time or size savings for this insight that is used for screen readers.
+   * @example {401 ms} PH1
+   * @example {112 kB} PH1
+   */
+  estimatedSavingsAria: 'Estimated savings for this insight: {PH1}',
+  /**
+   * @description Text to tell the user the estimated time and size savings for this insight that is used for screen readers.
+   * @example {401 ms} PH1
+   * @example {112 kB} PH2
+   */
+  estimatedSavingsTimingAndBytesAria: 'Estimated savings for this insight: {PH1} and {PH2}',
   /**
    * @description Used for screen-readers as a label on the button to expand an insight to view details
    * @example {LCP by phase} PH1
    */
-  viewDetails: 'View details for {PH1}',
+  viewDetails: 'View details for {PH1} insight.',
 } as const;
 
 const str_ = i18n.i18n.registerUIStrings('panels/timeline/components/insights/BaseInsightComponent.ts', UIStrings);
@@ -256,7 +268,7 @@ export abstract class BaseInsightComponent<T extends InsightModel> extends HTMLE
     return null;
   }
 
-  #getEstimatedSavingsString(): string|null {
+  #getEstimatedSavingsTextParts(): {bytesString?: string, timeString?: string} {
     const savingsTime = this.getEstimatedSavingsTime();
     const savingsBytes = this.getEstimatedSavingsBytes();
 
@@ -267,6 +279,37 @@ export abstract class BaseInsightComponent<T extends InsightModel> extends HTMLE
     if (savingsBytes) {
       bytesString = i18n.ByteUtilities.bytesToString(savingsBytes);
     }
+    return {
+      timeString,
+      bytesString,
+    };
+  }
+
+  #getEstimatedSavingsAriaLabel(): string|null {
+    const {bytesString, timeString} = this.#getEstimatedSavingsTextParts();
+
+    if (timeString && bytesString) {
+      return i18nString(UIStrings.estimatedSavingsTimingAndBytesAria, {
+        PH1: timeString,
+        PH2: bytesString,
+      });
+    }
+    if (timeString) {
+      return i18nString(UIStrings.estimatedSavingsAria, {
+        PH1: timeString,
+      });
+    }
+    if (bytesString) {
+      return i18nString(UIStrings.estimatedSavingsAria, {
+        PH1: bytesString,
+      });
+    }
+
+    return null;
+  }
+
+  #getEstimatedSavingsString(): string|null {
+    const {bytesString, timeString} = this.#getEstimatedSavingsTextParts();
 
     if (timeString && bytesString) {
       return i18nString(UIStrings.estimatedSavingsTimingAndBytes, {
@@ -335,6 +378,8 @@ export abstract class BaseInsightComponent<T extends InsightModel> extends HTMLE
     if (!this.#selected) {
       return Lit.nothing;
     }
+
+    const ariaLabel = `Ask AI about ${insightModel.title} insight`;
     // Only render the insight body content if it is selected.
     // To avoid re-rendering triggered from elsewhere.
     const content = this.renderContent();
@@ -351,6 +396,7 @@ export abstract class BaseInsightComponent<T extends InsightModel> extends HTMLE
               data-insights-ask-ai
               jslog=${VisualLogging.action(`timeline.insight-ask-ai.${this.internalName}`).track({click: true})}
               @click=${this.#askAIButtonClick}
+              aria-label=${ariaLabel}
             >Ask AI</devtools-button>
           </div>
         `: Lit.nothing}
@@ -369,6 +415,13 @@ export abstract class BaseInsightComponent<T extends InsightModel> extends HTMLE
       closed: !this.#selected,
     });
     const estimatedSavingsString = this.#getEstimatedSavingsString();
+    const estimatedSavingsAriaLabel = this.#getEstimatedSavingsAriaLabel();
+
+    let ariaLabel = `${i18nString(UIStrings.viewDetails, {PH1: this.#model.title})}`;
+    if (estimatedSavingsAriaLabel) {
+      // space prefix is deliberate to add a gap after the view details text
+      ariaLabel += ` ${estimatedSavingsAriaLabel}`;
+    }
 
     // clang-format off
     const output = html`
@@ -379,7 +432,7 @@ export abstract class BaseInsightComponent<T extends InsightModel> extends HTMLE
           tabIndex="0"
           role="button"
           aria-expanded=${this.#selected}
-          aria-label=${i18nString(UIStrings.viewDetails, {PH1: this.#model.title})}
+          aria-label=${ariaLabel}
         >
           ${this.#renderHoverIcon(this.#selected)}
           <h3 class="insight-title">${this.#model?.title}</h3>