Skip to content

fix: Bubble charts a11y issue #199

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
georgylobko merged 1 commit into from
Apr 7, 2026
Merged

fix: Bubble charts a11y issue #199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/cartesian-chart/__tests__/cartesian-chart-series.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const allSeries: CartesianChartProps.SeriesOptions[] = [
{ type: "line", name: "Line", data: [{ x: 1, y: 6 }], color: "5" },
{ type: "scatter", name: "Scatter", data: [{ x: 1, y: 7 }], color: "6" },
{ type: "spline", name: "Spline", data: [{ x: 1, y: 8 }], color: "7" },
{ type: "bubble", name: "Bubble", data: [{ x: 1, y: 9, z: 5 }], color: "8" },
{ type: "bubble", name: "Bubble", data: [{ x: 1, y: 9, size: 5 }], color: "8" },
{ type: "x-threshold", name: "X threshold", value: 1, color: "9" },
{ type: "y-threshold", name: "Y threshold", value: 9, color: "10" },
];
Expand Down
25 changes: 22 additions & 3 deletions src/core/__tests__/chart-core-navigation-cartesian.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@
// SPDX-License-Identifier: Apache-2.0

import highcharts from "highcharts";
import Highcharts from "highcharts";
import { range } from "lodash";
import { vi } from "vitest";

import { KeyCode } from "@cloudscape-design/component-toolkit/internal";

import "highcharts/highcharts-more";
import "highcharts/modules/accessibility";
import { CoreChartTestProps, createChartWrapper, renderChart } from "./common";

Expand Down Expand Up @@ -56,6 +58,18 @@ const seriesShort3: Highcharts.SeriesOptionsType[] = [
],
},
];
const seriesShort4: Highcharts.SeriesOptionsType[] = [
...seriesShort3,
{
type: "bubble",
name: "Bubble series 1",
data: [
{ x: 1, y: 21, z: 31 },
{ x: 3, y: 23, z: 32 },
{ x: 4, y: 24, z: 33 },
],
},
];
const seriesLong1: Highcharts.SeriesOptionsType[] = [
{
type: "line",
Expand Down Expand Up @@ -601,21 +615,26 @@ describe("CoreChart: navigation, cartesian charts", () => {
const { wrapper } = renderChart({
...commonProps(),
options: {
series: seriesShort2,
series: seriesShort4,
xAxis: { title: { text: "X" }, valueFormatter: (value) => `x${value}` },
yAxis: { title: { text: "Y" }, valueFormatter: (value) => `y${value}` },
},
sizeAxis: { title: "Size axis", valueFormatter: (value) => `size${value}` },
});

focusApplication();
keyDown(KeyCode.home);

expect(describeFocusedElement()).toBe("button:x1[true,false]");
expect(wrapper.findTooltip()!.getElement().textContent).toBe("x1Line series 1y11Line series 2y21");
expect(wrapper.findTooltip()!.getElement().textContent).toBe(
"x1Line series 1y11Line series 2y21Line series 3y31Bubble series 1Yy21Size axissize31",
);

keyDown(KeyCode.down);

expect(describeFocusedElement()).toBe("button:x1 y11, Line series 1[true,false]");
expect(wrapper.findTooltip()!.getElement().textContent).toBe("x1Line series 1y11Line series 2y21");
expect(wrapper.findTooltip()!.getElement().textContent).toBe(
"x1Line series 1y11Line series 2y21Line series 3y31Bubble series 1Yy21Size axissize31",
);
});
});
5 changes: 4 additions & 1 deletion src/core/chart-core.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,10 @@ export function InternalCoreChart({
keyboardNavigation: options.accessibility?.keyboardNavigation ?? { enabled: !keyboardNavigation },
point: {
// Point description formatter is overridden to respect custom axes value formatters.
descriptionFormatter: (point) => getPointAccessibleDescription(point, labels),
descriptionFormatter: (point) =>
getPointAccessibleDescription(point, labels, {
sizeAxis: castArray(rest.sizeAxis as CoreChartProps.SizeAxisOptions[]),
}),
...options.accessibility?.point,
},
},
Expand Down
4 changes: 2 additions & 2 deletions src/core/components/core-tooltip.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ import { ChartAPI } from "../chart-api";
import { getFormatter } from "../formatters";
import { BaseI18nStrings, CoreChartProps } from "../interfaces";
import {
getBubbleSeriesSizeAxis,
getPointColor,
getPointId,
getSeriesColor,
getSeriesId,
getSeriesMarkerType,
isPointVisible,
isXThreshold,
matchSizeAxis,
} from "../utils";

import styles from "../styles.css.js";
Expand Down Expand Up @@ -318,7 +318,7 @@ function getBubblePointDetails(item: MatchedItem, sizeAxis: readonly CoreChartPr

// Size axis is a custom abstraction built to support bubble series title and formatter for z (size) values.
// We match size axes by ID if provided, or take the first defined axis instead.
const matchedSizeAxis = sizeAxis.find((a) => a.id === getBubbleSeriesSizeAxis(item.point.series)) ?? sizeAxis[0];
const matchedSizeAxis = matchSizeAxis(sizeAxis, item.point.series);
if (matchedSizeAxis) {
// Highcharts bubble size is represented by point.z value, which is however not present in the internal point type -
// so we take it from point's options instead.
Expand Down
21 changes: 19 additions & 2 deletions src/core/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,11 +368,28 @@ export function getChartAccessibleDescription(chart: Highcharts.Chart) {
return chart.options.lang?.accessibility?.chartContainerLabel ?? "";
}

export function getPointAccessibleDescription(point: Highcharts.Point, labels: ChartLabels) {
export function matchSizeAxis(sizeAxis: readonly CoreChartProps.SizeAxisOptions[], series: Highcharts.Series) {
return sizeAxis.find((a) => a.id === getBubbleSeriesSizeAxis(series)) ?? sizeAxis[0];
}

export function getPointAccessibleDescription(
point: Highcharts.Point,
labels: ChartLabels,
additionalProps?: {
sizeAxis?: readonly CoreChartProps.SizeAxisOptions[];
},
) {
if (point.series.xAxis && point.series.yAxis) {
const formattedX = getFormatter(point.series.xAxis)(point.x);
const formattedY = getFormatter(point.series.yAxis)(point.y);
return `${formattedX} ${formattedY}, ${point.series.name}`;
let formattedSize = "";
if (additionalProps?.sizeAxis && point.series.type === "bubble") {
const matchedSizeAxis = matchSizeAxis(additionalProps.sizeAxis, point.series);
const size = point.options.z ?? null;
const sizeFormatter = getFormatter(matchedSizeAxis);
Copy link
Copy Markdown
Member

@pan-kot pan-kot Apr 7, 2026

Choose a reason for hiding this comment

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

nit: would it make sense to make size and formatter extraction in the util?

export function getMatchedSizeAxisProps(sizeAxis, point) {
  const series = point.series;
  const matched = sizeAxis?.find((a) => a.id === getBubbleSeriesSizeAxis(series)) ?? sizeAxis?.[0];
  if (!matched) {
    return null;
  }
  return { size: point.options.z ?? null, formatter: getFormatter(matched) };
}

Copy link
Copy Markdown
Member

@pan-kot pan-kot Apr 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

or maybe just return the formatted size?

Copy link
Copy Markdown
Member

@pan-kot pan-kot Apr 7, 2026

Choose a reason for hiding this comment

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

export function getFormattedSize(sizeAxis, point) {
  const series = point.series;
  const matched = sizeAxis?.find((a) => a.id === getBubbleSeriesSizeAxis(series)) ?? sizeAxis?.[0];
  if (!matched) {
    return null;
  }
  const size = point.options.z ?? null;
  const formatter = getFormatter(matched);
  return formatter(size);
}

Copy link
Copy Markdown
Member Author

@georgylobko georgylobko Apr 7, 2026

Choose a reason for hiding this comment

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

I'd say we would barely benefit from it, because we need the matchedSizeAxis title in the tooltip, while we don't need it in the getPointAccessibleDescription, so I would keep it as is for now. No strong opinion here, but if we need to reuse or extend it at some point in future, we can always do this

formattedSize = sizeFormatter(size);
}
return `${formattedX} ${formattedY}${formattedSize ? " " + formattedSize : ""}, ${point.series.name}`;
} else if (point.series.type === "pie") {
const segmentLabel = labels.chartSegmentLabel ? `${labels.chartSegmentLabel} ` : "";
return `${segmentLabel}${point.name}, ${point.y}. ${point.series.name}`;
Expand Down
Loading