[Seedless Onboarding]: Settings password form #2644

usame-algan · 2023-10-17T09:29:29Z

What it solves

Resolves #2519

How this PR fixes it

Updates the design for the password form
Adds a password strength check
Uses useWallet on the settings page to determine display logic
Dispatches events on success/fail when submitting the form

ToDos

- Extract strings
Update settings page automatically when logging in with google
Dispatch notifications on success/fail

How to test it

TBD

Screenshots

Checklist

I've tested the branch on mobile 📱
I've documented how it affects the analytics (if at all) 📊
I've written a unit/e2e test for it (if applicable) 🧑‍💻

github-actions · 2023-10-17T09:29:43Z

Branch preview

✅ Deploy successful!

https://settings_form--walletweb.review-wallet-web.5afe.dev

github-actions · 2023-10-17T09:33:08Z

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type	Occurrences	Fixable
Errors	0	0
Warnings	0	0
Ignored	0	N/A

Result: ✅ success
Annotations: 0 total

^{Report generated by eslint-plus-action}

…fications on success/error

github-actions · 2023-10-17T13:14:59Z

Coverage report

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	74.46% (-0.14% 🔻)	9339/12543
🔴	Branches	48.5% (-0.21% 🔻)	1870/3856
🔴	Functions	56.31% (-0.14% 🔻)	1370/2433
🟡	Lines	76.02% (-0.12% 🔻)	8443/11106

Show new covered files 🐣

St.^❔	File	Statements	Branches	Functions	Lines
🔴	`...` / PasswordForm.tsx	56.86%	30%	37.5%	59.57%
🔴	`...` / helper.ts	28.13%	0%	0%	31.03%
🟡	`...` / PasswordInput.tsx	50%	0%	0%	62.5%

Test suite run success

998 tests passing in 141 suites.

Report generated by 🧪jest coverage report action from 5615d17

schmanu · 2023-10-18T16:16:46Z

src/components/settings/SignerAccountMFA/PasswordForm.tsx

+  const dispatch = useAppDispatch()
+  const mpcCoreKit = useMPC()
+  const [passwordStrength, setPasswordStrength] = useState<PasswordStrength>(PasswordStrength.weak)
+  const [passwordsMatch, setPasswordsMatch] = useState<boolean>(false)


Should we move this to the react hook form validation?

schmanu · 2023-10-18T16:17:33Z

src/components/settings/SignerAccountMFA/PasswordForm.tsx

+                        onChange: (event: ChangeEvent<HTMLInputElement>) => {
+                          const confirmNewPW = getValues(PasswordFieldNames.confirmPassword)
+                          const value = event.target.value
+                          setPasswordsMatch(value !== '' && value === confirmNewPW)


As mentioned above: Should we use the form validation for this?

schmanu · 2023-10-18T16:23:24Z

src/components/settings/SignerAccountMFA/PasswordForm.tsx

+                        passwordStrength === PasswordStrength.strong
+                          ? css.strongPassword
+                          : passwordStrength === PasswordStrength.medium
+                          ? css.mediumPassword
+                          : css.weakPassword


We have two big condition blocks like this:
For the css class and for the label, should we add a Record<PasswordStrength, { label: string, cssClass: string}> mapping?

Good idea! I've extracted them into a const. I had to add a ts-ignore for the className though because css module imports don't seem to be typed.

schmanu · 2023-10-18T16:24:55Z

src/components/settings/SignerAccountMFA/PasswordForm.tsx

+}
+
+// At least 12 characters, one lowercase, one uppercase, one number, one symbol
+const strongPassword = new RegExp('(?=.*[a-z])(?=.*[A-Z])(?=.*[0-9])(?=.*[^A-Za-z0-9])(?=.{12,})')


Let's add tests for this component or at least for these RegEx as RegEx are quite error prone.

I extracted the logic into a helper function and wrote a few tests for it.

schmanu · 2023-10-18T16:27:08Z

src/components/settings/SignerAccountMFA/index.tsx


-  if (mpcCoreKit?.status !== COREKIT_STATUS.LOGGED_IN) {
+  if (wallet?.label !== ONBOARD_MPC_MODULE_LABEL) {


Nit: I think we are using this check quite frequently across the app.
Should we add a small helper isSocialLogin(wallet)?

schmanu

The form looks great now!

I have a small suggestion towards how to get rid of the ts-ignore annotations in an elegant way.

schmanu · 2023-10-20T08:31:53Z

src/components/settings/SignerAccountMFA/PasswordForm.tsx

+type PasswordStrengthMap = Record<PasswordStrength, { label: string; className: string }>
+
+const passwordStrengthMap: PasswordStrengthMap = {
+  [PasswordStrength.strong]: {
+    label: 'Strong',
+    className: 'strongPassword',
+  },
+  [PasswordStrength.medium]: {
+    label: 'Medium',
+    className: 'mediumPassword',
+  },
+  [PasswordStrength.weak]: {
+    label: 'Weak',
+    className: 'weakPassword',
+  },
+}


If you rewrite the PasswordStrengthMap and cast it to const it will fix the type errors below and we can remove the ts-ignores.

I think it also makes sense here as the passwordStrengthMap is effectively readonly and won't be mutated.

Suggested change

type PasswordStrengthMap = Record<PasswordStrength, { label: string; className: string }>

const passwordStrengthMap: PasswordStrengthMap = {

[PasswordStrength.strong]: {

label: 'Strong',

className: 'strongPassword',

},

[PasswordStrength.medium]: {

label: 'Medium',

className: 'mediumPassword',

},

[PasswordStrength.weak]: {

label: 'Weak',

className: 'weakPassword',

},

}

const passwordStrengthMap = {

[PasswordStrength.strong]: {

label: 'Strong',

className: 'strongPassword',

},

[PasswordStrength.medium]: {

label: 'Medium',

className: 'mediumPassword',

},

[PasswordStrength.weak]: {

label: 'Weak',

className: 'weakPassword',

},

} as const

schmanu · 2023-10-20T08:32:40Z

src/components/settings/SignerAccountMFA/PasswordForm.tsx

+                      alignItems="center"
+                      gap={1}
+                      mt={1}
+                      // @ts-ignore


We can remove this if we change the map's type slightly (see comment above)

schmanu · 2023-10-20T08:51:41Z

src/components/settings/SignerAccountMFA/helper.ts

  } catch (e) {
    const error = asError(e)
    logError(ErrorCodes._304, error.message)
+
+    dispatch(
+      showNotification({


Should we display the error inside the modal instead wrapped inside an <ErrorMessage>?
Not sure what is more consistent here.

I added a comment in Figma to check with Design

schmanu

Vamos! 🚀

usame-algan added 2 commits October 16, 2023 16:05

feat: Implement new form layout

53984fb

fix: Add password strength check, add cancel button

193e06d

usame-algan added 4 commits October 17, 2023 12:11

fix: Use RHF isSubmitting flag, move validate logic into onChange

5e854ef

fix: Update form dynamically when connecting/disconnecting, show noti…

ffbdf20

…fications on success/error

Merge remote-tracking branch 'origin/web3authcoresdk' into settings-form

196e469

fix: Update password match when typing in a new password

ec93102

usame-algan requested a review from schmanu October 17, 2023 15:25

usame-algan added 3 commits October 17, 2023 17:25

fix: Allow medium and strong passwords

d2484e9

Merge remote-tracking branch 'origin/web3authcoresdk' into settings-form

f4598a2

fix: reset form state, fix accordion expand

6ab2ae0

schmanu reviewed Oct 18, 2023

View reviewed changes

usame-algan added 3 commits October 19, 2023 09:26

fix: simplify validation, extract regex logic and test

3dca1c5

refactor: Extract isSocialLoginWallet

d61abc6

Merge remote-tracking branch 'origin/web3authcoresdk' into settings-form

d0b8112

usame-algan requested a review from schmanu October 19, 2023 07:42

fix: Failing test

4379603

schmanu reviewed Oct 20, 2023

View reviewed changes

usame-algan added 2 commits October 20, 2023 12:20

fix: Remove ts-ignore

5406014

fix: Dont reset form fields on error

5615d17

usame-algan requested a review from schmanu October 20, 2023 14:53

schmanu approved these changes Oct 23, 2023

View reviewed changes

usame-algan merged commit 0140ec0 into web3authcoresdk Oct 23, 2023
7 of 8 checks passed

usame-algan deleted the settings-form branch October 23, 2023 08:31

github-actions bot locked and limited conversation to collaborators Oct 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Seedless Onboarding]: Settings password form #2644

[Seedless Onboarding]: Settings password form #2644

usame-algan commented Oct 17, 2023 •

edited

Loading

github-actions bot commented Oct 17, 2023 •

edited

Loading

github-actions bot commented Oct 17, 2023 •

edited

Loading

github-actions bot commented Oct 17, 2023 •

edited

Loading

schmanu Oct 18, 2023

schmanu Oct 18, 2023

schmanu Oct 18, 2023

usame-algan Oct 19, 2023

schmanu Oct 18, 2023

usame-algan Oct 19, 2023

schmanu Oct 18, 2023

usame-algan Oct 19, 2023

schmanu left a comment

schmanu Oct 20, 2023

schmanu Oct 20, 2023

schmanu Oct 20, 2023

usame-algan Oct 20, 2023

schmanu left a comment


		if (mpcCoreKit?.status !== COREKIT_STATUS.LOGGED_IN) {
		if (wallet?.label !== ONBOARD_MPC_MODULE_LABEL) {

[Seedless Onboarding]: Settings password form #2644

[Seedless Onboarding]: Settings password form #2644

Conversation

usame-algan commented Oct 17, 2023 • edited Loading

What it solves

How this PR fixes it

ToDos

How to test it

Screenshots

Checklist

github-actions bot commented Oct 17, 2023 • edited Loading

Branch preview

github-actions bot commented Oct 17, 2023 • edited Loading

ESLint Summary View Full Report

github-actions bot commented Oct 17, 2023 • edited Loading

Coverage report

Test suite run success

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schmanu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schmanu left a comment

Choose a reason for hiding this comment

usame-algan commented Oct 17, 2023 •

edited

Loading

github-actions bot commented Oct 17, 2023 •

edited

Loading

github-actions bot commented Oct 17, 2023 •

edited

Loading

github-actions bot commented Oct 17, 2023 •

edited

Loading