Skip to content

Conversation

amyleadem
Copy link
Contributor

@amyleadem amyleadem commented Dec 19, 2023

Summary

  • Added known issues to component pages.
  • Updated the data keys to use snake case instead of camel case

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

  1. All issues in the Moving from usability test findings to issues/actions doc are included in the known issues section of component pages.
  2. Alert appears as expected on the top of each page that has known issues.
  3. The link in the alert works.
  4. In the "All known issues for [component name] are tracked in GitHub." copy:
    1. Confirm that the GitHub link points to a list of issues tagged with the appropriate "package" label.
    2. Confirm that the "[component name]" is correct for the page.
  5. Issue summaries are accurate, easy to understand, and follow consistent format.
  6. Issue links work as expected.
  7. Changelogs exist and are free from error.

Testing checklist

  • All components have changelog entries
  • Known issues are appearing in Character count, Combo box, Date picker, File input, Input mask, and Validation

Copy link
Contributor Author

@amyleadem amyleadem left a 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?

Comment on lines -56 to -59
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>.
Copy link
Contributor Author

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

@amyleadem amyleadem marked this pull request as ready for review January 25, 2024 16:12
@amyleadem amyleadem requested a review from jaclinec January 25, 2024 16:12
amyleadem added 2 commits January 29, 2024 11:32
- Determined that it is a doc update and should not be included
@amyleadem
Copy link
Contributor Author

@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?

@finekatie
Copy link
Contributor

Hi @amyleadem , I took a cursory look at the pages and everything looks good to me.
One question about a description of an issue on the date picker page:
"The focus indicator is not visible around the calendar icon in high contrast mode. "

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.
@sarah-sch do you have any other input?

Copy link
Contributor

@finekatie finekatie left a 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!

Comment on lines 12 to 18
- 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
Copy link
Contributor Author

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

Copy link
Contributor

@sarah-sch sarah-sch left a 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.

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.
Copy link
Contributor

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"

Copy link
Contributor Author

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

@amyleadem
Copy link
Contributor Author

@sarah-sch I've made the requested updates. Please let me know if you need any changes!

@amyleadem amyleadem requested a review from sarah-sch March 5, 2024 19:42
Copy link
Contributor

@sarah-sch sarah-sch 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 making those changes, @amyleadem - looks good!

Copy link
Contributor

@mejiaj mejiaj left a 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.

Copy link
Contributor

@mejiaj mejiaj Mar 12, 2024

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.

@mejiaj
Copy link
Contributor

mejiaj commented Mar 12, 2024

Build issues

Locally, I've run npm start and separately npm run test. Zero issues found.

Copy link
Contributor

@annepetersen annepetersen left a 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"?

@mejiaj
Copy link
Contributor

mejiaj commented Mar 13, 2024

@annepetersen, good catch!

  • Added Date picker latest updates in 2084a0d
  • Added Some to Validation known issue in d54f846

Double-checked other component changelogs as well.

@mejiaj mejiaj requested a review from annepetersen March 13, 2024 13:58
Copy link
Contributor

@annepetersen annepetersen left a comment

Choose a reason for hiding this comment

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

LGTM: thanks James!

@mejiaj
Copy link
Contributor

mejiaj commented Mar 13, 2024

Updated changelog date in 10fd817 to get ready to merge.

@mejiaj
Copy link
Contributor

mejiaj commented Mar 13, 2024

This is ready to merge. I've run npm run test locally and found zero issues.

@thisisdano
Copy link
Contributor

Merging over pa11y crashes

@thisisdano thisisdano merged commit c8055da into main Mar 18, 2024
@thisisdano thisisdano deleted the al-known-issues branch March 18, 2024 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

USWDS-Site - Add known issues to component pages
8 participants