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.

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

Corrects the element that aria- props are applied to #855

Merged
merged 4 commits into from Dec 21, 2022

Conversation

mellis481
Copy link
Contributor

@mellis481 mellis481 commented Jul 27, 2022

aria- attributes should be applied to the <table> element. Instead, they are applied to the great grandparent of the <table> element where they serve no purpose. eg:

image

This PR moves them to the correct element. As a result, screen readers will correctly announce the table.

This PR does NOT change where the data- props are added (still on the great grandparent of the <table> element).

ref: UIEN-973

@vercel
Copy link

vercel bot commented Jul 27, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
table ✅ Ready (Inspect) Visit Preview Sep 6, 2022 at 1:18PM (UTC)

@codecov
Copy link

codecov bot commented Jul 28, 2022

Codecov Report

Merging #855 (116136b) into master (e3124cf) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 116136b differs from pull request most recent head 1a02db6. Consider uploading reports for the commit 1a02db6 to get more accurate results

@@           Coverage Diff           @@
##           master     #855   +/-   ##
=======================================
  Coverage   99.40%   99.40%           
=======================================
  Files          36       36           
  Lines        1004     1005    +1     
  Branches      304      304           
=======================================
+ Hits          998      999    +1     
  Misses          6        6           
Impacted Files Coverage Δ
src/Table.tsx 99.21% <100.00%> (+<0.01%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@mellis481
Copy link
Contributor Author

@afc163 This fixes one of the most glaring a11y issues with react-component/table. Can you please merge it?

@mellis481
Copy link
Contributor Author

@zombieJ @afc163 Let's get this merged please.

@yhy-1
Copy link

yhy-1 commented Sep 9, 2022

@afc163 @zombieJ Hello, can you review this PR to solve accessibility issue.

@mellis481
Copy link
Contributor Author

mellis481 commented Oct 20, 2022

@zombieJ @afc163 Bumping again. This is a non-breaking change that addresses a relatively large accessibility defect (the inability to label a table).

@afc163 afc163 requested a review from zombieJ October 21, 2022 02:54
@mellis481
Copy link
Contributor Author

@zombieJ Can you merge this please?

@Ke1sy
Copy link

Ke1sy commented Dec 20, 2022

@zombieJ @afc163 could you please merge this PR? It is very important for my team. Thanks!

@afc163 afc163 merged commit 0504dc6 into react-component:master Dec 21, 2022
@mellis481 mellis481 deleted the aria-label-fix branch December 21, 2022 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants