-
Notifications
You must be signed in to change notification settings - Fork 24
Add accessibility check page #141
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
Add accessibility check page #141
Conversation
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.
Blocking: I don't think this really fits under "testing" - imo, accessibility is too important to NOT be at the top level. Could you please add it to this page as a new section called "Accessibility" with its own toctree.
Question: Currently the page reads as being targeted for developers to make their apps compliant. Is this an avenue of community contribution (e.g. testing for compliance) that we want to encourage the community to adopt?
FYI: Juan is on vacation this week so he'll review the suggestions next week. |
Add table of contents to Contributors page
Thanks @s-makin for having a look at this. I moved the page under Contributors and added the toctable to the index page. Does it look good to you?
Even if the guide is targeted to app maintainers, addressing accessibility issues in existing projects is also a great contribution that we should encourage. |
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.
Sorry for the delay in round 2, I was on PTO last week. Thanks for moving the page. I have done a more in-depth review this time around and have some additional questions and suggestions (and a couple of nits).
Co-authored-by: Sally <[email protected]>
Co-authored-by: Sally <[email protected]>
Co-authored-by: Sally <[email protected]>
Co-authored-by: Sally <[email protected]>
Co-authored-by: Sally <[email protected]>
Co-authored-by: Sally <[email protected]>
@s-makin Thanks a lot for your review. I incorporated your suggestions, except the mention to Server. Please let me know if you disagree with my rationale. |
Your rationale makes sense to me, I marked that point as resolved. The other changes you made all look great to me! The only point I'm still confused on is the "modals" on line 101. I don't know what that actually means -- although someone familiar with UI design might already know, new contributors that we want to encourage into UX testing may not. I'd suggest either finding a more commonly-used replacement word, or if there isn't one, adding a glossary definition for it. |
@s-makin Oops, missed that one. "Dialogs" alone is sufficient, just addressed it :) |
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.
Thanks for all your hard work on this, it's good to be able to feature accessibility so prominently! In follow-up work I'll add supporting text on related pages to drive traffic to this page and feature it alongside all the other ways to contribute :)
I've been working on an accessibility review guide together with @msuchane for the last couple of months. Even if it started as an internal guide for accessibility audits, we think there's value in publishing this to the community, so anyone can use it to review their contributions and also provide feedback.
This page provides guidance mainly on visual checks, keyboard navigation and screen reader; both what to look out for and how to use the assistive technologies themselves for testing.
@msuchane I decided in the end to keep the table for the screen reader shortcuts: it's more compact, which I think makes more sense given the guide is rather lengthy already. Let me know if you disagree.
@rkratky @s-makin Would love to hear your thoughts.
UDENG-7522