Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 118 additions & 0 deletions component/src/charts/__tests__/chart-aria.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import { render, screen, cleanup } from "@testing-library/react";
import { describe, it, expect, vi, afterEach } from "vitest";

// One place that verifies every chart gives BaseChart a descriptive aria-label
// (was the generic "Chart visualization" fallback) and honours a caller
// override. echarts/core + container size are stubbed so the charts mount.
vi.mock("echarts/core", () => {
const init = vi.fn(() => ({
setOption: vi.fn(),
resize: vi.fn(),
dispose: vi.fn(),
on: vi.fn(),
off: vi.fn(),
showLoading: vi.fn(),
hideLoading: vi.fn(),
}));
const stub = {
use: vi.fn(),
init,
registerTheme: vi.fn(),
registerMap: vi.fn(),
getMap: vi.fn(() => ({})),
format: { encodeHTML: (s: string) => s },
};
return { ...stub, default: stub };
});
vi.mock("@/hooks/useContainerSize", () => ({
useContainerSize: () => ({ width: 600, height: 400, containerRef: vi.fn() }),
}));

import { PieChart } from "../pie-chart";
import { RadarChart } from "../radar-chart";
import { GaugeChart } from "../gauge-chart";
import { SankeyChart } from "../sankey-chart";
import { TreemapChart } from "../treemap-chart";
import { SunburstChart } from "../sunburst-chart";
import { CirclePackingChart } from "../circle-packing-chart";
import { GanttChart } from "../gantt-chart";
import { ChoroplethChart } from "../choropleth-chart";

const cases: Array<[string, React.ReactElement, RegExp]> = [
[
"pie",
<PieChart data={[{ name: "A", value: 1 }]} />,
/Pie chart with 1 segments/,
],
[
"radar",
<RadarChart
data={{
indicators: [{ name: "X", max: 10 }],
series: [{ name: "S", values: [5] }],
}}
/>,
/Radar chart comparing 1 series across 1 axes/,
],
[
"gauge",
<GaugeChart data={[{ value: 50 }]} />,
/Gauge showing 50 of 0 to 100/,
],
[
"sankey",
<SankeyChart
data={{
nodes: [{ name: "A" }, { name: "B" }],
links: [{ source: "A", target: "B", value: 1 }],
}}
/>,
/Sankey diagram with 2 nodes and 1 links/,
],
[
"treemap",
<TreemapChart data={[{ name: "A", value: 1 }]} />,
/Treemap with 1 top-level items/,
],
[
"sunburst",
<SunburstChart data={[{ name: "A", value: 1 }]} />,
/Sunburst chart with 1 top-level segments/,
],
[
"circle-packing",
<CirclePackingChart data={[{ name: "A", value: 1 }]} />,
/Circle-packing chart with 1 top-level groups/,
],
[
"gantt",
<GanttChart data={[{ task: "T", start: 0, end: 1 }]} />,
/Gantt chart with 1 tasks/,
],
[
"choropleth",
<ChoroplethChart data={[{ name: "United States", value: 1 }]} />,
/Choropleth map with 1 regions/,
],
];

afterEach(cleanup);

describe("chart aria descriptions", () => {
it.each(cases)("%s gets a descriptive aria-label", (_name, el, re) => {
render(el);
expect(screen.getByTestId("base-chart").getAttribute("aria-label")).toMatch(
re,
);
});

it("honours a caller-provided ariaDescription override", () => {
render(
<PieChart data={[{ name: "A", value: 1 }]} ariaDescription="Custom" />,
);
expect(screen.getByTestId("base-chart")).toHaveAttribute(
"aria-label",
"Custom",
);
});
});
11 changes: 10 additions & 1 deletion component/src/charts/choropleth-chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ function ChoroplethChart({
minColor = "#fff7d6",
maxColor = "#993404",
showLabels = false,
ariaDescription,
...rest
}: ChoroplethChartProps) {
const [mapRegistered, setMapRegistered] = useState(false);
Expand Down Expand Up @@ -206,7 +207,15 @@ function ChoroplethChart({
showLabels,
]);

return <BaseChart options={options} {...rest} />;
return (
<BaseChart
options={options}
ariaDescription={
ariaDescription ?? `Choropleth map with ${data.length} regions`
}
{...rest}
/>
);
}

export { ChoroplethChart };
10 changes: 9 additions & 1 deletion component/src/charts/circle-packing-chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ function CirclePackingChart({
padding = 3,
stylingRules,
paramValues,
ariaDescription,
...rest
}: CirclePackingChartProps) {
const { width, height, containerRef } = useContainerSize();
Expand Down Expand Up @@ -266,7 +267,14 @@ function CirclePackingChart({

return (
<div ref={containerRef} className="h-full w-full">
<BaseChart options={options} {...rest} />
<BaseChart
options={options}
ariaDescription={
ariaDescription ??
`Circle-packing chart with ${data.length} top-level groups`
}
Comment on lines +272 to +275

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fallback group count can be wrong for single-root hierarchical input.

At Line 272, data.length reports 1 when input is a single root node with many children, so the aria label undercounts top-level groups.

Proposed fix
+  const topLevelGroupCount =
+    data.length === 1 && data[0]?.children?.length
+      ? data[0].children.length
+      : data.length;
+
   return (
     <div ref={containerRef} className="h-full w-full">
       <BaseChart
         options={options}
         ariaDescription={
           ariaDescription ??
-          `Circle-packing chart with ${data.length} top-level groups`
+          `Circle-packing chart with ${topLevelGroupCount} top-level groups`
         }
         {...rest}
       />
     </div>
   );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ariaDescription={
ariaDescription ??
`Circle-packing chart with ${data.length} top-level groups`
}
const topLevelGroupCount =
data.length === 1 && data[0]?.children?.length
? data[0].children.length
: data.length;
return (
<div ref={containerRef} className="h-full w-full">
<BaseChart
options={options}
ariaDescription={
ariaDescription ??
`Circle-packing chart with ${topLevelGroupCount} top-level groups`
}
{...rest}
/>
</div>
);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@component/src/charts/circle-packing-chart.tsx` around lines 272 - 275, The
ariaDescription fallback at the Circle-packing chart component is using
data.length to count top-level groups, but this is incorrect for hierarchical
input where a single root node contains multiple child groups. Instead of using
data.length directly in the aria description, check if the first element in data
has children (indicating a hierarchical structure), and if so, count the
children of that root node; otherwise, use data.length as the fallback. This
ensures the aria label accurately reflects the actual number of top-level groups
visible in the chart regardless of whether the input is flat or hierarchical.

{...rest}
/>
</div>
);
}
Expand Down
11 changes: 10 additions & 1 deletion component/src/charts/gantt-chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ function GanttChart({
showGridLines = true,
stylingRules,
paramValues,
ariaDescription,
...rest
}: GanttChartProps) {
const options = useMemo((): EChartsOption => {
Expand Down Expand Up @@ -323,7 +324,15 @@ function GanttChart({
paramValues,
]);

return <BaseChart options={options} {...rest} />;
return (
<BaseChart
options={options}
ariaDescription={
ariaDescription ?? `Gantt chart with ${data.length} tasks`
}
{...rest}
/>
);
}

export { GanttChart };
10 changes: 9 additions & 1 deletion component/src/charts/gauge-chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ function GaugeChart({
thresholdZones: thresholdZonesJson,
stylingRules,
paramValues,
ariaDescription,
...rest
}: GaugeChartProps) {
const { width, height, containerRef } = useContainerSize();
Expand Down Expand Up @@ -205,7 +206,14 @@ function GaugeChart({

return (
<div ref={containerRef} className="h-full w-full">
<BaseChart options={options} {...rest} />
<BaseChart
options={options}
ariaDescription={
ariaDescription ??
`Gauge showing ${data[0]?.value ?? 0} of ${min} to ${max}`
}
{...rest}
/>
</div>
);
}
Expand Down
9 changes: 8 additions & 1 deletion component/src/charts/pie-chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ function PieChart({
donutCenterText,
stylingRules,
paramValues,
ariaDescription,
...rest
}: PieChartProps) {
const { width, height, containerRef } = useContainerSize();
Expand Down Expand Up @@ -187,7 +188,13 @@ function PieChart({

return (
<div ref={containerRef} className="h-full w-full">
<BaseChart options={options} {...rest} />
<BaseChart
options={options}
ariaDescription={
ariaDescription ?? `Pie chart with ${data.length} segments`
}
Comment on lines +193 to +195

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use rendered segment count in the fallback aria description.

Line 194 uses data.length, but the rendered pie can be grouped by topN, so screen readers may get an incorrect segment count.

Suggested fix
-        ariaDescription={
-          ariaDescription ?? `Pie chart with ${data.length} segments`
-        }
+        ariaDescription={
+          ariaDescription ??
+          `Pie chart with ${topN > 0 && data.length > topN ? topN + 1 : data.length} segments`
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ariaDescription={
ariaDescription ?? `Pie chart with ${data.length} segments`
}
ariaDescription={
ariaDescription ??
`Pie chart with ${topN > 0 && data.length > topN ? topN + 1 : data.length} segments`
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@component/src/charts/pie-chart.tsx` around lines 193 - 195, The
ariaDescription fallback template in the pie chart component is using
data.length to report the segment count, but when topN grouping is applied, the
actual rendered number of segments may be different from the raw data length.
Replace data.length in the ariaDescription with the length of the actual
rendered/processed data that accounts for the topN parameter to ensure screen
readers announce the correct number of segments that are actually displayed in
the pie chart.

{...rest}
/>
</div>
);
}
Expand Down
10 changes: 9 additions & 1 deletion component/src/charts/radar-chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ function RadarChart({
showValues = false,
stylingRules,
paramValues,
ariaDescription,
...rest
}: RadarChartProps) {
const { width, height, containerRef } = useContainerSize();
Expand Down Expand Up @@ -152,7 +153,14 @@ function RadarChart({

return (
<div ref={containerRef} className="h-full w-full">
<BaseChart options={options} {...rest} />
<BaseChart
options={options}
ariaDescription={
ariaDescription ??
`Radar chart comparing ${data.series.length} series across ${data.indicators.length} axes`
}
{...rest}
/>
</div>
);
}
Expand Down
27 changes: 24 additions & 3 deletions component/src/charts/sankey-chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ function SankeyChart({
nodeGap = 8,
stylingRules,
paramValues,
ariaDescription,
...rest
}: SankeyChartProps) {
const { width, height, containerRef } = useContainerSize();
Expand All @@ -76,7 +77,11 @@ function SankeyChart({
data: data.nodes,
links: stylingRules?.length
? data.links.map((link) => {
const resolvedColor = resolveItemColor(link.value, stylingRules, paramValues);
const resolvedColor = resolveItemColor(
link.value,
stylingRules,
paramValues,
);
return {
...link,
lineStyle: resolvedColor ? { color: resolvedColor } : {},
Expand All @@ -99,11 +104,27 @@ function SankeyChart({
},
],
};
}, [data, orient, showLabels, nodeWidth, nodeGap, compact, stylingRules, paramValues]);
}, [
data,
orient,
showLabels,
nodeWidth,
nodeGap,
compact,
stylingRules,
paramValues,
]);

return (
<div ref={containerRef} className="h-full w-full">
<BaseChart options={options} {...rest} />
<BaseChart
options={options}
ariaDescription={
ariaDescription ??
`Sankey diagram with ${data.nodes.length} nodes and ${data.links.length} links`
}
{...rest}
/>
</div>
);
}
Expand Down
10 changes: 9 additions & 1 deletion component/src/charts/sunburst-chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ function SunburstChart({
highlightOnHover = true,
stylingRules,
paramValues,
ariaDescription,
...rest
}: SunburstChartProps) {
const { width, height, containerRef } = useContainerSize();
Expand Down Expand Up @@ -196,7 +197,14 @@ function SunburstChart({

return (
<div ref={containerRef} className="h-full w-full">
<BaseChart options={options} {...rest} />
<BaseChart
options={options}
ariaDescription={
ariaDescription ??
`Sunburst chart with ${data.length} top-level segments`
}
{...rest}
/>
</div>
);
}
Expand Down
9 changes: 8 additions & 1 deletion component/src/charts/treemap-chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ function TreemapChart({
colorSaturation = "medium",
stylingRules,
paramValues,
ariaDescription,
...rest
}: TreemapChartProps) {
const { width, height, containerRef } = useContainerSize();
Expand Down Expand Up @@ -174,7 +175,13 @@ function TreemapChart({

return (
<div ref={containerRef} className="h-full w-full">
<BaseChart options={options} {...rest} />
<BaseChart
options={options}
ariaDescription={
ariaDescription ?? `Treemap with ${data.length} top-level items`
}
{...rest}
/>
</div>
);
}
Expand Down
Loading