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 d6e7f57fc92..799fce3003c 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts @@ -31,7 +31,6 @@ import { ValueFormatter, } from '@superset-ui/core'; import { SupersetTheme, isThemeDark } from '@apache-superset/core/theme'; -import { getContrastingColor } from '@superset-ui/core'; import type { CallbackDataParams, DefaultStatesMixin, @@ -143,39 +142,28 @@ export const getBaselineSeriesForStream = ( }; }; -export function optimizeBarLabelPlacement( +export function transformNegativeLabelsPosition( series: SeriesOption, isHorizontal: boolean, ): TimeseriesDataRecord[] { /* - * Adjusts label position for all values in bar series - * Positions labels inside bars at appropriate edges to avoid axis overlap + * Adjusts label position for negative values in bar series * @param series - Array of series options * @param isHorizontal - Whether chart is horizontal - * @returns data with adjusted label positions for all values + * @returns data with adjusted label positions for negative values */ const transformValue = (value: any) => { const [xValue, yValue] = Array.isArray(value) ? value : [null, null]; const axisValue = isHorizontal ? xValue : yValue; - if (axisValue === null || axisValue === undefined) { - return value; - } - - // Use inside positioning for all bar charts to avoid axis overlap - const labelPosition = - axisValue < 0 - ? isHorizontal - ? 'insideLeft' - : 'insideBottom' - : isHorizontal - ? 'insideRight' - : 'insideTop'; - - return { - value, - label: { position: labelPosition }, - }; + return axisValue < 0 + ? { + value, + label: { + position: 'outside', + }, + } + : value; }; return (series.data as TimeseriesDataRecord[]).map(transformValue); @@ -388,7 +376,7 @@ export function transformSeries( ), } : seriesType === 'bar' && !stack - ? { data: optimizeBarLabelPlacement(series, isHorizontal) } + ? { data: transformNegativeLabelsPosition(series, isHorizontal) } : null : null), connectNulls, @@ -422,11 +410,8 @@ export function transformSeries( symbolSize: markerSize, label: { show: !!showValue, - position: stack ? 'inside' : isHorizontal ? 'right' : 'top', - color: - stack || seriesType === 'bar' - ? getContrastingColor(String(itemStyle.color)) - : theme?.colorText, + position: isHorizontal ? 'right' : 'top', + 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/Timeseries/transformers.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformers.test.ts index 7a2cb392337..54da9c14ea0 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 @@ -22,24 +22,20 @@ import { supersetTheme } from '@apache-superset/core/theme'; import type { SeriesOption } from 'echarts'; import { EchartsTimeseriesSeriesType } from '../../src'; import { TIMESERIES_CONSTANTS } from '../../src/constants'; -import { - LegendOrientation, - EchartsTimeseriesChartProps, -} from '../../src/types'; +import { LegendOrientation } from '../../src/types'; import { transformSeries, - optimizeBarLabelPlacement, + transformNegativeLabelsPosition, getPadding, } from '../../src/Timeseries/transformers'; import transformProps from '../../src/Timeseries/transformProps'; +import { EchartsTimeseriesChartProps } from '../../src/types'; import * as seriesUtils from '../../src/utils/series'; -// Mock the colorScale function to return different colors based on key -const mockColorScale = jest.fn((key: string) => { - if (key === 'test-key') return '#1f77b4'; // blue - if (key === 'series-key') return '#ff7f0e'; // orange - return '#2ca02c'; // green for any other key -}) as unknown as CategoricalColorScale; +// Mock the colorScale function +const mockColorScale = jest.fn( + (key: string, sliceId?: number) => `color-for-${key}-${sliceId}`, +) as unknown as CategoricalColorScale; describe('transformSeries', () => { const series = { name: 'test-series' }; @@ -53,8 +49,7 @@ describe('transformSeries', () => { const result = transformSeries(series, mockColorScale, 'test-key', opts); - expect(mockColorScale).toHaveBeenCalledWith('test-key', 1); - expect((result as any)?.itemStyle.color).toBe('#1f77b4'); + expect((result as any)?.itemStyle.color).toBe('color-for-test-key-1'); }); test('should use seriesKey if timeShiftColor is not enabled', () => { @@ -66,8 +61,7 @@ describe('transformSeries', () => { const result = transformSeries(series, mockColorScale, 'test-key', opts); - expect(mockColorScale).toHaveBeenCalledWith('series-key', 2); - expect((result as any)?.itemStyle.color).toBe('#ff7f0e'); + expect((result as any)?.itemStyle.color).toBe('color-for-series-key-2'); }); test('should apply border styles for bar series with connectNulls', () => { @@ -129,8 +123,8 @@ describe('transformSeries', () => { }); }); -describe('optimizeBarLabelPlacement', () => { - test('label position for non-stacked vertical charts', () => { +describe('transformNegativeLabelsPosition', () => { + test('label position bottom of negative value no Horizontal', () => { const isHorizontal = false; const series: SeriesOption = { data: [ @@ -143,12 +137,15 @@ describe('optimizeBarLabelPlacement', () => { type: EchartsTimeseriesSeriesType.Bar, stack: undefined, }; - const result = optimizeBarLabelPlacement(series, isHorizontal); - expect((result as any)[0].label.position).toBe('insideTop'); - expect((result as any)[1].label.position).toBe('insideTop'); - expect((result as any)[2].label.position).toBe('insideBottom'); - expect((result as any)[3].label.position).toBe('insideBottom'); - expect((result as any)[4].label.position).toBe('insideTop'); + const result = + Array.isArray(series.data) && series.type === 'bar' && !series.stack + ? transformNegativeLabelsPosition(series, isHorizontal) + : series.data; + expect((result as any)[0].label).toBe(undefined); + expect((result as any)[1].label).toBe(undefined); + expect((result as any)[2].label.position).toBe('outside'); + expect((result as any)[3].label.position).toBe('outside'); + expect((result as any)[4].label).toBe(undefined); }); test('label position left of negative value is Horizontal', () => { @@ -165,12 +162,15 @@ describe('optimizeBarLabelPlacement', () => { stack: undefined, }; - const result = optimizeBarLabelPlacement(series, isHorizontal); - expect((result as any)[0].label.position).toBe('insideRight'); - expect((result as any)[1].label.position).toBe('insideLeft'); - expect((result as any)[2].label.position).toBe('insideRight'); - expect((result as any)[3].label.position).toBe('insideLeft'); - expect((result as any)[4].label.position).toBe('insideLeft'); + const result = + Array.isArray(series.data) && series.type === 'bar' && !series.stack + ? transformNegativeLabelsPosition(series, isHorizontal) + : series.data; + expect((result as any)[0].label).toBe(undefined); + expect((result as any)[1].label.position).toBe('outside'); + expect((result as any)[2].label).toBe(undefined); + expect((result as any)[3].label.position).toBe('outside'); + expect((result as any)[4].label.position).toBe('outside'); }); test('label position to line type', () => { @@ -192,7 +192,7 @@ describe('optimizeBarLabelPlacement', () => { !series.stack && series.type !== 'line' && series.type === 'bar' - ? optimizeBarLabelPlacement(series, isHorizontal) + ? transformNegativeLabelsPosition(series, isHorizontal) : series.data; expect((result as any)[0].label).toBe(undefined); expect((result as any)[1].label).toBe(undefined); @@ -215,34 +215,15 @@ describe('optimizeBarLabelPlacement', () => { stack: 'obs', }; - const result = optimizeBarLabelPlacement(series, isHorizontal); - expect((result as any)[0].label.position).toBe('insideTop'); - expect((result as any)[1].label.position).toBe('insideTop'); - expect((result as any)[2].label.position).toBe('insideBottom'); - expect((result as any)[3].label.position).toBe('insideBottom'); - expect((result as any)[4].label.position).toBe('insideTop'); - }); - - test('label position for horizontal stacked charts', () => { - const isHorizontal = true; - const series: SeriesOption = { - data: [ - [1, 2020], - [-3, 2021], - [2, 2022], - [-4, 2023], - [-6, 2024], - ], - type: EchartsTimeseriesSeriesType.Bar, - stack: 'obs', - }; - - const result = optimizeBarLabelPlacement(series, isHorizontal); - expect((result as any)[0].label.position).toBe('insideRight'); - expect((result as any)[1].label.position).toBe('insideLeft'); - expect((result as any)[2].label.position).toBe('insideRight'); - expect((result as any)[3].label.position).toBe('insideLeft'); - expect((result as any)[4].label.position).toBe('insideLeft'); + const result = + Array.isArray(series.data) && series.type === 'bar' && !series.stack + ? transformNegativeLabelsPosition(series, isHorizontal) + : series.data; + expect((result as any)[0].label).toBe(undefined); + expect((result as any)[1].label).toBe(undefined); + expect((result as any)[2].label).toBe(undefined); + expect((result as any)[3].label).toBe(undefined); + expect((result as any)[4].label).toBe(undefined); }); });