[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 <[email protected]> Commit-Queue: Adriana Ixba <[email protected]>
ChromeDevTools · Jun 27, 2024 · 7b9a977 · 7b9a977
1 parent 6d0b33c
commit 7b9a977
Show file tree

Hide file tree

Showing 5 changed files with 99 additions and 10 deletions.
diff --git 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,15 +134,15 @@ 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;
     }
 
     // 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
@@ -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
@@ -9,16 +9,19 @@ import sidebarInsightStyles from './sidebarInsight.css.js';
 
 export interface InsightDetails {
   title: string;
+  expanded: boolean;
 }
 
 export class SidebarInsight extends HTMLElement {
   static readonly litTagName = LitHtml.literal`devtools-performance-sidebar-insight`;
   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
@@ -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
@@ -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;
 }