Skip to content
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 - Form templates: Add guidance for labelling required fields #1834

Merged
merged 25 commits into from May 10, 2023

Conversation

bonnieAcameron
Copy link
Contributor

@bonnieAcameron bonnieAcameron commented Oct 14, 2022

Summary

Added documentation about using required fields in forms.

Context

@jaclinec conducted research on best practices for required fields within forms and shared them here. We worked together to find the best place and format for this required guidance.

Preview link

Preview link:

Testing and review

Please:

  • check for consistent formatting
  • let us know if the structure makes sense
  • let us know if the location of the new instructions for required fields makes sense
  • let us know if we need additional information in the subheading for usability guidance, for example: "required fields usability guidance"

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.

I'm able to see the guidance on the Form templates preview.

For some reason clicking on Components in preview returns a 404 for me.

 ---

The guidance itself looks good to me.

_includes/accessibility.html Outdated Show resolved Hide resolved
_includes/accessibility.html Outdated Show resolved Hide resolved
_includes/accessibility.html Outdated Show resolved Hide resolved
_includes/accessibility.html Outdated Show resolved Hide resolved
_includes/accessibility.html Outdated Show resolved Hide resolved
_includes/accessibility.html Outdated Show resolved Hide resolved
_includes/accessibility.html Outdated Show resolved Hide resolved
Copy link
Contributor

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

@bonnieAcameron @mejiaj @jaclinec
Guidance looks good! I added some suggestions in the comments above.

Additionally, I believe it would make sense to update our form templates examples to match these recommendations – specifically, add the (optional) tag to our optional fields, and make the fields in the the password reset form and sign in form required (This is at least my understanding of our guidance).

I also think it would be helpful to include a short note about required fields with a link to this guidance in the usability sections of our Address form, Name form, Password reset form, Sign-in form pages.

Let me know if you need any help with anything or if you have questions!

bonnieAcameron and others added 10 commits October 21, 2022 15:03
Co-authored-by: James Mejia <james.mejia@gsa.gov>
Co-authored-by: James Mejia <james.mejia@gsa.gov>
Co-authored-by: James Mejia <james.mejia@gsa.gov>
Co-authored-by: James Mejia <james.mejia@gsa.gov>
Co-authored-by: James Mejia <james.mejia@gsa.gov>
Co-authored-by: James Mejia <james.mejia@gsa.gov>
@bonnieAcameron bonnieAcameron marked this pull request as ready for review October 21, 2022 19:36
@bonnieAcameron
Copy link
Contributor Author

re-requesting reviews!

@amyleadem
Copy link
Contributor

amyleadem commented Mar 24, 2023

@bonnieAcameron @jaclinec @mejiaj
This should be ready for another review!

Note: The build is currently failing because of an error that seems to be independent of this issue, so we can move forward with review. Resolved with 8043f40

@amyleadem amyleadem changed the title Guidance: Support labelling required fields USWDS-Site - Form templates: Add guidance for labelling required fields Mar 24, 2023
@bonnieAcameron
Copy link
Contributor Author

It looks great! I have two observations/suggestions:

  1. The "notes" are styled in a yellow block, and this actually makes the text in the block seem more important than the lists they follow, when in actuality they are more of an aside or exception. Can we style these with a less "important" color?
  2. Can we put the "examples" in the usability guidance list on a new line? For example:

Use text descriptions. Combine red asterisks (*) with a text description at the top of the form instructing the user of its meaning.

Example: “A red asterisk (*) indicates a required field.”

@jaclinec
Copy link
Contributor

I agree with Bonnie's comments and otherwise think this looks good!

@amyleadem
Copy link
Contributor

amyleadem commented Mar 30, 2023

@jaclinec @bonnieAcameron

I wanted to share an alternative solution. I renamed the section from "Usability guidance" to "Identifying required fields". Additionally, I removed the yellow background from the note and italicized the text. Below is a preview of what it could look like (I haven't yet pushed up any code). What do you think?

image

@bonnieAcameron
Copy link
Contributor Author

That's the way to go in my opinion, @amyleadem ! It looks great, and I like the title. It does a much better job than "usability guidance".

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.

I've added Form component to preview links in PR description because it also appears there.

Everything looks good, but we should merge #2037 before this PR.

I've tested for:

  • Typos
  • A11Y issues
  • Ran markup through HTML code sniffer

Copy link
Contributor

Choose a reason for hiding this comment

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

Was this file deleted because none of this applies anymore?

Nevermind, I see an a11y section on both guidance pages.

@amyleadem
Copy link
Contributor

@mejiaj I have merged the form changelogs together so that they can be shared across both the component form and template form pages. Re-requesting your review.

@amyleadem amyleadem requested a review from mejiaj March 31, 2023 18:03
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! Added a suggestion, but doesn't block or require a re-review.

Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be nice to have a comment at the top that says what pages this data appears on.

Copy link
Contributor

@amyleadem amyleadem Apr 6, 2023

Choose a reason for hiding this comment

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

Great idea. Added a commented note in 0c2c662

@thisisdano thisisdano changed the base branch from main to release-3.5.0 May 8, 2023 18:25
@thisisdano thisisdano added the Needs: Changelog We need to add a changelog item for this change label May 8, 2023
@thisisdano thisisdano mentioned this pull request May 8, 2023
24 tasks
@thisisdano
Copy link
Member

I'll draw up a changelog PR for this change.
Changelog PR: #2096

@thisisdano thisisdano merged commit 1b42c0f into release-3.5.0 May 10, 2023
8 checks passed
@thisisdano thisisdano deleted the bc-updated-forms-guidance branch May 10, 2023 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Changelog We need to add a changelog item for this change
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

5 participants