[pull] master from apache:master

superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts

@@ -59,6 +59,7 @@ import {
   LegendOrientation,
   Refs,
 } from '../types';
+import { BarValueLabelPosition } from '../Timeseries/types';
 import { parseAxisBound } from '../utils/controls';
 import { safeParseEChartOptions } from '../utils/safeEChartOptionsParser';
 import {
@@ -102,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';
@@ -519,6 +522,7 @@ export default function transformProps(
         areaOpacity: opacity,
         seriesType,
         showValue,
+        valueLabelPosition: BarValueLabelPosition.OutsideEnd,
         onlyTotal,
         stack: Boolean(stack),
         stackIdSuffix: '\na',
@@ -608,6 +612,7 @@ export default function transformProps(
         areaOpacity: opacityB,
         seriesType: seriesTypeB,
         showValue: showValueB,
+        valueLabelPosition: BarValueLabelPosition.OutsideEnd,
         onlyTotal: onlyTotalB,
         stack: Boolean(stackB),
         stackIdSuffix: '\nb',
@@ -661,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);

superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.test.tsx

@@ -40,6 +40,7 @@ import {
 } from '../types';
 import EchartsTimeseries from './EchartsTimeseries';
 import {
+  BarValueLabelPosition,
   EchartsTimeseriesSeriesType,
   OrientationType,
   type EchartsTimeseriesFormData,
@@ -159,6 +160,7 @@ const defaultFormData: EchartsTimeseriesFormData & {
   xAxisLabelRotation: 0,
   xAxisLabelInterval: 0,
   showValue: false,
+  valueLabelPosition: BarValueLabelPosition.Auto,
   onlyTotal: false,
   showExtraControls: true,
   percentageThreshold: 0,

superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/constants.ts

@@ -23,6 +23,7 @@ import {
 import { t } from '@apache-superset/core/translation';
 import { LegendOrientation, LegendType } from '../types';
 import {
+  BarValueLabelPosition,
   OrientationType,
   EchartsTimeseriesSeriesType,
   EchartsTimeseriesFormData,
@@ -88,6 +89,10 @@ export const DEFAULT_FORM_DATA: EchartsTimeseriesFormData = {
   xAxisLabelInterval: defaultXAxis.xAxisLabelInterval,
   groupby: [],
   showValue: false,
+  // Legacy charts saved before this field existed have no valueLabelPosition
+  // in form_data and must keep their pre-existing Outside End placement;
+  // Auto is opt-in via the Value label position control, not the default.
+  valueLabelPosition: BarValueLabelPosition.OutsideEnd,
   labelPosition: 'auto',
   onlyTotal: false,
   percentageThreshold: 0,

superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts

@@ -47,6 +47,7 @@ import {
   NumberFormats,
 } from '@superset-ui/core';
 import { GenericDataType } from '@apache-superset/core/common';
+import { isThemeDark } from '@apache-superset/core/theme';
 import {
   extractExtraMetrics,
   getOriginalSeries,
@@ -63,6 +64,7 @@ import {
   EchartsTimeseriesChartProps,
   EchartsTimeseriesFormData,
   EchartsTimeseriesSeriesType,
+  BarValueLabelPosition,
   OrientationType,
   TimeseriesChartTransformedProps,
 } from './types';
@@ -124,8 +126,11 @@ import {
 } from '../constants';
 import { getDefaultTooltip } from '../utils/tooltip';
 import {
+  createDedupXAxisFormatter,
+  createSpacedXAxisFormatter,
   getPercentFormatter,
   getTooltipTimeFormatter,
+  getXAxisDomain,
   getXAxisFormatter,
   getYAxisFormatter,
 } from '../utils/formatters';
@@ -295,6 +300,7 @@ export default function transformProps(
     seriesType,
     showLegend,
     showValue,
+    valueLabelPosition,
     size,
     labelPosition,
     colorByPrimaryAxis,
@@ -333,6 +339,8 @@ export default function transformProps(
     zoomable,
     stackDimension,
   }: EchartsTimeseriesFormData = { ...DEFAULT_FORM_DATA, ...formData };
+  const resolvedValueLabelPosition =
+    valueLabelPosition ?? BarValueLabelPosition.OutsideEnd;
 
   const refs: Refs = {};
   const groupBy = ensureIsArray(groupby);
@@ -743,6 +751,7 @@ export default function transformProps(
               labelMap?.[seriesName]?.[0],
             ) ?? defaultFormatter),
         showValue,
+        valueLabelPosition: resolvedValueLabelPosition,
         onlyTotal,
         totalStackedValues: sortedTotalValues,
         showValueIndexes,
@@ -1207,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(
@@ -1379,6 +1371,10 @@ export default function transformProps(
 
   const echartOptions: EChartsCoreOption = {
     useUTC: true,
+    ...(seriesType === EchartsTimeseriesSeriesType.Bar &&
+    resolvedValueLabelPosition === BarValueLabelPosition.Auto
+      ? { darkMode: isThemeDark(theme) }
+      : {}),
     grid: {
       ...defaultGrid,
       ...padding,