-
Notifications
You must be signed in to change notification settings - Fork 128
chore(refactor): refactors A11Y screen reader description generation into chart selector factories #2699
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.
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
walterra
merged 55 commits into
elastic:main
from
walterra:a11y-refactor-screen-reader-descriptions
Sep 15, 2025
Merged
chore(refactor): refactors A11Y screen reader description generation into chart selector factories #2699
walterra
merged 55 commits into
elastic:main
from
walterra:a11y-refactor-screen-reader-descriptions
Sep 15, 2025
Conversation
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
…criptions and axis information
…ture and remove unnecessary wrapper div
…owing ChartSelectors pattern
…and single values
… reader selectors
… reader selectors
…troducing generic ChartLabelData interface
…specific code area
…ngs with clearer naming
…ription component
…erated descriptions
ffdc472
to
a71cbe3
Compare
markov00
reviewed
Sep 4, 2025
packages/charts/src/chart_types/goal_chart/state/selectors/get_screen_reader_data.ts
Outdated
Show resolved
Hide resolved
andrimal
reviewed
Sep 4, 2025
packages/charts/src/chart_types/goal_chart/state/selectors/get_screen_reader_data.ts
Show resolved
Hide resolved
6 tasks
markov00
approved these changes
Sep 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes looks good to me! Thanks now goal chart is more readable
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
In general Elastic Charts has strict separation of concerns and individual charts are not aware of each other. How screen reader summaries were created was a bit of an exemption with chart type checks and customization within the screen reader components. To prepare for more detailed screen reader descriptions for individual charts, this PR adds an optional
getScreenReaderData
selector to the chart selector factories. This allows to create individual chart descriptions using a generic shared screen reader component instead of the current hard coded logic.Details
ChartSelectors
patternChartLabelData
interface to make screen reader components chart-agnostic (used by the goal chart)div
wrappers with semantic HTML elements likefigcaption
canvas
elementsRefactoring examples:
packages/charts/src/components/accessibility/screen_reader_summary.tsx
no longer includes chart specific logic (likegoalChartData
). Instead this can be now found in the chart specific selector inpackages/charts/src/chart_types/goal_chart/state/selectors/get_screen_reader_data.ts
.ScreenReaderPartitionTable
(specific to the partition chart) was moved from the commonaccessbility
directory to a chart specific place inpartition_chart
.Note: Now that we have baseline e2e a11y tests on CI, we can make a refactor like this with more confidence. The PR passes CI without any updates necessary to the a11y assertions.
Screencast of Safari with A11Y VoiceOver on a basic line chart with current description:
a11y-elastic-charts-0001.mp4
Issues
Part of elastic/kibana#36386.
Checklist
closes #123
,fixes #123
)