-
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: Validation - Add clarity to guidance #1851
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.
This PR looks good! Added a suggestion for improving external-link mixin in USWDS.
| a { | ||
| text-indent: initial; | ||
| } |
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 see this section uses usa-content-list. The external-link mixin could benefit from this change too
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.
Opened issue https://github.com/uswds/uswds-site/issues/1960 to address this
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.
Hi @amyleadem! I added a few clarifying questions and 1 copy edit. For the clarifying questions, take them with a grain of salt, since I'm a 099 level newbie to validation, and I'm assuming anyone reading this will at least have a 101 level understanding. I just flagged places where I felt there could be a bit more explanation.
|
@bonnieAcameron Thanks for the feedback. I added some slight tweaks and some responses to your comments. Let me know if you see anything that needs changing. |
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.
Just a quick reference to the code block, and it looks great!
|
@bonnieAcameron Added a reference note in 9ec48cb. Let me know if you want 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.
the "have yet to be completed" phrasing might be confusing for ESL readers. Kick it on back to me if you agree and we'll get this wrapped for ya!
Co-authored-by: Bonnie Cameron <96838068+bonnieAcameron@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!
Summary
Updated the guidance on the validation page to:
#from thedata-validation-elementvalue.Related issue
Closes #1846
Preview link
Preview link: Validation guidance
Problem statement
data-validation-element. (Related PR: #5001).regular expressionexternal link on this page.Solution
data-validation-element="#validate-code"todata-validation-element="validate-code"when-to-use.md.usa-content-list ol lito resolve style issue.Testing and review
To review:
Before opening this PR, make sure you’ve done whichever of these applies to you:
git pull origin [base branch]to pull in the most recent updates from your base and check for merge conflicts. (Often, the base branch isdevelop).npm testand confirm that all tests pass.