Copilot commented on code in PR #44147:
URL: https://github.com/apache/superset/pull/44147#discussion_r4165762501
##########
superset-frontend/plugins/plugin-chart-echarts/src/Pie/transformProps.ts:
##########
@@ -600,8 +610,10 @@ export default function transformProps(
legendOrientation,
showLegend,
theme,
- false,
+ false, // zoomable — Pie charts do not use the zoom control
legendState,
+ undefined, // padding — Pie passes width instead
+ Math.min(width, 250), // horizontalLegendWidth: cap at 250px so long
names don't consume the entire row
Review Comment:
Passing the entire chart width as the text width does not reserve space for
the legend icon and item gap. On a narrow chart, a label slightly shorter than
`width` is therefore considered untruncated even though the complete legend
item is wider than the chart, preserving the raw clipping this change is meant
to fix. Derive the text budget from the available legend width after
subtracting the symbol/gaps (and cover a narrow chart case).
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts:
##########
@@ -932,6 +949,59 @@ export function getLegendProps(
const getLegendWidth = (paddingWidth: number) =>
Math.max(paddingWidth - MARGIN_GUTTER, MIN_LEGEND_WIDTH);
+ /**
+ * Returns a legend tooltip config that:
+ * 1. Only appears when the label is actually truncated (name wider than
maxTextWidth)
+ * 2. Positions the tooltip ABOVE the legend item to avoid overlapping the
chart
+ *
+ * Accessibility note: this tooltip is hover-only; keyboard and screen-reader
+ * users cannot currently discover the untruncated name. A non-hover
+ * affordance (e.g. aria-label or title attribute on the legend item) is
+ * tracked as a separate enhancement.
Review Comment:
This knowingly makes the full legend name hover-only while truncation is
automatically enabled for every pie chart. Keyboard-only users cannot hover the
canvas legend, so long names become unavailable to them. Please provide an
equivalent focus/accessible-name path, or avoid automatic truncation until the
full value is exposed non-visually.
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts:
##########
@@ -932,6 +949,59 @@ export function getLegendProps(
const getLegendWidth = (paddingWidth: number) =>
Math.max(paddingWidth - MARGIN_GUTTER, MIN_LEGEND_WIDTH);
+ /**
+ * Returns a legend tooltip config that:
+ * 1. Only appears when the label is actually truncated (name wider than
maxTextWidth)
+ * 2. Positions the tooltip ABOVE the legend item to avoid overlapping the
chart
+ *
+ * Accessibility note: this tooltip is hover-only; keyboard and screen-reader
+ * users cannot currently discover the untruncated name. A non-hover
+ * affordance (e.g. aria-label or title attribute on the legend item) is
+ * tracked as a separate enhancement.
+ */
+ const makeLegendTooltip = (maxTextWidth: number): any => ({
+ show: true,
+ confine: false, // allow tooltip to render above the canvas boundary
+ position: (
+ _pos: [number, number],
+ _params: unknown,
+ _el: unknown,
+ elRect: { x: number; y: number; width: number; height: number },
+ size: { contentSize: [number, number]; viewSize: [number, number] },
+ ) => {
+ const tooltipWidth = size.contentSize[0];
+ const tooltipHeight = size.contentSize[1];
+ // Center horizontally over the hovered legend item
+ const x = Math.max(
+ Math.min(
+ elRect.x + elRect.width / 2 - tooltipWidth / 2,
+ size.viewSize[0] - tooltipWidth,
+ ),
+ 0,
+ );
+ // Place above the legend item; negative y appears above canvas with
confine:false
+ const y = elRect.y - tooltipHeight - 8;
+ return [x, y];
Review Comment:
For a top legend, `elRect.y` is near the chart’s top edge, so this
calculation commonly returns a negative coordinate. Unlike the chart tooltip in
`src/utils/tooltip.ts:34-44`, this legend tooltip does not set `appendToBody`;
`confine: false` does not move the tooltip out of the chart DOM, so the tooltip
can be clipped or rendered offscreen—the exact orientation this PR targets.
Reuse the existing append-to-body behavior and clamp or flip the tooltip below
when there is no room above.
##########
superset/translations/messages.pot:
##########
@@ -25,14 +25,14 @@ msgid ""
msgstr ""
"Project-Id-Version: Superset VERSION\n"
"Report-Msgid-Bugs-To: EMAIL@ADDRESS\n"
-"POT-Creation-Date: 2026-10-01 12:46+0300\n"
+"POT-Creation-Date: 2026-10-02 17:23+0530\n"
"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
"Language-Team: LANGUAGE <[email protected]>\n"
"MIME-Version: 1.0\n"
"Content-Type: text/plain; charset=utf-8\n"
"Content-Transfer-Encoding: 8bit\n"
-"Generated-By: Babel 2.17.0\n"
+"Generated-By: Babel 2.10.3\n"
Review Comment:
This catalog was regenerated with Babel 2.10.3 even though
`superset/translations/requirements.txt:17-20` explicitly pins Babel 2.17.0 to
prevent spurious catalog diffs. The resulting update clears many established
translations across the locale files (for example “New chart”, export labels,
and candlestick descriptions), so localized users would fall back to English.
Please rerun `babel_update.sh` in the pinned translation environment and commit
only the intended new msgids.
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts:
##########
@@ -932,6 +949,59 @@ export function getLegendProps(
const getLegendWidth = (paddingWidth: number) =>
Math.max(paddingWidth - MARGIN_GUTTER, MIN_LEGEND_WIDTH);
+ /**
+ * Returns a legend tooltip config that:
+ * 1. Only appears when the label is actually truncated (name wider than
maxTextWidth)
+ * 2. Positions the tooltip ABOVE the legend item to avoid overlapping the
chart
+ *
+ * Accessibility note: this tooltip is hover-only; keyboard and screen-reader
+ * users cannot currently discover the untruncated name. A non-hover
+ * affordance (e.g. aria-label or title attribute on the legend item) is
+ * tracked as a separate enhancement.
+ */
+ const makeLegendTooltip = (maxTextWidth: number): any => ({
Review Comment:
The new helper introduces an explicit `any`, contrary to the repository’s
TypeScript type-safety rule. The legend option already exposes the tooltip
shape, so retaining that type here will also validate the callback signatures
instead of bypassing them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
