feat: redesign approval editor #3614

schmanu · 2024-04-25T13:15:06Z

What it solves

Resolves #3607

TODO

Maintain order of approvals as they are in the transaction or fix update function

How this PR fixes it

Changes layout / design of approval editor
Groups approvals by spender address
Fetches and displays contract name for spender address

How to test it

Use any app that requires approvals OR transaction builder to craft specific approval payloads
Observe the approval editor

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 · 2024-04-25T13:15:23Z

Branch preview

✅ Deploy successful!

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

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

github-actions · 2024-04-25T13:18:31Z

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-04-25T13:20:28Z

📦 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`	`1006.5 KB` (🟡 +184 B)

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!

Five Pages Changed Size

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

Page	Size (compressed)	First Load
`/apps`	`50.06 KB` (🟡 +3.47 KB)	1.03 MB
`/apps/custom`	`41.68 KB` (🟡 +3.47 KB)	1.02 MB
`/apps/open`	`78.6 KB` (🟡 +3.47 KB)	1.06 MB
`/home`	`63.24 KB` (🟡 +3.47 KB)	1.04 MB
`/settings/safe-apps`	`25.33 KB` (🟡 +3.47 KB)	1.01 MB

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-04-25T13:21:43Z

Coverage report

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	79.33% (+0.26% 🔼)	11296/14239
🔴	Branches	58.76% (+0.23% 🔼)	2674/4551
🟡	Functions	66.45% (+0.35% 🔼)	1830/2754
🟢	Lines	80.61% (+0.28% 🔼)	10176/12623

Show new covered files 🐣

St.^❔	File	Statements	Branches	Functions	Lines
🟢	`...` / EditableApprovalItem.tsx	95.65%	100%	75%	100%
🟢	`...` / SpenderField.tsx	100%	100%	100%	100%

Show files with reduced coverage 🔻

St.^❔	File	Statements	Branches	Functions	Lines
🟡	`...` / web3.ts	70.83% (-0.6% 🔻)	25%	57.14%	67.57% (-0.85% 🔻)
🟡	`...` / safe-messages.ts	79.66% (-1.59% 🔻)	72% (-2.07% 🔻)	100%	77.36% (-0.82% 🔻)
🟢	`...` / SignMessage.tsx	89.89% (-1.29% 🔻)	83.33% (+6.06% 🔼)	66.67% (-2.56% 🔻)	90.7% (-1.22% 🔻)
🟢	`...` / ApprovalEditorForm.tsx	100%	50% (-25% 🔻)	100%	100%
🟢	`...` / ApprovalValueField.tsx	88.46% (-6.99% 🔻)	78.57% (+7.14% 🔼)	87.5% (-12.5% 🔻)	88% (-7.24% 🔻)
🟢	`...` / ApprovalItem.tsx	92.31% (-7.69% 🔻)	33.33% (-66.67% 🔻)	75% (-25% 🔻)	90.91% (-9.09% 🔻)
🔴	`...` / index.tsx	44.83% (-0.33% 🔻)	0%	0%	46.43% (-0.24% 🔻)
🟢	`...` / SendAmountBlock.tsx	87.5% (-12.5% 🔻)	0%	0% (-100% 🔻)	100%
🟢	`...` / index.tsx	75% (-25% 🔻)	100%	0% (-100% 🔻)	100%

Test suite run success

1422 tests passing in 198 suites.

Report generated by 🧪jest coverage report action from 2084973

compojoom · 2024-04-25T13:53:05Z

works nice! No storybook?

src/components/tx/ApprovalEditor/ApprovalValueField.tsx

src/components/tx/ApprovalEditor/ApprovalEditorForm.tsx

src/components/tx/ApprovalEditor/ApprovalItem.tsx

usame-algan · 2024-04-26T12:33:30Z

src/components/tx/ApprovalEditor/EditableApprovalItem.tsx

+    <Stack direction="row" alignItems="center" gap={2} className={css.approvalField}>
+      <Box display="flex" flexDirection="row" alignItems="center" gap="4px">
+        <TokenIcon size={32} logoUri={approval.tokenInfo?.logoUri} tokenSymbol={approval.tokenInfo?.symbol} />
+        <Typography>{approval.tokenInfo?.symbol || shortenAddress(approval.tokenAddress)}</Typography>


Imo this makes the alignment in the Box worse than before especially if we show a shortened address. Can it even happen that there is no symbol for a Token?

One alternative would be to show the Symbol as a Suffix to the value i.e. Set allowance to 1 COW but it seems hard to achieve in the input element. Maybe @TanyaEfremova has another idea?

@usame-algan I think adding a suffix to the label w

orks!

I think we will have to convert these labels to template strings as it should be e.g. "Set WETH allowance to 5"
Or "Increase SAFE allowance by 20"

src/components/tx/ApprovalEditor/index.tsx

github-actions · 2024-04-29T09:47: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}

usame-algan

Just have a small adjustment for the readOnly mode but looks good otherwise 👍

src/components/tx/ApprovalEditor/EditableApprovalItem.tsx

src/components/tx/ApprovalEditor/ApprovalValueField.tsx

github-actions · 2024-05-02T14:12:00Z

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}

liliya-soroka · 2024-05-03T10:21:14Z

Change the text under what is Allow access to the text from the design

liliya-soroka · 2024-05-03T10:33:27Z

Question about the spender truncated address view.
Could we truncate the address only for the low-resolution?

schmanu · 2024-05-03T11:05:34Z

Change the text under what is Allow access to the text from the design

As part of the review this was changed. Tanya also confirmed this change: See here

liliya-soroka · 2024-05-03T11:24:11Z

Validation error confusing

github-actions · 2024-05-07T06:45:52Z

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}

feat: redesign approval editor

2622482

schmanu requested a review from usame-algan April 25, 2024 13:15

schmanu marked this pull request as draft April 25, 2024 13:26

fix: store position in transaction within approval infos

ebc696e

usame-algan reviewed Apr 26, 2024

View reviewed changes

schmanu marked this pull request as ready for review April 29, 2024 06:56

fix: review issues and bug with increaseAllowance

5546787

fix: tests

27fac17

schmanu requested a review from usame-algan May 2, 2024 07:57

usame-algan approved these changes May 2, 2024

View reviewed changes

src/components/tx/ApprovalEditor/EditableApprovalItem.tsx Outdated Show resolved Hide resolved

src/components/tx/ApprovalEditor/ApprovalValueField.tsx Show resolved Hide resolved

src/components/tx/ApprovalEditor/ApprovalValueField.tsx Show resolved Hide resolved

fix: suggested changes

17d0a42

fix: shortAddress for small screens only

3ecb731

schmanu added 2 commits May 3, 2024 13:33

fix: show validation errors in approval editor

4e64644

test: fix test expectation

2084973

schmanu merged commit 1e1597d into dev May 7, 2024
14 checks passed

schmanu deleted the feat/redesign-approval-editor branch May 7, 2024 07:48

github-actions bot locked and limited conversation to collaborators May 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: redesign approval editor #3614

feat: redesign approval editor #3614

schmanu commented Apr 25, 2024 •

edited

github-actions bot commented Apr 25, 2024 •

edited

github-actions bot commented Apr 25, 2024 •

edited

github-actions bot commented Apr 25, 2024 •

edited

github-actions bot commented Apr 25, 2024 •

edited

compojoom commented Apr 25, 2024

usame-algan Apr 26, 2024

usame-algan Apr 26, 2024

TanyaEfremova Apr 26, 2024

schmanu Apr 29, 2024

github-actions bot commented Apr 29, 2024 •

edited

usame-algan left a comment

github-actions bot commented May 2, 2024 •

edited

liliya-soroka commented May 3, 2024

liliya-soroka commented May 3, 2024

schmanu commented May 3, 2024

liliya-soroka commented May 3, 2024

github-actions bot commented May 7, 2024

feat: redesign approval editor #3614

feat: redesign approval editor #3614

Conversation

schmanu commented Apr 25, 2024 • edited

What it solves

TODO

How this PR fixes it

How to test it

Screenshots

Checklist

github-actions bot commented Apr 25, 2024 • edited

Branch preview

github-actions bot commented Apr 25, 2024 • edited

ESLint Summary View Full Report

github-actions bot commented Apr 25, 2024 • edited

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

⚠️ Global Bundle Size Increased

Five Pages Changed Size

github-actions bot commented Apr 25, 2024 • edited

Coverage report

Test suite run success

compojoom commented Apr 25, 2024

usame-algan Apr 26, 2024

Choose a reason for hiding this comment

usame-algan Apr 26, 2024

Choose a reason for hiding this comment

TanyaEfremova Apr 26, 2024

Choose a reason for hiding this comment

schmanu Apr 29, 2024

Choose a reason for hiding this comment

github-actions bot commented Apr 29, 2024 • edited

ESLint Summary View Full Report

usame-algan left a comment

Choose a reason for hiding this comment

github-actions bot commented May 2, 2024 • edited

ESLint Summary View Full Report

liliya-soroka commented May 3, 2024

liliya-soroka commented May 3, 2024

schmanu commented May 3, 2024

liliya-soroka commented May 3, 2024

github-actions bot commented May 7, 2024

ESLint Summary View Full Report

schmanu commented Apr 25, 2024 •

edited

github-actions bot commented Apr 25, 2024 •

edited

github-actions bot commented Apr 25, 2024 •

edited

github-actions bot commented Apr 25, 2024 •

edited

github-actions bot commented Apr 25, 2024 •

edited

github-actions bot commented Apr 29, 2024 •

edited

github-actions bot commented May 2, 2024 •

edited