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

Fix accessibility table examples #23821

Merged

Conversation

forsti0506
Copy link
Contributor

Hello,

I'd like to commit some fixes to the accessibility of the angular examples. In my opinion many people are copying these examples and therefore it is important to provide accessible examples.

First I added an arrow to expand tables, because otherwise the row is only accessible by clicking with a mouse!

I hope you have time to review my first Pull request!

The `nbsp; is needed, because for example NVDA doesn't recognize it, if it is used without any content!

Second Part is to enable focusing scrollable regions to ensure that screen an tabusers can access them!

Thanks in Advance!

Best regards,

Martin

@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Oct 22, 2021
@andrewseguin andrewseguin removed the cla: yes PR author has agreed to Google's Contributor License Agreement label Dec 28, 2021
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Dec 29, 2021
Copy link
Contributor

@andrewseguin andrewseguin left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. Just one thing to change and we can get these changes in.

@forsti0506
Copy link
Contributor Author

@andrewseguin Maybe I missed the suggested change, but I can't see the thing to change?

@andrewseguin
Copy link
Contributor

Odd my comment didn't get through, maybe because it's not an affected line. Be sure to change [attr.colspan]="columnsToDisplay.length" to use columnsToDisplayWithExpand instead so that the bottom border stretches across all cells

@forsti0506
Copy link
Contributor Author

I added the fix.

Maybe one question from me: I saw there is an experimental section too for table. Should I apply the fix there too in this PR?

@forsti0506 forsti0506 force-pushed the fix-accessibility-table-examples branch from 35480f5 to 21c1079 Compare February 28, 2022 21:22
@forsti0506 forsti0506 requested a review from andrewseguin March 8, 2022 20:19
@andrewseguin
Copy link
Contributor

Yes - that'd be great if you can add the fix to the MDC based versions. We're working on making those the primary versions now

@forsti0506 forsti0506 force-pushed the fix-accessibility-table-examples branch from 7446dc9 to 1a49cf5 Compare April 3, 2022 12:35
@google-cla google-cla bot added cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels Apr 3, 2022
@forsti0506 forsti0506 force-pushed the fix-accessibility-table-examples branch from 1a49cf5 to 21c1079 Compare April 3, 2022 12:43
@google-cla google-cla bot added cla: yes PR author has agreed to Google's Contributor License Agreement and removed cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla labels Apr 3, 2022
@forsti0506
Copy link
Contributor Author

@andrewseguin I added the features for the mdc examples.

I'm not sure how to fix the CircleCi errors, because the build fails because of my master merge!

@andrewseguin
Copy link
Contributor

It looks like one of the issues is that table-sticky-complex-flex-example.html has the beginning div switched to a section but not the closing div

@forsti0506
Copy link
Contributor Author

Yes thanks: closing tag is fixed, but I think the merge is blocked, because of the commit message for the master merge!

@forsti0506 forsti0506 force-pushed the fix-accessibility-table-examples branch from 16bb33e to 9339019 Compare April 4, 2022 17:47
@andrewseguin andrewseguin added the merge: fix commit message When the PR is merged, rewrites/fixups of the commit messages are needed label Apr 5, 2022
@devversion devversion removed request for devversion and a team April 9, 2022 13:40
@andrewseguin andrewseguin added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release target: major This PR is targeted for the next major release and removed target: patch This PR is targeted for the next patch release labels Apr 21, 2022
@andrewseguin andrewseguin merged commit 8dfbbed into angular:master Apr 21, 2022
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators May 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement merge: fix commit message When the PR is merged, rewrites/fixups of the commit messages are needed target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants