-
Notifications
You must be signed in to change notification settings - Fork 195
USWDS-Site: Add known issues to component pages #2402
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
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.
@jaclinec I have added the known issues to the component pages listed in the preview links in the PR description. I had a few questions for you. Can you take a look when you get a chance and let me know what you think?
alert: true | ||
alert-class: info | ||
alert-heading: Current accessibility issues | ||
alert-content: In late 2022, we found some combo box <a href="#accessibility-combo-box">usability issues</a> when we were testing with people who use screen readers and alternative input devices. For more information, see <a href="https://github.com/uswds/uswds-site/issues/1898">the issue on GitHub</a>. |
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.
Note
Replaced this alert with the standard "known issues" alert
- Determined that it is a doc update and should not be included
@jaclinec I have responded to your comments. Please let me know if you need any other changes! @sarah-sch and/or @finekatie, can you proof the copy for the known issues entries on the pages listed above and let me know if you need any changes? @mejiaj and/or @mahoneycm, can you confirm that the code updates look good? |
Hi @amyleadem , I took a cursory look at the pages and everything looks good to me. I was wondering if there was a plainer way to say it, but if "focus indicator" is a common phrase or term we use, we should leave 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.
I commented before I added review. Apologize for doing backwards!
_data/issues/date-picker.yml
Outdated
- date: 2023-09-29 | ||
summary: The focus indicator is not visible around the calendar icon in high contrast mode. | ||
summaryAdditional: | ||
affectsKeyboard: true | ||
affectsHighContrast: true | ||
githubRepo: uswds | ||
githubNumber: 5535 |
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.
Note
Removed this because this issue will be resolved with 3.8.0
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.
In addition to change I left in line, please change the contribute/join GitHub language in the known issues sections to read as follows:
Help fix these issues or add details on GitHub. Don’t have an account? Please join our community to share feedback and contribute to USWDS.
_data/issues/validation.yml
Outdated
issue_number: 5642 | ||
- date: 2023-10-25 | ||
summary: Users are confused about the purpose of the validation component. | ||
summary_additional: The component does not match user expectations for how their information should be validated in a form input. |
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.
Suggest changing "expectations for" to "expectations of"
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 made this update in 3deeefa
@sarah-sch I've made the requested updates. Please let me know if you need any changes! |
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 making those changes, @amyleadem - looks good!
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.
LGTM, just need to update date
fields before merge.
_data/issues/README.md
Outdated
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.
Tip
Renamed to README.md
in d299fc3. Simpler, follows GitHub guidance, and (unverified) is supposed to improve display when viewing online.
Build issuesLocally, I've run |
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.
Date picker doesn't seem to have the known issues addition in its latest updates section
Validation: shouldn't the second known issue start with "Some users" rather than just "Users"?
@annepetersen, good catch! Double-checked other component changelogs as well. |
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.
LGTM: thanks James!
Updated changelog date in 10fd817 to get ready to merge. |
This is ready to merge. I've run |
Merging over pa11y crashes |
Summary
The known issues were listed in the Moving from usability test findings to issues/actions document. (Google doc 🔒)
Note
Build issues fixed in 0d4cbe5.
Related issue
Closes #2375
Preview link
Problem statement
Add the known issues listed in the Moving from usability test findings to issues/actions document. (Google doc 🔒)
Testing and review
Testing checklist