-
Notifications
You must be signed in to change notification settings - Fork 428
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
[Multichain] feat: redesign of multiaccounts and context menu for multiaccounts #4125
Changes from 5 commits
cc59938
1c59e54
8437fbc
9020775
d15bdb6
5e3bb59
de3c784
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,107 @@ | ||||||
import type { MouseEvent } from 'react' | ||||||
import { useState, type ReactElement } from 'react' | ||||||
import ListItemIcon from '@mui/material/ListItemIcon' | ||||||
import IconButton from '@mui/material/IconButton' | ||||||
import MoreVertIcon from '@mui/icons-material/MoreVert' | ||||||
import MenuItem from '@mui/material/MenuItem' | ||||||
import ListItemText from '@mui/material/ListItemText' | ||||||
|
||||||
import EntryDialog from '@/components/address-book/EntryDialog' | ||||||
import EditIcon from '@/public/images/common/edit.svg' | ||||||
import PlusIcon from '@/public/images/common/plus.svg' | ||||||
import ContextMenu from '@/components/common/ContextMenu' | ||||||
import { trackEvent, OVERVIEW_EVENTS, OVERVIEW_LABELS } from '@/services/analytics' | ||||||
import { SvgIcon } from '@mui/material' | ||||||
import { AppRoutes } from '@/config/routes' | ||||||
import router from 'next/router' | ||||||
import { CreateSafeOnNewChain } from '@/features/multichain/components/CreateSafeOnNewChain' | ||||||
|
||||||
enum ModalType { | ||||||
RENAME = 'rename', | ||||||
ADD_CHAIN = 'add_chain', | ||||||
} | ||||||
|
||||||
const defaultOpen = { [ModalType.RENAME]: false, [ModalType.ADD_CHAIN]: false } | ||||||
|
||||||
const MultiAccountContextMenu = ({ | ||||||
name, | ||||||
address, | ||||||
chainIds, | ||||||
}: { | ||||||
name: string | ||||||
address: string | ||||||
chainIds: string[] | ||||||
}): ReactElement => { | ||||||
const [anchorEl, setAnchorEl] = useState<HTMLElement | undefined>() | ||||||
clovisdasilvaneto marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
const [open, setOpen] = useState<typeof defaultOpen>(defaultOpen) | ||||||
|
||||||
const trackingLabel = | ||||||
router.pathname === AppRoutes.welcome.accounts ? OVERVIEW_LABELS.login_page : OVERVIEW_LABELS.sidebar | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is only needed in |
||||||
|
||||||
const handleOpenContextMenu = (e: MouseEvent<HTMLButtonElement, globalThis.MouseEvent>) => { | ||||||
e.stopPropagation() | ||||||
setAnchorEl(e.currentTarget) | ||||||
} | ||||||
|
||||||
const handleCloseContextMenu = (event: MouseEvent) => { | ||||||
event.stopPropagation() | ||||||
setAnchorEl(undefined) | ||||||
} | ||||||
|
||||||
const handleOpenModal = | ||||||
(type: keyof typeof open, event: typeof OVERVIEW_EVENTS.SIDEBAR_RENAME | typeof OVERVIEW_EVENTS.SIDEBAR_RENAME) => | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Can we use the enum directly for the type? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also we have |
||||||
(e: MouseEvent) => { | ||||||
handleCloseContextMenu(e) | ||||||
setOpen((prev) => ({ ...prev, [type]: true })) | ||||||
|
||||||
trackEvent({ ...event, label: trackingLabel }) | ||||||
} | ||||||
|
||||||
const handleCloseModal = () => { | ||||||
setOpen(defaultOpen) | ||||||
} | ||||||
|
||||||
return ( | ||||||
<> | ||||||
<IconButton data-testid="safe-options-btn" edge="end" size="small" onClick={handleOpenContextMenu}> | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
<MoreVertIcon sx={({ palette }) => ({ color: palette.border.main })} /> | ||||||
</IconButton> | ||||||
<ContextMenu anchorEl={anchorEl} open={!!anchorEl} onClose={handleCloseContextMenu}> | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
<MenuItem onClick={handleOpenModal(ModalType.RENAME, OVERVIEW_EVENTS.SIDEBAR_RENAME)}> | ||||||
<ListItemIcon> | ||||||
<SvgIcon component={EditIcon} inheritViewBox fontSize="small" color="success" /> | ||||||
</ListItemIcon> | ||||||
<ListItemText data-testid="rename-btn">Rename</ListItemText> | ||||||
</MenuItem> | ||||||
|
||||||
<MenuItem onClick={handleOpenModal(ModalType.ADD_CHAIN, OVERVIEW_EVENTS.ADD_NEW_NETWORK)}> | ||||||
<ListItemIcon> | ||||||
<SvgIcon component={PlusIcon} inheritViewBox fontSize="small" color="primary" /> | ||||||
</ListItemIcon> | ||||||
<ListItemText data-testid="add-chain-btn">Add another network</ListItemText> | ||||||
</MenuItem> | ||||||
</ContextMenu> | ||||||
|
||||||
{open[ModalType.RENAME] && ( | ||||||
<EntryDialog | ||||||
handleClose={handleCloseModal} | ||||||
defaultValues={{ name, address }} | ||||||
chainIds={chainIds} | ||||||
disableAddressInput | ||||||
/> | ||||||
)} | ||||||
|
||||||
{open[ModalType.ADD_CHAIN] && ( | ||||||
<CreateSafeOnNewChain | ||||||
onClose={handleCloseModal} | ||||||
currentName={name} | ||||||
deployedChainIds={chainIds} | ||||||
open | ||||||
safeAddress={address} | ||||||
/> | ||||||
)} | ||||||
</> | ||||||
) | ||||||
} | ||||||
|
||||||
export default MultiAccountContextMenu |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,6 +44,12 @@ | |
background-color: var(--color-background-light) !important; | ||
} | ||
|
||
.currentListItem.multiListItem { | ||
border-left-width: 1px; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you don't need this first line, since the next one overwrites this one 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is the single account component which uses only There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @schmanu I mean, in the same selector, line 49, you overwrite it with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah sorry I misunderstood. I thought you meant the entire block not just the line -.- |
||
border: 1px solid var(--color-border-light); | ||
background-color: none; | ||
} | ||
|
||
.listItem :global .MuiAccordion-root, | ||
.listItem :global .MuiAccordion-root:hover > .MuiAccordionSummary-root { | ||
background-color: transparent; | ||
|
@@ -54,48 +60,18 @@ | |
} | ||
|
||
.listItem.subItem { | ||
border: none; | ||
margin-bottom: 0px; | ||
border-radius: 0px; | ||
} | ||
|
||
.subItem:before { | ||
content: ''; | ||
display: block; | ||
width: 8px; | ||
height: 1px; | ||
background: var(--color-border-light); | ||
left: 0; | ||
top: 50%; | ||
position: absolute; | ||
} | ||
|
||
.subItem.currentListItem { | ||
border: none; | ||
} | ||
|
||
.subItem.currentListItem:before { | ||
background: var(--color-secondary-light); | ||
height: 1px; | ||
margin-bottom: 8px; | ||
} | ||
|
||
.subItem .borderLeft { | ||
top: 0; | ||
bottom: 0; | ||
position: absolute; | ||
border-left: 1px solid var(--color-border-light); | ||
border-radius: 6px; | ||
border: 1px solid var(--color-border-light); | ||
} | ||
.subItem.currentListItem .borderLeft { | ||
border-left: 1px solid var(--color-secondary-light); | ||
} | ||
|
||
.subItem:last-child { | ||
border-left: none; | ||
} | ||
|
||
.subItem:last-child .borderLeft { | ||
top: 0%; | ||
bottom: 50%; | ||
border-left: 4px solid var(--color-secondary-light); | ||
} | ||
|
||
.listItem > :first-child { | ||
|
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.
about the architecture of this component, since it's a button that shows a dropdown with some options which triggers a modal, would be amazing if we can separate the dropdown in generic renderProps component which provides the
openMenu
function to the rendered child and takes an array of options? something like:in this case,
items
would be a common type like this: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.
The architecture also comes from the MUI component library. I do not think we can easily rewrite it to this.
So I'll stick to how the examples use the component for the time being.