Refactor: unify confirmations in the transaction flow #4177

clovisdasilvaneto · 2024-09-16T08:44:45Z

What it solves

Unify all the Review screens from the tx-flow floder.

How this PR fixes it

It introduces a refactor in the way how we handle the confirmation views on the SignOrExecuteForm component. This PR introduces the first part of the refactor, by creating a central component to handle the presentation of the Review screens. It also makes all modals from the approval owner flow to use the same SettingsChange component (which was part of the ReviewOwner component.

How to test it

1- Go to the add signer flow (in a modal)
2- Go to the confirmation screen.

You will see the new screen shows a loading screen before the confirmation screen, it happens because we had to move some data outside the SignOrExecuteForm component, and these data was asynchronous, so in order to prevent some glitch in the screen, we show the loading component before we have load the necessary data to show in the component.

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-09-16T08:44:58Z

Branch preview

✅ Deploy successful!

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

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

github-actions · 2024-09-16T08:45:58Z

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-09-16T08:48:33Z

📦 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`	`1011.89 KB` (🟡 +158 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!

Four 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`	`48.42 KB` (-1 B)	1.04 MB
`/apps/custom`	`40.06 KB` (-1 B)	1.03 MB
`/balances/nfts`	`19.2 KB` (-1 B)	1.01 MB
`/stake`	`596 B` (🟢 -1 B)	1012.47 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-09-16T08:51:20Z

Coverage report

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	74.19% (+0.6% 🔼)	13746/18527
🔴	Branches	52.77% (+1.35% 🔼)	3397/6437
🔴	Functions	58.04% (+0.89% 🔼)	2011/3465
🟡	Lines	75.86% (+0.55% 🔼)	12485/16459

Show new covered files 🐣

St.^❔	File	Statements	Branches	Functions	Lines
🟢	`...` / SignOrExecuteForm.tsx	85.54%	78.75%	42.86%	84.81%
🟢	`...` / index.tsx	92.31%	61.9%	100%	100%
🟢	`...` / index.tsx	95%	85.71%	100%	100%
🟢	`...` / ChangeSignerSetupWarning.tsx	100%	100%	100%	100%
🟢	`...` / useIsMultichainSafe.ts	90.91%	100%	66.67%	100%
🟢	`...` / context.ts	100%	100%	100%	100%
🟢	`...` / index.tsx	100%	100%	100%	100%
🟢	`...` / context.tsx	100%	100%	100%	100%
🟢	`...` / index.tsx	100%	100%	100%	100%
🟢	`...` / utils.ts	100%	100%	100%	100%
🟢	`...` / SignOrExecuteSkeleton.tsx	100%	100%	100%	100%
🟡	`...` / index.tsx	62.16%	33.33%	42.86%	61.11%
🟢	`...` / mockData.ts	100%	100%	100%	100%
🟢	`...` / mockData.ts	100%	100%	100%	100%

Show files with reduced coverage 🔻

St.^❔	File	Statements	Branches	Functions	Lines
🟡	`...` / hooks.ts	79.26% (-1.78% 🔻)	68.29% (-4.43% 🔻)	70% (-0.59% 🔻)	79.51% (-1.62% 🔻)
🔴	`...` / SafeTxProvider.tsx	27.5% (-1.45% 🔻)	0%	36.36% (-3.64% 🔻)	27.78% (-1.63% 🔻)
🟡	src/pages/_app.tsx	69.33% (-0.4% 🔻)	42.86%	0%	68.06% (-0.44% 🔻)
🔴	`...` / index.tsx	30.08% (-4.88% 🔻)	0% (-1.96% 🔻)	0% (-2.63% 🔻)	33.04% (-4.46% 🔻)

Test suite run success

1582 tests passing in 213 suites.

Report generated by 🧪jest coverage report action from 0c1b25e

src/components/tx-flow/flows/RecoverAccount/RecoverAccountFlowReview.tsx

src/components/tx-flow/flows/SignMessageOnChain/ReviewSignMessageOnChain.test.tsx

src/components/tx/SignOrExecuteForm/SignOrExecuteForm.tsx

src/components/tx/SignOrExecuteForm/hooks.ts

src/components/tx/confirmation-views/SettingsChange/index.ts

src/components/tx/confirmation-views/SettingsChange/utils.ts

src/components/tx/confirmation-views/index.tsx

src/components/tx/confirmation-views/utils.tsx

github-actions · 2024-09-16T11:31:09Z

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	1	0
Ignored	0	N/A

Result: ✅ success
Annotations: 1 total

[warning] react-hooks/exhaustive-deps

verifies the list of dependencies for Hooks like useEffect and similar

src/components/tx/SignOrExecuteForm/hooks.ts Line 52 - React Hook useAsync has a missing dependency: 'safe.chainId'. Either include it or remove the dependency array.

^{Report generated by eslint-plus-action}

github-actions · 2024-09-17T11:42:27Z

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}

src/components/tx/SignOrExecuteForm/index.tsx

src/components/tx/SignOrExecuteForm/hooks.ts

usame-algan · 2024-09-17T13:42:53Z

src/components/tx/SignOrExecuteForm/hooks.ts

+    const proposeTx: TxActions['proposeTx'] = async (safeTx, txId, origin) => {
+      assertTx(safeTx)
+      return _propose(wallet?.address || safe.owners[0].value, safeTx, txId, origin)
+    }


Does this need to be a separate function just for the assertTx?

src/components/tx/DecodedTx/index.tsx

github-actions · 2024-09-18T07:43:09Z

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-09-19T08:49:38Z

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-09-19T13:23:46Z

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-09-19T17:03:55Z

any reason we moved approve at the top again ?
We moved approve under main information for the purpose some time ago - #3855

github-actions · 2024-09-20T12:04:46Z

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-26T13:46:41Z

Confirmation Screens Refactor

…ow screen

liliya-soroka · 2024-10-25T10:17:38Z

Replace owner:
A new owner name is missing from the previous step

francovenica · 2024-10-31T00:45:05Z

The issue of the owner name in the adding and replacing was fixed.

New issue: "Add signer" form shows the new owner twice
Just try to add a new owner and reach the review page

francovenica · 2024-11-01T01:57:25Z

The issue is fixed:

LGTM

clovisdasilvaneto marked this pull request as ready for review September 16, 2024 08:44

clovisdasilvaneto requested review from katspaugh and usame-algan September 16, 2024 08:47

katspaugh reviewed Sep 16, 2024

View reviewed changes

katspaugh changed the title ~~Chore: Refactor confirmation transactions flow~~ Refactor: unify confirmations in the transaction flow Sep 16, 2024

clovisdasilvaneto requested a review from katspaugh September 16, 2024 11:39

clovisdasilvaneto force-pushed the chore/refactor-transaction-flow branch from 1615043 to e8f3d2e Compare September 16, 2024 11:45

katspaugh force-pushed the dev branch from 10ac4f4 to 43c7da0 Compare September 16, 2024 12:37

clovisdasilvaneto force-pushed the chore/refactor-transaction-flow branch from c26dac4 to 80f3df1 Compare September 16, 2024 14:33

clovisdasilvaneto force-pushed the chore/refactor-transaction-flow branch from 7250d54 to 4148903 Compare September 17, 2024 12:51

clovisdasilvaneto requested review from usame-algan and removed request for usame-algan September 17, 2024 12:52

usame-algan reviewed Sep 17, 2024

View reviewed changes

katspaugh approved these changes Sep 17, 2024

View reviewed changes

usame-algan force-pushed the chore/refactor-transaction-flow branch from 5d7b041 to 20c52f2 Compare September 18, 2024 13:46

clovisdasilvaneto force-pushed the chore/refactor-transaction-flow branch from 1840103 to 49e771f Compare September 19, 2024 13:22

clovisdasilvaneto mentioned this pull request Sep 19, 2024

Refactor: Unify changeThresholdReview screen #4212

Merged

3 tasks

clovisdasilvaneto mentioned this pull request Sep 20, 2024

Refactor: unify confirmBatch screen #4217

Merged

3 tasks

clovisdasilvaneto and others added 16 commits October 18, 2024 12:08

fix: do not propose transaction for contrafactual safes

a122ac3

fix: cypress drain e2e test

9b1bf83

fix: remove duplicated data in the DecodedTx component

83d99a8

fix: Add back showMethodCall and keep prop drilling it

3f258e9

fix: do not show method call for approve transactions

cd96870

fix: add showMethodCall into the confirmationView component

522b8a8

fix: pass isApproval down to the confirmation view component

4138686

fix: Add isCreation check inside DecodedTx to render partial summary

214b48a

fix: Update snapshot and mock hex data generation to be even length

f7e79b9

chore: generated snapshots

f79c5af

chore: refactor changeThresholdReview screen (#4212)

689c862

Approval editor

e7ceec3

Fix error display in confirmation screen

528d06f

chore: unify confirmBatch screen (#4217)

cac3e67

fix(account-flow-import): change import src in the recover account fl…

47c3960

…ow screen

fix(eslint): eslint hook dependencies

360f9d4

clovisdasilvaneto force-pushed the chore/refactor-transaction-flow branch from f3d527b to 360f9d4 Compare October 21, 2024 15:54

clovisdasilvaneto added 4 commits October 22, 2024 14:22

fix(unit-tests): change txDetails mocked data in unit tests

7b517a6

fix(eslint): change operators order

ea4becd

Merge branch 'dev' into chore/refactor-transaction-flow

94b8e80

fix(unit-tests): mock useSafeAddress hook

ed04a74

clovisdasilvaneto added 2 commits October 28, 2024 09:53

Merge branch 'dev' into chore/refactor-transaction-flow

26ee5b6

fix(settings-change): add address name in the change owner screen

40f3ea0

fix(settings-change): duplicated owner name in the add owner flow

0c1b25e

clovisdasilvaneto merged commit aad2bb5 into dev Nov 1, 2024
15 checks passed

clovisdasilvaneto deleted the chore/refactor-transaction-flow branch November 1, 2024 10:04

github-actions bot locked and limited conversation to collaborators Nov 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor: unify confirmations in the transaction flow #4177

Refactor: unify confirmations in the transaction flow #4177

clovisdasilvaneto commented Sep 16, 2024

github-actions bot commented Sep 16, 2024 •

edited

Loading

github-actions bot commented Sep 16, 2024 •

edited

Loading

github-actions bot commented Sep 16, 2024 •

edited

Loading

github-actions bot commented Sep 16, 2024 •

edited

Loading

github-actions bot commented Sep 16, 2024 •

edited

Loading

github-actions bot commented Sep 17, 2024 •

edited

Loading

usame-algan Sep 17, 2024

github-actions bot commented Sep 18, 2024 •

edited

Loading

github-actions bot commented Sep 19, 2024 •

edited

Loading

github-actions bot commented Sep 19, 2024 •

edited

Loading

liliya-soroka commented Sep 19, 2024 •

edited

Loading

github-actions bot commented Sep 20, 2024 •

edited

Loading

notion-workspace bot commented Sep 26, 2024

liliya-soroka commented Oct 25, 2024 •

edited

Loading

francovenica commented Oct 31, 2024

francovenica commented Nov 1, 2024

Refactor: unify confirmations in the transaction flow #4177

Refactor: unify confirmations in the transaction flow #4177

Conversation

clovisdasilvaneto commented Sep 16, 2024

What it solves

How this PR fixes it

How to test it

Screenshots

Checklist

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

Branch preview

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

ESLint Summary View Full Report

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

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

⚠️ Global Bundle Size Increased

Four Pages Changed Size

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

Coverage report

Test suite run success

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

ESLint Summary View Full Report

[warning] react-hooks/exhaustive-deps

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

ESLint Summary View Full Report

usame-algan Sep 17, 2024

Choose a reason for hiding this comment

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

ESLint Summary View Full Report

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

ESLint Summary View Full Report

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

ESLint Summary View Full Report

liliya-soroka commented Sep 19, 2024 • edited Loading

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

ESLint Summary View Full Report

notion-workspace bot commented Sep 26, 2024

liliya-soroka commented Oct 25, 2024 • edited Loading

francovenica commented Oct 31, 2024

francovenica commented Nov 1, 2024

github-actions bot commented Sep 16, 2024 •

edited

Loading

github-actions bot commented Sep 16, 2024 •

edited

Loading

github-actions bot commented Sep 16, 2024 •

edited

Loading

github-actions bot commented Sep 16, 2024 •

edited

Loading

github-actions bot commented Sep 16, 2024 •

edited

Loading

github-actions bot commented Sep 17, 2024 •

edited

Loading

github-actions bot commented Sep 18, 2024 •

edited

Loading

github-actions bot commented Sep 19, 2024 •

edited

Loading

github-actions bot commented Sep 19, 2024 •

edited

Loading

liliya-soroka commented Sep 19, 2024 •

edited

Loading

github-actions bot commented Sep 20, 2024 •

edited

Loading

liliya-soroka commented Oct 25, 2024 •

edited

Loading