-
Notifications
You must be signed in to change notification settings - Fork 140
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
USWDS-Site: Add search accessibility tests page #2694
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.
It's looking good! Here are a few edits:
This check "A mouse is not required to perform actions in the search component" has the wrong WCAG success criteria.
I think it's just Keyboard https://www.w3.org/WAI/WCAG21/Understanding/keyboard.html
Search field...adjust on mobile check needs to have "orientation" as it's success criteria https://www.w3.org/WAI/standards-guidelines/wcag/new-in-21/#134-orientation-aa
Screen reader announces that search results will be displayed.
That success criteria should be for "on input" https://www.w3.org/WAI/WCAG22/Understanding/on-input.html
also
"where" needs to be "will" in this sentence: "When you complete a search using a screen reader, you are informed where search results where (WILL) appear."
Committing some text changes.
Committing a change to the subheader
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.
I committed some text tweaks and also left a query about a test I think could use additional clarity.
@sarah-sch Thanks for making those changes! I updated the focus indicator summary to match your recommendation and it's ready for you re-review! |
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.
Looking good. I added some questions and comments below. Please let me know if you have any questions
Deleting extraneous text
Adding missing period.
Fixing comma splice
Adding a missing period
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 the thorough discussion on these items, team. I had a few more items for your review. Please let me know if you have any questions.
…1y-checklist-search
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.
This is getting close! I found a couple other small edits, but hopefully they are quick to resolve.
Also, could you pull in main
to (hopefully) resolve the build errors?
Co-authored-by: Amy Leadem <93996430+amyleadem@users.noreply.github.com>
Co-authored-by: Amy Leadem <93996430+amyleadem@users.noreply.github.com>
Co-authored-by: Amy Leadem <93996430+amyleadem@users.noreply.github.com>
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.
Really close! Just a couple of small things I found after doing another read through
Co-authored-by: Amy Leadem <93996430+amyleadem@users.noreply.github.com>
Co-authored-by: Amy Leadem <93996430+amyleadem@users.noreply.github.com>
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.
Looks good to me!
Note
The build errors should be resolved after we merge in #2710 Done!
…swds-site into cm-a11y-checklist-search
…1y-checklist-search
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.
Looking good! I approve! Food for thought for WAY later in the future. Wondering if we will be able to begin adding a section for speech dictation, so we don't have to combine the label in name criteria for screen reader and speech.
Summary
Added accessibility tests page for the search component.
Important
We should update changelog dates before merging.
Related issue
Closes #2626
Preview link
Resources
Testing and review
3.2.3
wcag criterion is accurate and link works