Repository navigation
Conversation
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
…ly, without useMemo
aqandrew
marked this pull request as ready for review
October 7, 2026 23:38
aslilac
reviewed
Oct 8, 2026
…ooltipContent No caller passes a ref, so attach the internal ref directly instead of merging it with the prop.
Jesswinters
approved these changes
Oct 10, 2026
Jesswinters
left a comment
Contributor
There was a problem hiding this comment.
LGTM! This is a great addition. I left a couple suggestions for improvements ✨
…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.
aslilac
approved these changes
Oct 15, 2026
aqandrew
deleted the
aqandrew/deq-408-application-roleapplication-is-used-inappropriately
branch
October 15, 2026 21:02
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
This PR was generated by Coder Agents on behalf of @aqandrew.
Fixes DEQ-408 (axe
custom-role-applicationon the user engagement chart). Also fixes DEQ-342recharts'
accessibilityLayersetsrole="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) withrole="group"andaria-roledescription="chart". It also gives each chart an accessible name and description through recharts'title/descprops.accessibilityLayerstays, so keyboard users can still move the tooltip with the arrow keys. To make that work for screen readers,ChartContainernow renders a politerole="status"live region that is always mounted.ChartTooltipContentbuilds 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:
playfunctions. recharts' keyboard navigation depends on real layout (getBoundingClientRect), so jsdom unit tests can't exercise it.<desc>is checked with a selector.dom-accessibility-apidoesn't compute descriptions from SVG<desc>, although Chromium does per SVG-AAM.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: withaccessibilityLayer, the<svg>getstabIndex = props.tabIndex ?? 0,role = props.role ?? "application", andonKeyDown/onFocushandlers. The handlers are attached whatever the role is, sorole="group"keeps ArrowLeft/ArrowRight working.AccessibilityManager.spoofMouse) goes through the sametriggeredAfterMouseMovepath as the mouse, so the tooltip shows the same content for both.container/Surface.js:41: thetitleanddescprops render as<title>and<desc>children of the<svg>. These give the SVG its accessible name and description.component/Tooltip.js:contentis always rendered and receives anactiveprop. The tooltip wrapper (TooltipBoundingBox,tabIndex=-1) is a sibling of the<svg>and isvisibility: hiddenwhen inactive. That makes it unreliable as a live region, so announcements need their own element that is always mounted.accessibilityLayer:UserEngagementChart(flagged),ActiveUserChart,LicenseSeatConsumptionChart. All three are fixed.Changes
Chart.tsx:ChartContainerholds the announcement state and renders an always-mountedsr-onlylive region next toResponsiveContainer.ChartTooltipContentsets the announcement while active and clears it when inactive.accessibilityLayer; addrole="group",aria-roledescription="chart",title(the section name), anddesc(date range plus "Use the left and right arrow keys to read each day.").Tests (red, then green)
playfunctions on the existing stories, which run in Chromium through@storybook/addon-vitest:<desc>. There is noapplicationrole. Focusing the chart and pressing ArrowRight updates the live region.components/Chartdefault 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
grouprather thanimg, becauseimghides the content and conflicts with the focusable, arrow-key-driven chart.figurehas no interaction meaning, andgraphics-documenthas poor screen-reader support.aria-roledescription="chart", so screen readers say "chart" rather than "grouping".ChartContextvalue inChartContainer, so announcement updates don't re-render everyuseChartconsumer.components/Chartisn't covered by the React Compiler.role="status", so it can be queried by role.activeandpayload. 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.<desc>rather than withtoHaveAccessibleDescription(see FE10 above).Known tradeoffs
deschint covers this.