LG-4448 Implement rules of use for new users#4968
Conversation
|
cc @juliaelman @anniehirshman-gsa The before and after for your review |
stevegsa
force-pushed
the
stevegsa-new-users-rules-of-use
branch
from
532ecbf to
5c24a23
Compare
anniehirshman-gsa
left a comment
anniehirshman-gsa
left a comment
There was a problem hiding this comment.
@stevegsa per our conversation on Slack, thanks for your willingness to incorporate a few design changes here. Wanted to suggest these visual adjustments for consistency with the USWDS and the login.gov design system:
- Remove horizontal rule below
h1for consistency with other pages in the IdP flow - Remove blue box around checkbox and use simple checkbox from the login design system, including the error state on the checkbox if the user clicks the "Submit" button without checking the box.
- The USWDS recommends using dropdown inputs only for 7+ options (see guidance), so I would recommend reverting back to the original design of the language selection (radio button group) to align with that guidance.
Let me know if you have any questions, and thanks so much in advance!
stevegsa
force-pushed
the
stevegsa-new-users-rules-of-use
branch
from
4d3017c to
90d4b15
Compare
|
@anniehirshman-gsa new screenshot attached |
zachmargolis
left a comment
zachmargolis
left a comment
There was a problem hiding this comment.
LGTM... but would really love for the deploy feature flag issue to be addressed before merging
config/locales/account/en.yml
Outdated
There was a problem hiding this comment.
Was there consensus on whether we're calling this "Rules of Use" or "Terms of Use" ? We use "Terms" here, and "Rules" in #5040.
There was a problem hiding this comment.
I posted the discrepancy in slack and they said roll with it
config/locales/account/en.yml
Outdated
There was a problem hiding this comment.
In #5040, we embed a link to the rules in the text. Should we be doing that here? Otherwise, it kinda feels like we're asking the user to agree to something without giving them a chance to read what they're agreeing to. Or will we update the text "Security Practices and Privacy Act Statement" to also include "Rules of Use" ?
There was a problem hiding this comment.
This was part of the discrepancies I posted in slack. It appears they want the link "Security Practices and Privacy Act Statement" to go to the new "Rules of Use". There are other tabs there for the security practices and privacy act statement. It does seem odd.
stevegsa
force-pushed
the
stevegsa-new-users-rules-of-use
branch
from
63f4e61 to
2e130ba
Compare
|
Thanks for adding the feature flag 👍 |
|
FYI #5066 changed how locale YAML files are normalized, so you might want to rebase / merge to check for any local changes. # git fetch origin
# git rebase origin/main
yarn install
make normalize_yaml |
stevegsa
force-pushed
the
stevegsa-new-users-rules-of-use
branch
from
7d16c7b to
83dab68
Compare
| <div class="margin-bottom-3"> | ||
| <%= f.check_box :terms_accepted, { class: 'usa-checkbox__input', | ||
| required: true, aria: { invalid: false } }, true, false %> | ||
| <label for="user_terms_accepted" class="usa-checkbox__label"> | ||
| <%= t('sign_up.terms') %> | ||
| <%= new_window_link_to(t('titles.rules_of_use'), MarketingSite.rules_of_use_url) %> | ||
| </label> | ||
| <div class="usa-error-message usa-error-message--with-icon display-if-invalid" role="alert"> | ||
| <%= t('sign_up.agree_to_rules_of_use_and_continue') %> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Should we only be showing this if rules_of_use_enabled config is 'true'? (I expect we might also need to not add the corresponding JavaScript if disabled)
4 participants
Todo: need to add more specs and break this up into two deploys since agreement to terms will not work with old and new servers
Before
After