fix(charts): revert: improve negative stacked bar label positioning and accessibility (#37405) (#38484)

Co-authored-by: Kamil Gabryjelski <kamil.gabryjelski@gmail.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

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

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);
   });
 });