mirror of
https://github.com/apache/superset.git
synced 2026-09-08 16:25:03 +00:00
fix(echarts): prevent x-axis time labels from overlapping (#43669)
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
32fe4f6612
commit
ba50bdff23
+17
-33
@@ -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);
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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];
|
||||
}
|
||||
|
||||
+52
@@ -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).
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user