Skip to content
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

Jump to bottom

feat(a11y): add basic aria-label to canvas element #1084

Merged
merged 12 commits into from
Mar 25, 2021
Merged

feat(a11y): add basic aria-label to canvas element #1084

merged 12 commits into from
Mar 25, 2021

Conversation

rshen91
Copy link
Contributor

@rshen91 rshen91 commented Mar 22, 2021 •
edited
Loading

Summary

This PR adds Chart to the canvas element for Cartesian charts. Tested with VoiceOver and safari. Added a jest puppeteer test for the accessibility object model showing that the alternative text is indeed in the AOM.

Checklist

Delete any items that are not applicable to this PR.

  • Unit tests were updated or added to match the most common scenarios

@rshen91 rshen91 mentioned this pull request Mar 22, 2021
Closed
2 tasks
@rshen91 rshen91 changed the title feat(xy_charts): add static alternative text for xy_charts feat: add static alternative text for xy_charts Mar 22, 2021
@rshen91 rshen91 added :accessibility Accessibility related issue :xy Bar/Line/Area chart related labels Mar 23, 2021
@rshen91
Copy link
Contributor Author

rshen91 commented Mar 23, 2021

jenkins test this

myasonik
myasonik approved these changes Mar 23, 2021
View reviewed changes
Copy link

@myasonik myasonik left a comment

Choose a reason for hiding this comment

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

It's a start!

It doesn't solve Charts' accessibility issues but it takes the first step.

src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx Outdated Show resolved Hide resolved
@rshen91 rshen91 marked this pull request as ready for review March 23, 2021 21:52
@codecov-io
Copy link

Codecov Report

Merging #1084 (36a71d8) into master (7e2a5e1) will increase coverage by 0.43%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1084      +/-   ##
==========================================
+ Coverage   71.55%   71.98%   +0.43%     
==========================================
  Files         381      397      +16     
  Lines       11914    12229     +315     
  Branches     2588     2636      +48     
==========================================
+ Hits         8525     8803     +278     
- Misses       3358     3387      +29     
- Partials       31       39       +8
Flag Coverage Δ
unittests 71.55% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../chart_types/xy_chart/renderer/canvas/xy_chart.tsx 94.59% <ø> (ø)
src/mocks/series/utils.ts 100.00% <0.00%> (ø)
src/mocks/specs/index.ts 100.00% <0.00%> (ø)
src/mocks/series/series.ts 91.22% <0.00%> (ø)
src/mocks/series/data.ts 100.00% <0.00%> (ø)
src/mocks/specs/specs.ts 83.33% <0.00%> (ø)
src/mocks/theme.ts 86.66% <0.00%> (ø)
src/mocks/utils.ts 100.00% <0.00%> (ø)
src/mocks/scale/scale.ts 77.77% <0.00%> (ø)
src/mocks/store/store.ts 92.59% <0.00%> (ø)
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7e2a5e1...36a71d8. Read the comment docs.

@markov00 markov00 added wip work in progress and removed wip work in progress labels Mar 24, 2021
@rshen91 rshen91 removed the :xy Bar/Line/Area chart related label Mar 25, 2021
markov00
markov00 approved these changes Mar 25, 2021
View reviewed changes
Copy link
Member

@markov00 markov00 left a comment

Choose a reason for hiding this comment

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

Good to merge after solving merge conflicts

@markov00 markov00 changed the title feat: add static alternative text for xy_charts feat(a11y): add basic aria-label to canvas element Mar 25, 2021
@rshen91 rshen91 merged commit 1a5aef7 into elastic:master Mar 25, 2021
github-actions bot pushed a commit that referenced this pull request Mar 26, 2021
@semantic-release-bot
chore(release): 26.1.0 [skip ci]
58e6b5f 
# [26.1.0](v26.0.0...v26.1.0) (2021-03-26)

### Features

* **a11y:** add basic aria-label to canvas element ([#1084](#1084)) ([1a5aef7](1a5aef7))
* **xy_charts:** render legend inside the chart ([#1031](#1031)) ([ba88122](ba88122)), closes [#861](#861)
@nickofthyme
Copy link
Collaborator

🎉 This PR is included in version 26.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@nickofthyme nickofthyme added the released Issue released publicly label Mar 26, 2021
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this pull request Feb 10, 2022
@semantic-release-bot
chore(release): 26.1.0 [skip ci]
fd910a5 
# [26.1.0](elastic/elastic-charts@v26.0.0...v26.1.0) (2021-03-26)

### Features

* **a11y:** add basic aria-label to canvas element ([opensearch-project#1084](elastic/elastic-charts#1084)) ([d4b3e4f](elastic/elastic-charts@d4b3e4f))
* **xy_charts:** render legend inside the chart ([opensearch-project#1031](elastic/elastic-charts#1031)) ([b271d09](elastic/elastic-charts@b271d09)), closes [opensearch-project#861](elastic/elastic-charts#861)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@myasonik myasonik myasonik approved these changes

@markov00 markov00 markov00 approved these changes

@nickofthyme nickofthyme Awaiting requested review from nickofthyme

@monfera monfera Awaiting requested review from monfera

Assignees
No one assigned
Labels
:accessibility Accessibility related issue released Issue released publicly
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@rshen91 @codecov-io @nickofthyme @markov00 @myasonik