feat: Add 'Even X-Axis Spacing' toggle to chart settings

This change introduces a new toggle in the settings dialog of
explore-simple-sk, allowing users to space X-axis points
  evenly, regardless of their numerical value.

   - Adds an md-switch for 'Even X-Axis Spacing' to the settings dialog template.
   - Updates plot-google-chart-sk's 'useDiscreteAxis' property when the toggle changes.
   - Dispatches an 'even-x-axis-spacing-changed' event with the new value and graph_index.
   - Updates explore-multi-sk and report-page-sk to listen for this event and synchronize the state across all child
     explore-simple-sk instances.
   - Adds unit tests for the new toggle and event handling in explore-simple-sk, explore-multi-sk, and report-page-sk.
   - Updates mock factories in tests to support necessary methods like setUseDiscreteAxis and render.

Bug: 418808644
Change-Id: I9073bd5d4409ed288022e9b72dd921b8e256e8eb
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/1120909
Commit-Queue: Eduardo Yap <[email protected]>
Reviewed-by: Tony Seaward <[email protected]>

Author: Eduardo Yap

perf/modules/explore-multi-sk/explore-multi-sk.ts

@@ -110,6 +110,9 @@ export class State {
   dayRange: number = -1;
 
   dateAxis: boolean = false;
+
+  // boolean indicating if the chart should space x-axis points evenly, treating them as categories.
+  evenXAxisSpacing: boolean = false;
 }
 
 export class ExploreMultiSk extends ElementSk {
@@ -733,7 +736,8 @@ export class ExploreMultiSk extends ElementSk {
         id="graphContainer"
         @x-axis-toggled=${ele.syncXAxisLabel}
         @range-changing-in-multi=${ele.syncRange}
-        @selection-changing-in-multi=${ele.syncChartSelection}></div>
+        @selection-changing-in-multi=${ele.syncChartSelection}
+        @even-x-axis-spacing-changed=${ele.syncEvenXAxisSpacing}></div>
       <pagination-sk
         offset=${ele.state.pageOffset}
         page_size=${ele.state.pageSize}
@@ -1339,6 +1343,25 @@ export class ExploreMultiSk extends ElementSk {
     });
   }
 
+  private syncEvenXAxisSpacing(e: CustomEvent): void {
+    const newValue = e.detail.value;
+    if (this.state.evenXAxisSpacing === newValue) {
+      return;
+    }
+    this.state.evenXAxisSpacing = newValue;
+    this.exploreElements.forEach((graph) => {
+      // Skip graph that sent the event.
+      if (graph.state.graph_index !== e.detail.graph_index) {
+        graph.state.evenXAxisSpacing = newValue;
+        graph.setUseDiscreteAxis(newValue);
+        graph.render();
+      }
+    });
+    if (this.stateHasChanged) {
+      this.stateHasChanged();
+    }
+  }
+
   private addStateToExplore(
     explore: ExploreSimpleSk,
     graphConfig: GraphConfig,
@@ -1375,6 +1398,7 @@ export class ExploreMultiSk extends ElementSk {
       hide_paramset: true,
       graph_index: index,
       doNotQueryData: doNotQueryData,
+      evenXAxisSpacing: this.state.evenXAxisSpacing,
     };
     explore.state = newState;
   }

perf/modules/explore-multi-sk/explore-multi-sk_test.ts

@@ -570,6 +570,47 @@ describe('ExploreMultiSk', () => {
     });
   });
 
+  describe('Even X-Axis Spacing Sync', () => {
+    beforeEach(async () => {
+      await setupElement();
+      // Add a couple of graphs for testing sync
+      element['addEmptyGraph']();
+      element['addEmptyGraph']();
+      element['exploreElements'][0].state.graph_index = 0;
+      element['exploreElements'][1].state.graph_index = 1;
+      // Add to DOM for event propagation
+      element['graphDiv']!.appendChild(element['exploreElements'][0]);
+      element['graphDiv']!.appendChild(element['exploreElements'][1]);
+    });
+
+    it('syncs enableDiscrete state to other graphs', async () => {
+      const graph1 = element['exploreElements'][0];
+      const graph2 = element['exploreElements'][1];
+
+      const spy1 = sinon.spy(graph1, 'setUseDiscreteAxis');
+      const spy2 = sinon.spy(graph2, 'setUseDiscreteAxis');
+
+      // Simulate event from graph1
+      const event = new CustomEvent('even-x-axis-spacing-changed', {
+        detail: { value: true, graph_index: 0 },
+        bubbles: true,
+      });
+      graph1.dispatchEvent(event);
+
+      assert.isTrue(element.state.evenXAxisSpacing, 'MultiSk state should be updated');
+      assert.isTrue(spy1.notCalled, 'Source graph setUseDiscreteAxis should not be called by sync');
+      assert.isTrue(spy2.calledOnceWith(true), 'Target graph setUseDiscreteAxis should be called');
+      assert.isTrue(graph2.state.evenXAxisSpacing, 'Target graph state should be updated');
+    });
+
+    it('initializes new graphs with the current enableDiscrete state', () => {
+      element.state.evenXAxisSpacing = true;
+      const newGraph = element['addEmptyGraph']()!;
+      element['addStateToExplore'](newGraph, new GraphConfig(), false);
+      assert.isTrue(newGraph.state.evenXAxisSpacing);
+    });
+  });
+
   describe('Pagination', () => {
     beforeEach(async () => {
       await setupElement();
@@ -659,6 +700,7 @@ describe('ExploreMultiSk', () => {
         horizontal_zoom: false,
         graph_index: 0,
         doNotQueryData: false,
+        evenXAxisSpacing: false,
       };
 
       element['addStateToExplore'](simpleSk, new GraphConfig(), false);

perf/modules/explore-simple-sk/explore-simple-sk.ts

@@ -364,6 +364,9 @@ export class State {
 
   // boolean indicating if the element should disable querying of data from backend.
   doNotQueryData: boolean = false;
+
+  // boolean indicating if the chart should space x-axis points evenly, treating them as categories.
+  evenXAxisSpacing: boolean = false;
 }
 
 // TODO(jcgregorio) Move to a 'key' module.
@@ -747,6 +750,17 @@ export class ExploreSimpleSk extends ElementSk implements KeyboardShortcutHandle
                 Switch Zoom Direction
               </label>
             </li>
+            <li>
+              <label>
+                <md-switch
+                  form="form"
+                  id="even-x-axis-spacing-switch"
+                  ?selected="${ele._state.evenXAxisSpacing}"
+                  @change=${(e: InputEvent) =>
+                    ele.switchEvenXAxisSpacing(e.target as MdSwitch)}></md-switch>
+                Even X-Axis Spacing
+              </label>
+            </li>
           </ul>
         </div>
       </md-dialog>
@@ -758,6 +772,7 @@ export class ExploreSimpleSk extends ElementSk implements KeyboardShortcutHandle
           .domain=${ele._state.domain}
           ${ref(ele.googleChartPlot)}
           .highlightAnomalies=${ele._state.highlight_anomalies}
+          .useDiscreteAxis=${ele._state.evenXAxisSpacing}
           @plot-data-select=${ele.onChartSelect}
           @plot-data-mouseover=${ele.onChartOver}
           @plot-data-mousedown=${ele.onChartMouseDown}
@@ -1578,6 +1593,26 @@ export class ExploreSimpleSk extends ElementSk implements KeyboardShortcutHandle
     this.dispatchEvent(new CustomEvent('switch-zoom', { detail: detail, bubbles: true }));
   }
 
+  private switchEvenXAxisSpacing(target: MdSwitch | null) {
+    if (!target) return;
+    this._state.evenXAxisSpacing = target.selected;
+    this.setUseDiscreteAxis(this._state.evenXAxisSpacing);
+    this.render();
+    this.dispatchEvent(
+      new CustomEvent('even-x-axis-spacing-changed', {
+        detail: { value: this._state.evenXAxisSpacing, graph_index: this._state.graph_index },
+        bubbles: true,
+      })
+    );
+    this._stateHasChanged();
+  }
+
+  public setUseDiscreteAxis(value: boolean) {
+    if (this.googleChartPlot.value) {
+      this.googleChartPlot.value.useDiscreteAxis = value;
+    }
+  }
+
   private closeQueryDialog(): void {
     this.queryDialog!.close();
     this._dialogOn = false;

perf/modules/explore-simple-sk/explore-simple-sk_test.ts

@@ -1192,3 +1192,82 @@ describe('Keyboard Shortcuts', () => {
     assert.isTrue(tooltip.openExistingBug.calledOnce, 'e key should trigger openExistingBug');
   });
 });
+
+describe('Even X-Axis Spacing toggle', () => {
+  let explore: ExploreSimpleSk;
+  let switchEl: MdSwitch;
+  let eventSpy: sinon.SinonSpy;
+  let setUseDiscreteAxisSpy: sinon.SinonSpy;
+
+  beforeEach(async () => {
+    fetchMock.get(/.*\/_\/initpage\/.*/, {
+      dataframe: { paramset: {} },
+    });
+    fetchMock.get('/_/login/status', {
+      email: '[email protected]',
+      roles: ['editor'],
+    });
+    setUseDiscreteAxisSpy = sinon.spy(ExploreSimpleSk.prototype, 'setUseDiscreteAxis');
+    explore = setUpElementUnderTest<ExploreSimpleSk>('explore-simple-sk')();
+    await waitForRender(explore);
+    const settingsDialog = explore.querySelector('#settings-dialog') as MdDialog;
+    switchEl = settingsDialog.querySelector('#even-x-axis-spacing-switch') as MdSwitch;
+    eventSpy = sinon.spy();
+    explore.addEventListener('even-x-axis-spacing-changed', eventSpy);
+  });
+
+  afterEach(() => {
+    setUseDiscreteAxisSpy.restore();
+  });
+
+  it('should have the switch element', () => {
+    assert.exists(switchEl);
+  });
+
+  it('should be unchecked by default', () => {
+    assert.isFalse(switchEl.selected);
+    assert.isFalse(explore.state.evenXAxisSpacing);
+  });
+
+  it('should update state and fire event when toggled on', async () => {
+    switchEl.selected = true;
+    switchEl.dispatchEvent(new Event('change'));
+
+    await waitForRender(explore);
+
+    assert.isTrue(explore.state.evenXAxisSpacing);
+    assert.isTrue(setUseDiscreteAxisSpy.calledOnceWith(true));
+    assert.isTrue(eventSpy.calledOnce);
+
+    const event = eventSpy.firstCall.args[0];
+
+    assert.equal(event.type, 'even-x-axis-spacing-changed');
+    assert.deepEqual(event.detail, { value: true, graph_index: 0 });
+  });
+
+  it('should update state and fire event when toggled off', async () => {
+    // Turn it on first
+
+    switchEl.selected = true;
+    switchEl.dispatchEvent(new Event('change'));
+
+    await waitForRender(explore);
+
+    eventSpy.resetHistory();
+
+    // Turn it off
+    switchEl.selected = false;
+    switchEl.dispatchEvent(new Event('change'));
+
+    await waitForRender(explore);
+
+    assert.isFalse(explore.state.evenXAxisSpacing);
+    assert.isTrue(setUseDiscreteAxisSpy.lastCall.calledWith(false));
+    assert.isTrue(eventSpy.calledOnce);
+
+    const event = eventSpy.firstCall.args[0];
+
+    assert.equal(event.type, 'even-x-axis-spacing-changed');
+    assert.deepEqual(event.detail, { value: false, graph_index: 0 });
+  });
+});

perf/modules/plot-google-chart-sk/plot-google-chart-sk.ts

@@ -280,12 +280,6 @@ export class PlotGoogleChartSk extends LitElement {
   constructor() {
     super();
     this.addEventListeners();
-    // TODO(eduardoyap): Adding this for test purposes. Should be added as a toggle once
-    // the feature is verified.
-    const urlParams = new URLSearchParams(window.location.search);
-    if (urlParams.get('enable_discrete') === 'true') {
-      this.useDiscreteAxis = true;
-    }
   }
 
   connectedCallback(): void {

perf/modules/report-page-sk/report-page-sk.ts

@@ -159,6 +159,7 @@ export class ReportPageSk extends ElementSk {
       @x-axis-toggled=${ele.syncXAxisLabel}
       @range-changing-in-multi=${ele.syncExtendRangeOnSummaryBar}
       @selection-changing-in-multi=${ele.syncChartSelection}
+      @even-x-axis-spacing-changed=${ele.syncEvenXAxisSpacing}
       @open-anomaly-chart=${(e: CustomEvent<Anomaly>) =>
         ele.anomaliesTable!.openAnomalyChartListener(e)}></div>
   `;
@@ -479,6 +480,21 @@ export class ReportPageSk extends ElementSk {
     });
   }
 
+  // This ensures that all charts use the same x-axis type (continuous or discrete).
+  private syncEvenXAxisSpacing(e: CustomEvent): void {
+    const newValue = e.detail.value;
+    const graphs = this.graphDiv!.querySelectorAll('explore-simple-sk');
+    graphs.forEach((graphNode) => {
+      const graph = graphNode as ExploreSimpleSk;
+      // Skip graph that sent the event.
+      if (graph.state.graph_index !== e.detail.graph_index) {
+        graph.state.evenXAxisSpacing = newValue;
+        graph.setUseDiscreteAxis(newValue);
+        graph.render();
+      }
+    });
+  }
+
   // findRequestedAnomalies returns a list of requested anomaly objects .
   // This is for only loading selected untriaged anomaly graphs in the first place.
   findRequestedAnomalies(): Anomaly[] {