From 7b9a97746cf9bf1ddc400ea4f732bf54638622dc Mon Sep 17 00:00:00 2001
From: Adriana Ixba <aixba@google.com>
Date: Thu, 27 Jun 2024 18:28:15 +0000
Subject: [PATCH] [RPP] Make sidebar insight clickable to toggle.

When clicked, the insight will expand to show its contents.

Bug:349700397
Change-Id: I758c279b0de7bd9ae2553a7d03aeed1d1af1221f
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5661588
Reviewed-by: Alina Varkki <alinavarkki@chromium.org>
Commit-Queue: Adriana Ixba <aixba@google.com>
---
 .../panels/timeline/components/Sidebar.ts     | 37 +++++++++++++---
 .../components/SidebarInsight.test.ts         | 43 ++++++++++++++++++-
 .../timeline/components/SidebarInsight.ts     | 13 +++++-
 .../panels/timeline/components/sidebar.css    |  2 +-
 .../timeline/components/sidebarInsight.css    | 14 +++++-
 5 files changed, 99 insertions(+), 10 deletions(-)

diff --git a/front_end/panels/timeline/components/Sidebar.ts b/front_end/panels/timeline/components/Sidebar.ts
index 722d197df60..b8d1871f06b 100644
--- a/front_end/panels/timeline/components/Sidebar.ts
+++ b/front_end/panels/timeline/components/Sidebar.ts
@@ -34,6 +34,14 @@ enum InsightsCategories {
   OTHER = 'Other',
 }
 
+export class ToggleSidebarInsights extends Event {
+  static readonly eventName = 'toggleinsightclick';
+
+  constructor() {
+    super(ToggleSidebarInsights.eventName, {bubbles: true, composed: true});
+  }
+}
+
 export class SidebarWidget extends UI.SplitWidget.SplitWidget {
   #sidebarExpanded: boolean = false;
   #sidebarUI = new SidebarUI();
@@ -81,6 +89,7 @@ export class SidebarUI extends HTMLElement {
   #activeTab: SidebarTabsName = SidebarTabsName.INSIGHTS;
   selectedCategory: InsightsCategories = InsightsCategories.ALL;
   #expanded: boolean = false;
+  #lcpPhasesExpanded: boolean = false;
 
   #traceParsedData?: TraceEngine.Handlers.Types.TraceParseData|null;
   #inpMetric: {
@@ -125,7 +134,7 @@ export class SidebarUI extends HTMLElement {
         {phase: 'Time to first byte', timing: ttfb, percent: `${(100 * ttfb / timing).toFixed(0)}%`},
         {phase: 'Resource load delay', timing: loadDelay, percent: `${(100 * loadDelay / timing).toFixed(0)}%`},
         {phase: 'Resource load duration', timing: loadTime, percent: `${(100 * loadTime / timing).toFixed(0)}%`},
-        {phase: 'Resource render delay', timing: renderDelay, percent: `${(100 * ttfb / timing).toFixed(0)}%`},
+        {phase: 'Resource render delay', timing: renderDelay, percent: `${(100 * renderDelay / timing).toFixed(0)}%`},
       ];
       return phaseData;
     }
@@ -133,7 +142,7 @@ export class SidebarUI extends HTMLElement {
     // If the lcp is text, we only have ttfb and render delay.
     const phaseData = [
       {phase: 'Time to first byte', timing: ttfb, percent: `${(100 * ttfb / timing).toFixed(0)}%`},
-      {phase: 'Resource render delay', timing: renderDelay, percent: `${(100 * ttfb / timing).toFixed(0)}%`},
+      {phase: 'Resource render delay', timing: renderDelay, percent: `${(100 * renderDelay / timing).toFixed(0)}%`},
     ];
     return phaseData;
   }
@@ -158,6 +167,8 @@ export class SidebarUI extends HTMLElement {
     }
     this.#insights = insights;
     this.#phaseData = this.getLCPInsightData();
+    // Reset toggled insights.
+    this.#lcpPhasesExpanded = false;
     void ComponentHelpers.ScheduledRender.scheduleRender(this, this.#renderBound);
   }
 
@@ -277,6 +288,12 @@ export class SidebarUI extends HTMLElement {
         this.#clsMetric.clsScoreClassification);
   }
 
+  #toggleLCPPhaseClick(): void {
+    this.#lcpPhasesExpanded = !this.#lcpPhasesExpanded;
+    this.dispatchEvent(new ToggleSidebarInsights());
+    void ComponentHelpers.ScheduledRender.scheduleRender(this, this.#renderBound);
+  }
+
   #renderInsightsForCategory(insightsCategory: InsightsCategories): LitHtml.TemplateResult {
     switch (insightsCategory) {
       case InsightsCategories.ALL:
@@ -286,12 +303,12 @@ export class SidebarUI extends HTMLElement {
             ${this.#renderLCPMetric()}
             ${this.#renderCLSMetric()}
           </div>
-          <div class="insights">${this.#renderLCPPhases()}</div>
+          <div class="insights" @click=${this.#toggleLCPPhaseClick}>${this.#renderLCPPhases()}</div>
         `;
       case InsightsCategories.LCP:
         return LitHtml.html`
           ${this.#renderLCPMetric()}
-          <div class="insights">${this.#renderLCPPhases()}</div>
+          <div class="insights" @click=${this.#toggleLCPPhaseClick}>${this.#renderLCPPhases()}</div>
         `;
       case InsightsCategories.CLS:
         return LitHtml.html`${this.#renderCLSMetric()}`;
@@ -307,10 +324,12 @@ export class SidebarUI extends HTMLElement {
     const showLCPPhases = this.#phaseData ? this.#phaseData.length > 0 : false;
 
     // clang-format off
-    return LitHtml.html`${
+    if (this.#lcpPhasesExpanded) {
+      return LitHtml.html`${
         showLCPPhases ? LitHtml.html`
         <${SidebarInsight.SidebarInsight.litTagName} .data=${{
             title: lcpTitle,
+            expanded: this.#lcpPhasesExpanded,
           } as SidebarInsight.InsightDetails}>
           <div slot="insight-description" class="insight-description">
             Each
@@ -328,6 +347,14 @@ export class SidebarUI extends HTMLElement {
             </dl>
           </div>
         </${SidebarInsight.SidebarInsight}>` : LitHtml.nothing}`;
+    }
+      return LitHtml.html`
+      <${SidebarInsight.SidebarInsight.litTagName} .data=${{
+            title: lcpTitle,
+            expanded: this.#lcpPhasesExpanded,
+          } as SidebarInsight.InsightDetails}>
+        </${SidebarInsight.SidebarInsight}>`;
+
     // clang-format on
   }
 
diff --git a/front_end/panels/timeline/components/SidebarInsight.test.ts b/front_end/panels/timeline/components/SidebarInsight.test.ts
index 6f9af94f4c3..f878c80b70a 100644
--- a/front_end/panels/timeline/components/SidebarInsight.test.ts
+++ b/front_end/panels/timeline/components/SidebarInsight.test.ts
@@ -14,7 +14,7 @@ describeWithEnvironment('SidebarInsight', () => {
 
   it('renders insight title', async () => {
     const component = new SidebarInsight();
-    component.data = {title: 'LCP by Phase'};
+    component.data = {title: 'LCP by Phase', expanded: true};
     renderElementIntoDOM(component);
 
     await coordinator.done();
@@ -24,4 +24,45 @@ describeWithEnvironment('SidebarInsight', () => {
     assert.isNotNull(titleElement);
     assert.deepEqual(titleElement.textContent, 'LCP by Phase');
   });
+
+  it('renders only insight title when not toggled', async () => {
+    const component = new SidebarInsight();
+    component.data = {title: 'LCP by Phase', expanded: false};
+    renderElementIntoDOM(component);
+
+    await coordinator.done();
+
+    assert.isNotNull(component.shadowRoot);
+    const titleElement = component.shadowRoot.querySelector<HTMLElement>('.insight-title');
+    assert.isNotNull(titleElement);
+    assert.deepEqual(titleElement.textContent, 'LCP by Phase');
+
+    // Should not contain the description and content slots.
+    const slotElements = component.shadowRoot.querySelectorAll<HTMLSlotElement>('slot');
+    assert.isEmpty(slotElements);
+  });
+
+  it('renders title, description and content when toggled', async () => {
+    const component = new SidebarInsight();
+    component.data = {title: 'LCP by Phase', expanded: true};
+    renderElementIntoDOM(component);
+
+    await coordinator.done();
+
+    assert.isNotNull(component.shadowRoot);
+    const titleElement = component.shadowRoot.querySelector<HTMLElement>('.insight-title');
+    assert.isNotNull(titleElement);
+    assert.deepEqual(titleElement.textContent, 'LCP by Phase');
+
+    const slotElements = component.shadowRoot.querySelectorAll<HTMLSlotElement>('slot');
+    assert.isNotEmpty(slotElements);
+
+    const descriptionSlot = slotElements[0];
+    assert.isNotNull(descriptionSlot);
+    assert.strictEqual(descriptionSlot.name, 'insight-description');
+
+    const contentSlot = slotElements[1];
+    assert.isNotNull(contentSlot);
+    assert.strictEqual(contentSlot.name, 'insight-content');
+  });
 });
diff --git a/front_end/panels/timeline/components/SidebarInsight.ts b/front_end/panels/timeline/components/SidebarInsight.ts
index 9f86952758a..a660f52adb0 100644
--- a/front_end/panels/timeline/components/SidebarInsight.ts
+++ b/front_end/panels/timeline/components/SidebarInsight.ts
@@ -9,6 +9,7 @@ import sidebarInsightStyles from './sidebarInsight.css.js';
 
 export interface InsightDetails {
   title: string;
+  expanded: boolean;
 }
 
 export class SidebarInsight extends HTMLElement {
@@ -16,9 +17,11 @@ export class SidebarInsight extends HTMLElement {
   readonly #shadow = this.attachShadow({mode: 'open'});
   readonly #boundRender = this.#render.bind(this);
   #insightTitle: string = '';
+  #expanded: boolean = false;
 
   set data(data: InsightDetails) {
     this.#insightTitle = data.title;
+    this.#expanded = data.expanded;
   }
 
   connectedCallback(): void {
@@ -27,12 +30,20 @@ export class SidebarInsight extends HTMLElement {
   }
 
   #render(): void {
-    const output = LitHtml.html`
+    let output: LitHtml.TemplateResult;
+    if (!this.#expanded) {
+      output = LitHtml.html`
+        <div class="insight closed">
+            <h3 class="insight-title">${this.#insightTitle}</h3>
+        </div>`;
+    } else {
+      output = LitHtml.html`
         <div class="insight">
             <h3 class="insight-title">${this.#insightTitle}</h3>
             <slot name="insight-description"></slot>
             <slot name="insight-content"></slot>
         </div>`;
+    }
     LitHtml.render(output, this.#shadow, {host: this});
   }
 }
diff --git a/front_end/panels/timeline/components/sidebar.css b/front_end/panels/timeline/components/sidebar.css
index cf620296da1..392a7c8539b 100644
--- a/front_end/panels/timeline/components/sidebar.css
+++ b/front_end/panels/timeline/components/sidebar.css
@@ -126,7 +126,7 @@ devtools-select-menu {
 }
 
 .insight-description {
-  border-bottom: 1px solid var(--color-input-outline);
+  border-bottom: 1px solid var(--sys-color-outline);
   padding-bottom: 10px;
 }
 
diff --git a/front_end/panels/timeline/components/sidebarInsight.css b/front_end/panels/timeline/components/sidebarInsight.css
index 201a8b5264b..2c6cb4024ec 100644
--- a/front_end/panels/timeline/components/sidebarInsight.css
+++ b/front_end/panels/timeline/components/sidebarInsight.css
@@ -9,13 +9,23 @@
   width: auto;
   height: auto;
   margin: 10px 0;
-  border: 1px solid var(--box-shadow-outline-color);
   padding: 10px;
   border-radius: 3px;
   overflow: hidden;
+  border: 1px solid var(--sys-color-outline);
+  background-color: var(--sys-color-base);
+
+  &.closed {
+    background-color: var(--sys-color-neutral-container);
+    border: none;
+  }
+
+  &.closed:hover {
+    background-color: var(--sys-color-state-disabled-container);
+  }
 }
 
 .insight-title {
-  color: var(--color-text-primary);
+  color: var(--sys-color-on-base);
   margin-block: 3px;
 }