This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 924343e3ee6 fix(gantt): prevent y-axis category labels from being
clipped (#44321)
924343e3ee6 is described below
commit 924343e3ee6048e494c5082c331a692ab9b56c34
Author: Amin Ghadersohi <[email protected]>
AuthorDate: Thu Oct 1 12:58:52 2026 +1000
fix(gantt): prevent y-axis category labels from being clipped (#44321)
---
.../plugin-chart-echarts/src/Gantt/constants.ts | 3 +
.../src/Gantt/transformProps.ts | 35 ++--
.../plugin-chart-echarts/src/utils/series.ts | 65 +++++--
.../test/Gantt/transformProps.test.ts | 213 ++++++++++++++++++++-
.../plugin-chart-echarts/test/utils/series.test.ts | 70 +++++++
5 files changed, 356 insertions(+), 30 deletions(-)
diff --git
a/superset-frontend/plugins/plugin-chart-echarts/src/Gantt/constants.ts
b/superset-frontend/plugins/plugin-chart-echarts/src/Gantt/constants.ts
index 83b06da12b2..c1187885660 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Gantt/constants.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Gantt/constants.ts
@@ -23,6 +23,9 @@ export const ELEMENT_HEIGHT_SCALE = 0.85 as const;
// reserved by hand. These drive that calculation in transformProps.
export const CATEGORY_LABEL_GAP = 8 as const;
export const MAX_CATEGORY_LABEL_WIDTH_RATIO = 0.25 as const;
+// ECharts' `overflow: 'truncate'` treats a label box as 1px narrower than its
+// `width`, so a box sized to exactly the measured text width truncates it.
+export const CATEGORY_LABEL_TRUNCATE_GAP = 1 as const;
export enum Dimension {
StartTime = 'startTime',
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 e24205013d3..c8249c16f4b 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Gantt/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Gantt/transformProps.ts
@@ -49,7 +49,7 @@ import {
getLegendProps,
getLegendScrollDataIndex,
groupData,
- measureTextWidth,
+ measureTextInkWidth,
} from '../utils/series';
import { resolveLegendLayout } from '../utils/legendLayout';
import {
@@ -62,6 +62,7 @@ import { convertInteger } from '../utils/convertInteger';
import { getTooltipLabels } from '../utils/tooltip';
import {
CATEGORY_LABEL_GAP,
+ CATEGORY_LABEL_TRUNCATE_GAP,
Dimension,
ELEMENT_HEIGHT_SCALE,
MAX_CATEGORY_LABEL_WIDTH_RATIO,
@@ -217,6 +218,14 @@ export default function transformProps(chartProps:
EchartsGanttChartProps) {
const categoryLines: { yAxis: number; name?: string }[] = [];
let sum = 0;
let prevSum = 0;
+ let maxCategoryLabelWidth = 0;
+
+ // The category labels are drawn via the markLine `label` below, so its font
+ // must match the one measureTextInkWidth measures with (the theme's
+ // fontSizeSM / fontFamily), or the reserved grid area can end up narrower
+ // than the rendered text.
+ const categoryLabelFontSize = theme.fontSizeSM;
+ const categoryLabelFontFamily = theme.fontFamily;
Array.from(seriesInCategoriesMap.entries()).forEach(([key, map]) => {
sum += map.size;
@@ -228,6 +237,15 @@ export default function transformProps(chartProps:
EchartsGanttChartProps) {
name,
});
+ if (name) {
+ // Reserve the rendered glyphs' ink extent rather than only the advance
+ // width, which previously left labels clipped by a few pixels.
+ maxCategoryLabelWidth = Math.max(
+ maxCategoryLabelWidth,
+ measureTextInkWidth(name, theme),
+ );
+ }
+
borderLines.push({ yAxis: seriesCount - sum });
prevSum = sum;
@@ -240,13 +258,9 @@ export default function transformProps(chartProps:
EchartsGanttChartProps) {
// category cannot eat the chart; anything past the cap is truncated with an
// ellipsis by the label itself.
const categoryLabelWidth = Math.min(
- Math.ceil(
- categoryLines.reduce(
- (maxWidth, { name }) =>
- name ? Math.max(maxWidth, measureTextWidth(name, theme)) : maxWidth,
- 0,
- ),
- ),
+ maxCategoryLabelWidth > 0
+ ? Math.ceil(maxCategoryLabelWidth) + CATEGORY_LABEL_TRUNCATE_GAP
+ : 0,
Math.floor(width * MAX_CATEGORY_LABEL_WIDTH_RATIO),
);
@@ -367,9 +381,8 @@ export default function transformProps(chartProps:
EchartsGanttChartProps) {
position: 'start',
formatter: '{b}',
color: theme.colorText,
- // Must match the font size measureTextWidth assumes above, or the
- // reserved categoryLabelWidth won't match what actually renders.
- fontSize: theme.fontSizeSM,
+ fontSize: categoryLabelFontSize,
+ fontFamily: categoryLabelFontFamily,
width: categoryLabelWidth,
overflow: 'truncate',
},
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 fa83470fa1e..09dfca1f9b4 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
@@ -74,12 +74,31 @@ 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;
-// Keyed on every distinct legend label and (since Gantt's category names
-// share this cache too) every distinct category name ever measured, so an
-// unbounded cache could grow with high-cardinality data over a long session.
-// Cap it and evict the least-recently-used entry once full.
+// Keyed on every distinct legend label ever measured, so an unbounded cache
+// could grow with high-cardinality data over a long session. Cap it and evict
+// the least-recently-used entry once full.
const TEXT_WIDTH_CACHE_MAX_SIZE = 2000;
const TEXT_WIDTH_CACHE = new Map<string, number>();
+// Approximate glyph width as a fraction of the font size, used to estimate
+// text width when canvas measurement is unavailable (e.g. SSR).
+const APPROX_CHAR_WIDTH_RATIO = 0.62;
+
+function getTextMeasureContext(
+ theme: SupersetTheme,
+): CanvasRenderingContext2D | null {
+ if (typeof document === 'undefined') {
+ return null;
+ }
+ const context = document.createElement('canvas').getContext('2d');
+ if (context) {
+ context.font = `${theme.fontSizeSM}px ${theme.fontFamily}`;
+ }
+ return context;
+}
+
+function estimateTextWidth(text: string, theme: SupersetTheme): number {
+ return text.length * theme.fontSizeSM * APPROX_CHAR_WIDTH_RATIO;
+}
type LegendDataItem =
| string
@@ -116,16 +135,10 @@ export function measureTextWidth(text: string, theme:
SupersetTheme): number {
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));
- }
- }
+ const context = getTextMeasureContext(theme);
+ const width = context
+ ? context.measureText(text).width
+ : estimateTextWidth(text, theme);
if (TEXT_WIDTH_CACHE.size >= TEXT_WIDTH_CACHE_MAX_SIZE) {
const oldestKey = TEXT_WIDTH_CACHE.keys().next().value;
@@ -137,6 +150,30 @@ export function measureTextWidth(text: string, theme:
SupersetTheme): number {
return width;
}
+/**
+ * Measures the horizontal space `text` occupies when rendered at the theme's
+ * small font size, taking the larger of the advance width and the painted ink
+ * extent. Glyphs such as italics, or glyphs whose ink overhangs the pen
+ * advance, can paint past the advance width, so reserving only the advance
+ * width can clip them. Falls back to an approximate width when canvas is
+ * unavailable (e.g. SSR), or to the advance width when the browser does not
+ * report bounding-box metrics.
+ */
+export function measureTextInkWidth(
+ text: string,
+ theme: SupersetTheme,
+): number {
+ const context = getTextMeasureContext(theme);
+ if (!context) {
+ return estimateTextWidth(text, theme);
+ }
+ const metrics = context.measureText(text);
+ const inkWidth =
+ (metrics.actualBoundingBoxLeft ?? 0) +
+ (metrics.actualBoundingBoxRight ?? 0);
+ return Math.max(metrics.width, inkWidth);
+}
+
function hasLegendLabel(item: LegendDataItem): boolean {
if (item === null || item === undefined) {
return false;
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 8441514d591..9e69c70d32a 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
@@ -259,11 +259,8 @@ describe('Gantt transformProps', () => {
position: 'start',
formatter: '{b}',
color: 'rgba(0,0,0,0.88)',
- // Must match the font size measureTextWidth assumed when reserving
- // `categoryLabelWidth`, or the rendered label overflows the room
- // that was reserved for it.
fontSize: supersetTheme.fontSizeSM,
- // room reserved for the category name, truncated past the cap
+ fontFamily: supersetTheme.fontFamily,
width: expect.any(Number),
overflow: 'truncate',
},
@@ -278,6 +275,212 @@ describe('Gantt transformProps', () => {
});
});
+describe('category label width reservation', () => {
+ test('reserves the ink extent of the widest label, not just its narrower
advance width', () => {
+ // Simulates a glyph whose visible ink extends past the metrics.width
+ // advance value returned by measureText (e.g. italics, or glyphs whose
+ // ink overhangs the pen advance).
+ // The grid must grow to fit the ink extent, or the previous
+ // "prevent cut off" fix (#39137) regresses back to clipped labels.
+ const getContext = jest.spyOn(HTMLCanvasElement.prototype, 'getContext');
+ try {
+ // "first" always measures narrower than "second" so "second" alone
+ // drives maxCategoryLabelWidth in both runs below.
+ getContext.mockReturnValue({
+ measureText: (text: string) =>
+ text === 'second'
+ ? {
+ width: 10,
+ actualBoundingBoxLeft: 0,
+ actualBoundingBoxRight: 10,
+ }
+ : { width: 5, actualBoundingBoxLeft: 0, actualBoundingBoxRight: 5
},
+ } as never);
+ const withoutOverhang = transformProps(
+ new ChartProps(chartPropsConfig) as EchartsGanttChartProps,
+ );
+
+ getContext.mockReturnValue({
+ measureText: (text: string) =>
+ text === 'second'
+ ? {
+ width: 10,
+ actualBoundingBoxLeft: 5,
+ actualBoundingBoxRight: 45,
+ }
+ : { width: 5, actualBoundingBoxLeft: 0, actualBoundingBoxRight: 5
},
+ } as never);
+ const withOverhang = transformProps(
+ new ChartProps(chartPropsConfig) as EchartsGanttChartProps,
+ );
+
+ const noOverhangLeft = (
+ withoutOverhang.echartOptions.grid as { left: number }
+ ).left;
+ const overhangLeft = (withOverhang.echartOptions.grid as { left: number
})
+ .left;
+
+ // Ink extent for "second" is 5 + 45 = 50 vs. its 10px advance width;
+ // the reserved space must grow by the full 40px difference.
+ expect(overhangLeft - noOverhangLeft).toBe(40);
+ } finally {
+ getContext.mockRestore();
+ }
+ });
+
+ test('reserves the advance width when bounding-box metrics are absent', ()
=> {
+ const getContext = jest.spyOn(HTMLCanvasElement.prototype, 'getContext');
+ try {
+ getContext.mockReturnValue({
+ measureText: (text: string) =>
+ text === 'second'
+ ? {
+ width: 10,
+ actualBoundingBoxLeft: 0,
+ actualBoundingBoxRight: 10,
+ }
+ : { width: 5, actualBoundingBoxLeft: 0, actualBoundingBoxRight: 5
},
+ } as never);
+ const withMetrics = transformProps(
+ new ChartProps(chartPropsConfig) as EchartsGanttChartProps,
+ );
+
+ // Same advance widths, but no actualBoundingBox* fields at all.
+ getContext.mockReturnValue({
+ measureText: (text: string) => ({ width: text === 'second' ? 10 : 5 }),
+ } as never);
+ const withoutMetrics = transformProps(
+ new ChartProps(chartPropsConfig) as EchartsGanttChartProps,
+ );
+
+ const withMetricsLeft = (
+ withMetrics.echartOptions.grid as { left: number }
+ ).left;
+ const withoutMetricsLeft = (
+ withoutMetrics.echartOptions.grid as { left: number }
+ ).left;
+
+ expect(withoutMetricsLeft).not.toBeNaN();
+ expect(withoutMetricsLeft).toBe(withMetricsLeft);
+ } finally {
+ getContext.mockRestore();
+ }
+ });
+
+ test('reserves an approximate width when canvas is unavailable', () => {
+ const getContext = jest
+ .spyOn(HTMLCanvasElement.prototype, 'getContext')
+ .mockReturnValue(null);
+ try {
+ const transformed = transformProps(
+ new ChartProps(chartPropsConfig) as EchartsGanttChartProps,
+ );
+ const categoryLabelSeries = (
+ transformed.echartOptions.series as Array<{
+ markLine?: { label?: { show?: boolean; width?: number } };
+ }>
+ ).find(series => series.markLine?.label?.show);
+
+ // "second" is the widest category name (6 chars) at ~0.62em per char,
+ // plus the 1px ECharts reserves before truncating.
+ expect(categoryLabelSeries?.markLine?.label?.width).toBe(
+ Math.ceil('second'.length * supersetTheme.fontSizeSM * 0.62) + 1,
+ );
+ } finally {
+ getContext.mockRestore();
+ }
+ });
+
+ test('sizes the label box so ECharts does not truncate the widest name', ()
=> {
+ // ECharts' `overflow: 'truncate'` only keeps text that fits in
+ // `width - 1`, so a box exactly as wide as the text cuts off its last
+ // glyph. The widest name here measures an integral 142px.
+ const getContext = jest
+ .spyOn(HTMLCanvasElement.prototype, 'getContext')
+ .mockReturnValue({
+ measureText: (text: string) =>
+ text === 'second'
+ ? {
+ width: 142,
+ actualBoundingBoxLeft: 0,
+ actualBoundingBoxRight: 141,
+ }
+ : {
+ width: 50,
+ actualBoundingBoxLeft: 0,
+ actualBoundingBoxRight: 50,
+ },
+ } as never);
+ try {
+ const transformed = transformProps(
+ new ChartProps({
+ ...chartPropsConfig,
+ width: 1000,
+ }) as EchartsGanttChartProps,
+ );
+ const categoryLabelSeries = (
+ transformed.echartOptions.series as Array<{
+ markLine?: { label?: { show?: boolean; width?: number } };
+ }>
+ ).find(series => series.markLine?.label?.show);
+ const labelWidth = categoryLabelSeries?.markLine?.label?.width ?? 0;
+
+ expect(labelWidth - 1).toBeGreaterThanOrEqual(142);
+ } finally {
+ getContext.mockRestore();
+ }
+ });
+
+ test('measures label width using the exact font the label is rendered in',
() => {
+ let capturedFont = '';
+ const getContext = jest.spyOn(HTMLCanvasElement.prototype, 'getContext');
+ try {
+ getContext.mockReturnValue({
+ set font(value: string) {
+ capturedFont = value;
+ },
+ get font() {
+ return capturedFont;
+ },
+ measureText: (text: string) => ({
+ width: text.length * 7,
+ actualBoundingBoxLeft: 0,
+ actualBoundingBoxRight: text.length * 7,
+ }),
+ } as never);
+
+ const transformed = transformProps(
+ new ChartProps(chartPropsConfig) as EchartsGanttChartProps,
+ );
+
+ expect(capturedFont).toBe(
+ `${supersetTheme.fontSizeSM}px ${supersetTheme.fontFamily}`,
+ );
+
+ const categoryLabelSeries = (
+ transformed.echartOptions.series as Array<{
+ markLine?: {
+ label?: {
+ show?: boolean;
+ fontSize?: number;
+ fontFamily?: string;
+ };
+ };
+ }>
+ ).find(series => series.markLine?.label?.show);
+
+ // The rendered label must use the same font as the measurement, or the
+ // reserved grid space can fall out of sync with the painted text again.
+ expect(categoryLabelSeries?.markLine?.label).toMatchObject({
+ fontSize: supersetTheme.fontSizeSM,
+ fontFamily: supersetTheme.fontFamily,
+ });
+ } finally {
+ getContext.mockRestore();
+ }
+ });
+});
+
describe('legend sorting', () => {
const createChartProps = (overrides = {}) =>
new ChartProps({
@@ -424,7 +627,7 @@ test('reserves grid room for category names so they are not
clipped (#38844)', (
expect(categoryMarkLine.markLine.label.overflow).toBe('truncate');
expect(categoryMarkLine.markLine.label.width).toBeGreaterThan(0);
expect(categoryMarkLine.markLine.label.width).toBeLessThanOrEqual(800 *
0.25);
- // the label must render at the font size measureTextWidth assumed above,
+ // the label must render at the font size used for measurement above,
// or the reserved width won't match what actually gets drawn (#43189)
expect(categoryMarkLine.markLine.label.fontSize).toBe(
supersetTheme.fontSizeSM,
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 c5b0efc4fa3..ab57ad0903c 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
@@ -44,6 +44,7 @@ import {
getMinAndMaxFromBounds,
capTickMarks,
getTemporalTickValues,
+ measureTextInkWidth,
measureTextWidth,
sanitizeHtml,
sortAndFilterSeries,
@@ -2210,3 +2211,72 @@ describe('measureTextWidth caching', () => {
expect(measureText).toHaveBeenCalledTimes(2);
});
});
+
+describe('measureTextInkWidth', () => {
+ let getContext: jest.SpyInstance;
+
+ afterEach(() => {
+ getContext.mockRestore();
+ });
+
+ test('uses the ink extent when it exceeds the advance width', () => {
+ getContext = jest
+ .spyOn(HTMLCanvasElement.prototype, 'getContext')
+ .mockReturnValue({
+ measureText: () => ({
+ width: 10,
+ actualBoundingBoxLeft: 5,
+ actualBoundingBoxRight: 45,
+ }),
+ } as never);
+ expect(measureTextInkWidth('label', theme)).toBe(50);
+ });
+
+ test('keeps the advance width when it exceeds the ink extent', () => {
+ getContext = jest
+ .spyOn(HTMLCanvasElement.prototype, 'getContext')
+ .mockReturnValue({
+ measureText: () => ({
+ width: 30,
+ actualBoundingBoxLeft: 0,
+ actualBoundingBoxRight: 20,
+ }),
+ } as never);
+ expect(measureTextInkWidth('label', theme)).toBe(30);
+ });
+
+ test('falls back to the advance width when bounding-box metrics are absent',
() => {
+ getContext = jest
+ .spyOn(HTMLCanvasElement.prototype, 'getContext')
+ .mockReturnValue({ measureText: () => ({ width: 42 }) } as never);
+ const width = measureTextInkWidth('label', theme);
+ expect(width).not.toBeNaN();
+ expect(width).toBe(42);
+ });
+
+ test('falls back to an approximate width when canvas is unavailable', () => {
+ getContext = jest
+ .spyOn(HTMLCanvasElement.prototype, 'getContext')
+ .mockReturnValue(null);
+ expect(measureTextInkWidth('label', theme)).toBeCloseTo(
+ 'label'.length * theme.fontSizeSM * 0.62,
+ );
+ });
+
+ test('measures with the theme small font', () => {
+ let capturedFont = '';
+ getContext = jest
+ .spyOn(HTMLCanvasElement.prototype, 'getContext')
+ .mockReturnValue({
+ set font(value: string) {
+ capturedFont = value;
+ },
+ get font() {
+ return capturedFont;
+ },
+ measureText: () => ({ width: 1 }),
+ } as never);
+ measureTextInkWidth('label', theme);
+ expect(capturedFont).toBe(`${theme.fontSizeSM}px ${theme.fontFamily}`);
+ });
+});
