-
Notifications
You must be signed in to change notification settings - Fork 1
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
[Web][UMA-1191]Disable removal of default account and add Lock button. #2429
base: main
Are you sure you want to change the base?
[Web][UMA-1191]Disable removal of default account and add Lock button. #2429
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
8b51dd9
to
55f9e90
Compare
@@ -144,6 +144,9 @@ export const accountsSlice = createSlice({ | |||
if (!state.defaultAccount) { | |||
state.defaultAccount = hideConfidentialData(state.items[0]); | |||
} | |||
if (state.items.length === 0) { |
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.
state.items.length === 0
=> !state.items.length
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.
What's the point of this 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.
To handle to scenario when user deletes all accounts.
@@ -99,13 +105,14 @@ export const AccountSelectorModal = () => { | |||
|
|||
const onRemove = (type: string, accounts: Account[]) => { | |||
const account = accounts[0]; | |||
const isLast = accounts.length === implicitAccounts.length; | |||
|
|||
const isDefaultAccount = accounts.some(a => a.address.pkh === defaultAccount.address.pkh); |
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.
const isDefaultAccount = accounts.some(a => a.address.pkh === defaultAccount.address.pkh); | |
const isDefaultAccount = account.address.pkh === defaultAccount.address.pkh |
if (isDefaultAccount) { | ||
return ( | ||
removeDefaultAccountDescription + | ||
"\n\n<b>Make sure your mnemonic phrase is securely saved. Losing this phrase could result in permanent loss of access to your data.</b>" |
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.
Make sure your mnemonic phrase is securely saved. Losing this phrase could result in permanent loss of access to your data.
may not be related to the social default account
@@ -50,6 +52,10 @@ export const AccountSelectorModal = () => { | |||
const removeMnemonic = useRemoveMnemonic(); | |||
const removeNonMnemonic = useRemoveNonMnemonic(); | |||
const { openWith, goBack, onClose } = useDynamicModalContext(); | |||
const defaultAccount = useAppSelector(state => state.accounts.defaultAccount); |
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.
Please create a reusable useGetDefaultAccount
hook in packages/state/src/hooks/getAccountData.ts
and handle the edge case logic inside the hook
description="Before you log out, ensure your mnemonic phrase is securely saved. Without it, you may permanently lose access to your account and data." | ||
description={ | ||
"Before you log out, ensure your mnemonic phrase is securely saved. Without it, you may permanently lose access to your account and data. \n" + | ||
"Loging out will remove all personal data (including saved contacts, password and accounts) from your device." + |
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.
Why not use removeDefaultAccountDescription
?
@@ -7,23 +7,45 @@ import { type MenuItems } from "./types"; | |||
type GenericMenuProps = { | |||
title?: string; | |||
menuItems: MenuItems; | |||
bottomMenuItems?: MenuItems; |
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.
No need to pass additional props
icon: <LogoutIcon />, | ||
onClick: () => openModal(<LogoutModal />), | ||
}, | ||
]; | ||
|
||
return <GenericMenu menuItems={[coreMenuItems, themeMenuItems, logoutMenuItems]} />; | ||
return ( | ||
<GenericMenu bottomMenuItems={[logoutMenuItems]} menuItems={[coreMenuItems, themeMenuItems]} /> |
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.
<GenericMenu bottomMenuItems={[logoutMenuItems]} menuItems={[coreMenuItems, themeMenuItems]} /> | |
return <GenericMenu menuItems={[coreMenuItems, themeMenuItems, logoutMenuItems]} />; |
@@ -74,15 +83,18 @@ export const Menu = () => { | |||
onClick: toggleColorMode, | |||
rightElement: <Switch isChecked={colorMode === "dark"} onChange={toggleColorMode} />, | |||
}, | |||
lockUmami, |
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.
Please move this under the logoutMenuItems
as a first item
}; | ||
|
||
export const GenericMenu = ({ title, menuItems }: GenericMenuProps) => ( | ||
export const GenericMenu = ({ title, menuItems, bottomMenuItems }: GenericMenuProps) => ( |
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.
export const GenericMenu = ({ title, menuItems, bottomMenuItems }: GenericMenuProps) => ( | |
export const GenericMenu = ({ title, menuItems }: GenericMenuProps) => ( | |
<DrawerContentWrapper title={title}> | |
<VStack | |
gap={{ base: "18px", md: "24px" }} | |
height="100%" | |
marginTop={title ? { base: "36px", md: "40px" } : 0} | |
divider={<Divider />} | |
spacing="0" | |
> | |
{menuItems.map((items, index) => ( | |
<Box | |
key={index} | |
width="full" | |
_last={{ | |
display: "flex", | |
flexDirection: "column", | |
justifyContent: "space-between", | |
height: "100%", | |
}} | |
> | |
{items.map(item => ( | |
<MenuItem key={item.label} {...item} /> | |
))} | |
</Box> | |
))} | |
</VStack> | |
</DrawerContentWrapper> | |
); |
description="Before you log out, ensure your mnemonic phrase is securely saved. Without it, you may permanently lose access to your account and data." | ||
description={ | ||
"Before you log out, ensure your mnemonic phrase is securely saved. Without it, you may permanently lose access to your account and data. \n" + | ||
"Loging out will remove all personal data (including saved contacts, password and accounts) from your device." + |
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.
Why not use removeDefaultAccountDescription
?
Proposed changes
Task link
Types of changes
Steps to reproduce
Add two accounts, and remove the default one. Then close the tab and open a new tab, it asks for password and after entering the password it says mnemonic could not be found.
Also no provision for Lock Umami.
Screenshots
Add the screenshots of how the app used to look like and how it looks now
|

| |
Checklist