Skip to content

fix(site/src): replace role="application" on recharts charts - #30375

Merged
aqandrew merged 6 commits into
mainfrom
aqandrew/deq-408-application-roleapplication-is-used-inappropriately
Oct 15, 2026
Merged

aqandrew merged 6 commits into
mainfrom
aqandrew/deq-408-application-roleapplication-is-used-inappropriately

Conversation

@aqandrew

@aqandrew aqandrew commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

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.
Implementation plan

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.

recharts' accessibilityLayer defaults the chart <svg> to
role="application", which axe flags (custom-role-application) and
which hides chart content from screen readers.

Override the role with role="group" and aria-roledescription="chart",
keep accessibilityLayer for the arrow-key tooltip, and label each chart
with recharts' title/desc props. ChartContainer now renders a polite
live region that mirrors the active tooltip text so keyboard
navigation is announced.

Fixes DEQ-408
@linear-code

linear-code Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

DEQ-408

DEQ-342

@aqandrew
aqandrew marked this pull request as ready for review October 7, 2026 23:38
Comment thread site/src/components/Chart/Chart.tsx Outdated
Comment thread site/src/components/Chart/Chart.tsx Outdated
…ooltipContent

No caller passes a ref, so attach the internal ref directly instead of
merging it with the prop.

@Jesswinters Jesswinters left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! This is a great addition. I left a couple suggestions for improvements ✨

Comment thread site/src/pages/DeploymentSettingsPage/OverviewPage/UserEngagementChart.tsx Outdated
Comment thread site/src/components/Chart/Chart.tsx Outdated
…ng render

Build the live-region text from the same label and item values the
tooltip renders, and sync it with an effect keyed on that string. This
replaces the effect that ran on every render and read the tooltip text
from the DOM.
Announcement updates re-render ChartContainer. Without a stable context
value, each update also re-renders every useChart consumer. Callers pass
a module-level config, so the memo holds.
@aqandrew
aqandrew requested a review from aslilac October 15, 2026 00:09
@aqandrew
aqandrew merged commit 5261723 into main Oct 15, 2026
26 of 27 checks passed
@aqandrew
aqandrew deleted the aqandrew/deq-408-application-roleapplication-is-used-inappropriately branch October 15, 2026 21:02
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 15, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants