-
Notifications
You must be signed in to change notification settings - Fork 148
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 list accessibility tests page #2746
Conversation
…blish-list-checklist-items
…blish-list-checklist-items
…uswds/uswds-site into rc-publish-list-checklist-items merge branch
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.
@RachelCorsino thanks for creating this! I can confirm the page works and looks as expected. I updated the PR description preview link so it goes to the new page.
Add a minor comment about updating the date before final review/merge, otherwise LGTM.
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.
Great work on this @RachelCorsino ! It's looking great. Kudos on the changelogs as well! I left a note to update those once this is approved to be merged but you won't have to worry about that right now.
Requesting two small changes to resolve some visual discrepancies.
| title: List accessibility tests | ||
| type: component | ||
| items: | ||
| - date: 2024-07-08 |
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.
todo: We'll want to update this date to the date that the PR will be merged once it's approved.
_data/changelogs/component-list.yml
Outdated
| @@ -2,6 +2,11 @@ title: List | |||
| type: component | |||
| changelogURL: | |||
| items: | |||
| - date: 2024-07-08 | |||
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.
todo: We'll want to update this date to the date that the PR will be merged once it's approved.
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.
@RachelCorsino Looks really good! Thanks for taking this on.
I've added some small notes and questions below (Some of them will need to be answered by @alex-hull and @amycole501). Let me know if you have any questions.
|
All fixes have been made and PR should be ready to go now! This PR is not passing all test due to Snyk alert issue, which as been fixed in the following PR: USWDS-Site: Updates Snyk Alerts #2751 |
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 the quick fixes, @RachelCorsino. I found one more reference to 3.6.1 that should be updated. I've added a suggestion in the comment below.
Other than that, just waiting to hear back from the accessibility team about the 2.4.6 checklist item.
Looking good!
List "Accessibility Guidance" sectionAfter re-reviewing, I remembered that we added the "accessibility guidance" section to the Prose component page in order to display the accessibility page callout section. For consistency, it's probably a good idea to add it here too. I was able to do this by creating a
|
|
Resolved in 25bc6e4
|
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 great! Thanks Rachel 🎉
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! Thanks for your work on this @RachelCorsino.
…blish-list-checklist-items
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! Approved on my end.
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.
Updating changelog dates


Summary
Added accessibility tests page for the list component.
Related issue
Closes #2727
Preview link
Preview link - [
/components/list/accessibility-tests]Testing and review
Follow these steps: