From 1e7d781354e96d8edfb04caa3d8e69b716f8316a Mon Sep 17 00:00:00 2001 From: Richard Fogaca Nienkotter <63572350+richardfogaca@users.noreply.github.com> Date: Wed, 25 Mar 2026 09:38:31 -0300 Subject: [PATCH] fix(echarts): prevent plain legend clipping in dashboards (#38675) (cherry picked from commit 12aca720746d70a13e0f76c24380ef4bdb69118f) --- .../src/Bubble/transformProps.ts | 29 +- .../src/Funnel/transformProps.ts | 29 +- .../src/Gantt/transformProps.ts | 48 ++- .../src/Graph/transformProps.ts | 29 +- .../src/MixedTimeseries/transformProps.ts | 64 ++- .../src/Pie/transformProps.ts | 33 +- .../src/Radar/transformProps.ts | 31 +- .../src/Timeseries/transformProps.ts | 128 ++++-- .../src/utils/legendLayout.ts | 56 +++ .../plugin-chart-echarts/src/utils/series.ts | 372 +++++++++++++++++- .../test/Gantt/transformProps.test.ts | 65 +++ .../MixedTimeseries/transformProps.test.ts | 63 +++ .../test/Pie/transformProps.test.ts | 46 +++ .../Timeseries/Bar/transformProps.test.ts | 274 ++++++++++++- .../test/Timeseries/transformProps.test.ts | 37 +- .../test/utils/series.test.ts | 312 ++++++++++++++- 16 files changed, 1514 insertions(+), 102 deletions(-) create mode 100644 superset-frontend/plugins/plugin-chart-echarts/src/utils/legendLayout.ts diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Bubble/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Bubble/transformProps.ts index ec671b94893..1f11498a358 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Bubble/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Bubble/transformProps.ts @@ -31,6 +31,7 @@ import { EchartsBubbleChartProps, EchartsBubbleFormData } from './types'; import { DEFAULT_FORM_DATA, MINIMUM_BUBBLE_SIZE } from './constants'; import { defaultGrid } from '../defaults'; import { getLegendProps, getMinAndMaxFromBounds } from '../utils/series'; +import { resolveLegendLayout } from '../utils/legendLayout'; import { Refs } from '../types'; import { parseAxisBound } from '../utils/controls'; import { getDefaultTooltip } from '../utils/tooltip'; @@ -172,6 +173,20 @@ export default function transformProps(chartProps: EchartsBubbleChartProps) { const xAxisFormatter = getNumberFormatter(xAxisFormat); const yAxisFormatter = getNumberFormatter(yAxisFormat); const tooltipSizeFormatter = getNumberFormatter(tooltipSizeFormat); + const legendData = Array.from(legends).sort((a: string, b: string) => { + if (!legendSort) return 0; + return legendSort === 'asc' ? a.localeCompare(b) : b.localeCompare(a); + }); + const { effectiveLegendMargin, effectiveLegendType } = resolveLegendLayout({ + chartHeight: height, + chartWidth: width, + legendItems: legendData, + legendMargin, + orientation: legendOrientation, + show: showLegend, + theme, + type: legendType, + }); const [xAxisMin, xAxisMax] = (xAxisBounds || []).map(parseAxisBound); const [yAxisMin, yAxisMax] = (yAxisBounds || []).map(parseAxisBound); @@ -181,7 +196,7 @@ export default function transformProps(chartProps: EchartsBubbleChartProps) { legendOrientation, true, false, - legendMargin, + effectiveLegendMargin, true, 'Left', convertInteger(yAxisTitleMargin), @@ -230,11 +245,13 @@ export default function transformProps(chartProps: EchartsBubbleChartProps) { type: logYAxis ? AxisType.Log : AxisType.Value, }, legend: { - ...getLegendProps(legendType, legendOrientation, showLegend, theme), - data: Array.from(legends).sort((a: string, b: string) => { - if (!legendSort) return 0; - return legendSort === 'asc' ? a.localeCompare(b) : b.localeCompare(a); - }), + ...getLegendProps( + effectiveLegendType, + legendOrientation, + showLegend, + theme, + ), + data: legendData, }, tooltip: { show: !inContextMenu, diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Funnel/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Funnel/transformProps.ts index 58c0a697402..87189c355a3 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Funnel/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Funnel/transformProps.ts @@ -46,6 +46,7 @@ import { getLegendProps, sanitizeHtml, } from '../utils/series'; +import { resolveLegendLayout } from '../utils/legendLayout'; import { defaultGrid } from '../defaults'; import { DEFAULT_LEGEND_FORM_DATA, OpacityEnum } from '../constants'; import { getDefaultTooltip } from '../utils/tooltip'; @@ -241,11 +242,25 @@ export default function transformProps( textBorderColor: theme.colorBgBase, textBorderWidth: 1, }; + const legendData = keys.sort((a: string, b: string) => { + if (!legendSort) return 0; + return legendSort === 'asc' ? a.localeCompare(b) : b.localeCompare(a); + }); + const { effectiveLegendMargin, effectiveLegendType } = resolveLegendLayout({ + chartHeight: height, + chartWidth: width, + legendItems: legendData, + legendMargin, + orientation: legendOrientation, + show: showLegend, + theme, + type: legendType, + }); const series: FunnelSeriesOption[] = [ { type: VizType.Funnel, - ...getChartPadding(showLegend, legendOrientation, legendMargin), + ...getChartPadding(showLegend, legendOrientation, effectiveLegendMargin), animation: true, minSize: '0%', maxSize: '100%', @@ -298,11 +313,13 @@ export default function transformProps( }, }, legend: { - ...getLegendProps(legendType, legendOrientation, showLegend, theme), - data: keys.sort((a: string, b: string) => { - if (!legendSort) return 0; - return legendSort === 'asc' ? a.localeCompare(b) : b.localeCompare(a); - }), + ...getLegendProps( + effectiveLegendType, + legendOrientation, + showLegend, + theme, + ), + data: legendData, }, series, }; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Gantt/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Gantt/transformProps.ts index 997cbab1397..1a9b4d565a1 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Gantt/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Gantt/transformProps.ts @@ -43,8 +43,13 @@ import { EchartsGanttFormData, } from './types'; import { DEFAULT_FORM_DATA, TIMESERIES_CONSTANTS } from '../constants'; -import { Refs } from '../types'; -import { getLegendProps, groupData } from '../utils/series'; +import { LegendOrientation, Refs } from '../types'; +import { + getHorizontalLegendAvailableWidth, + getLegendProps, + groupData, +} from '../utils/series'; +import { resolveLegendLayout } from '../utils/legendLayout'; import { getTooltipTimeFormatter, getXAxisFormatter, @@ -246,7 +251,7 @@ export default function transformProps(chartProps: EchartsGanttChartProps) { } const padding = getPadding( - showLegend && seriesMap.size > 1, + showLegend, legendOrientation, false, zoomable, @@ -345,6 +350,40 @@ export default function transformProps(chartProps: EchartsGanttChartProps) { if (!legendSort) return 0; return legendSort === 'asc' ? a.localeCompare(b) : b.localeCompare(a); }); + const { legendLayout, effectiveLegendType } = resolveLegendLayout({ + availableWidth: + legendOrientation === LegendOrientation.Top || + legendOrientation === LegendOrientation.Bottom + ? getHorizontalLegendAvailableWidth({ + chartWidth: width, + orientation: legendOrientation, + padding, + zoomable, + }) + : undefined, + chartHeight: height, + chartWidth: width, + legendItems: legendData, + legendMargin, + orientation: legendOrientation, + show: showLegend, + theme, + type: legendType, + }); + if (legendLayout.effectiveMargin !== undefined) { + const adjustedPadding = getPadding( + showLegend, + legendOrientation, + false, + zoomable, + legendLayout.effectiveMargin, + !!xAxisTitle, + 'Left', + convertInteger(yAxisTitleMargin), + convertInteger(xAxisTitleMargin), + ); + Object.assign(padding, adjustedPadding); + } const tooltipFormatterMap = { [GenericDataType.Numeric]: tooltipValuesFormatter, @@ -374,12 +413,13 @@ export default function transformProps(chartProps: EchartsGanttChartProps) { }, legend: { ...getLegendProps( - legendType, + effectiveLegendType, legendOrientation, showLegend, theme, zoomable, legendState, + padding, ), data: legendData, }, diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Graph/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Graph/transformProps.ts index 8204351f30f..9fee03ae3d3 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Graph/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Graph/transformProps.ts @@ -42,6 +42,7 @@ import { getLegendProps, sanitizeHtml, } from '../utils/series'; +import { resolveLegendLayout } from '../utils/legendLayout'; import { getDefaultTooltip } from '../utils/tooltip'; import { Refs } from '../types'; @@ -298,6 +299,20 @@ export default function transformProps( }); const categoryList = [...categories]; + const legendData = categoryList.sort((a: string, b: string) => { + if (!legendSort) return 0; + return legendSort === 'asc' ? a.localeCompare(b) : b.localeCompare(a); + }); + const { effectiveLegendMargin, effectiveLegendType } = resolveLegendLayout({ + chartHeight: height, + chartWidth: width, + legendItems: legendData, + legendMargin, + orientation: legendOrientation, + show: showLegend, + theme, + type: legendType, + }); const series: GraphSeriesOption[] = [ { zoom: DEFAULT_GRAPH_SERIES_OPTION.zoom, @@ -324,7 +339,7 @@ export default function transformProps( edgeSymbol: parseEdgeSymbol(edgeSymbol), edgeSymbolSize: baseEdgeWidth * 2, selectedMode, - ...getChartPadding(showLegend, legendOrientation, legendMargin), + ...getChartPadding(showLegend, legendOrientation, effectiveLegendMargin), animation: DEFAULT_GRAPH_SERIES_OPTION.animation, label: { ...DEFAULT_GRAPH_SERIES_OPTION.label, @@ -353,11 +368,13 @@ export default function transformProps( }, }, legend: { - ...getLegendProps(legendType, legendOrientation, showLegend, theme), - data: categoryList.sort((a: string, b: string) => { - if (!legendSort) return 0; - return legendSort === 'asc' ? a.localeCompare(b) : b.localeCompare(a); - }), + ...getLegendProps( + effectiveLegendType, + legendOrientation, + showLegend, + theme, + ), + data: legendData, }, series, }; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts index f4db0afc3d9..b1655e38d1d 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts @@ -55,6 +55,7 @@ import { import { EchartsTimeseriesSeriesType, ForecastSeriesEnum, + LegendOrientation, Refs, } from '../types'; import { parseAxisBound } from '../utils/controls'; @@ -67,10 +68,12 @@ import { extractTooltipKeys, getAxisType, getColtypesMapping, + getHorizontalLegendAvailableWidth, getLegendProps, getMinAndMaxFromBounds, getOverMaxHiddenFormatter, } from '../utils/series'; +import { resolveLegendLayout } from '../utils/legendLayout'; import { extractAnnotationLabels, getAnnotationData, @@ -583,13 +586,57 @@ export default function transformProps( convertInteger(yAxisTitleMargin) !== 0; const addXAxisTitleOffset = !!xAxisTitle && convertInteger(xAxisTitleMargin) !== 0; + const baseChartPadding = getPadding( + showLegend, + legendOrientation, + addYAxisTitleOffset, + zoomable, + legendMargin, + addXAxisTitleOffset, + yAxisTitlePosition, + convertInteger(yAxisTitleMargin), + convertInteger(xAxisTitleMargin), + ); + const legendData = series + .filter( + entry => + extractForecastSeriesContext((entry.name || '') as string).type === + ForecastSeriesEnum.Observation, + ) + .map(entry => entry.name) + .filter((name): name is string => Boolean(name)) + .concat(extractAnnotationLabels(annotationLayers)) + .sort((a: string, b: string) => { + if (!legendSort) return 0; + return legendSort === 'asc' ? a.localeCompare(b) : b.localeCompare(a); + }); + const { effectiveLegendMargin, effectiveLegendType } = resolveLegendLayout({ + availableWidth: + legendOrientation === LegendOrientation.Top || + legendOrientation === LegendOrientation.Bottom + ? getHorizontalLegendAvailableWidth({ + chartWidth: width, + orientation: legendOrientation, + padding: baseChartPadding, + zoomable, + }) + : undefined, + chartHeight: height, + chartWidth: width, + legendItems: legendData, + legendMargin, + orientation: legendOrientation, + show: showLegend, + theme, + type: legendType, + }); const chartPadding = getPadding( showLegend, legendOrientation, addYAxisTitleOffset, zoomable, - legendMargin, + effectiveLegendMargin, addXAxisTitleOffset, yAxisTitlePosition, convertInteger(yAxisTitleMargin), @@ -753,7 +800,7 @@ export default function transformProps( }, legend: { ...getLegendProps( - legendType, + effectiveLegendType, legendOrientation, showLegend, theme, @@ -761,18 +808,7 @@ export default function transformProps( legendState, chartPadding, ), - data: series - .filter( - entry => - extractForecastSeriesContext((entry.name || '') as string).type === - ForecastSeriesEnum.Observation, - ) - .map(entry => entry.id || entry.name || '') - .concat(extractAnnotationLabels(annotationLayers)) - .sort((a: string, b: string) => { - if (!legendSort) return 0; - return legendSort === 'asc' ? a.localeCompare(b) : b.localeCompare(a); - }), + data: legendData, }, series: dedupSeries(reorderForecastSeries(series) as SeriesOption[]), toolbox: { diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Pie/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/transformProps.ts index 3a66e61910e..6a355f6e753 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Pie/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/transformProps.ts @@ -48,6 +48,7 @@ import { getLegendProps, sanitizeHtml, } from '../utils/series'; +import { resolveLegendLayout } from '../utils/legendLayout'; import { defaultGrid } from '../defaults'; import { convertInteger } from '../utils/convertInteger'; import { getDefaultTooltip } from '../utils/tooltip'; @@ -380,11 +381,27 @@ export default function transformProps( show: showLabels, color: theme.colorText, }; + const legendData = transformedData + .map(datum => datum.name) + .sort((a: string, b: string) => { + if (!legendSort) return 0; + return legendSort === 'asc' ? a.localeCompare(b) : b.localeCompare(a); + }); + const { effectiveLegendMargin, effectiveLegendType } = resolveLegendLayout({ + chartHeight: height, + chartWidth: width, + legendItems: legendData, + legendMargin, + orientation: legendOrientation, + show: showLegend, + theme, + type: legendType, + }); const chartPadding = getChartPadding( showLegend, legendOrientation, - legendMargin, + effectiveLegendMargin, ); const series: PieSeriesOption[] = [ @@ -444,13 +461,13 @@ export default function transformProps( }, }, legend: { - ...getLegendProps(legendType, legendOrientation, showLegend, theme), - data: transformedData - .map(datum => datum.name) - .sort((a: string, b: string) => { - if (!legendSort) return 0; - return legendSort === 'asc' ? a.localeCompare(b) : b.localeCompare(a); - }), + ...getLegendProps( + effectiveLegendType, + legendOrientation, + showLegend, + theme, + ), + data: legendData, }, graphic: showTotal ? { diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Radar/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Radar/transformProps.ts index 142bc51de45..a906ee9b177 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Radar/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Radar/transformProps.ts @@ -45,6 +45,7 @@ import { getColtypesMapping, getLegendProps, } from '../utils/series'; +import { resolveLegendLayout } from '../utils/legendLayout'; import { defaultGrid } from '../defaults'; import { Refs } from '../types'; import { getDefaultTooltip } from '../utils/tooltip'; @@ -313,11 +314,27 @@ export default function transformProps( min, }; }); + const legendData = Array.from(columnsLabelMap.keys()).sort( + (a: string, b: string) => { + if (!legendSort) return 0; + return legendSort === 'asc' ? a.localeCompare(b) : b.localeCompare(a); + }, + ); + const { effectiveLegendMargin, effectiveLegendType } = resolveLegendLayout({ + chartHeight: height, + chartWidth: width, + legendItems: legendData, + legendMargin, + orientation: legendOrientation, + show: showLegend, + theme, + type: legendType, + }); const series: RadarSeriesOption[] = [ { type: 'radar', - ...getChartPadding(showLegend, legendOrientation, legendMargin), + ...getChartPadding(showLegend, legendOrientation, effectiveLegendMargin), animation: false, emphasis: { label: { @@ -354,11 +371,13 @@ export default function transformProps( formatter: NormalizedTooltipFormater, }, legend: { - ...getLegendProps(legendType, legendOrientation, showLegend, theme), - data: Array.from(columnsLabelMap.keys()).sort((a: string, b: string) => { - if (!legendSort) return 0; - return legendSort === 'asc' ? a.localeCompare(b) : b.localeCompare(a); - }), + ...getLegendProps( + effectiveLegendType, + legendOrientation, + showLegend, + theme, + ), + data: legendData, }, series, radar: { 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 f7d90cc3f5a..e53070ceb81 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts @@ -63,7 +63,13 @@ import { TimeseriesChartTransformedProps, } from './types'; import { DEFAULT_FORM_DATA } from './constants'; -import { ForecastSeriesEnum, ForecastValue, Refs } from '../types'; +import { + ForecastSeriesEnum, + ForecastValue, + LegendOrientation, + LegendType, + Refs, +} from '../types'; import { parseAxisBound } from '../utils/controls'; import { calculateLowerLogTick, @@ -74,9 +80,11 @@ import { extractTooltipKeys, getAxisType, getColtypesMapping, + getHorizontalLegendAvailableWidth, getLegendProps, getMinAndMaxFromBounds, } from '../utils/series'; +import { resolveLegendLayout } from '../utils/legendLayout'; import { extractAnnotationLabels, getAnnotationData, @@ -647,29 +655,10 @@ export default function transformProps( onLegendScroll, } = hooks; - const addYAxisLabelOffset = - !!yAxisTitle && convertInteger(yAxisTitleMargin) !== 0; - const addXAxisLabelOffset = - !!xAxisTitle && convertInteger(xAxisTitleMargin) !== 0; - const padding = getPadding( - showLegend, - legendOrientation, - addYAxisLabelOffset, - zoomable, - legendMargin, - addXAxisLabelOffset, - yAxisTitlePosition, - convertInteger(yAxisTitleMargin), - convertInteger(xAxisTitleMargin), - isHorizontal, - ); - const legendData = colorByPrimaryAxis && groupBy.length === 0 && series.length > 0 - ? // When colorByPrimaryAxis is enabled, show only primary axis values (deduped + filtered) - (() => { + ? (() => { const firstSeries = series[0]; - // For horizontal charts the category is at index 1, for vertical at index 0 const primaryAxisIndex = isHorizontal ? 1 : 0; if (firstSeries && Array.isArray(firstSeries.data)) { const names = (firstSeries.data as any[]) @@ -692,8 +681,7 @@ export default function transformProps( } return []; })() - : // Otherwise show original series names - rawSeries + : rawSeries .filter( entry => extractForecastSeriesContext(entry.name || '').type === @@ -701,6 +689,84 @@ export default function transformProps( ) .map(entry => entry.name || '') .concat(extractAnnotationLabels(annotationLayers)); + const addYAxisLabelOffset = + !!yAxisTitle && convertInteger(yAxisTitleMargin) !== 0; + const addXAxisLabelOffset = + !!xAxisTitle && convertInteger(xAxisTitleMargin) !== 0; + + const sortedLegendData = [...legendData].sort((a: string, b: string) => { + if (!legendSort) return 0; + return legendSort === 'asc' ? a.localeCompare(b) : b.localeCompare(a); + }); + const colorByPrimaryAxisLegendData = legendData.map(name => ({ + name, + icon: 'roundRect', + })); + const getLegendLayout = (candidateLegendMargin?: string | number | null) => { + const padding = getPadding( + showLegend, + legendOrientation, + addYAxisLabelOffset, + zoomable, + candidateLegendMargin, + addXAxisLabelOffset, + yAxisTitlePosition, + convertInteger(yAxisTitleMargin), + convertInteger(xAxisTitleMargin), + isHorizontal, + ); + + return resolveLegendLayout({ + availableWidth: + legendOrientation === LegendOrientation.Top || + legendOrientation === LegendOrientation.Bottom + ? getHorizontalLegendAvailableWidth({ + chartWidth: width, + orientation: legendOrientation, + padding, + zoomable, + }) + : undefined, + chartHeight: height, + chartWidth: width, + legendItems: + colorByPrimaryAxis && groupBy.length === 0 + ? colorByPrimaryAxisLegendData + : sortedLegendData, + legendMargin: candidateLegendMargin, + orientation: legendOrientation, + show: showLegend, + showSelectors: !(colorByPrimaryAxis && groupBy.length === 0), + theme, + type: legendType, + }); + }; + const initialLegendLayout = getLegendLayout(legendMargin); + const legendLayout = + isHorizontal && + legendOrientation === LegendOrientation.Bottom && + initialLegendLayout.effectiveLegendType === LegendType.Plain + ? getLegendLayout(initialLegendLayout.effectiveLegendMargin) + : initialLegendLayout; + const { effectiveLegendType } = legendLayout; + const effectiveLegendMargin = + isHorizontal && + legendOrientation === LegendOrientation.Bottom && + legendLayout.effectiveLegendType === LegendType.Scroll + ? legendMargin + : legendLayout.effectiveLegendMargin; + const padding = getPadding( + showLegend, + legendOrientation, + addYAxisLabelOffset, + zoomable, + effectiveLegendMargin, + addXAxisLabelOffset, + yAxisTitlePosition, + convertInteger(yAxisTitleMargin), + convertInteger(xAxisTitleMargin), + isHorizontal, + ); let xAxis: any = { type: xAxisType, @@ -910,7 +976,7 @@ export default function transformProps( }, legend: { ...getLegendProps( - legendType, + effectiveLegendType, legendOrientation, showLegend, theme, @@ -921,18 +987,8 @@ export default function transformProps( scrollDataIndex: legendIndex || 0, data: colorByPrimaryAxis && groupBy.length === 0 - ? // When colorByPrimaryAxis, configure legend items with roundRect icons - legendData.map(name => ({ - name, - icon: 'roundRect', - })) - : // Otherwise use normal legend data - legendData.sort((a: string, b: string) => { - if (!legendSort) return 0; - return legendSort === 'asc' - ? a.localeCompare(b) - : b.localeCompare(a); - }), + ? colorByPrimaryAxisLegendData + : sortedLegendData, // Disable legend selection and buttons when colorByPrimaryAxis is enabled ...(colorByPrimaryAxis && groupBy.length === 0 ? { diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/legendLayout.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/legendLayout.ts new file mode 100644 index 00000000000..c4a8740e0c1 --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/legendLayout.ts @@ -0,0 +1,56 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import type { SupersetTheme } from '@apache-superset/core/theme'; +import { LegendOrientation, LegendType } from '../types'; +import { getLegendLayoutResult, LegendLayoutResult } from './series'; + +type LegendDataItem = + | string + | number + | null + | undefined + | { name?: string | number | null }; + +export type ResolvedLegendLayout = { + effectiveLegendMargin?: string | number | null; + effectiveLegendType: LegendType; + legendLayout: LegendLayoutResult; +}; + +export function resolveLegendLayout(args: { + availableHeight?: number; + availableWidth?: number; + chartHeight: number; + chartWidth: number; + legendItems?: LegendDataItem[]; + legendMargin?: string | number | null; + orientation: LegendOrientation; + show: boolean; + showSelectors?: boolean; + theme: SupersetTheme; + type: LegendType; +}): ResolvedLegendLayout { + const legendLayout = getLegendLayoutResult(args); + + return { + effectiveLegendMargin: legendLayout.effectiveMargin ?? args.legendMargin, + effectiveLegendType: legendLayout.effectiveType, + legendLayout, + }; +} diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts index 882612ec33f..8e243167474 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts @@ -55,6 +55,364 @@ function isDefined(value: T | undefined | null): boolean { return value !== undefined && value !== null; } +const DEFAULT_LEGEND_ITEM_GAP = 10; +const DEFAULT_LEGEND_ICON_WIDTH = 25; +const LEGEND_ICON_LABEL_GAP = 5; +const LEGEND_HORIZONTAL_SIDE_GUTTER = 16; +const LEGEND_HORIZONTAL_ROW_HEIGHT = 24; +const LEGEND_HORIZONTAL_MAX_ROWS = 2; +const LEGEND_HORIZONTAL_MAX_HEIGHT_RATIO = 0.25; +const LEGEND_VERTICAL_SIDE_GUTTER = 16; +const LEGEND_VERTICAL_ROW_HEIGHT = 24; +const LEGEND_VERTICAL_MAX_WIDTH_RATIO = 0.4; +const LEGEND_SELECTOR_GAP = 10; +const LEGEND_MARGIN_GUTTER = 45; +// ECharts does not expose pre-render measurements for plain legends, so these +// values intentionally overestimate selector space to avoid clipping. +const ESTIMATED_LEGEND_SELECTOR_WIDTH = 112; +const LEGEND_TEXT_WIDTH_CACHE = new Map(); + +type LegendDataItem = + | string + | number + | null + | undefined + | { name?: string | number | null }; + +export type LegendLayoutResult = { + effectiveMargin?: number; + effectiveType: LegendType; +}; + +const SCROLL_LEGEND_LAYOUT: LegendLayoutResult = { + effectiveType: LegendType.Scroll, +}; + +function getLegendLabel(item: LegendDataItem): string { + if (typeof item === 'string' || typeof item === 'number') { + return String(item); + } + + if (item?.name === undefined || item.name === null) { + return ''; + } + + return String(item.name); +} + +function measureLegendTextWidth(text: string, theme: SupersetTheme): number { + const cacheKey = `${theme.fontFamily}:${theme.fontSizeSM}:${text}`; + const cachedWidth = LEGEND_TEXT_WIDTH_CACHE.get(cacheKey); + if (cachedWidth !== undefined) { + return cachedWidth; + } + + let width = text.length * theme.fontSizeSM * 0.62; + + if (typeof document !== 'undefined') { + const canvas = document.createElement('canvas'); + const context = canvas.getContext('2d'); + if (context) { + context.font = `${theme.fontSizeSM}px ${theme.fontFamily}`; + ({ width } = context.measureText(text)); + } + } + + LEGEND_TEXT_WIDTH_CACHE.set(cacheKey, width); + return width; +} + +function hasLegendLabel(item: LegendDataItem): boolean { + if (item === null || item === undefined) { + return false; + } + + if (typeof item === 'object') { + return item.name !== null && item.name !== undefined; + } + + return true; +} + +function getLegendLabels(items: LegendDataItem[]): string[] { + return items.filter(hasLegendLabel).map(getLegendLabel); +} + +function getLegendItemWidths(labels: string[], theme: SupersetTheme): number[] { + return labels.map( + label => + DEFAULT_LEGEND_ICON_WIDTH + + LEGEND_ICON_LABEL_GAP + + measureLegendTextWidth(label, theme), + ); +} + +function estimateHorizontalLegendRows( + labels: string[], + chartWidth: number, + showSelectors: boolean, + theme: SupersetTheme, +): number { + const availableWidth = Math.max( + chartWidth - LEGEND_HORIZONTAL_SIDE_GUTTER, + 0, + ); + if (availableWidth === 0) { + return Infinity; + } + + const legendItemWidths = getLegendItemWidths(labels, theme); + + if (legendItemWidths.length === 0) { + if (showSelectors && ESTIMATED_LEGEND_SELECTOR_WIDTH > availableWidth) { + return Infinity; + } + return 1; + } + + let rows = 1; + let rowWidth = 0; + + for (const itemWidth of legendItemWidths) { + if (itemWidth > availableWidth) { + return Infinity; + } + + const nextWidth = + rowWidth === 0 + ? itemWidth + : rowWidth + DEFAULT_LEGEND_ITEM_GAP + itemWidth; + if (rowWidth > 0 && nextWidth > availableWidth) { + rows += 1; + rowWidth = itemWidth; + } else { + rowWidth = nextWidth; + } + } + + if (showSelectors) { + if (ESTIMATED_LEGEND_SELECTOR_WIDTH > availableWidth) { + return Infinity; + } + const selectorWidth = + rowWidth === 0 + ? ESTIMATED_LEGEND_SELECTOR_WIDTH + : rowWidth + LEGEND_SELECTOR_GAP + ESTIMATED_LEGEND_SELECTOR_WIDTH; + if (selectorWidth > availableWidth) { + rows += 1; + } + } + + return rows; +} + +export function getHorizontalLegendAvailableWidth({ + chartWidth, + orientation, + padding, + zoomable = false, +}: { + chartWidth: number; + orientation: LegendOrientation.Top | LegendOrientation.Bottom; + padding?: LegendPaddingType; + zoomable?: boolean; +}): number { + let availableWidth = chartWidth - (padding?.left ?? 0); + + if (orientation === LegendOrientation.Top && zoomable) { + availableWidth -= TIMESERIES_CONSTANTS.legendTopRightOffset; + } + + return Math.max(availableWidth, 0); +} + +function getLongestLegendLabelWidth( + labels: string[], + theme: SupersetTheme, +): number { + return labels.reduce( + (maxWidth, label) => + Math.max(maxWidth, measureLegendTextWidth(label, theme)), + 0, + ); +} + +function isHorizontalLegendOrientation( + orientation: LegendOrientation, +): orientation is LegendOrientation.Top | LegendOrientation.Bottom { + return ( + orientation === LegendOrientation.Top || + orientation === LegendOrientation.Bottom + ); +} + +function getHorizontalPlainLegendLayout({ + availableHeight, + availableWidth, + currentMargin, + legendLabels, + orientation, + showSelectors, + theme, +}: { + availableHeight: number; + availableWidth: number; + currentMargin: number; + legendLabels: string[]; + orientation: LegendOrientation.Top | LegendOrientation.Bottom; + showSelectors: boolean; + theme: SupersetTheme; +}): LegendLayoutResult { + const rowCount = estimateHorizontalLegendRows( + legendLabels, + availableWidth, + showSelectors, + theme, + ); + const requiredMargin = + defaultLegendPadding[orientation] + + Math.max(0, rowCount - 1) * LEGEND_HORIZONTAL_ROW_HEIGHT; + const maxLegendHeight = + availableHeight > 0 + ? availableHeight * LEGEND_HORIZONTAL_MAX_HEIGHT_RATIO + : Infinity; + + if ( + !Number.isFinite(rowCount) || + rowCount > LEGEND_HORIZONTAL_MAX_ROWS || + requiredMargin > maxLegendHeight + ) { + return SCROLL_LEGEND_LAYOUT; + } + + return { + effectiveMargin: Math.max(currentMargin, requiredMargin), + effectiveType: LegendType.Plain, + }; +} + +function getVerticalPlainLegendLayout({ + availableHeight, + availableWidth, + currentMargin, + legendLabels, + showSelectors, + theme, +}: { + availableHeight: number; + availableWidth: number; + currentMargin: number; + legendLabels: string[]; + showSelectors: boolean; + theme: SupersetTheme; +}): LegendLayoutResult { + if (legendLabels.length === 0) { + return { + effectiveMargin: currentMargin, + effectiveType: LegendType.Plain, + }; + } + + const selectorHeight = showSelectors + ? LEGEND_VERTICAL_ROW_HEIGHT + LEGEND_SELECTOR_GAP + : 0; + const effectiveAvailableHeight = Math.max( + availableHeight - LEGEND_VERTICAL_SIDE_GUTTER - selectorHeight, + 0, + ); + const rowsPerColumn = Math.floor( + (effectiveAvailableHeight + DEFAULT_LEGEND_ITEM_GAP) / + (LEGEND_VERTICAL_ROW_HEIGHT + DEFAULT_LEGEND_ITEM_GAP), + ); + const requiredSelectorMargin = showSelectors + ? ESTIMATED_LEGEND_SELECTOR_WIDTH + LEGEND_VERTICAL_SIDE_GUTTER + : 0; + const requiredMargin = Math.ceil( + Math.max( + getLongestLegendLabelWidth(legendLabels, theme) + LEGEND_MARGIN_GUTTER, + requiredSelectorMargin, + ), + ); + const maxLegendWidth = + availableWidth > 0 + ? availableWidth * LEGEND_VERTICAL_MAX_WIDTH_RATIO + : Infinity; + + if ( + rowsPerColumn <= 0 || + legendLabels.length > rowsPerColumn || + requiredMargin > maxLegendWidth + ) { + return SCROLL_LEGEND_LAYOUT; + } + + return { + effectiveMargin: Math.max(currentMargin, requiredMargin), + effectiveType: LegendType.Plain, + }; +} + +export function getLegendLayoutResult({ + availableHeight, + availableWidth, + chartHeight, + chartWidth, + legendItems = [], + legendMargin, + orientation, + show, + showSelectors = true, + theme, + type, +}: { + // Raw chart dimensions. Use availableWidth/availableHeight when other chart + // UI elements reserve legend space before ECharts lays out the legend. + availableHeight?: number; + availableWidth?: number; + chartHeight: number; + chartWidth: number; + legendItems?: LegendDataItem[]; + legendMargin?: string | number | null; + orientation: LegendOrientation; + show: boolean; + showSelectors?: boolean; + theme: SupersetTheme; + type: LegendType; +}): LegendLayoutResult { + if (!show || type !== LegendType.Plain) { + return { effectiveType: type }; + } + + const resolvedLegendMargin = + typeof legendMargin === 'number' + ? legendMargin + : defaultLegendPadding[orientation]; + const legendLabels = getLegendLabels(legendItems); + const resolvedAvailableWidth = availableWidth ?? chartWidth; + const resolvedAvailableHeight = availableHeight ?? chartHeight; + + if (isHorizontalLegendOrientation(orientation)) { + return getHorizontalPlainLegendLayout({ + availableHeight: resolvedAvailableHeight, + availableWidth: resolvedAvailableWidth, + currentMargin: resolvedLegendMargin, + legendLabels, + orientation, + showSelectors, + theme, + }); + } + + return getVerticalPlainLegendLayout({ + availableHeight: resolvedAvailableHeight, + availableWidth: resolvedAvailableWidth, + currentMargin: resolvedLegendMargin, + legendLabels, + showSelectors, + theme, + }); +} + export function extractDataTotalValues( data: DataRecord[], opts: { @@ -438,12 +796,6 @@ export function getLegendProps( legendState?: LegendState, padding?: LegendPaddingType, ): LegendComponentOption { - const isHorizontal = - orientation === LegendOrientation.Top || - orientation === LegendOrientation.Bottom; - - const effectiveType = - type === LegendType.Scroll || !isHorizontal ? type : LegendType.Scroll; const legend: LegendComponentOption = { orient: [LegendOrientation.Top, LegendOrientation.Bottom].includes( orientation, @@ -451,7 +803,7 @@ export function getLegendProps( ? 'horizontal' : 'vertical', show, - type: effectiveType, + type, selected: legendState ?? {}, selector: ['all', 'inverse'], selectorLabel: { @@ -491,10 +843,16 @@ export function getLegendProps( if (padding?.left) { legend.left = padding.left; } + if (type === LegendType.Plain) { + legend.right = 0; + } break; case LegendOrientation.Top: legend.top = 0; legend.right = zoomable ? TIMESERIES_CONSTANTS.legendTopRightOffset : 0; + if (type === LegendType.Plain && padding?.left) { + legend.left = padding.left; + } break; default: legend.top = 0; diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Gantt/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Gantt/transformProps.test.ts index fd0545d5e99..f260959e4d2 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/Gantt/transformProps.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/Gantt/transformProps.test.ts @@ -303,4 +303,69 @@ describe('legend sorting', () => { const legendData = (result.echartOptions.legend as any).data; expect(legendData).toEqual(['series value 2', 'series value 1']); }); + + test('falls back to scroll for plain legends with an overlong legend item', () => { + const props = new ChartProps({ + ...chartPropsConfig, + width: 320, + formData: { + ...formData, + legendType: LegendType.Plain, + }, + queriesData: [ + { + data: [ + { + startTime: Date.UTC(2025, 1, 1, 13, 0, 0), + endTime: Date.UTC(2025, 1, 1, 14, 0, 0), + 'Y Axis': 'first', + tooltip_column: 'tooltip value 1', + series: + 'This is a ridiculously long legend label that should switch to scroll', + }, + { + startTime: Date.UTC(2025, 1, 1, 18, 0, 0), + endTime: Date.UTC(2025, 1, 1, 20, 0, 0), + 'Y Axis': 'second', + tooltip_column: 'tooltip value 2', + series: 'short label', + }, + ], + colnames: [ + 'startTime', + 'endTime', + 'Y Axis', + 'tooltip_column', + 'series', + ], + }, + ], + }); + + const result = transformProps(props as EchartsGanttChartProps); + + expect((result.echartOptions.legend as any).type).toBe(LegendType.Scroll); + }); + + test('keeps legend visibility driven by showLegend for single-series charts', () => { + const props = new ChartProps({ + ...chartPropsConfig, + queriesData: [ + { + data: [queriesData[0].data[0]], + colnames: [ + 'startTime', + 'endTime', + 'Y Axis', + 'tooltip_column', + 'series', + ], + }, + ], + }); + + const result = transformProps(props as EchartsGanttChartProps); + + expect((result.echartOptions.legend as any).show).toBe(true); + }); }); 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 e8707d0c8c2..81b3f367084 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 @@ -19,8 +19,10 @@ import { AnnotationStyle, AnnotationType, + AnnotationSourceType, DataRecord, FormulaAnnotationLayer, + IntervalAnnotationLayer, VizType, ChartDataResponseResult, } from '@superset-ui/core'; @@ -366,6 +368,67 @@ test('legend margin: right orientation sets grid.right correctly', () => { expect((transformed.echartOptions.grid as any).right).toEqual(270); }); +test('should exclude unnamed annotation helper series from legend data', () => { + const interval: IntervalAnnotationLayer = { + annotationType: AnnotationType.Interval, + name: 'My Interval', + show: true, + showLabel: true, + sourceType: AnnotationSourceType.Table, + titleColumn: '', + timeColumn: 'start', + intervalEndColumn: 'end', + descriptionColumns: [], + style: AnnotationStyle.Dashed, + value: 2, + }; + + const annotationData = { + 'My Interval': { + columns: ['start', 'end', 'title'], + records: [ + { + start: 2000, + end: 3000, + title: 'My Title', + }, + ], + }, + }; + + const chartProps = createEchartsTimeseriesTestChartProps< + EchartsMixedTimeseriesFormData, + EchartsMixedTimeseriesProps + >({ + ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS, + defaultQueriesData: [], + formData: { + ...formData, + annotationLayers: [interval], + showLegend: true, + showQueryIdentifiers: true, + }, + queriesData: [ + createTestQueryData(defaultQueryRows, { + label_map: defaultLabelMap, + annotation_data: annotationData, + }), + createTestQueryData(defaultQueryRows, { + label_map: defaultLabelMap, + annotation_data: annotationData, + }), + ], + }); + const transformed = transformProps(chartProps); + + expect((transformed.echartOptions.legend as any).data).toEqual([ + 'sum__num (Query A), girl', + 'sum__num (Query A), boy', + 'sum__num (Query B), girl', + 'sum__num (Query B), boy', + ]); +}); + test('should add a formula annotation when X-axis column has dataset-level label', () => { const formula: FormulaAnnotationLayer = { name: 'My Formula', diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Pie/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Pie/transformProps.test.ts index 78b35ddbc65..5f2c508562c 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/Pie/transformProps.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/Pie/transformProps.test.ts @@ -29,6 +29,7 @@ import type { } from 'echarts/types/src/util/types'; import transformProps, { parseParams } from '../../src/Pie/transformProps'; import { EchartsPieChartProps, PieChartDataItem } from '../../src/Pie/types'; +import { LegendOrientation, LegendType } from '../../src/types'; describe('Pie transformProps', () => { const formData: SqlaFormData = { @@ -84,6 +85,51 @@ describe('Pie transformProps', () => { }), ); }); + + test('falls back to scroll for plain legends with overlong labels', () => { + const longLegendChartProps = new ChartProps({ + formData: { + colorScheme: 'bnbColors', + datasource: '3__table', + granularity_sqla: 'ds', + metric: 'sum__num', + groupby: ['category'], + viz_type: 'pie', + legendType: LegendType.Plain, + legendOrientation: LegendOrientation.Top, + showLegend: true, + } as SqlaFormData, + width: 320, + height: 600, + queriesData: [ + { + data: [ + { + category: 'This is a very long pie legend label one', + sum__num: 10, + }, + { + category: 'This is a very long pie legend label two', + sum__num: 20, + }, + { + category: 'This is a very long pie legend label three', + sum__num: 30, + }, + ], + }, + ], + theme: supersetTheme, + }); + + const transformed = transformProps( + longLegendChartProps as EchartsPieChartProps, + ); + + expect((transformed.echartOptions.legend as any).type).toBe( + LegendType.Scroll, + ); + }); }); describe('formatPieLabel', () => { 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 ec5bdac5a8c..4d87320849f 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 @@ -16,12 +16,62 @@ * specific language governing permissions and limitations * under the License. */ -import { ChartProps, SqlaFormData } from '@superset-ui/core'; +import { + ChartDataResponseResult, + ChartProps, + DataRecord, + SqlaFormData, +} from '@superset-ui/core'; +import { GenericDataType } from '@apache-superset/core/common'; import { supersetTheme } from '@apache-superset/core/theme'; -import { EchartsTimeseriesChartProps } from '../../../src/types'; +import type { + GridComponentOption, + LegendComponentOption, +} from 'echarts/components'; +import { + EchartsTimeseriesChartProps, + LegendOrientation, + LegendType, +} from '../../../src/types'; import transformProps from '../../../src/Timeseries/transformProps'; import { DEFAULT_FORM_DATA } from '../../../src/Timeseries/constants'; -import { EchartsTimeseriesSeriesType } from '../../../src/Timeseries/types'; +import { + EchartsTimeseriesFormData, + OrientationType, + EchartsTimeseriesSeriesType, +} from '../../../src/Timeseries/types'; +import { getPadding } from '../../../src/Timeseries/transformers'; +import { + getHorizontalLegendAvailableWidth, + getLegendLayoutResult, +} from '../../../src/utils/series'; +import { createEchartsTimeseriesTestChartProps } from '../../helpers'; + +function createTestQueryData( + data: DataRecord[], + overrides?: Partial, +): ChartDataResponseResult { + return { + annotation_data: null, + cache_key: null, + cache_timeout: null, + cached_dttm: null, + queried_dttm: null, + data, + colnames: [], + coltypes: [], + error: null, + is_cached: false, + query: '', + rowcount: data.length, + sql_rowcount: data.length, + stacktrace: null, + status: 'success', + from_dttm: null, + to_dttm: null, + ...overrides, + }; +} describe('Bar Chart X-axis Time Formatting', () => { const baseFormData: SqlaFormData = { @@ -570,6 +620,48 @@ describe('Bar Chart X-axis Time Formatting', () => { expect(legendData).toContain('C'); }); + test('should preserve source order for color-by-primary-axis legends when label sorting is enabled', () => { + const unsortedCategoricalData = [ + { + data: [ + { category: 'Zulu', value: 100 }, + { category: 'Alpha', value: 150 }, + { category: 'Mike', value: 200 }, + ], + colnames: ['category', 'value'], + coltypes: ['STRING', 'BIGINT'], + }, + ]; + + const formData = { + ...baseFormData, + colorByPrimaryAxis: true, + groupby: [], + legendSort: 'asc', + x_axis: 'category', + metric: 'value', + }; + + const chartProps = new ChartProps({ + ...baseChartPropsConfig, + queriesData: unsortedCategoricalData, + formData, + }); + + const transformedProps = transformProps( + chartProps as unknown as EchartsTimeseriesChartProps, + ); + + const legend = transformedProps.echartOptions.legend as { + data: { name: string }[]; + }; + expect(legend.data.map(item => item.name)).toEqual([ + 'Zulu', + 'Alpha', + 'Mike', + ]); + }); + test('should deduplicate legend entries when x-axis has repeated values', () => { const repeatedData = [ { @@ -634,4 +726,180 @@ describe('Bar Chart X-axis Time Formatting', () => { expect(hiddenSeries.length).toBe(3); }); }); + + describe('Legend layout regressions', () => { + const getBottomLegendLayout = ( + chartWidth: number, + legendItems: string[], + legendMargin?: string | number | null, + ) => + getLegendLayoutResult({ + availableWidth: getHorizontalLegendAvailableWidth({ + chartWidth, + orientation: LegendOrientation.Bottom, + padding: getPadding( + true, + LegendOrientation.Bottom, + false, + false, + legendMargin, + false, + undefined, + undefined, + undefined, + true, + ), + }), + chartHeight: baseChartPropsConfig.height, + chartWidth, + legendItems, + legendMargin, + orientation: LegendOrientation.Bottom, + show: true, + theme: supersetTheme, + type: LegendType.Plain, + }); + + test('should fall back to scroll for horizontal bottom legends after margin expansion reduces available width', () => { + const legendLabels = [ + 'This is a long sales legend', + 'This is a long marketing legend', + 'This is a long operations legend', + ]; + const longLegendData: ChartDataResponseResult[] = [ + createTestQueryData( + [ + { + [legendLabels[0]]: 100, + [legendLabels[1]]: null, + [legendLabels[2]]: null, + __timestamp: 1609459200000, + }, + { + [legendLabels[0]]: null, + [legendLabels[1]]: 150, + [legendLabels[2]]: null, + __timestamp: 1612137600000, + }, + { + [legendLabels[0]]: null, + [legendLabels[1]]: null, + [legendLabels[2]]: 200, + __timestamp: 1614556800000, + }, + ], + { + colnames: [...legendLabels, '__timestamp'], + coltypes: [ + GenericDataType.Numeric, + GenericDataType.Numeric, + GenericDataType.Numeric, + GenericDataType.Temporal, + ], + }, + ), + ]; + const regressionFormData: EchartsTimeseriesFormData = { + ...(baseFormData as EchartsTimeseriesFormData), + metric: legendLabels, + orientation: OrientationType.Horizontal, + legendOrientation: LegendOrientation.Bottom, + legendType: LegendType.Plain, + showLegend: true, + }; + const baselineChartProps = createEchartsTimeseriesTestChartProps< + EchartsTimeseriesFormData, + EchartsTimeseriesChartProps + >({ + defaultFormData: regressionFormData, + defaultVizType: 'echarts_timeseries_bar', + defaultQueriesData: longLegendData, + width: baseChartPropsConfig.width, + height: baseChartPropsConfig.height, + }); + const baselineTransformed = transformProps(baselineChartProps); + const legendItems = ( + (baselineTransformed.echartOptions.legend as LegendComponentOption) + .data as Array + ).map(item => (typeof item === 'string' ? item : item.name)); + let chartWidth: number | undefined; + let expandedLegendMargin: number | null = null; + + for (let width = 300; width <= 700; width += 1) { + const initialLayout = getBottomLegendLayout(width, legendItems, null); + + if (initialLayout.effectiveType !== LegendType.Plain) { + continue; + } + + const refinedLayout = getBottomLegendLayout( + width, + legendItems, + initialLayout.effectiveMargin ?? null, + ); + + if (refinedLayout.effectiveType === LegendType.Scroll) { + chartWidth = width; + expandedLegendMargin = initialLayout.effectiveMargin ?? null; + break; + } + } + + expect(chartWidth).toBeDefined(); + expect(expandedLegendMargin).not.toBeNull(); + const resolvedChartWidth = chartWidth ?? baseChartPropsConfig.width; + + const chartProps = createEchartsTimeseriesTestChartProps< + EchartsTimeseriesFormData, + EchartsTimeseriesChartProps + >({ + defaultFormData: regressionFormData, + defaultVizType: 'echarts_timeseries_bar', + defaultQueriesData: longLegendData, + width: resolvedChartWidth, + height: baseChartPropsConfig.height, + }); + + const transformedProps = transformProps(chartProps); + const legend = transformedProps.echartOptions + .legend as LegendComponentOption; + const grid = transformedProps.echartOptions.grid as GridComponentOption; + const expectedPadding = getPadding( + true, + LegendOrientation.Bottom, + false, + false, + null, + false, + undefined, + undefined, + undefined, + true, + ); + [expectedPadding.bottom, expectedPadding.left] = [ + expectedPadding.left, + expectedPadding.bottom, + ]; + const expandedPadding = getPadding( + true, + LegendOrientation.Bottom, + false, + false, + expandedLegendMargin, + false, + undefined, + undefined, + undefined, + true, + ); + [expandedPadding.bottom, expandedPadding.left] = [ + expandedPadding.left, + expandedPadding.bottom, + ]; + + expect(legend.type).toBe(LegendType.Scroll); + expect(grid.bottom).toBe(expectedPadding.bottom); + expect(grid.bottom).not.toBe(expandedPadding.bottom); + }); + }); }); diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts index 9020c811871..9e147a4f245 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts @@ -37,7 +37,8 @@ import { OrientationType, EchartsTimeseriesFormData, } from '../../src/Timeseries/types'; -import { StackControlsValue } from '../../src/constants'; +import { StackControlsValue, TIMESERIES_CONSTANTS } from '../../src/constants'; +import { LegendOrientation, LegendType } from '../../src/types'; import { DEFAULT_FORM_DATA } from '../../src/Timeseries/constants'; import { createEchartsTimeseriesTestChartProps } from '../helpers'; import { BASE_TIMESTAMP, createTestData } from './helpers'; @@ -898,6 +899,40 @@ describe('legend sorting', () => { 'Boston', ]); }); + + test('falls back to scroll for zoomable top legends when toolbox space reduces available width', () => { + const narrowLegendData = [ + createTestQueryData( + createTestData( + [ + { + Alpha: 1, + Beta: 2, + Gamma: 3, + }, + ], + { intervalMs: 300000000 }, + ), + ), + ]; + const chartProps = createTestChartProps({ + width: 190 + TIMESERIES_CONSTANTS.legendTopRightOffset, + formData: { + ...formData, + legendType: LegendType.Plain, + legendOrientation: LegendOrientation.Top, + showLegend: true, + zoomable: true, + }, + queriesData: narrowLegendData, + }); + + const transformed = transformProps(chartProps); + + expect((transformed.echartOptions.legend as any).type).toBe( + LegendType.Scroll, + ); + }); }); const timeCompareFormData: SqlaFormData = { diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts index 88c58f34837..f50b1933d5d 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { SortSeriesType } from '@superset-ui/chart-controls'; +import { LegendPaddingType, SortSeriesType } from '@superset-ui/chart-controls'; import { AxisType, DataRecord, @@ -51,6 +51,71 @@ import { import { defaultLegendPadding } from '../../src/defaults'; import { NULL_STRING } from '../../src/constants'; +const { + getHorizontalLegendAvailableWidth, + getLegendLayoutResult, +}: { + getHorizontalLegendAvailableWidth: (args: { + chartWidth: number; + orientation: LegendOrientation.Top | LegendOrientation.Bottom; + padding?: LegendPaddingType; + zoomable?: boolean; + }) => number; + getLegendLayoutResult: (args: { + availableHeight?: number; + availableWidth?: number; + chartHeight: number; + chartWidth: number; + legendItems?: ( + | string + | number + | null + | undefined + | { name?: string | number | null } + )[]; + legendMargin?: string | number | null; + orientation: LegendOrientation; + show: boolean; + showSelectors?: boolean; + theme: typeof theme; + type: LegendType; + }) => { + effectiveMargin?: number; + effectiveType: LegendType; + }; +} = require('../../src/utils/series'); + +const { + resolveLegendLayout, +}: { + resolveLegendLayout: (args: { + availableHeight?: number; + availableWidth?: number; + chartHeight: number; + chartWidth: number; + legendItems?: ( + | string + | number + | null + | undefined + | { name?: string | number | null } + )[]; + legendMargin?: string | number | null; + orientation: LegendOrientation; + show: boolean; + showSelectors?: boolean; + theme: typeof theme; + type: LegendType; + }) => { + effectiveLegendMargin?: string | number | null; + effectiveLegendType: LegendType; + legendLayout: { + effectiveMargin?: number; + effectiveType: LegendType; + }; + }; +} = require('../../src/utils/legendLayout'); + const expectedThemeProps = { selector: ['all', 'inverse'], selected: {}, @@ -891,19 +956,20 @@ describe('getLegendProps', () => { }); }); - test('should default plain legends to scroll for bottom orientation', () => { + test('should return the correct props for plain type with bottom orientation', () => { expect( getLegendProps(LegendType.Plain, LegendOrientation.Bottom, false, theme), ).toEqual({ show: false, bottom: 0, + right: 0, orient: 'horizontal', - type: 'scroll', + type: 'plain', ...expectedThemeProps, }); }); - test('should default plain legends to scroll for top orientation', () => { + test('should return the correct props for plain type with top orientation', () => { expect( getLegendProps(LegendType.Plain, LegendOrientation.Top, false, theme), ).toEqual({ @@ -911,12 +977,248 @@ describe('getLegendProps', () => { top: 0, right: 0, orient: 'horizontal', - type: 'scroll', + type: 'plain', ...expectedThemeProps, }); }); }); +test('getLegendLayoutResult keeps plain horizontal legends when they fit within two rows', () => { + expect( + getLegendLayoutResult({ + chartHeight: 400, + chartWidth: 800, + legendItems: ['Alpha', 'Beta', 'Gamma', 'Delta'], + legendMargin: null, + orientation: LegendOrientation.Top, + show: true, + theme, + type: LegendType.Plain, + }), + ).toEqual({ + effectiveMargin: defaultLegendPadding[LegendOrientation.Top], + effectiveType: LegendType.Plain, + }); +}); + +test('getLegendLayoutResult adds extra margin for wrapped plain horizontal legends', () => { + const layout = getLegendLayoutResult({ + chartHeight: 400, + chartWidth: 640, + legendItems: [ + 'This is a long legend label', + 'Another long legend label', + 'Third long legend label', + ], + legendMargin: null, + orientation: LegendOrientation.Top, + show: true, + theme, + type: LegendType.Plain, + }); + + expect(layout).toMatchObject({ + effectiveType: LegendType.Plain, + }); + expect(layout.effectiveMargin).toBeGreaterThan( + defaultLegendPadding[LegendOrientation.Top], + ); +}); + +test('getLegendLayoutResult falls back to scroll when horizontal plain legends exceed two rows', () => { + expect( + getLegendLayoutResult({ + chartHeight: 400, + chartWidth: 240, + legendItems: [ + 'This is a long legend label', + 'Another long legend label', + 'Third long legend label', + ], + legendMargin: null, + orientation: LegendOrientation.Top, + show: true, + theme, + type: LegendType.Plain, + }), + ).toEqual({ + effectiveType: LegendType.Scroll, + }); +}); + +test('getLegendLayoutResult falls back to scroll when a single horizontal plain legend item exceeds available width', () => { + expect( + getLegendLayoutResult({ + chartHeight: 400, + chartWidth: 260, + legendItems: [ + 'This is a ridiculously long legend label that should not fit on one line', + ], + legendMargin: null, + orientation: LegendOrientation.Top, + show: true, + theme, + type: LegendType.Plain, + }), + ).toEqual({ + effectiveType: LegendType.Scroll, + }); +}); + +test('getLegendLayoutResult falls back to scroll when reserved horizontal width reduces plain legend capacity', () => { + const availableWidth = getHorizontalLegendAvailableWidth({ + chartWidth: 265, + orientation: LegendOrientation.Top, + padding: { left: 20 }, + zoomable: true, + }); + + expect( + getLegendLayoutResult({ + availableWidth, + chartHeight: 400, + chartWidth: 265, + legendItems: ['Alpha', 'Beta', 'Gamma'], + legendMargin: null, + orientation: LegendOrientation.Top, + show: true, + theme, + type: LegendType.Plain, + }), + ).toEqual({ + effectiveType: LegendType.Scroll, + }); +}); + +test('getLegendLayoutResult falls back to scroll when horizontal legend selectors alone exceed available width', () => { + expect( + getLegendLayoutResult({ + chartHeight: 400, + chartWidth: 95, + legendItems: ['A'], + legendMargin: null, + orientation: LegendOrientation.Top, + show: true, + theme, + type: LegendType.Plain, + }), + ).toEqual({ + effectiveType: LegendType.Scroll, + }); +}); + +test('getLegendLayoutResult keeps plain vertical legends when they fit within a single column', () => { + expect( + getLegendLayoutResult({ + chartHeight: 400, + chartWidth: 800, + legendItems: ['Alpha', 'Beta', 'Gamma'], + legendMargin: null, + orientation: LegendOrientation.Left, + show: true, + theme, + type: LegendType.Plain, + }), + ).toEqual({ + effectiveMargin: defaultLegendPadding[LegendOrientation.Left], + effectiveType: LegendType.Plain, + }); +}); + +test('getLegendLayoutResult adds extra margin for wide vertical plain legends', () => { + const layout = getLegendLayoutResult({ + chartHeight: 400, + chartWidth: 800, + legendItems: ['This is a very long legend label'], + legendMargin: null, + orientation: LegendOrientation.Left, + show: true, + theme, + type: LegendType.Plain, + }); + + expect(layout).toMatchObject({ + effectiveType: LegendType.Plain, + }); + expect(layout.effectiveMargin).toBeGreaterThan( + defaultLegendPadding[LegendOrientation.Left], + ); +}); + +test('getLegendLayoutResult falls back to scroll when vertical plain legends exceed one column', () => { + expect( + getLegendLayoutResult({ + chartHeight: 160, + chartWidth: 800, + legendItems: ['Alpha', 'Beta', 'Gamma', 'Delta', 'Epsilon'], + legendMargin: null, + orientation: LegendOrientation.Left, + show: true, + theme, + type: LegendType.Plain, + }), + ).toEqual({ + effectiveType: LegendType.Scroll, + }); +}); + +test('getLegendLayoutResult falls back to scroll when vertical plain legend selectors exceed available width', () => { + expect( + getLegendLayoutResult({ + chartHeight: 400, + chartWidth: 300, + legendItems: ['A', 'B', 'C'], + legendMargin: null, + orientation: LegendOrientation.Left, + show: true, + theme, + type: LegendType.Plain, + }), + ).toEqual({ + effectiveType: LegendType.Scroll, + }); +}); + +test('getLegendLayoutResult counts empty-string legend labels when estimating layout', () => { + expect( + getLegendLayoutResult({ + chartHeight: 400, + chartWidth: 116, + legendItems: ['', 'A', 'B', 'C', 'D'], + legendMargin: null, + orientation: LegendOrientation.Top, + show: true, + showSelectors: false, + theme, + type: LegendType.Plain, + }), + ).toEqual({ + effectiveType: LegendType.Scroll, + }); +}); + +test('resolveLegendLayout returns both raw and effective legend layout values', () => { + expect( + resolveLegendLayout({ + chartHeight: 400, + chartWidth: 800, + legendItems: ['Alpha', 'Beta'], + legendMargin: null, + orientation: LegendOrientation.Top, + show: true, + theme, + type: LegendType.Plain, + }), + ).toEqual({ + effectiveLegendMargin: defaultLegendPadding[LegendOrientation.Top], + effectiveLegendType: LegendType.Plain, + legendLayout: { + effectiveMargin: defaultLegendPadding[LegendOrientation.Top], + effectiveType: LegendType.Plain, + }, + }); +}); + describe('getChartPadding', () => { test('should handle top default', () => { expect(getChartPadding(true, LegendOrientation.Top)).toEqual({