mirror of
https://github.com/coder/coder.git
synced 2026-10-16 14:48:56 +08:00
Andrew Aquino
5261723186
> [!NOTE]
> This PR was generated by Coder Agents on behalf of @aqandrew.
Fixes DEQ-408 (axe `custom-role-application` on the user engagement
chart). Also fixes DEQ-342
recharts' `accessibilityLayer` sets `role="application"` on the chart
`<svg>` by default. This PR overrides it on all three charts that use it
(user engagement, template active users, license seat consumption) with
`role="group"` and `aria-roledescription="chart"`. It also gives each
chart an accessible name and description through recharts'
`title`/`desc` props.
`accessibilityLayer` stays, so keyboard users can still move the tooltip
with the arrow keys. To make that work for screen readers,
`ChartContainer` now renders a polite `role="status"` live region that
is always mounted. `ChartTooltipContent` builds the announcement during
render from the same label and item values the tooltip shows, and an
effect keyed on that string pushes it into the live region. Each point
is announced, for example "175 users January 02".
Not verified: NVDA behaviour (whether Tab focus switches to focus mode
so the arrows reach the chart), and an axe re-run on
`/deployment/overview`.
Frontend-review notes:
- **FE1:** the assertions live in story `play` functions. recharts'
keyboard navigation depends on real layout (`getBoundingClientRect`), so
jsdom unit tests can't exercise it.
- **FE10:** `<desc>` is checked with a selector. `dom-accessibility-api`
doesn't compute descriptions from SVG `<desc>`, although Chromium does
per SVG-AAM.
- **FE8:** the announcement string is derived during render. The one
remaining effect only pushes it into `ChartContainer`'s state, because a
component can't update another component's state while rendering.
<details>
<summary>Implementation plan</summary>
## Findings (recharts 2.15.4, verified in `node_modules`)
- `generateCategoricalChart.js:1937-1953`: with `accessibilityLayer`,
the `<svg>` gets `tabIndex = props.tabIndex ?? 0`, `role = props.role ??
"application"`, and `onKeyDown`/`onFocus` handlers. The handlers are
attached whatever the role is, so `role="group"` keeps
ArrowLeft/ArrowRight working.
- Keyboard navigation (`AccessibilityManager.spoofMouse`) goes through
the same `triggeredAfterMouseMove` path as the mouse, so the tooltip
shows the same content for both.
- `container/Surface.js:41`: the `title` and `desc` props render as
`<title>` and `<desc>` children of the `<svg>`. These give the SVG its
accessible name and description.
- `component/Tooltip.js`: `content` is always rendered and receives an
`active` prop. The tooltip wrapper (`TooltipBoundingBox`, `tabIndex=-1`)
is a sibling of the `<svg>` and is `visibility: hidden` when inactive.
That makes it unreliable as a live region, so announcements need their
own element that is always mounted.
- Three charts use `accessibilityLayer`: `UserEngagementChart`
(flagged), `ActiveUserChart`, `LicenseSeatConsumptionChart`. All three
are fixed.
## Changes
1. **Shared live region in `Chart.tsx`:** `ChartContainer` holds the
announcement state and renders an always-mounted `sr-only` live region
next to `ResponsiveContainer`. `ChartTooltipContent` sets the
announcement while active and clears it when inactive.
2. **Per-chart props:** keep `accessibilityLayer`; add `role="group"`,
`aria-roledescription="chart"`, `title` (the section name), and `desc`
(date range plus "Use the left and right arrow keys to read each day.").
## Tests (red, then green)
`play` functions on the existing stories, which run in Chromium through
`@storybook/addon-vitest`:
- **User engagement:** the chart is a named group with roledescription
"chart" and the expected `<desc>`. There is no `application` role.
Focusing the chart and pressing ArrowRight updates the live region.
- **The other two charts:** role and name only.
- **`components/Chart` default story:** without formatters, the
announcement is built from the label, the item name and the value
("January Users 186").
With the source changes reverted, all three tests fail.
## Decision log
- **Role:** `group` rather than `img`, because `img` hides the content
and conflicts with the focusable, arrow-key-driven chart. `figure` has
no interaction meaning, and `graphics-document` has poor screen-reader
support.
- **Keep the arrow-key tooltip** for keyboard users without a screen
reader, and announce changes through a live region.
- **Add `aria-roledescription="chart"`**, so screen readers say "chart"
rather than "grouping".
- **Memoize the `ChartContext` value** in `ChartContainer`, so
announcement updates don't re-render every `useChart` consumer.
`components/Chart` isn't covered by the React Compiler.
- **Deviations from the plan:**
- The live region gained `role="status"`, so it can be queried by role.
- The plan keyed the announcement effect on `active` and `payload`. The
first version instead ran on every render and read the text from the
DOM. Following review, the string is now derived from the label and item
values (strings and numbers only), and the effect depends on that
string.
- The test focuses the chart directly instead of pressing Tab, because
the first Tab stop is the "How we calculate" button.
- The description is checked on `<desc>` rather than with
`toHaveAccessibleDescription` (see FE10 above).
- The planned "live region empties on leave" assertion was dropped,
because recharts doesn't clear the tooltip on blur.
## Known tradeoffs
- **Mouse hover:** moving the mouse over the chart also triggers
announcements for screen-reader users, once per point.
- **NVDA:** NVDA users need to be in focus mode for the arrow keys to
reach the chart. The `desc` hint covers this.
- **No data table:** this PR doesn't add a table alternative to the
chart, so the WCAG 1.1.1 risk for the data itself remains.
</details>