[Multichain] feat: redesign of multiaccounts and context menu for multiaccounts #4125

schmanu · 2024-08-30T16:15:55Z

What this PR changes

Redesigns how sub-items are displayed under multi-accounts.
Adds context menu for multi accounts offering renaming and adding a new network
Some UX fixes around the context menu

How to test it

Open the new Sidebar and observe multi chain accounts
Try to rename an account

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) 🧑‍💻

- Redesigns how sub-items are displayed under multi-accounts. - Adds context menu for multi accounts offering renaming and adding a new network - Some UX fixes around the context menu

github-actions · 2024-08-30T16:16:10Z

Branch preview

✅ Deploy successful!

Website:
https://feat_multi_account_context--walletweb.review.5afe.dev/home?safe=eth:0xA77DE01e157f9f57C7c4A326eeE9C4874D0598b6

Storybook:
https://feat_multi_account_context--walletweb.review.5afe.dev/storybook/

notion-workspace · 2024-08-30T16:16:25Z

Multichain Safes v1: Contextmenu for Multichain Safe & UI updated for sidebar

github-actions · 2024-08-30T16:18:43Z

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}

github-actions · 2024-08-30T16:21:29Z

📦 Next.js Bundle Analysis for safe-wallet-web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

⚠️ Global Bundle Size Increased

Page	Size (compressed)
`global`	`979.08 KB` (🟡 +38.67 KB)

Details

The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!

Nineteen Pages Changed Size

The following pages changed size from the code in this PR compared to its base branch:

Page	Size (compressed)	First Load
`/`	`507 B` (🟢 -24.36 KB)	979.57 KB
`/address-book`	`26.11 KB` (🟡 +3 B)	1005.19 KB
`/apps/open`	`54.74 KB` (🟡 +1 B)	1.01 MB
`/balances`	`30.64 KB` (-3 B)	1009.72 KB
`/home`	`60.28 KB` (🟡 +31 B)	1.01 MB
`/new-safe/advanced-create`	`36.51 KB` (🟡 +1.37 KB)	1015.59 KB
`/new-safe/create`	`35.76 KB` (🟡 +1.36 KB)	1014.84 KB
`/new-safe/load`	`16.38 KB` (🟢 -7 B)	995.46 KB
`/settings/modules`	`9.76 KB` (🟢 -1 B)	988.84 KB
`/settings/setup`	`35.96 KB` (🟡 +1 B)	1015.03 KB
`/share/safe-app`	`9.72 KB` (🟢 -2 B)	988.8 KB
`/terms`	`11.24 KB` (🟡 +60 B)	990.32 KB
`/transactions`	`73.47 KB` (🟡 +1 B)	1.03 MB
`/transactions/history`	`73.44 KB` (🟡 +1 B)	1.03 MB
`/transactions/messages`	`39.12 KB` (-1 B)	1018.2 KB
`/transactions/msg`	`29.5 KB` (-1 B)	1008.58 KB
`/transactions/queue`	`31.22 KB` (🟡 +1 B)	1010.3 KB
`/transactions/tx`	`21.05 KB` (🟡 +1 B)	1000.13 KB
`/welcome`	`6.77 KB` (🟢 -1 B)	985.84 KB

Details

Only the gzipped size is provided here based on an expert tip.

First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

Next to the size is how much the size has increased or decreased compared with the base branch of this PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this.

github-actions · 2024-08-30T16:22:51Z

Coverage report

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	77.94% (+0.01% 🔼)	12313/15798
🔴	Branches	58.04% (+0.02% 🔼)	3181/5481
🟡	Functions	65.14% (+0.02% 🔼)	1934/2969
🟡	Lines	79.41% (+0.01% 🔼)	11117/14000

Show files with reduced coverage 🔻

St.^❔	File	Statements	Branches	Functions	Lines
🟢	`...` / index.tsx	95.45% (-4.55% 🔻)	50% (-16.67% 🔻)	100%	95.24% (-4.76% 🔻)

Test suite run success

1525 tests passing in 206 suites.

Report generated by 🧪jest coverage report action from de3c784

src/components/address-book/EntryDialog/index.tsx

src/components/sidebar/SafeListContextMenu/MultiAccountContextMenu.tsx

clovisdasilvaneto · 2024-09-09T13:29:10Z

src/components/sidebar/SafeListContextMenu/MultiAccountContextMenu.tsx

+      <IconButton data-testid="safe-options-btn" edge="end" size="small" onClick={handleOpenContextMenu}>
+        <MoreVertIcon sx={({ palette }) => ({ color: palette.border.main })} />
+      </IconButton>
+      <ContextMenu anchorEl={anchorEl} open={!!anchorEl} onClose={handleCloseContextMenu}>


Suggested change

<ContextMenu anchorEl={anchorEl} open={!!anchorEl} onClose={handleCloseContextMenu}>

<ContextMenu anchorEl={anchorEl.current}>

clovisdasilvaneto · 2024-09-09T13:30:25Z

src/components/sidebar/SafeListContextMenu/MultiAccountContextMenu.tsx

+
+  return (
+    <>
+      <IconButton data-testid="safe-options-btn" edge="end" size="small" onClick={handleOpenContextMenu}>


Suggested change

<IconButton data-testid="safe-options-btn" edge="end" size="small" onClick={handleOpenContextMenu}>

<IconButton data-testid="safe-options-btn" edge="end" size="small" onClick={handleOpenContextMenu} ref={anchorEl}>

clovisdasilvaneto · 2024-09-09T13:38:18Z

src/components/sidebar/SafeListContextMenu/MultiAccountContextMenu.tsx

+
+const defaultOpen = { [ModalType.RENAME]: false, [ModalType.ADD_CHAIN]: false }
+
+const MultiAccountContextMenu = ({


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:

<DropdownMenu items={items}> {(openMenu) => ( <IconButton data-testid="safe-options-btn" edge="end" size="small" onClick={handleOpenContextMenu}> <MoreVertIcon sx={({ palette }) => ({ color: palette.border.main })} /> </IconButton> )} </DropdownMenu>

in this case, items would be a common type like this:

type item = { onClick: (e: MouseEvent) => void, label: string icon?: React.ReactNode }[]

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.

clovisdasilvaneto · 2024-09-09T13:41:48Z

src/components/welcome/MyAccounts/MultiAccountItem.tsx

+                <Box>
+                  <Typography fontSize="14px">Multichain account on:</Typography>
+                  {safes.map((safeItem) => (
+                    <Box p="4px 0px" key={safeItem.chainId}>
+                      <ChainIndicator chainId={safeItem.chainId} />
+                    </Box>
+                  ))}
+                </Box>


Would be nice if we can move it to be a separated component outside the MultiAccountItem? So we just need to call the reference of the component inside the tooltip instead of re-creating the component every-time the parent has a re-render?

clovisdasilvaneto · 2024-09-09T13:43:17Z

src/components/welcome/MyAccounts/styles.module.css

@@ -44,6 +44,12 @@
  background-color: var(--color-background-light) !important;
 }

+.currentListItem.multiListItem {
+  border-left-width: 1px;


I think you don't need this first line, since the next one overwrites this one 🤔

There is the single account component which uses only currentListItem without the multiListItem css class.
I think its needed there.

@schmanu I mean, in the same selector, line 49, you overwrite it with border: 1px solid var(--color-border-light) 🤔

Ah sorry I misunderstood. I thought you meant the entire block not just the line -.-
You are right. I will remove it :)

github-actions · 2024-09-11T12:07:18Z

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}

…/multi-account-context

github-actions · 2024-09-12T13:46:54Z

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}

notion-workspace · 2024-09-12T14:15:03Z

Show error message if no network is available to replay the Safe

usame-algan · 2024-09-17T09:38:38Z

src/components/sidebar/SafeListContextMenu/MultiAccountContextMenu.tsx

+  }
+
+  const handleOpenModal =
+    (type: keyof typeof open, event: typeof OVERVIEW_EVENTS.SIDEBAR_RENAME | typeof OVERVIEW_EVENTS.SIDEBAR_RENAME) =>


Suggested change

(type: keyof typeof open, event: typeof OVERVIEW_EVENTS.SIDEBAR_RENAME | typeof OVERVIEW_EVENTS.SIDEBAR_RENAME) =>

(type: ModalType, event: typeof OVERVIEW_EVENTS.ADD_NEW_NETWORK | typeof OVERVIEW_EVENTS.SIDEBAR_RENAME) =>

Can we use the enum directly for the type?

Also we have OVERVIEW_EVENTS.SIDEBAR_RENAME twice in here

usame-algan · 2024-09-17T09:41:08Z

src/components/sidebar/SafeListContextMenu/MultiAccountContextMenu.tsx

+  const trackingLabel =
+    router.pathname === AppRoutes.welcome.accounts ? OVERVIEW_LABELS.login_page : OVERVIEW_LABELS.sidebar


This is only needed in handleOpenModal so I suggest moving it there.

src/store/addressBookSlice.ts

clovisdasilvaneto

✅

github-actions · 2024-09-17T14:47:13Z

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}

…/multi-account-context

feat: redesign of multiaccounts and context menu for multiaccounts

cc59938

- Redesigns how sub-items are displayed under multi-accounts. - Adds context menu for multi accounts offering renaming and adding a new network - Some UX fixes around the context menu

schmanu changed the title ~~feat: redesign of multiaccounts and context menu for multiaccounts~~ [Multichain] feat: redesign of multiaccounts and context menu for multiaccounts Sep 9, 2024

clovisdasilvaneto reviewed Sep 9, 2024

View reviewed changes

src/components/address-book/EntryDialog/index.tsx Outdated Show resolved Hide resolved

clovisdasilvaneto reviewed Sep 9, 2024

View reviewed changes

src/components/sidebar/SafeListContextMenu/MultiAccountContextMenu.tsx Show resolved Hide resolved

clovisdasilvaneto reviewed Sep 9, 2024

View reviewed changes

fix: introduce upsertMUltichainAddressBookEntry reducer

1c59e54

refactor: extract component

8437fbc

schmanu requested a review from clovisdasilvaneto September 11, 2024 12:45

Merge remote-tracking branch 'origin/epic/multichain-safes' into feat…

9020775

…/multi-account-context

fix: show error if no chains available

d15bdb6

usame-algan reviewed Sep 17, 2024

View reviewed changes

usame-algan approved these changes Sep 17, 2024

View reviewed changes

clovisdasilvaneto approved these changes Sep 17, 2024

View reviewed changes

fix: review issues

5e3bb59

Merge remote-tracking branch 'origin/epic/multichain-safes' into feat…

de3c784

…/multi-account-context

schmanu merged commit 6555c95 into epic/multichain-safes Sep 17, 2024
14 checks passed

schmanu deleted the feat/multi-account-context branch September 17, 2024 16:01

github-actions bot locked and limited conversation to collaborators Sep 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Multichain] feat: redesign of multiaccounts and context menu for multiaccounts #4125

[Multichain] feat: redesign of multiaccounts and context menu for multiaccounts #4125

schmanu commented Aug 30, 2024

github-actions bot commented Aug 30, 2024 •

edited

Loading

notion-workspace bot commented Aug 30, 2024

github-actions bot commented Aug 30, 2024

github-actions bot commented Aug 30, 2024 •

edited

Loading

github-actions bot commented Aug 30, 2024 •

edited

Loading

clovisdasilvaneto Sep 9, 2024

clovisdasilvaneto Sep 9, 2024

clovisdasilvaneto Sep 9, 2024 •

edited

Loading

schmanu Sep 11, 2024

clovisdasilvaneto Sep 9, 2024

clovisdasilvaneto Sep 9, 2024

schmanu Sep 11, 2024

clovisdasilvaneto Sep 17, 2024

schmanu Sep 17, 2024

github-actions bot commented Sep 11, 2024 •

edited

Loading

github-actions bot commented Sep 12, 2024 •

edited

Loading

notion-workspace bot commented Sep 12, 2024

usame-algan Sep 17, 2024

usame-algan Sep 17, 2024

usame-algan Sep 17, 2024

clovisdasilvaneto left a comment

github-actions bot commented Sep 17, 2024 •

edited

Loading

	<ContextMenu anchorEl={anchorEl} open={!!anchorEl} onClose={handleCloseContextMenu}>
	<ContextMenu anchorEl={anchorEl.current}>

	<IconButton data-testid="safe-options-btn" edge="end" size="small" onClick={handleOpenContextMenu}>
	<IconButton data-testid="safe-options-btn" edge="end" size="small" onClick={handleOpenContextMenu} ref={anchorEl}>


		const defaultOpen = { [ModalType.RENAME]: false, [ModalType.ADD_CHAIN]: false }

		const MultiAccountContextMenu = ({

	(type: keyof typeof open, event: typeof OVERVIEW_EVENTS.SIDEBAR_RENAME \| typeof OVERVIEW_EVENTS.SIDEBAR_RENAME) =>
	(type: ModalType, event: typeof OVERVIEW_EVENTS.ADD_NEW_NETWORK \| typeof OVERVIEW_EVENTS.SIDEBAR_RENAME) =>

		const trackingLabel =
		router.pathname === AppRoutes.welcome.accounts ? OVERVIEW_LABELS.login_page : OVERVIEW_LABELS.sidebar

[Multichain] feat: redesign of multiaccounts and context menu for multiaccounts #4125

[Multichain] feat: redesign of multiaccounts and context menu for multiaccounts #4125

Conversation

schmanu commented Aug 30, 2024

What this PR changes

How to test it

Screenshots

Checklist

github-actions bot commented Aug 30, 2024 • edited Loading

Branch preview

notion-workspace bot commented Aug 30, 2024

github-actions bot commented Aug 30, 2024

ESLint Summary View Full Report

github-actions bot commented Aug 30, 2024 • edited Loading

📦 Next.js Bundle Analysis for safe-wallet-web

⚠️ Global Bundle Size Increased

Nineteen Pages Changed Size

github-actions bot commented Aug 30, 2024 • edited Loading

Coverage report

Test suite run success

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clovisdasilvaneto Sep 9, 2024 • edited Loading

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

github-actions bot commented Sep 11, 2024 • edited Loading

ESLint Summary View Full Report

github-actions bot commented Sep 12, 2024 • edited Loading

ESLint Summary View Full Report

notion-workspace bot commented Sep 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clovisdasilvaneto left a comment

Choose a reason for hiding this comment

github-actions bot commented Sep 17, 2024 • edited Loading

ESLint Summary View Full Report

github-actions bot commented Aug 30, 2024 •

edited

Loading

github-actions bot commented Aug 30, 2024 •

edited

Loading

github-actions bot commented Aug 30, 2024 •

edited

Loading

clovisdasilvaneto Sep 9, 2024 •

edited

Loading

github-actions bot commented Sep 11, 2024 •

edited

Loading

github-actions bot commented Sep 12, 2024 •

edited

Loading

github-actions bot commented Sep 17, 2024 •

edited

Loading