From 5d05596b4d3bc730aba3211e4e6316ade51f60bd Mon Sep 17 00:00:00 2001
From: Usame Algan <5880855+usame-algan@users.noreply.github.com>
Date: Mon, 23 Oct 2023 17:10:45 +0200
Subject: [PATCH] fix: Address design review, restructure components (#2683)

---
 .../common/ConnectWallet/PasswordRecovery.tsx |  2 +-
 .../ExportMPCAccountModal.tsx                 |  2 +-
 .../SocialSignerExport}/index.tsx             | 10 ++--
 .../SocialSignerExport}/styles.module.css     |  0
 .../SocialSignerMFA}/PasswordInput.tsx        |  0
 .../SocialSignerMFA}/helper.ts                |  0
 .../SocialSignerMFA/index.test.tsx}           |  2 +-
 .../SocialSignerMFA/index.tsx}                | 30 ++++++-----
 .../SocialSignerMFA}/styles.module.css        |  0
 .../settings/SecurityLogin/index.tsx          | 51 +++++++++++++++++++
 .../settings/SignerAccountMFA/index.tsx       | 23 ---------
 .../sidebar/SidebarNavigation/config.tsx      |  8 +--
 .../welcome/WelcomeLogin/WalletLogin.tsx      |  2 +-
 .../__tests__/WalletLogin.test.tsx            |  2 -
 src/config/routes.ts                          |  4 +-
 src/pages/settings/security-login.tsx         | 23 +++++++++
 src/pages/settings/signer-account.tsx         | 50 ------------------
 17 files changed, 106 insertions(+), 103 deletions(-)
 rename src/components/settings/{ExportMPCAccount => SecurityLogin/SocialSignerExport}/ExportMPCAccountModal.tsx (98%)
 rename src/components/settings/{ExportMPCAccount => SecurityLogin/SocialSignerExport}/index.tsx (72%)
 rename src/components/settings/{ExportMPCAccount => SecurityLogin/SocialSignerExport}/styles.module.css (100%)
 rename src/components/settings/{SignerAccountMFA => SecurityLogin/SocialSignerMFA}/PasswordInput.tsx (100%)
 rename src/components/settings/{SignerAccountMFA => SecurityLogin/SocialSignerMFA}/helper.ts (100%)
 rename src/components/settings/{SignerAccountMFA/PasswordForm.test.tsx => SecurityLogin/SocialSignerMFA/index.test.tsx} (96%)
 rename src/components/settings/{SignerAccountMFA/PasswordForm.tsx => SecurityLogin/SocialSignerMFA/index.tsx} (89%)
 rename src/components/settings/{SignerAccountMFA => SecurityLogin/SocialSignerMFA}/styles.module.css (100%)
 create mode 100644 src/components/settings/SecurityLogin/index.tsx
 delete mode 100644 src/components/settings/SignerAccountMFA/index.tsx
 create mode 100644 src/pages/settings/security-login.tsx
 delete mode 100644 src/pages/settings/signer-account.tsx

diff --git a/src/components/common/ConnectWallet/PasswordRecovery.tsx b/src/components/common/ConnectWallet/PasswordRecovery.tsx
index 053a533a27..d5c47fff6c 100644
--- a/src/components/common/ConnectWallet/PasswordRecovery.tsx
+++ b/src/components/common/ConnectWallet/PasswordRecovery.tsx
@@ -13,7 +13,7 @@ import {
 import { useState } from 'react'
 import Track from '../Track'
 import { FormProvider, useForm } from 'react-hook-form'
-import PasswordInput from '@/components/settings/SignerAccountMFA/PasswordInput'
+import PasswordInput from '@/components/settings/SecurityLogin/SocialSignerMFA/PasswordInput'
 
 type PasswordFormData = {
   password: string
diff --git a/src/components/settings/ExportMPCAccount/ExportMPCAccountModal.tsx b/src/components/settings/SecurityLogin/SocialSignerExport/ExportMPCAccountModal.tsx
similarity index 98%
rename from src/components/settings/ExportMPCAccount/ExportMPCAccountModal.tsx
rename to src/components/settings/SecurityLogin/SocialSignerExport/ExportMPCAccountModal.tsx
index efffab0b71..b51a9c7282 100644
--- a/src/components/settings/ExportMPCAccount/ExportMPCAccountModal.tsx
+++ b/src/components/settings/SecurityLogin/SocialSignerExport/ExportMPCAccountModal.tsx
@@ -5,7 +5,7 @@ import { Box, Button, DialogContent, DialogTitle, IconButton, TextField, Typogra
 import { useContext, useState } from 'react'
 import { useForm } from 'react-hook-form'
 import { Visibility, VisibilityOff, Close } from '@mui/icons-material'
-import css from './styles.module.css'
+import css from '@/components/settings/SecurityLogin/SocialSignerExport/styles.module.css'
 import ErrorCodes from '@/services/exceptions/ErrorCodes'
 import { logError } from '@/services/exceptions'
 import ErrorMessage from '@/components/tx/ErrorMessage'
diff --git a/src/components/settings/ExportMPCAccount/index.tsx b/src/components/settings/SecurityLogin/SocialSignerExport/index.tsx
similarity index 72%
rename from src/components/settings/ExportMPCAccount/index.tsx
rename to src/components/settings/SecurityLogin/SocialSignerExport/index.tsx
index d8c2b43601..46900f325a 100644
--- a/src/components/settings/ExportMPCAccount/index.tsx
+++ b/src/components/settings/SecurityLogin/SocialSignerExport/index.tsx
@@ -1,19 +1,19 @@
 import { Alert, Box, Button, Typography } from '@mui/material'
 import { useState } from 'react'
-import ExportMPCAccountModal from './ExportMPCAccountModal'
+import ExportMPCAccountModal from '@/components/settings/SecurityLogin/SocialSignerExport/ExportMPCAccountModal'
 
-const ExportMPCAccount = () => {
+const SocialSignerExport = () => {
   const [isModalOpen, setIsModalOpen] = useState(false)
 
   return (
     <>
       <Box display="flex" flexDirection="column" gap={2} alignItems="flex-start">
         <Typography>
-          Accounts created via Google can be exported and imported to any non-custodial wallet outside of Safe.
+          Signers created via Google can be exported and imported to any non-custodial wallet outside of Safe.
         </Typography>
         <Alert severity="warning">
           Never disclose your keys or seed phrase to anyone. If someone gains access to them, they have full access over
-          your signer account.
+          your social login signer.
         </Alert>
         <Button color="primary" variant="contained" disabled={isModalOpen} onClick={() => setIsModalOpen(true)}>
           Reveal private key
@@ -24,4 +24,4 @@ const ExportMPCAccount = () => {
   )
 }
 
-export default ExportMPCAccount
+export default SocialSignerExport
diff --git a/src/components/settings/ExportMPCAccount/styles.module.css b/src/components/settings/SecurityLogin/SocialSignerExport/styles.module.css
similarity index 100%
rename from src/components/settings/ExportMPCAccount/styles.module.css
rename to src/components/settings/SecurityLogin/SocialSignerExport/styles.module.css
diff --git a/src/components/settings/SignerAccountMFA/PasswordInput.tsx b/src/components/settings/SecurityLogin/SocialSignerMFA/PasswordInput.tsx
similarity index 100%
rename from src/components/settings/SignerAccountMFA/PasswordInput.tsx
rename to src/components/settings/SecurityLogin/SocialSignerMFA/PasswordInput.tsx
diff --git a/src/components/settings/SignerAccountMFA/helper.ts b/src/components/settings/SecurityLogin/SocialSignerMFA/helper.ts
similarity index 100%
rename from src/components/settings/SignerAccountMFA/helper.ts
rename to src/components/settings/SecurityLogin/SocialSignerMFA/helper.ts
diff --git a/src/components/settings/SignerAccountMFA/PasswordForm.test.tsx b/src/components/settings/SecurityLogin/SocialSignerMFA/index.test.tsx
similarity index 96%
rename from src/components/settings/SignerAccountMFA/PasswordForm.test.tsx
rename to src/components/settings/SecurityLogin/SocialSignerMFA/index.test.tsx
index dbd754b54e..4a2bde113d 100644
--- a/src/components/settings/SignerAccountMFA/PasswordForm.test.tsx
+++ b/src/components/settings/SecurityLogin/SocialSignerMFA/index.test.tsx
@@ -1,4 +1,4 @@
-import { _getPasswordStrength, PasswordStrength } from '@/components/settings/SignerAccountMFA/PasswordForm'
+import { _getPasswordStrength, PasswordStrength } from '@/components/settings/SecurityLogin/SocialSignerMFA/index'
 
 describe('_getPasswordStrength', () => {
   it('should return weak if the value has fewer than 9 characters', () => {
diff --git a/src/components/settings/SignerAccountMFA/PasswordForm.tsx b/src/components/settings/SecurityLogin/SocialSignerMFA/index.tsx
similarity index 89%
rename from src/components/settings/SignerAccountMFA/PasswordForm.tsx
rename to src/components/settings/SecurityLogin/SocialSignerMFA/index.tsx
index b681bbc437..43a3545b84 100644
--- a/src/components/settings/SignerAccountMFA/PasswordForm.tsx
+++ b/src/components/settings/SecurityLogin/SocialSignerMFA/index.tsx
@@ -15,16 +15,17 @@ import {
 import { MPC_WALLET_EVENTS } from '@/services/analytics/events/mpcWallet'
 import { useState, useMemo, type ChangeEvent } from 'react'
 import { FormProvider, useForm } from 'react-hook-form'
-import { enableMFA } from './helper'
+import { enableMFA } from '@/components/settings/SecurityLogin/SocialSignerMFA/helper'
 import CheckIcon from '@/public/images/common/check-filled.svg'
 import LockWarningIcon from '@/public/images/common/lock-warning.svg'
-import PasswordInput from '@/components/settings/SignerAccountMFA/PasswordInput'
-import css from './styles.module.css'
+import PasswordInput from '@/components/settings/SecurityLogin/SocialSignerMFA/PasswordInput'
+import css from '@/components/settings/SecurityLogin/SocialSignerMFA/styles.module.css'
 import BarChartIcon from '@/public/images/common/bar-chart.svg'
 import ShieldIcon from '@/public/images/common/shield.svg'
 import ShieldOffIcon from '@/public/images/common/shield-off.svg'
 import useMPC from '@/hooks/wallets/mpc/useMPC'
 import { useAppDispatch } from '@/store'
+import ExpandMoreIcon from '@mui/icons-material/ExpandMore'
 
 enum PasswordFieldNames {
   oldPassword = 'oldPassword',
@@ -76,7 +77,7 @@ const passwordStrengthMap = {
   },
 } as const
 
-export const PasswordForm = () => {
+const SocialSignerMFA = () => {
   const dispatch = useAppDispatch()
   const mpcCoreKit = useMPC()
   const [passwordStrength, setPasswordStrength] = useState<PasswordStrength>(PasswordStrength.weak)
@@ -110,7 +111,8 @@ export const PasswordForm = () => {
     setPasswordStrength(PasswordStrength.weak)
   }
 
-  const passwordsMatch = watch(PasswordFieldNames.newPassword) === watch(PasswordFieldNames.confirmPassword)
+  const confirmPassword = watch(PasswordFieldNames.confirmPassword)
+  const passwordsMatch = watch(PasswordFieldNames.newPassword) === confirmPassword && confirmPassword !== ''
 
   const isSubmitDisabled =
     !passwordsMatch ||
@@ -123,11 +125,11 @@ export const PasswordForm = () => {
       <form onSubmit={handleSubmit(onSubmit)}>
         <Box display="flex" flexDirection="column" gap={3} alignItems="baseline">
           <Typography>
-            Protect your account with a password. It will be used to restore access to your Safe in another browser or
-            on another device.
+            Protect your social login signer with a password. It will be used to restore access in another browser or on
+            another device.
           </Typography>
-          <Accordion defaultExpanded={isPasswordSet}>
-            <AccordionSummary>
+          <Accordion>
+            <AccordionSummary expandIcon={<ExpandMoreIcon />}>
               <Box display="flex" alignItems="center" gap={1}>
                 <SvgIcon component={CheckIcon} sx={{ color: isPasswordSet ? 'success.main' : 'border.light' }} />
                 <Typography fontWeight="bold">Password</Typography>
@@ -138,7 +140,7 @@ export const PasswordForm = () => {
                 <Grid item container xs={12} md={7} gap={3} p={3}>
                   {isPasswordSet && (
                     <>
-                      <Typography>You already have a recovery password setup.</Typography>
+                      <Typography>You already have a password setup.</Typography>
                       <FormControl fullWidth>
                         <PasswordInput
                           name={PasswordFieldNames.oldPassword}
@@ -178,7 +180,7 @@ export const PasswordForm = () => {
                       {passwordStrengthMap[passwordStrength].label} password
                     </Typography>
                     <Typography variant="body2" color="text.secondary" mt={1}>
-                      Include at least 8 or more characters, a number, an uppercase letter and a symbol
+                      Include at least 9 or more characters, a number, an uppercase letter and a symbol
                     </Typography>
                   </FormControl>
 
@@ -232,8 +234,8 @@ export const PasswordForm = () => {
                     </Typography>
                     <ol className={css.list}>
                       <Typography component="li" variant="body2">
-                        You will have to input this password if you need to recover access to Safe in another browser or
-                        on another device.
+                        You will have to input this password if you login with this social login signer in another
+                        browser or on another device.
                       </Typography>
                       <Typography component="li" variant="body2">
                         We suggest to use a password manager or to write the password down on a piece of paper and
@@ -250,3 +252,5 @@ export const PasswordForm = () => {
     </FormProvider>
   )
 }
+
+export default SocialSignerMFA
diff --git a/src/components/settings/SignerAccountMFA/styles.module.css b/src/components/settings/SecurityLogin/SocialSignerMFA/styles.module.css
similarity index 100%
rename from src/components/settings/SignerAccountMFA/styles.module.css
rename to src/components/settings/SecurityLogin/SocialSignerMFA/styles.module.css
diff --git a/src/components/settings/SecurityLogin/index.tsx b/src/components/settings/SecurityLogin/index.tsx
new file mode 100644
index 0000000000..bb493cb90d
--- /dev/null
+++ b/src/components/settings/SecurityLogin/index.tsx
@@ -0,0 +1,51 @@
+import { Grid, Paper, Typography } from '@mui/material'
+import SocialSignerMFA from '@/components/settings/SecurityLogin/SocialSignerMFA'
+import SocialSignerExport from '@/components/settings/SecurityLogin/SocialSignerExport'
+import useWallet from '@/hooks/wallets/useWallet'
+import { isSocialLoginWallet } from '@/services/mpc/module'
+
+const SecurityLogin = () => {
+  const wallet = useWallet()
+
+  const isSocialLogin = isSocialLoginWallet(wallet?.label)
+
+  if (!isSocialLogin) {
+    return (
+      <Paper sx={{ p: 4 }}>
+        <Typography>You are currently not logged in with a social login signer.</Typography>
+      </Paper>
+    )
+  }
+
+  return (
+    <>
+      <Paper sx={{ p: 4 }}>
+        <Grid container spacing={3}>
+          <Grid item lg={4} xs={12}>
+            <Typography variant="h4" fontWeight="bold" mb={1}>
+              Multi-factor Authentication
+            </Typography>
+          </Grid>
+
+          <Grid item xs>
+            <SocialSignerMFA />
+          </Grid>
+        </Grid>
+      </Paper>
+      <Paper sx={{ p: 4, mt: 2 }}>
+        <Grid container spacing={3}>
+          <Grid item lg={4} xs={12}>
+            <Typography variant="h4" fontWeight="bold" mb={1}>
+              Social login signer export
+            </Typography>
+          </Grid>
+          <Grid item xs>
+            <SocialSignerExport />
+          </Grid>
+        </Grid>
+      </Paper>
+    </>
+  )
+}
+
+export default SecurityLogin
diff --git a/src/components/settings/SignerAccountMFA/index.tsx b/src/components/settings/SignerAccountMFA/index.tsx
deleted file mode 100644
index 07d8b2745b..0000000000
--- a/src/components/settings/SignerAccountMFA/index.tsx
+++ /dev/null
@@ -1,23 +0,0 @@
-import { Box, Typography } from '@mui/material'
-
-import { PasswordForm } from './PasswordForm'
-import useWallet from '@/hooks/wallets/useWallet'
-import { isSocialLoginWallet } from '@/services/mpc/module'
-
-const SignerAccountMFA = () => {
-  const wallet = useWallet()
-
-  const isSocialLogin = isSocialLoginWallet(wallet?.label)
-
-  if (!isSocialLogin) {
-    return (
-      <Box>
-        <Typography>You are currently not logged in with a social account</Typography>
-      </Box>
-    )
-  }
-
-  return <PasswordForm />
-}
-
-export default SignerAccountMFA
diff --git a/src/components/sidebar/SidebarNavigation/config.tsx b/src/components/sidebar/SidebarNavigation/config.tsx
index 3132f3c584..20924e1f9a 100644
--- a/src/components/sidebar/SidebarNavigation/config.tsx
+++ b/src/components/sidebar/SidebarNavigation/config.tsx
@@ -96,6 +96,10 @@ export const settingsNavItems = [
     label: 'Spending limits',
     href: AppRoutes.settings.spendingLimits,
   },
+  {
+    label: 'Security & Login',
+    href: AppRoutes.settings.securityLogin,
+  },
   {
     label: 'Safe Apps',
     href: AppRoutes.settings.safeApps.index,
@@ -108,10 +112,6 @@ export const settingsNavItems = [
     label: 'Environment variables',
     href: AppRoutes.settings.environmentVariables,
   },
-  {
-    label: 'Signer account',
-    href: AppRoutes.settings.signerAccount,
-  },
 ]
 
 export const generalSettingsNavItems = [
diff --git a/src/components/welcome/WelcomeLogin/WalletLogin.tsx b/src/components/welcome/WelcomeLogin/WalletLogin.tsx
index f1657a7ae2..102b0de6ef 100644
--- a/src/components/welcome/WelcomeLogin/WalletLogin.tsx
+++ b/src/components/welcome/WelcomeLogin/WalletLogin.tsx
@@ -19,7 +19,7 @@ const WalletLogin = ({ onLogin }: { onLogin: () => void }) => {
 
   const isSocialLogin = isSocialLoginWallet(wallet?.label)
 
-  if (wallet !== null && isSocialLogin) {
+  if (wallet !== null && !isSocialLogin) {
     return (
       <Box sx={{ width: '100%' }}>
         <Track {...CREATE_SAFE_EVENTS.CONTINUE_TO_CREATION} label={wallet.label}>
diff --git a/src/components/welcome/WelcomeLogin/__tests__/WalletLogin.test.tsx b/src/components/welcome/WelcomeLogin/__tests__/WalletLogin.test.tsx
index e7f7ce97bb..a846dd5f8d 100644
--- a/src/components/welcome/WelcomeLogin/__tests__/WalletLogin.test.tsx
+++ b/src/components/welcome/WelcomeLogin/__tests__/WalletLogin.test.tsx
@@ -1,7 +1,6 @@
 import { act, render, waitFor } from '@/tests/test-utils'
 import * as useConnectWallet from '@/components/common/ConnectWallet/useConnectWallet'
 import * as useWallet from '@/hooks/wallets/useWallet'
-import * as mpcModule from '@/services/mpc/module'
 import WalletLogin from '../WalletLogin'
 import { hexZeroPad } from '@ethersproject/bytes'
 import { type EIP1193Provider } from '@web3-onboard/common'
@@ -21,7 +20,6 @@ describe('WalletLogin', () => {
       label: 'MetaMask',
       provider: {} as unknown as EIP1193Provider,
     })
-    jest.spyOn(mpcModule, 'isSocialLoginWallet').mockReturnValue(true)
     jest.spyOn(useConnectWallet, 'default').mockReturnValue(jest.fn())
 
     const result = render(<WalletLogin onLogin={mockOnLogin} />)
diff --git a/src/config/routes.ts b/src/config/routes.ts
index 48713b5ea9..34b6848e00 100644
--- a/src/config/routes.ts
+++ b/src/config/routes.ts
@@ -1,6 +1,5 @@
 export const AppRoutes = {
   '404': '/404',
-  _offline: '/_offline',
   welcome: '/welcome',
   terms: '/terms',
   privacy: '/privacy',
@@ -11,6 +10,7 @@ export const AppRoutes = {
   cookie: '/cookie',
   addressBook: '/address-book',
   addOwner: '/addOwner',
+  _offline: '/_offline',
   apps: {
     open: '/apps/open',
     index: '/apps',
@@ -27,8 +27,8 @@ export const AppRoutes = {
   },
   settings: {
     spendingLimits: '/settings/spending-limits',
-    signerAccount: '/settings/signer-account',
     setup: '/settings/setup',
+    securityLogin: '/settings/security-login',
     notifications: '/settings/notifications',
     modules: '/settings/modules',
     index: '/settings',
diff --git a/src/pages/settings/security-login.tsx b/src/pages/settings/security-login.tsx
new file mode 100644
index 0000000000..856af7d8a1
--- /dev/null
+++ b/src/pages/settings/security-login.tsx
@@ -0,0 +1,23 @@
+import type { NextPage } from 'next'
+import Head from 'next/head'
+
+import SettingsHeader from '@/components/settings/SettingsHeader'
+import SecurityLogin from '@/components/settings/SecurityLogin'
+
+const SecurityLoginPage: NextPage = () => {
+  return (
+    <>
+      <Head>
+        <title>{'Safe{Wallet} – Settings – Security & Login'}</title>
+      </Head>
+
+      <SettingsHeader />
+
+      <main>
+        <SecurityLogin />
+      </main>
+    </>
+  )
+}
+
+export default SecurityLoginPage
diff --git a/src/pages/settings/signer-account.tsx b/src/pages/settings/signer-account.tsx
deleted file mode 100644
index 13876f2a2d..0000000000
--- a/src/pages/settings/signer-account.tsx
+++ /dev/null
@@ -1,50 +0,0 @@
-import { Grid, Paper, Typography } from '@mui/material'
-
-import type { NextPage } from 'next'
-import Head from 'next/head'
-
-import SettingsHeader from '@/components/settings/SettingsHeader'
-import SignerAccountMFA from '@/components/settings/SignerAccountMFA'
-import ExportMPCAccount from '@/components/settings/ExportMPCAccount'
-
-const SignerAccountPage: NextPage = () => {
-  return (
-    <>
-      <Head>
-        <title>{'Safe{Wallet} – Settings – Signer account'}</title>
-      </Head>
-
-      <SettingsHeader />
-
-      <main>
-        <Paper sx={{ p: 4 }}>
-          <Grid container spacing={3}>
-            <Grid item lg={4} xs={12}>
-              <Typography variant="h4" fontWeight="bold" mb={1}>
-                Multi-factor Authentication
-              </Typography>
-            </Grid>
-
-            <Grid item xs>
-              <SignerAccountMFA />
-            </Grid>
-          </Grid>
-        </Paper>
-        <Paper sx={{ p: 4, mt: 2 }}>
-          <Grid container spacing={3}>
-            <Grid item lg={4} xs={12}>
-              <Typography variant="h4" fontWeight="bold" mb={1}>
-                Account export
-              </Typography>
-            </Grid>
-            <Grid item xs>
-              <ExportMPCAccount />
-            </Grid>
-          </Grid>
-        </Paper>
-      </main>
-    </>
-  )
-}
-
-export default SignerAccountPage