-
Notifications
You must be signed in to change notification settings - Fork 210
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
feat(ux): Update 2fa signin screens/reset PW copy, create HeadingPrimary #17982
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.
Adding some early comments about some inconsistent terminology that was missed in Figma review - we should verify with Eduardo/Jeff.
confirm-totp-reset-password-input-label-v2 = Enter 6-digit code | ||
confirm-totp-reset-password-use-different-account = Use a different account | ||
confirm-recovery-code-reset-password-input-label = Enter 10-character code |
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 catch that l10n was missing for these strings.
packages/fxa-settings/src/pages/Signin/SigninTotpCode/index.tsx
Outdated
Show resolved
Hide resolved
packages/fxa-settings/src/pages/Signin/SigninTotpCode/index.tsx
Outdated
Show resolved
Hide resolved
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.
My comment about terminology is non-blocking since there's no change to current state, and we can update in a follow-up once we have a decision from UX. This should be good to go once the functional tests are updated and pass. Thank you for you work on this!
signin-recovery-code-sub-heading = Enter backup code | ||
signin-recovery-code-instruction-v2 = Enter one of the one-time use backup codes you saved during two-step authentication setup. |
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.
Are we no longer using the term "backup authentication code"?
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.
Recommendation from UX was to simplify to "backup codes" as we update these flows to add an SMS recovery method. Is there terminology reference material for Mozilla accounts localization?
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.
We have a terminology we use across projects. We added "backup authentication code" based on this previous discussion: https://mozilla-hub.atlassian.net/browse/FXA-4423
We have roughly ~50ish instances where we use that term currently:
https://pontoon.mozilla.org/ja/mozilla-accounts/all-resources/?search=backup+authentication+code&string=254617
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.
@bcolsson Thank you for the catch. We just had a meeting, and we want to go with what we typically go for with these terms for the SMS work and may review to change them all in the future but for now we will go with "backup authentication codes" and "two-step authentication" (instead of two-factor).
b506061
to
dcd156c
Compare
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.
Added a couple of copy questions, non-blocking.
</FtlMsg> | ||
</h1> | ||
<FtlMsg id="inline-recovery-key-setup-hint-header"> | ||
<HeadingPrimary>Security recommendation</HeadingPrimary> |
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.
Should the heading be the same for all steps? Either "Security recommendation" or "Secure your account"?
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 call out. I remember running this one by Eduardo before - the first step is a recommendation. If they're on step 2, they're in the process of doing the thing.
@@ -138,23 +139,19 @@ const AccountRecoveryConfirmKey = ({ | |||
}; | |||
|
|||
return ( | |||
<AppLayout> | |||
<AppLayout cardClass="card-base"> |
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.
Note to self: remember to check for new mentions of "card-base" when finalizing FXA-10613
signin-recovery-code-input-label = Enter 10-digit backup authentication code | ||
signin-recovery-code-heading = Sign in | ||
signin-recovery-code-sub-heading = Enter backup authentication code | ||
signin-recovery-code-instruction-v2 = Enter one of the one-time use backup authentication codes you saved during two-step authentication setup. |
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 think one-time-use
should have hyphens between all words.
|
||
<FtlMsg id="signin-recovery-code-instruction-v2"> | ||
<p className="mt-2 text-sm"> | ||
Enter one of the one-time use backup authentication codes you saved |
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.
Ditto here for "one-time-use"
Because: * We are prepping for SMS work and want to make some small UX improvements This commit: * Creates HeadingPrimary for our grey h1 text at the top of our flows that we are moving towards * Updates copy and styling for 2FA signin and reset password closes FXA-10210
Because:
This commit:
closes FXA-10210
See UX channel, there's a slight variance to what's in the ticket, Figma, and what's here, but this should be what's discussed in thread, and I will add notes on that other ticket mentioned with follow ups: