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

Adds ability to define a scope #859

Merged
merged 11 commits into from
Oct 25, 2022

Conversation

SpaNb4
Copy link
Contributor

@SpaNb4 SpaNb4 commented Aug 5, 2022

In this PR I added:

  • an optional colScope and rowScope props to the table columns;
  • background color for th elements in tbody so that the user can more easily identify the table headers;
  • if the title for some th in thead is not specified, then I change th to td to fix the error
    Screenshot
    image

So, by adding these props, we give developers an ability to improve accessibility in cases where the browser incorrectly determines the header cell type. And this, combined with other fixes, can help users with a screen reader to better understand and navigate through tables.

Examples that describe this a11y topic in more detail: Simple table, Table with two-level header

Without the scope attribute, tables that use rc-table may not meet the WCAG 1.3.1 success criteria.
Ref: https://www.w3.org/TR/WCAG20-TECHS/F91.html

update README.md

add colgroup example

combine two scope examples
@vercel
Copy link

vercel bot commented Aug 5, 2022

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

Name Status Preview Updated
table ✅ Ready (Inspect) Visit Preview Oct 20, 2022 at 5:30PM (UTC)

@mellis481
Copy link
Contributor

@afc163 Can we get this merged too please?

@codecov
Copy link

codecov bot commented Aug 17, 2022

Codecov Report

Merging #859 (feaa987) into master (2c6f53d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #859   +/-   ##
=======================================
  Coverage   99.41%   99.41%           
=======================================
  Files          36       36           
  Lines        1017     1022    +5     
  Branches      304      307    +3     
=======================================
+ Hits         1011     1016    +5     
  Misses          6        6           
Impacted Files Coverage Δ
src/Cell/index.tsx 99.10% <ø> (ø)
src/Body/BodyRow.tsx 97.91% <100.00%> (+0.04%) ⬆️
src/Body/index.tsx 100.00% <100.00%> (ø)
src/Header/Header.tsx 100.00% <100.00%> (ø)
src/Header/HeaderRow.tsx 100.00% <100.00%> (ø)

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

@mellis481
Copy link
Contributor

@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 please.

README.md Outdated Show resolved Hide resolved
src/Header/HeaderRow.tsx Outdated Show resolved Hide resolved
src/Body/BodyRow.tsx Outdated Show resolved Hide resolved
@mellis481
Copy link
Contributor

@zombieJ @yoyo837 This PR adds some very valuable functionality. Can we get this merged?

@yoyo837 yoyo837 requested a review from zombieJ October 4, 2022 13:21
src/Body/index.tsx Outdated Show resolved Hide resolved
@SpaNb4
Copy link
Contributor Author

SpaNb4 commented Oct 14, 2022

@zombieJ @yoyo837 Is there anything I need to add or change to this PR?

@SpaNb4
Copy link
Contributor Author

SpaNb4 commented Oct 18, 2022

@zombieJ It seems to me that CI has started to fail, but this is not due to my changes

@SpaNb4
Copy link
Contributor Author

SpaNb4 commented Oct 18, 2022

And I think the problem is somewhere in package-lock.json, because before reinstalling node_modules, npm run compile worked fine for me

@zombieJ
Copy link
Member

zombieJ commented Oct 19, 2022

Please check CI. CI / compile failed.

@SpaNb4
Copy link
Contributor Author

SpaNb4 commented Oct 19, 2022

Please check CI. CI / compile failed.

I see, but the problem is not on my side. You will see the same error on the master branch
image

@zombieJ
Copy link
Member

zombieJ commented Oct 20, 2022

OK. Let me handle this~

@zombieJ
Copy link
Member

zombieJ commented Oct 20, 2022

Update #891

@zombieJ
Copy link
Member

zombieJ commented Oct 25, 2022

+ rc-table@7.28.0

Please help to create PR on antd also : )

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

5 participants