-
Notifications
You must be signed in to change notification settings - Fork 199
USWDS-Site: Settings documentation #1755
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
|
@bonnieAcameron @mejiaj I know this is larger than the scope of the original issue, so let me know if the solution should be reined in. Let me know if you have any thoughts/opinions! |
Hi Amy! This looks great! I added a couple of clarity suggestions and one grammatical update, but overall this is a super clear update, and definitely needed.
bonnieAcameron
left a comment
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 Amy! I edited the file with a few of my suggestions & a grammar update, but this is an excellent update to the settings page.
mejiaj
left a comment
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.
Nice job! This introduction seems clear and straightforward. Added some comments and questions on a few small things.
Co-authored-by: James Mejia <james.mejia@gsa.gov>
bonnieAcameron
left a comment
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! I've reviewed for style guide, typos, and accessibility and have a few small changes to suggest:
Line 25-If we're specifically talking about the USWDS Design System, then we capitalize it: the Design System
Line 25, 44-instead of saying "tables below," (directional language isn't best practice for accessibility) can we use active voice to say, you can find USWDS settings and their default values in the USWDS settings tables
@bonnieAcameron Great feedback! I've updated these items in a5c672f |
mejiaj
left a comment
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.
Good changes, I like the added clarity. Thanks @amyleadem!
bonnieAcameron
left a comment
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.
Reviewed for clarity, plain language (within reason) & typos.
Had a few ideas/clarifications, but other than that, fantastic changes.
include not found your not found configuration not found section
|
@bonnieAcameron I added some items to clarify in these commits: 2073839 and 840bffd.
|
bonnieAcameron
left a comment
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, Amy!
thisisdano
left a comment
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.
Nice improvement here
Summary
Removed redundant instructions for setting up the Sass file structure from the settings page.
Related issue
Closes #1701
Recommend releasing this update with the Phase 2 compile update in PR #1766
Preview link
Preview link: Settings page
Problem statement
Providing the same instructions in two different locations can cause confusion for the user and requires double maintenance. Additionally, users looking to configure their settings will receive more information than needed.
Solution
Replacing the instructions for setting up Sass files with a link to the Getting Started guide allows the user to focus on only configuring the settings.
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.