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/08/13 20:57:25 UTC
(superset) 03/03: fix(echarts): align bar label contrast paths
This is an automated email from the ASF dual-hosted git repository.
sadpandajoe pushed a commit to branch fix-bar-chart-label-placement
in repository https://gitbox.apache.org/repos/asf/superset.git
commit f395353984733a282cbc8d5ba97d6dc0257f1853
Author: sadpandajoe <jc...@gmail.com>
AuthorDate: Thu Aug 13 20:44:46 2026 +0000
fix(echarts): align bar label contrast paths
---
.../src/Timeseries/transformProps.ts | 6 ++--
.../src/Timeseries/transformers.ts | 9 +++--
.../test/MixedTimeseries/transformProps.test.ts | 32 +++++++++++++++++
.../test/Timeseries/Bar/transformProps.test.ts | 41 ++++++++++++++++++++++
.../test/Timeseries/transformers.test.ts | 31 +++++++++++++---
5 files changed, 110 insertions(+), 9 deletions(-)
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 de269e65f1b..7fc25fa5095 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@@ -328,6 +328,8 @@ export default function transformProps(
zoomable,
stackDimension,
}: EchartsTimeseriesFormData = { ...DEFAULT_FORM_DATA, ...formData };
+ const resolvedValueLabelPosition =
+ valueLabelPosition ?? BarValueLabelPosition.Auto;
const refs: Refs = {};
const groupBy = ensureIsArray(groupby);
@@ -738,7 +740,7 @@ export default function transformProps(
labelMap?.[seriesName]?.[0],
) ?? defaultFormatter),
showValue,
- valueLabelPosition,
+ valueLabelPosition: resolvedValueLabelPosition,
onlyTotal,
totalStackedValues: sortedTotalValues,
showValueIndexes,
@@ -1373,7 +1375,7 @@ export default function transformProps(
const echartOptions: EChartsCoreOption = {
useUTC: true,
...(seriesType === EchartsTimeseriesSeriesType.Bar &&
- valueLabelPosition === BarValueLabelPosition.Auto
+ resolvedValueLabelPosition === BarValueLabelPosition.Auto
? { darkMode: isThemeDark(theme) }
: {}),
grid: {
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts
index d2a6c2500c1..c83ff4298df 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts
@@ -537,6 +537,9 @@ export function transformSeries(
const isAutoBarLabel =
plotType === 'bar' && valueLabelPosition === BarValueLabelPosition.Auto;
+ const isInsideBarLabel =
+ plotType === 'bar' &&
+ valueLabelPosition !== BarValueLabelPosition.OutsideEnd;
return {
...series,
@@ -586,9 +589,9 @@ export function transformSeries(
: isHorizontal
? 'right'
: 'top',
- // ECharts derives contrast from the bar fill while the label is inside.
- // Returning x/y clears the attached position, selecting its outside fill.
- ...(isAutoBarLabel ? {} : { color: theme?.colorText }),
+ // ECharts derives contrast from the bar fill for inside positions.
+ // Auto x/y overflow clears the position, selecting its outside fill.
+ ...(isInsideBarLabel ? {} : { color: theme?.colorText }),
textBorderWidth: 0,
formatter: (params: any) => {
// don't show confidence band value labels, as they're already visible on the tooltip
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 e921fdf560c..c56120d4a89 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
@@ -216,6 +216,38 @@ test('bar value labels retain their legacy outside position', () => {
});
});
+test('negative bar values retain their legacy outside position', () => {
+ const negativeRows = [
+ { boy: -1, girl: -2, ds: 599616000000 },
+ { boy: -3, girl: -4, ds: 599916000000 },
+ ];
+ const negativeQueriesData = [
+ createTestQueryData(negativeRows, { label_map: defaultLabelMap }),
+ createTestQueryData(negativeRows, { label_map: defaultLabelMap }),
+ ];
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: negativeQueriesData,
+ formData: { ...formData, showValueB: true },
+ queriesData: negativeQueriesData,
+ });
+
+ const transformed = transformProps(chartProps);
+ const barSeries = (transformed.echartOptions.series as SeriesOption[]).filter(
+ (series): series is BarSeriesOption => series.type === 'bar',
+ );
+
+ expect(barSeries).not.toHaveLength(0);
+ barSeries.forEach(series => {
+ expect(series.data?.[0]).toMatchObject({
+ label: { position: 'bottom' },
+ });
+ });
+});
+
test('should transform chart props for viz with showQueryIdentifiers=false', () => {
const chartProps = createEchartsTimeseriesTestChartProps<
EchartsMixedTimeseriesFormData,
diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Bar/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Bar/transformProps.test.ts
index b56457b9ec0..50aa76c6ac2 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Bar/transformProps.test.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Bar/transformProps.test.ts
@@ -134,6 +134,47 @@ test('Auto Bar labels enable theme-aware ECharts contrast', () => {
expect(echartOptions.darkMode).toBe(false);
});
+test('legacy Bar labels without a saved position enable Auto contrast', () => {
+ const legacyFormData: Partial<EchartsTimeseriesFormData> = {
+ ...DEFAULT_FORM_DATA,
+ };
+ delete legacyFormData.valueLabelPosition;
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsTimeseriesFormData,
+ EchartsTimeseriesChartProps
+ >({
+ defaultFormData: legacyFormData as EchartsTimeseriesFormData,
+ defaultVizType: 'echarts_timeseries_bar',
+ formData: {
+ seriesType: EchartsTimeseriesSeriesType.Bar,
+ metrics: ['Sales'],
+ xAxis: '__timestamp',
+ showValue: true,
+ },
+ queriesData: [
+ createTestQueryData([{ Sales: 100, __timestamp: 1609459200000 }], {
+ colnames: ['Sales', '__timestamp'],
+ coltypes: [GenericDataType.Numeric, GenericDataType.Temporal],
+ }),
+ ],
+ });
+
+ expect(chartProps.formData).not.toHaveProperty('valueLabelPosition');
+ const { echartOptions } = transformProps(chartProps);
+ const [series] = echartOptions.series as BarSeriesOption[];
+
+ expect(typeof series.labelLayout).toBe('function');
+ expect(echartOptions.darkMode).toBe(false);
+
+ Reflect.set(chartProps.formData, 'valueLabelPosition', undefined);
+ const undefinedPositionOptions = transformProps(chartProps).echartOptions;
+ const [undefinedPositionSeries] =
+ undefinedPositionOptions.series as BarSeriesOption[];
+
+ expect(typeof undefinedPositionSeries.labelLayout).toBe('function');
+ expect(undefinedPositionOptions.darkMode).toBe(false);
+});
+
describe('Bar Chart X-axis Time Formatting', () => {
const baseFormData: SqlaFormData = {
...DEFAULT_FORM_DATA,
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 81584494c2b..708bf81b149 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
@@ -342,13 +342,36 @@ test.each([
) as BarSeriesOption;
expect(result.labelLayout).toBeUndefined();
- expect(result.label).toMatchObject({
- position: expected,
- color: supersetTheme.colorText,
- });
+ expect(result.label).toMatchObject({ position: expected });
+ if (position === BarValueLabelPosition.OutsideEnd) {
+ expect(result.label).toMatchObject({ color: supersetTheme.colorText });
+ } else {
+ expect(result.label).not.toHaveProperty('color');
+ }
},
);
+test('manual Outside End positions negative stacked segments below the bar', () => {
+ const result = transformSeries(
+ { name: 'test-series', type: 'bar', data: [[2026, -1]] },
+ mockColorScale,
+ 'test-key',
+ {
+ seriesType: EchartsTimeseriesSeriesType.Bar,
+ stack: StackControlsValue.Stack,
+ valueLabelPosition: BarValueLabelPosition.OutsideEnd,
+ },
+ ) as BarSeriesOption;
+
+ expect(result.data).toEqual([
+ {
+ value: [2026, -1],
+ label: { position: 'bottom' },
+ },
+ ]);
+ expect(result.labelLayout).toBeUndefined();
+});
+
test('Auto positions negative stacked segments at their inside end', () => {
const result = transformSeries(
{ name: 'test-series', type: 'bar', data: [[2026, -1]] },