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/18 23:16:11 UTC
(superset) branch fix-plain-legend-overlap-timeseries created (now 32da7f3b74d)
This is an automated email from the ASF dual-hosted git repository.
sadpandajoe pushed a change to branch fix-plain-legend-overlap-timeseries
in repository https://gitbox.apache.org/repos/asf/superset.git
at 32da7f3b74d fix(echarts): scope plain legend floor to timeseries
This branch includes the following new commits:
new 506424dd74a fix(echarts): reserve full margin for plain legends
new 07717dc89ef fix(echarts): account for non-legend chart padding
new 32da7f3b74d fix(echarts): scope plain legend floor to timeseries
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 01/03: fix(echarts): reserve full margin for plain legends
Posted by jl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sadpandajoe pushed a commit to branch fix-plain-legend-overlap-timeseries
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 506424dd74abd9c098eb95fb3f5b482dda25fbc8
Author: sadpandajoe <jc...@gmail.com>
AuthorDate: Tue Aug 18 22:28:31 2026 +0000
fix(echarts): reserve full margin for plain legends
---
RCA.md | 75 ++++++++++++++++++++++
.../plugin-chart-echarts/src/utils/series.ts | 7 +-
.../plugin-chart-echarts/test/utils/series.test.ts | 25 +++++++-
3 files changed, 103 insertions(+), 4 deletions(-)
diff --git a/RCA.md b/RCA.md
new file mode 100644
index 00000000000..2f08d431c46
--- /dev/null
+++ b/RCA.md
@@ -0,0 +1,75 @@
+## What Happened
+
+- **Verified:** A Time-Series chart with a top-oriented Plain legend and many
+ grouped series rendered the legend across more rows than the chart grid had
+ reserved.
+- **Verified:** The expected behavior was for the grid to begin below every
+ legend row. Instead, legend rows beyond the reserved margin were painted over
+ the plot area.
+
+## Root Cause
+
+- **Verified:** `superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts`
+ estimated the number of horizontal legend rows and calculated their full
+ `requiredMargin`, but limited that margin to 40% of the available chart
+ height. The same path always preserved `LegendType.Plain`, so ECharts rendered
+ every item without pagination even when the reserved grid margin was smaller
+ than the rendered legend.
+- **Verified:**
+ `superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts`
+ passes the resolved legend margin through `getPadding`, which becomes the
+ chart grid padding, and passes the preserved Plain type through
+ `getLegendProps`. The underestimated margin therefore directly exposed the
+ plot to the remaining legend rows.
+
+## Why It Wasn't Caught
+
+- **Verified:** Existing regression coverage ensured that an explicit Plain
+ selection was not silently changed to Scroll, but its representative legends
+ either fit or did not assert enough margin for all estimated rows.
+- **Verified:** The existing overflow test asserted the 40%-of-height cap
+ itself, so it encoded the behavior that caused the overlap rather than the
+ minimum plot-space constraint the cap was intended to provide.
+- **Inferred:** The layout tests treated preserving plot area and preserving the
+ selected legend type as separate concerns, leaving the interaction between an
+ unpaginated Plain legend and a capped grid margin uncovered.
+
+## The Fix
+
+- **Verified:**
+ `superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts` replaces
+ the horizontal 40% cap with an 80px minimum plot-space floor. It reserves the
+ full estimated legend margin whenever that leaves at least 80px for the plot;
+ for larger legends, it reserves all available height except that floor.
+- **Verified:** The horizontal path continues to return `LegendType.Plain`; no
+ Plain-to-Scroll fallback was added.
+- **Verified:**
+ `superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts` adds
+ a 40-item, 800-by-400 regression case whose estimated eleven rows require a
+ 260px margin, and updates the small-chart overflow expectation to preserve an
+ 80px plot area.
+- **Verified:** Vertical Plain legend layout is unchanged.
+
+## Latent Bugs Found
+
+- **Verified:** `getVerticalPlainLegendLayout` sizes left/right margins from the
+ longest label and selector width but does not account for the number of
+ vertically stacked items. A tall vertical Plain legend can therefore extend
+ beyond the plot. This is a separate orientation-specific defect and remains
+ unchanged.
+- **Verified:** The bottom-horizontal special case for
+ `effectiveLegendType === LegendType.Scroll` in
+ `superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts`
+ is no longer reachable as a Plain-to-Scroll fallback because Plain is always
+ preserved. It is still reachable for an explicitly selected Scroll legend,
+ so it is not wholly dead. Its fallback-specific rationale is stale, but the
+ branch remains unchanged to avoid broadening this fix.
+
+## Prevention
+
+- **Verified:** The new regression test ties the reserved horizontal margin to
+ the estimator's complete row count at realistic chart dimensions while also
+ asserting that the user-selected Plain type is preserved.
+- **Inferred:** Future legend layout changes should test the selected legend
+ type, the estimated item capacity, and the resulting grid margin together,
+ including a case that crosses any plot-preservation boundary.
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 cdfaaf78c18..8e90cf1eca3 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
@@ -60,7 +60,7 @@ const DEFAULT_LEGEND_ICON_WIDTH = 25;
const LEGEND_ICON_LABEL_GAP = 5;
const LEGEND_HORIZONTAL_SIDE_GUTTER = 16;
const LEGEND_HORIZONTAL_ROW_HEIGHT = 24;
-// Cap the reserved horizontal legend margin so an overflowing legend can't eat the plot.
+const MIN_HORIZONTAL_LEGEND_PLOT_SPACE = 80;
const MAX_LEGEND_MARGIN_RATIO = 0.4;
const LEGEND_VERTICAL_SIDE_GUTTER = 16;
const LEGEND_SELECTOR_GAP = 10;
@@ -271,7 +271,10 @@ function getHorizontalPlainLegendLayout({
Math.max(0, rowsForMargin - 1) * LEGEND_HORIZONTAL_ROW_HEIGHT;
const boundedMargin =
availableHeight > 0
- ? Math.min(requiredMargin, availableHeight * MAX_LEGEND_MARGIN_RATIO)
+ ? Math.min(
+ requiredMargin,
+ Math.max(availableHeight - MIN_HORIZONTAL_LEGEND_PLOT_SPACE, 0),
+ )
: requiredMargin;
return {
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 32f16acfe85..19104d6f864 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
@@ -1248,6 +1248,27 @@ test('getLegendLayoutResult keeps plain when horizontal plain legends exceed two
expect(layout.effectiveMargin).toBe(68);
});
+test('getLegendLayoutResult reserves every row for many horizontal plain legend items', () => {
+ const layout = getLegendLayoutResult({
+ chartHeight: 400,
+ chartWidth: 800,
+ legendItems: Array.from(
+ { length: 40 },
+ (_, index) => `Country ${index + 1}, Product`,
+ ),
+ legendMargin: null,
+ orientation: LegendOrientation.Top,
+ show: true,
+ theme,
+ type: LegendType.Plain,
+ });
+
+ expect(layout.effectiveType).toBe(LegendType.Plain);
+ // The estimator packs the items into ten rows and the selectors into an
+ // eleventh: 20px base padding + 10 additional 24px rows.
+ expect(layout.effectiveMargin).toBe(260);
+});
+
test('getLegendLayoutResult bounds reserved margin for overflowing horizontal legends so the plot is not collapsed', () => {
const chartHeight = 200;
const layout = getLegendLayoutResult({
@@ -1263,8 +1284,8 @@ test('getLegendLayoutResult bounds reserved margin for overflowing horizontal le
expect(layout.effectiveType).toBe(LegendType.Plain);
expect(Number.isFinite(layout.effectiveMargin)).toBe(true);
- // 40% of the 200px chart height.
- expect(layout.effectiveMargin).toBe(80);
+ // Reserve all but the 80px minimum plot space.
+ expect(layout.effectiveMargin).toBe(120);
});
test('getLegendLayoutResult bounds reserved margin for long vertical legend labels so the plot is not collapsed', () => {
(superset) 02/03: fix(echarts): account for non-legend chart padding
Posted by jl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sadpandajoe pushed a commit to branch fix-plain-legend-overlap-timeseries
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 07717dc89ef77258cddd20662be4a102883bb142
Author: sadpandajoe <jc...@gmail.com>
AuthorDate: Tue Aug 18 22:47:31 2026 +0000
fix(echarts): account for non-legend chart padding
---
RCA.md | 13 +++++--
.../src/Timeseries/transformProps.ts | 16 +++++++++
.../plugin-chart-echarts/src/utils/series.ts | 12 +++----
.../test/Timeseries/transformProps.test.ts | 40 ++++++++++++++++++++++
4 files changed, 72 insertions(+), 9 deletions(-)
diff --git a/RCA.md b/RCA.md
index 2f08d431c46..b93969d4257 100644
--- a/RCA.md
+++ b/RCA.md
@@ -39,8 +39,14 @@
- **Verified:**
`superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts` replaces
the horizontal 40% cap with an 80px minimum plot-space floor. It reserves the
- full estimated legend margin whenever that leaves at least 80px for the plot;
- for larger legends, it reserves all available height except that floor.
+ full estimated legend margin whenever that leaves at least 80px of genuinely
+ available height for the plot; for larger legends, it reserves the available
+ height except that floor.
+- **Verified:**
+ `superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts`
+ calculates the top and bottom padding required without a legend, including
+ zoom controls and axis-title offsets, and excludes those fixed reservations
+ before applying the plot-space floor.
- **Verified:** The horizontal path continues to return `LegendType.Plain`; no
Plain-to-Scroll fallback was added.
- **Verified:**
@@ -48,6 +54,9 @@
a 40-item, 800-by-400 regression case whose estimated eleven rows require a
260px margin, and updates the small-chart overflow expectation to preserve an
80px plot area.
+- **Verified:** A transform-level regression test covers a 200px-tall zoomable
+ chart with a many-item top Plain legend and verifies that the resulting grid
+ retains 80px of plot height after both top and bottom padding.
- **Verified:** Vertical Plain legend layout is unchanged.
## Latent Bugs Found
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 43e9a2f431b..e8daba62ae0 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@@ -1122,6 +1122,18 @@ export default function transformProps(
name,
icon: 'roundRect',
}));
+ const nonLegendPadding = getPadding(
+ false,
+ legendOrientation,
+ addYAxisLabelOffset,
+ zoomable,
+ undefined,
+ addXAxisLabelOffset,
+ yAxisTitlePosition,
+ convertInteger(yAxisTitleMargin),
+ convertInteger(xAxisTitleMargin),
+ isHorizontal,
+ );
const getLegendLayout = (candidateLegendMargin?: string | number | null) => {
const padding = getPadding(
showLegend,
@@ -1137,6 +1149,10 @@ export default function transformProps(
);
return resolveLegendLayout({
+ availableHeight: Math.max(
+ height - nonLegendPadding.top - nonLegendPadding.bottom,
+ 0,
+ ),
availableWidth:
legendOrientation === LegendOrientation.Top ||
legendOrientation === LegendOrientation.Bottom
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 8e90cf1eca3..cae2e11535a 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
@@ -60,6 +60,7 @@ const DEFAULT_LEGEND_ICON_WIDTH = 25;
const LEGEND_ICON_LABEL_GAP = 5;
const LEGEND_HORIZONTAL_SIDE_GUTTER = 16;
const LEGEND_HORIZONTAL_ROW_HEIGHT = 24;
+// Smallest plot height that still preserves the chart's visible structure.
const MIN_HORIZONTAL_LEGEND_PLOT_SPACE = 80;
const MAX_LEGEND_MARGIN_RATIO = 0.4;
const LEGEND_VERTICAL_SIDE_GUTTER = 16;
@@ -269,13 +270,10 @@ function getHorizontalPlainLegendLayout({
const requiredMargin =
defaultLegendPadding[orientation] +
Math.max(0, rowsForMargin - 1) * LEGEND_HORIZONTAL_ROW_HEIGHT;
- const boundedMargin =
- availableHeight > 0
- ? Math.min(
- requiredMargin,
- Math.max(availableHeight - MIN_HORIZONTAL_LEGEND_PLOT_SPACE, 0),
- )
- : requiredMargin;
+ const boundedMargin = Math.min(
+ requiredMargin,
+ Math.max(availableHeight - MIN_HORIZONTAL_LEGEND_PLOT_SPACE, 0),
+ );
return {
effectiveMargin: Math.max(currentMargin, boundedMargin),
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 ff44aa7d3b6..7968bc580c6 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,6 +37,7 @@ import {
import { GenericDataType } from '@apache-superset/core/common';
import { supersetTheme } from '@apache-superset/core/theme';
import type { SeriesOption } from 'echarts';
+import type { GridComponentOption } from 'echarts/components';
import transformProps from '../../src/Timeseries/transformProps';
import {
EchartsTimeseriesSeriesType,
@@ -1035,6 +1036,45 @@ test('honors an explicit List selection for zoomable top legends even when toolb
expect((transformed.echartOptions.legend as any).type).toBe(LegendType.Plain);
});
+test('preserves plot space for a small zoomable chart with a many-item Plain legend', () => {
+ const chartHeight = 200;
+ const manyLegendValues = Object.fromEntries(
+ Array.from({ length: 40 }, (_, index) => [
+ `Country ${index + 1}, Product`,
+ index + 1,
+ ]),
+ );
+ const chartProps = createTestChartProps({
+ height: chartHeight,
+ formData: {
+ ...formData,
+ legendType: LegendType.Plain,
+ legendOrientation: LegendOrientation.Top,
+ showLegend: true,
+ zoomable: true,
+ yAxisTitleMargin: 0,
+ yAxisTitlePosition: 'Left',
+ },
+ queriesData: [
+ createTestQueryData(
+ createTestData([manyLegendValues], { intervalMs: 300000000 }),
+ ),
+ ],
+ });
+
+ const transformed = transformProps(chartProps);
+ const grid = transformed.echartOptions.grid as GridComponentOption;
+
+ expect((transformed.echartOptions.legend as { type?: LegendType }).type).toBe(
+ LegendType.Plain,
+ );
+ expect(typeof grid.top).toBe('number');
+ expect(typeof grid.bottom).toBe('number');
+ expect(
+ chartHeight - (Number(grid.top) + Number(grid.bottom)),
+ ).toBeGreaterThanOrEqual(80);
+});
+
test('honors user-selected plain legend type for top orientation when space allows (#39540)', () => {
// Regression test for issue #39540: switching the legend type control from
// scroll to plain must reach the rendered ECharts config. Horizontal legends
(superset) 03/03: fix(echarts): scope plain legend floor to timeseries
Posted by jl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sadpandajoe pushed a commit to branch fix-plain-legend-overlap-timeseries
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 32da7f3b74da6ebc750769c3ccd1df92d4100469
Author: sadpandajoe <jc...@gmail.com>
AuthorDate: Tue Aug 18 23:01:59 2026 +0000
fix(echarts): scope plain legend floor to timeseries
---
RCA.md | 26 ++++++++----
.../src/Timeseries/transformProps.ts | 8 ++--
.../plugin-chart-echarts/src/utils/legendLayout.ts | 3 ++
.../plugin-chart-echarts/src/utils/series.ts | 28 +++++++++++--
.../test/MixedTimeseries/transformProps.test.ts | 48 ++++++++++++++++++++++
.../test/Timeseries/transformProps.test.ts | 41 ++++++++++++++++++
.../plugin-chart-echarts/test/utils/series.test.ts | 6 ++-
7 files changed, 143 insertions(+), 17 deletions(-)
diff --git a/RCA.md b/RCA.md
index b93969d4257..bcbd652179f 100644
--- a/RCA.md
+++ b/RCA.md
@@ -37,16 +37,17 @@
## The Fix
- **Verified:**
- `superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts` replaces
- the horizontal 40% cap with an 80px minimum plot-space floor. It reserves the
- full estimated legend margin whenever that leaves at least 80px of genuinely
- available height for the plot; for larger legends, it reserves the available
- height except that floor.
+ `superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts` adds an
+ opt-in 80px minimum plot-space floor for callers that provide their fixed,
+ non-legend height reservation. It reserves the full estimated legend margin
+ whenever that leaves the floor intact and otherwise uses the remaining space.
+ Callers without an accurate reservation retain the previous 40% ratio cap.
- **Verified:**
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts`
calculates the top and bottom padding required without a legend, including
- zoom controls and axis-title offsets, and excludes those fixed reservations
- before applying the plot-space floor.
+ zoom controls and axis-title offsets. For horizontal-axis charts it uses the
+ left padding that becomes bottom padding after the axis swap, then excludes
+ those final-axis fixed reservations before applying the plot-space floor.
- **Verified:** The horizontal path continues to return `LegendType.Plain`; no
Plain-to-Scroll fallback was added.
- **Verified:**
@@ -57,6 +58,10 @@
- **Verified:** A transform-level regression test covers a 200px-tall zoomable
chart with a many-item top Plain legend and verifies that the resulting grid
retains 80px of plot height after both top and bottom padding.
+- **Verified:** A second transform-level regression covers the same constraint
+ after horizontal axes swap left and bottom padding.
+- **Verified:** Mixed Timeseries coverage pins the pre-existing 40% fallback for
+ shared-helper callers that have not opted into the floor-based calculation.
- **Verified:** Vertical Plain legend layout is unchanged.
## Latent Bugs Found
@@ -73,6 +78,10 @@
preserved. It is still reachable for an explicitly selected Scroll legend,
so it is not wholly dead. Its fallback-specific rationale is stale, but the
branch remains unchanged to avoid broadening this fix.
+- **Verified:** Mixed Timeseries, Gantt, Pie, Funnel, Radar, Bubble, and Graph
+ share the legend-layout resolver but do not provide chart-specific fixed
+ height reservations. They retain the legacy ratio cap until each caller can
+ be migrated with its own layout accounting.
## Prevention
@@ -81,4 +90,5 @@
asserting that the user-selected Plain type is preserved.
- **Inferred:** Future legend layout changes should test the selected legend
type, the estimated item capacity, and the resulting grid margin together,
- including a case that crosses any plot-preservation boundary.
+ including a case that crosses any plot-preservation boundary and a sibling
+ caller that must retain legacy behavior.
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 e8daba62ae0..6ce86ed77c0 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@@ -1134,6 +1134,9 @@ export default function transformProps(
convertInteger(xAxisTitleMargin),
isHorizontal,
);
+ const nonLegendReservedHeight =
+ nonLegendPadding.top +
+ (isHorizontal ? nonLegendPadding.left : nonLegendPadding.bottom);
const getLegendLayout = (candidateLegendMargin?: string | number | null) => {
const padding = getPadding(
showLegend,
@@ -1149,10 +1152,6 @@ export default function transformProps(
);
return resolveLegendLayout({
- availableHeight: Math.max(
- height - nonLegendPadding.top - nonLegendPadding.bottom,
- 0,
- ),
availableWidth:
legendOrientation === LegendOrientation.Top ||
legendOrientation === LegendOrientation.Bottom
@@ -1170,6 +1169,7 @@ export default function transformProps(
? colorByPrimaryAxisLegendData
: sortedLegendData,
legendMargin: candidateLegendMargin,
+ nonLegendReservedHeight,
orientation: legendOrientation,
show: showLegend,
showSelectors: !(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
index b33caa235f6..7204210d491 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/legendLayout.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/legendLayout.ts
@@ -80,6 +80,9 @@ export function resolveLegendLayout(args: {
chartWidth: number;
legendItems?: LegendDataItem[];
legendMargin?: string | number | null;
+ // Only Timeseries opts into floor-based layout until sibling callers can
+ // provide their own accurate fixed-height reservations.
+ nonLegendReservedHeight?: number;
orientation: LegendOrientation;
show: boolean;
showSelectors?: boolean;
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 cae2e11535a..f613ba86cf8 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
@@ -246,6 +246,7 @@ function getHorizontalPlainLegendLayout({
availableWidth,
currentMargin,
legendLabels,
+ nonLegendReservedHeight,
orientation,
showSelectors,
theme,
@@ -254,6 +255,7 @@ function getHorizontalPlainLegendLayout({
availableWidth: number;
currentMargin: number;
legendLabels: string[];
+ nonLegendReservedHeight?: number;
orientation: LegendOrientation.Top | LegendOrientation.Bottom;
showSelectors: boolean;
theme: SupersetTheme;
@@ -270,10 +272,23 @@ function getHorizontalPlainLegendLayout({
const requiredMargin =
defaultLegendPadding[orientation] +
Math.max(0, rowsForMargin - 1) * LEGEND_HORIZONTAL_ROW_HEIGHT;
- const boundedMargin = Math.min(
- requiredMargin,
- Math.max(availableHeight - MIN_HORIZONTAL_LEGEND_PLOT_SPACE, 0),
- );
+ let boundedMargin: number;
+ if (nonLegendReservedHeight === undefined) {
+ boundedMargin =
+ availableHeight > 0
+ ? Math.min(requiredMargin, availableHeight * MAX_LEGEND_MARGIN_RATIO)
+ : requiredMargin;
+ } else {
+ boundedMargin = Math.min(
+ requiredMargin,
+ Math.max(
+ availableHeight -
+ nonLegendReservedHeight -
+ MIN_HORIZONTAL_LEGEND_PLOT_SPACE,
+ 0,
+ ),
+ );
+ }
return {
effectiveMargin: Math.max(currentMargin, boundedMargin),
@@ -328,6 +343,7 @@ export function getLegendLayoutResult({
chartWidth,
legendItems = [],
legendMargin,
+ nonLegendReservedHeight,
orientation,
show,
showSelectors = true,
@@ -342,6 +358,9 @@ export function getLegendLayoutResult({
chartWidth: number;
legendItems?: LegendDataItem[];
legendMargin?: string | number | null;
+ // The floor-based layout is opt-in while only Timeseries supplies accurate
+ // fixed padding; callers that omit it retain the legacy ratio cap.
+ nonLegendReservedHeight?: number;
orientation: LegendOrientation;
show: boolean;
showSelectors?: boolean;
@@ -366,6 +385,7 @@ export function getLegendLayoutResult({
availableWidth: resolvedAvailableWidth,
currentMargin: resolvedLegendMargin,
legendLabels,
+ nonLegendReservedHeight,
orientation,
showSelectors,
theme,
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 d2f8b584ef0..dfa9b2f5e14 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
@@ -43,6 +43,7 @@ import {
} from '../../src/MixedTimeseries/types';
import { createEchartsTimeseriesTestChartProps } from '../helpers';
import type { SeriesOption } from 'echarts';
+import type { GridComponentOption } from 'echarts/components';
type LabelFormatterParams = {
value: [number, number];
@@ -511,6 +512,53 @@ test('legend margin: top orientation sets grid.top correctly', () => {
expect((transformed.echartOptions.grid as any).top).toEqual(270);
});
+test('dense Plain legends retain the legacy margin cap for Mixed Timeseries', () => {
+ const seriesNames = Array.from(
+ { length: 40 },
+ (_, index) => `Country ${index + 1}, Product`,
+ );
+ const row = {
+ ds: 599616000000,
+ ...Object.fromEntries(
+ seriesNames.map((seriesName, index) => [seriesName, index + 1]),
+ ),
+ };
+ const labelMap = {
+ ds: ['ds'],
+ ...Object.fromEntries(
+ seriesNames.map(seriesName => [seriesName, [seriesName]]),
+ ),
+ };
+ const denseQueriesData = [
+ createTestQueryData([row], { label_map: labelMap }),
+ createTestQueryData([row], { label_map: labelMap }),
+ ];
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: denseQueriesData,
+ height: 200,
+ formData: {
+ ...formData,
+ legendType: LegendType.Plain,
+ showLegend: true,
+ zoomable: true,
+ },
+ queriesData: denseQueriesData,
+ });
+
+ const transformed = transformProps(chartProps);
+ const grid = transformed.echartOptions.grid as GridComponentOption;
+
+ expect((transformed.echartOptions.legend as { type?: LegendType }).type).toBe(
+ LegendType.Plain,
+ );
+ // Legacy behavior reserves 40% of 200px plus the fixed 20px top offset.
+ expect(grid.top).toBe(100);
+});
+
test('legend margin: bottom orientation sets grid.bottom correctly', () => {
const chartProps = createEchartsTimeseriesTestChartProps<
EchartsMixedTimeseriesFormData,
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 7968bc580c6..f69e1e64033 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
@@ -1075,6 +1075,47 @@ test('preserves plot space for a small zoomable chart with a many-item Plain leg
).toBeGreaterThanOrEqual(80);
});
+test('uses post-swap padding to preserve plot space for horizontal zoomable charts', () => {
+ const chartHeight = 200;
+ const manyLegendValues = Object.fromEntries(
+ Array.from({ length: 40 }, (_, index) => [
+ `Country ${index + 1}, Product`,
+ index + 1,
+ ]),
+ );
+ const chartProps = createTestChartProps({
+ height: chartHeight,
+ formData: {
+ ...formData,
+ legendType: LegendType.Plain,
+ legendOrientation: LegendOrientation.Top,
+ orientation: OrientationType.Horizontal,
+ showLegend: true,
+ zoomable: true,
+ yAxisTitle: 'Revenue',
+ yAxisTitleMargin: 50,
+ yAxisTitlePosition: 'Left',
+ },
+ queriesData: [
+ createTestQueryData(
+ createTestData([manyLegendValues], { intervalMs: 300000000 }),
+ ),
+ ],
+ });
+
+ const transformed = transformProps(chartProps);
+ const grid = transformed.echartOptions.grid as GridComponentOption;
+
+ expect((transformed.echartOptions.legend as { type?: LegendType }).type).toBe(
+ LegendType.Plain,
+ );
+ expect(typeof grid.top).toBe('number');
+ expect(typeof grid.bottom).toBe('number');
+ expect(
+ chartHeight - (Number(grid.top) + Number(grid.bottom)),
+ ).toBeGreaterThanOrEqual(80);
+});
+
test('honors user-selected plain legend type for top orientation when space allows (#39540)', () => {
// Regression test for issue #39540: switching the legend type control from
// scroll to plain must reach the rendered ECharts config. Horizontal legends
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 19104d6f864..78cd493d32f 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
@@ -76,6 +76,7 @@ const {
| { name?: string | number | null }
)[];
legendMargin?: string | number | null;
+ nonLegendReservedHeight?: number;
orientation: LegendOrientation;
show: boolean;
showSelectors?: boolean;
@@ -103,6 +104,7 @@ const {
| { name?: string | number | null }
)[];
legendMargin?: string | number | null;
+ nonLegendReservedHeight?: number;
orientation: LegendOrientation;
show: boolean;
showSelectors?: boolean;
@@ -1248,7 +1250,7 @@ test('getLegendLayoutResult keeps plain when horizontal plain legends exceed two
expect(layout.effectiveMargin).toBe(68);
});
-test('getLegendLayoutResult reserves every row for many horizontal plain legend items', () => {
+test('getLegendLayoutResult reserves every row when fixed padding is supplied', () => {
const layout = getLegendLayoutResult({
chartHeight: 400,
chartWidth: 800,
@@ -1257,6 +1259,7 @@ test('getLegendLayoutResult reserves every row for many horizontal plain legend
(_, index) => `Country ${index + 1}, Product`,
),
legendMargin: null,
+ nonLegendReservedHeight: 0,
orientation: LegendOrientation.Top,
show: true,
theme,
@@ -1276,6 +1279,7 @@ test('getLegendLayoutResult bounds reserved margin for overflowing horizontal le
chartWidth: 100,
legendItems: Array.from({ length: 100 }, (_, index) => `Series ${index}`),
legendMargin: null,
+ nonLegendReservedHeight: 0,
orientation: LegendOrientation.Top,
show: true,
theme,