You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jl...@apache.org on 2026/09/01 20:23:24 UTC

(superset) branch master updated: fix(echarts): prevent x-axis time labels from overlapping (#43669)

This is an automated email from the ASF dual-hosted git repository.

sadpandajoe pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new ba50bdff237 fix(echarts): prevent x-axis time labels from overlapping (#43669)
ba50bdff237 is described below

commit ba50bdff237e28dfd18f0792ffaf60d0c8ca9cae
Author: Joe Li <jo...@preset.io>
AuthorDate: Tue Sep 1 13:23:17 2026 -0700

    fix(echarts): prevent x-axis time labels from overlapping (#43669)
    
    Co-authored-by: Claude Sonnet 5 <no...@anthropic.com>
---
 .../src/MixedTimeseries/transformProps.ts          |  50 +++----
 .../src/Timeseries/transformProps.ts               |  56 +++-----
 .../plugins/plugin-chart-echarts/src/constants.ts  |   6 +
 .../plugin-chart-echarts/src/utils/formatters.ts   | 150 +++++++++++++++++++++
 .../test/MixedTimeseries/transformProps.test.ts    |  52 +++++++
 .../test/Timeseries/transformProps.test.ts         |  38 ++++++
 .../test/Timeseries/transformers.test.ts           |  49 +++++++
 7 files changed, 333 insertions(+), 68 deletions(-)

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
index 64cdc7b287a..1855b19a202 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
@@ -103,7 +103,9 @@ import {
 import { TIMEGRAIN_TO_TIMESTAMP, TIMESERIES_CONSTANTS } from '../constants';
 import { getDefaultTooltip } from '../utils/tooltip';
 import {
+  createSpacedXAxisFormatter,
   getTooltipTimeFormatter,
+  getXAxisDomain,
   getXAxisFormatter,
   getYAxisFormatter,
 } from '../utils/formatters';
@@ -664,44 +666,26 @@ export default function transformProps(
       ? getXAxisFormatter(xAxisTimeFormat, resolvedTimeGrain)
       : String;
 
+  // hideOverlap must stay off so the forced boundary label from showMaxLabel
+  // is never suppressed (#39899). The formatter itself dedupes consecutive
+  // identical labels and thins out labels that would otherwise visually
+  // collide, since hideOverlap can no longer do that for us.
   const showMaxLabel =
     xAxisType === AxisType.Time &&
     xAxisLabelRotation === 0 &&
     !!resolvedTimeGrain;
   const deduplicatedFormatter = showMaxLabel
-    ? (() => {
-        let lastLabel: string | undefined;
-        let lastValue: number | undefined;
-        const wrapper = (value: number | string) => {
-          // ECharts formats the labels in repeated ascending passes. Reset the
-          // dedup state when the sequence restarts so a forced boundary label
-          // (e.g. the min date) isn't blanked by the previous pass's last label
-          // when both format identically (e.g. a May-to-May range).
-          if (
-            typeof value === 'number' &&
-            lastValue !== undefined &&
-            value <= lastValue
-          ) {
-            lastLabel = undefined;
-          }
-          if (typeof value === 'number') {
-            lastValue = value;
-          }
-          const label =
-            typeof xAxisFormatter === 'function'
-              ? (xAxisFormatter as Function)(value)
-              : String(value);
-          if (label === lastLabel) {
-            return '';
-          }
-          lastLabel = label;
-          return label;
-        };
-        if (typeof xAxisFormatter === 'function' && 'id' in xAxisFormatter) {
-          (wrapper as any).id = (xAxisFormatter as any).id;
-        }
-        return wrapper;
-      })()
+    ? createSpacedXAxisFormatter(
+        xAxisFormatter,
+        ...getXAxisDomain(
+          [
+            rebasedDataA as Record<string, unknown>[],
+            rebasedDataB as Record<string, unknown>[],
+          ],
+          xAxisLabel,
+        ),
+        Math.max(width - 2 * TIMESERIES_CONSTANTS.gridOffsetLeft, 0),
+      )
     : xAxisFormatter;
 
   const yAxisTitleMarginPx = convertInteger(yAxisTitleMargin);
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
index 4882dfff5e1..169a7563128 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@@ -126,8 +126,11 @@ import {
 } from '../constants';
 import { getDefaultTooltip } from '../utils/tooltip';
 import {
+  createDedupXAxisFormatter,
+  createSpacedXAxisFormatter,
   getPercentFormatter,
   getTooltipTimeFormatter,
+  getXAxisDomain,
   getXAxisFormatter,
   getYAxisFormatter,
 } from '../utils/formatters';
@@ -1213,46 +1216,29 @@ export default function transformProps(
 
   // When showMaxLabel is true, ECharts may render a label at the axis
   // boundary that formats identically to the last data-point tick (e.g.
-  // "2005" appears twice with Year grain). Wrap the formatter to suppress
-  // consecutive duplicate labels.
+  // "2005" appears twice with Year grain), and hideOverlap must stay off so
+  // that forced boundary label is never suppressed (#39899). Wrap the
+  // formatter to suppress consecutive duplicate labels and to thin out
+  // labels that would otherwise visually collide, since hideOverlap can no
+  // longer do that for us. The spacing estimate assumes the axis runs along
+  // the bottom of the chart (pixel width, character width); a horizontal
+  // orientation chart puts the time axis on the side instead, so it falls
+  // back to dedup-only there.
   const showMaxLabel =
     xAxisType === AxisType.Time &&
     xAxisLabelRotation === 0 &&
     !!resolvedTimeGrain;
   const deduplicatedFormatter = showMaxLabel
-    ? (() => {
-        let lastLabel: string | undefined;
-        let lastValue: number | undefined;
-        const wrapper = (value: number | string) => {
-          // ECharts formats the labels in repeated ascending passes. Reset the
-          // dedup state when the sequence restarts so a forced boundary label
-          // (e.g. the min date) isn't blanked by the previous pass's last label
-          // when both format identically (e.g. a May-to-May range).
-          if (
-            typeof value === 'number' &&
-            lastValue !== undefined &&
-            value <= lastValue
-          ) {
-            lastLabel = undefined;
-          }
-          if (typeof value === 'number') {
-            lastValue = value;
-          }
-          const label =
-            typeof xAxisFormatter === 'function'
-              ? (xAxisFormatter as Function)(value)
-              : String(value);
-          if (label === lastLabel) {
-            return '';
-          }
-          lastLabel = label;
-          return label;
-        };
-        if (typeof xAxisFormatter === 'function' && 'id' in xAxisFormatter) {
-          (wrapper as any).id = (xAxisFormatter as any).id;
-        }
-        return wrapper;
-      })()
+    ? isHorizontal
+      ? createDedupXAxisFormatter(xAxisFormatter)
+      : createSpacedXAxisFormatter(
+          xAxisFormatter,
+          ...getXAxisDomain(
+            [rebasedData as Record<string, unknown>[]],
+            xAxisLabel,
+          ),
+          Math.max(width - 2 * TIMESERIES_CONSTANTS.gridOffsetLeft, 0),
+        )
     : xAxisFormatter;
 
   const temporalTickValues = resolveTemporalTickValues(
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/constants.ts b/superset-frontend/plugins/plugin-chart-echarts/src/constants.ts
index 2b94967ae00..7454da43b92 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/constants.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/constants.ts
@@ -52,6 +52,12 @@ export const TIMESERIES_CONSTANTS = {
   microChartHeight: 60,
   // One y-axis tick per this many pixels of chart height
   yAxisPixelsPerTick: 80,
+  // Rough average glyph width (px) used to estimate whether adjacent x-axis
+  // time labels would visually collide, since the real rendered width isn't
+  // known until ECharts lays out the axis.
+  xAxisLabelCharWidthPx: 7,
+  // Minimum gap (px) to keep between adjacent x-axis time labels.
+  xAxisLabelMinGapPx: 8,
 };
 
 export enum OpacityEnum {
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/formatters.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/formatters.ts
index 485bc2407ac..5fe4dd19560 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/formatters.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/formatters.ts
@@ -24,6 +24,7 @@ import {
   getTimeFormatter,
   isSavedMetric,
   NumberFormats,
+  NumberFormatter,
   QueryFormMetric,
   SMART_DATE_DETAILED_ID,
   SMART_DATE_ID,
@@ -32,6 +33,7 @@ import {
   TimeGranularity,
   ValueFormatter,
 } from '@superset-ui/core';
+import { TIMESERIES_CONSTANTS } from '../constants';
 
 export const getSmartDateDetailedFormatter = () =>
   getTimeFormatter(SMART_DATE_DETAILED_ID);
@@ -213,3 +215,151 @@ export function getXAxisFormatter(
   }
   return String;
 }
+
+type XAxisFormatterFn =
+  | TimeFormatter
+  | NumberFormatter
+  | StringConstructor
+  | ((value: number | string) => string);
+
+/**
+ * Wraps an x-axis time formatter so that consecutive ticks that format to
+ * identical text are blanked (e.g. the boundary label forced by
+ * showMaxLabel duplicating the last real tick).
+ *
+ * Use this instead of createSpacedXAxisFormatter when the axis geometry
+ * doesn't match the spacing model's horizontal-plot assumptions, e.g. a
+ * horizontal orientation chart, where the time axis runs vertically along
+ * the side of the chart rather than along the bottom.
+ */
+export function createDedupXAxisFormatter(
+  xAxisFormatter: XAxisFormatterFn | undefined,
+): (value: number | string) => string {
+  let lastLabel: string | undefined;
+  let lastValue: number | undefined;
+  const wrapper = (value: number | string) => {
+    // ECharts formats the labels in repeated ascending passes. Reset the
+    // dedup state when the sequence restarts so a forced boundary label
+    // (e.g. the min date) isn't blanked by the previous pass's last label
+    // when both format identically (e.g. a May-to-May range).
+    if (
+      typeof value === 'number' &&
+      lastValue !== undefined &&
+      value <= lastValue
+    ) {
+      lastLabel = undefined;
+    }
+    if (typeof value === 'number') {
+      lastValue = value;
+    }
+    const label =
+      typeof xAxisFormatter === 'function'
+        ? (xAxisFormatter as Function)(value)
+        : String(value);
+    if (label === lastLabel) {
+      return '';
+    }
+    lastLabel = label;
+    return label;
+  };
+  if (typeof xAxisFormatter === 'function' && 'id' in xAxisFormatter) {
+    (wrapper as { id?: unknown }).id = (xAxisFormatter as { id?: unknown }).id;
+  }
+  return wrapper;
+}
+
+/**
+ * Wraps an x-axis time formatter so that:
+ * - consecutive ticks that format to identical text are blanked (e.g. the
+ *   boundary label forced by showMaxLabel duplicating the last real tick).
+ * - ticks that would render close enough to visually collide with the
+ *   previously shown label are blanked, since disabling ECharts'
+ *   `hideOverlap` (required to keep the forced boundary label visible, see
+ *   #39899) also disables its native overlap suppression for every other
+ *   label on the axis.
+ *
+ * The forced axis boundary labels (domainMin/domainMax) are never blanked by
+ * the spacing check so they stay visible regardless of density.
+ */
+export function createSpacedXAxisFormatter(
+  xAxisFormatter: XAxisFormatterFn | undefined,
+  domainMin: number | undefined,
+  domainMax: number | undefined,
+  plotWidthPx: number,
+): (value: number | string) => string {
+  const pixelsPerMs =
+    domainMin !== undefined && domainMax !== undefined && domainMax > domainMin
+      ? plotWidthPx / (domainMax - domainMin)
+      : undefined;
+  let lastLabel: string | undefined;
+  let lastValue: number | undefined;
+  let lastShownValue: number | undefined;
+  const wrapper = (value: number | string) => {
+    // ECharts formats the labels in repeated ascending passes. Reset the
+    // dedup/spacing state when the sequence restarts so a forced boundary
+    // label (e.g. the min date) isn't blanked by the previous pass's state
+    // when both format identically (e.g. a May-to-May range).
+    if (
+      typeof value === 'number' &&
+      lastValue !== undefined &&
+      value <= lastValue
+    ) {
+      lastLabel = undefined;
+      lastShownValue = undefined;
+    }
+    if (typeof value === 'number') {
+      lastValue = value;
+    }
+    const label =
+      typeof xAxisFormatter === 'function'
+        ? (xAxisFormatter as Function)(value)
+        : String(value);
+    if (label === lastLabel) {
+      return '';
+    }
+    const isBoundary =
+      typeof value === 'number' && (value === domainMin || value === domainMax);
+    if (
+      !isBoundary &&
+      typeof value === 'number' &&
+      pixelsPerMs !== undefined &&
+      lastShownValue !== undefined &&
+      (value - lastShownValue) * pixelsPerMs <
+        label.length * TIMESERIES_CONSTANTS.xAxisLabelCharWidthPx +
+          TIMESERIES_CONSTANTS.xAxisLabelMinGapPx
+    ) {
+      return '';
+    }
+    lastLabel = label;
+    if (typeof value === 'number') {
+      lastShownValue = value;
+    }
+    return label;
+  };
+  if (typeof xAxisFormatter === 'function' && 'id' in xAxisFormatter) {
+    (wrapper as { id?: unknown }).id = (xAxisFormatter as { id?: unknown }).id;
+  }
+  return wrapper;
+}
+
+/**
+ * Computes the [min, max] of a temporal x-axis column across one or more
+ * data record arrays, for use with createSpacedXAxisFormatter.
+ */
+export function getXAxisDomain(
+  dataRecordArrays: Record<string, unknown>[][],
+  xAxisCol: string,
+): [number | undefined, number | undefined] {
+  let domainMin: number | undefined;
+  let domainMax: number | undefined;
+  dataRecordArrays.forEach(records => {
+    records.forEach(record => {
+      const value = record[xAxisCol];
+      if (typeof value === 'number') {
+        if (domainMin === undefined || value < domainMin) domainMin = value;
+        if (domainMax === undefined || value > domainMax) domainMax = value;
+      }
+    });
+  });
+  return [domainMin, domainMax];
+}
diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts
index 2288ff19386..be425d16d5f 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts
@@ -1379,6 +1379,58 @@ test('#39899 - x-axis dates do not overlap and last label stays visible at 0° r
   expect(axisLabel.hideOverlap).toBe(false);
 });
 
+test('#39899 - closely spaced x-axis time labels do not visually overlap (mixed)', () => {
+  const startTime = Date.UTC(2026, 0, 1);
+  const data = Array.from({ length: 20 }, (_, i) => ({
+    __timestamp: startTime + i * 60 * 1000,
+    sum__num: i,
+  }));
+  const queryData = createTestQueryData(data, {
+    colnames: ['__timestamp', 'sum__num'],
+    coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+    label_map: { __timestamp: ['__timestamp'], sum__num: ['sum__num'] },
+  });
+
+  const chartProps = createEchartsTimeseriesTestChartProps<
+    EchartsMixedTimeseriesFormData,
+    EchartsMixedTimeseriesProps
+  >({
+    ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+    width: 300,
+    height: 400,
+    defaultQueriesData: [queryData, queryData],
+    formData: {
+      ...formData,
+      x_axis: '__timestamp',
+      xAxisTimeFormat: '%Y-%m-%d %H:%M:%S',
+      metrics: ['sum__num'],
+      metricsB: ['sum__num'],
+      groupby: [],
+      groupbyB: [],
+      xAxisLabelRotation: 0,
+      timeGrainSqla: TimeGranularity.MINUTE,
+    },
+    queriesData: [queryData, queryData],
+  });
+
+  const { echartOptions } = transformProps(chartProps);
+  const { axisLabel } = echartOptions.xAxis as Record<string, any>;
+  const labels = data.map(({ __timestamp }) =>
+    axisLabel.formatter(__timestamp),
+  );
+
+  // hideOverlap must stay off so ECharts' own collision detection can never
+  // suppress the forced boundary label (#39899 must not regress).
+  expect(axisLabel.hideOverlap).toBe(false);
+  // The formatter itself must thin out labels that are too close together to
+  // render legibly in the available width.
+  expect(labels.filter(label => label === '').length).toBeGreaterThan(0);
+  // The first and last labels are the forced axis boundaries and must always
+  // stay visible.
+  expect(labels[0]).not.toBe('');
+  expect(labels[labels.length - 1]).not.toBe('');
+});
+
 test('regression #37921: multi-metric Query A with groupby does not duplicate first metric in series names', () => {
   // Regression test for https://github.com/apache/superset/issues/37921
   // ("Residual" follow-up to #37055).
diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts
index a24ff415857..45d782ff458 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts
@@ -3132,6 +3132,44 @@ test('applies gridlines to the value axis after a horizontal orientation swaps i
   expect((echartOptions.xAxis as any).splitLine.show).toBe(false);
 });
 
+test('#39899 - horizontal orientation does not over-thin the time axis labels', () => {
+  // The spacing formatter estimates label collisions using horizontal plot
+  // geometry (width, 7px/char). A horizontal chart swaps the time axis onto
+  // the side of the chart, where that geometry no longer applies, so the
+  // spacing formatter must not be used there.
+  const monthData = Array.from({ length: 24 }, (_, i) => ({
+    __timestamp: Date.UTC(2020, i, 1),
+    sales: i,
+  }));
+  const { echartOptions } = transformProps(
+    createTestChartProps({
+      formData: {
+        granularity_sqla: 'ds',
+        timeGrainSqla: TimeGranularity.MONTH,
+        xAxisTimeFormat: '%Y-%m',
+        seriesType: EchartsTimeseriesSeriesType.Bar,
+        orientation: OrientationType.Horizontal,
+      },
+      width: 800,
+      queriesData: [
+        createTestQueryData(monthData, {
+          colnames: ['__timestamp', 'sales'],
+          coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+        }),
+      ],
+    }),
+  );
+  // Horizontal swaps the axes, so the time axis ends up as yAxis.
+  const { axisLabel } = echartOptions.yAxis as Record<string, any>;
+  const labels = monthData.map(({ __timestamp }) =>
+    axisLabel.formatter(__timestamp),
+  );
+
+  // Every month is a distinct label, so none should be blanked by the
+  // spacing/dedup formatter on a horizontal chart.
+  expect(labels.filter(label => label === '')).toHaveLength(0);
+});
+
 test('boundary label alignment is dropped when the orientation moves the time axis to the side', () => {
   // The alignments position labels against the left and right edges of a
   // bottom axis. A horizontal chart swaps the axes, so applying them there
diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformers.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformers.test.ts
index ba49f4b3845..13a8638f180 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformers.test.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformers.test.ts
@@ -834,6 +834,55 @@ test('#39899 - x-axis dates do not overlap and last label stays visible at 0° r
   expect(axisLabel.hideOverlap).toBe(false);
 });
 
+test('#39899 - closely spaced x-axis time labels do not visually overlap', () => {
+  const formData = {
+    colorScheme: 'bnbColors',
+    datasource: '3__table',
+    granularity_sqla: 'ds',
+    timeGrainSqla: TimeGranularity.MINUTE,
+    x_axis_time_format: '%Y-%m-%d %H:%M:%S',
+    metric: 'sum__num',
+    viz_type: 'my_viz',
+  };
+  const startTime = new Date('2026-01-01T00:00:00Z').getTime();
+  const data = Array.from({ length: 20 }, (_, i) => ({
+    sum__num: i,
+    __timestamp: startTime + i * 60 * 1000,
+  }));
+  const chartProps = new ChartProps({
+    formData,
+    width: 300,
+    height: 400,
+    queriesData: [
+      {
+        data,
+        colnames: ['sum__num', '__timestamp'],
+        coltypes: [GenericDataType.Numeric, GenericDataType.Temporal],
+      },
+    ],
+    theme: supersetTheme,
+  });
+
+  const result = transformProps(
+    chartProps as unknown as EchartsTimeseriesChartProps,
+  );
+  const { axisLabel } = result.echartOptions.xAxis as Record<string, any>;
+  const labels = data.map(({ __timestamp }) =>
+    axisLabel.formatter(__timestamp),
+  );
+
+  // hideOverlap must stay off so ECharts' own collision detection can never
+  // suppress the forced boundary label (#39899 must not regress).
+  expect(axisLabel.hideOverlap).toBe(false);
+  // The formatter itself must thin out labels that are too close together to
+  // render legibly in the available width.
+  expect(labels.filter(label => label === '').length).toBeGreaterThan(0);
+  // The first and last labels are the forced axis boundaries and must always
+  // stay visible.
+  expect(labels[0]).not.toBe('');
+  expect(labels[labels.length - 1]).not.toBe('');
+});
+
 test('last x-axis date is visible and not cut off when rotated -45°', () => {
   const lastDataPointTimestamp = new Date('2026-12-01').getTime();
   const result = transformProps(