NFT Approval and SetApprovalForAll confirmation screens header updated by filipsekulic · Pull Request #15727 · MetaMask/metamask-extension

filipsekulic · 2022-08-30T13:42:54Z

Explanation

Updated the NFT Approval and SetApprovalForAll confirmation screens header.

More Information

Fixes Update the NFT Approval and SetApprovalForAll confirmation screens to use the new header component #15695
Related Header component for transaction confirmation screens #15614

Screenshots/Screencaps

Before

Before.mov

After

After.mov

Manual Testing Steps

1st case:

Open the test dapp https://metamask.github.io/test-dapp/
Under the "Collectibles" section, click on "Deploy"
Wait for transaction to be confirmed and click on "Mint"
Wait for transaction to be confirmed and click on "Approve" to get our NFT approval confirmation screen
Click on "Set Approval For All" to get our setApprovalForAll confirmation screen

2nd case:

Open the test dapp https://metamask.github.io/test-dapp/
Under the "Send tokens" section, click on "Create token"
Wait for the token to be created and click on "Approve tokens" or "Approve tokens without gas"

github-actions · 2022-08-30T13:43:11Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

mirjanaKukic · 2022-08-31T09:08:16Z

Verified by QA

metamaskbot · 2022-09-08T15:49:46Z

Builds ready [f2bf921]

builds: chrome, firefox, opera
builds (beta): chrome, firefox, opera
builds (flask): chrome, firefox, opera
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 2, 3, 4, 5, 6, 7
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1834 ± 64 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	97	173	120	18	9
		domContentLoaded	1629	2115	1810	132	63
		load	1652	2115	1834	134	64
		domInteractive	1629	2114	1810	132	63

highlights:

storybook

mirjanaKukic · 2022-09-09T10:00:41Z

Verified by QA

adonesky1 · 2022-09-09T16:24:42Z

Do we want this header show up for ERC20 approvals too? Probably right?

Screen.Recording.2022-09-09.at.11.24.12.AM.mov

adonesky1

Seems like we want to make this change for ERC20 approvals too?

metamaskbot · 2022-09-12T10:09:44Z

Builds ready [9b02d25]

builds: chrome, firefox, opera
builds (beta): chrome, firefox, opera
builds (flask): chrome, firefox, opera
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 2, 3, 4, 5, 6, 7
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1710 ± 59 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	89	170	115	19	9
		domContentLoaded	1573	2132	1688	121	58
		load	1573	2132	1710	122	59
		domInteractive	1573	2132	1688	121	58

highlights:

storybook

filipsekulic · 2022-09-12T13:00:15Z

Seems like we want to make this change for ERC20 approvals too?

Yes, I had checked it previously with @bschorchit before I made changes here (45ebf6ce6f448258652850fdcfa0fdb65ac91cd5) and updated the screencast (the after one).

metamaskbot · 2022-09-12T13:47:16Z

Builds ready [45ebf6c]

builds: chrome, firefox, opera
builds (beta): chrome, firefox, opera
builds (flask): chrome, firefox, opera
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 2, 3, 4, 5, 6, 7
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1940 ± 84 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	99	2080	223	426	205
		domContentLoaded	1631	2323	1918	173	83
		load	1631	2323	1940	175	84
		domInteractive	1631	2323	1918	173	83

highlights:

storybook

adonesky1 · 2022-09-15T17:03:33Z

@filipsekulic need a rebase here and then looks good to go!

filipsekulic · 2022-09-15T21:10:22Z

@filipsekulic need a rebase here and then looks good to go!

Done ✅

metamaskbot · 2022-09-15T21:11:22Z

Builds ready [b66f487]

builds: chrome, firefox, opera
builds (beta): chrome, firefox, opera
builds (flask): chrome, firefox, opera
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 2, 3, 4, 5, 6, 7
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1513 ± 76 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	107	214	139	30	14
		domContentLoaded	1262	1769	1501	151	72
		load	1262	1869	1513	159	76
		domInteractive	1262	1769	1501	151	72

highlights:

storybook

adonesky1

LGTM! Thanks Filip!

metamaskbot · 2022-09-21T13:04:04Z

Builds ready [8a85f7b]

builds: chrome, firefox, opera
builds (beta): chrome, firefox, opera
builds (flask): chrome, firefox, opera
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 2, 3, 4, 5, 6, 7
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1215 ± 27 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	86	1172	153	234	112
		domContentLoaded	1113	1367	1201	57	27
		load	1113	1367	1215	55	27
		domInteractive	1113	1367	1201	57	27

highlights:

storybook

filipsekulic self-assigned this Aug 30, 2022

filipsekulic marked this pull request as ready for review August 31, 2022 09:38

filipsekulic requested a review from a team as a code owner August 31, 2022 09:38

filipsekulic requested a review from danjm August 31, 2022 09:38

filipsekulic force-pushed the update-nft-approval-and-set-approval-for-all-header branch 2 times, most recently from b6c4124 to 387e65a Compare September 8, 2022 14:06

adonesky1 requested changes Sep 9, 2022

View reviewed changes

filipsekulic requested a review from adonesky1 September 12, 2022 13:00

filipsekulic mentioned this pull request Sep 12, 2022

Header component for transaction confirmation screens #15614

Merged

filipsekulic force-pushed the update-nft-approval-and-set-approval-for-all-header branch from 45ebf6c to f09d606 Compare September 15, 2022 19:48

adonesky1 previously approved these changes Sep 15, 2022

View reviewed changes

Filip Sekulic added 8 commits September 21, 2022 14:03

Updated header for NFT approval and Set approval for all screns

48a7979

Covered additional cases

1fe23a9

Code refactor

bee95c5

Added token standard condition

2ad655e

Removed unnecessary code

173366d

Updated header component

8f3c628

Enabled showing the new header for ERC20 approvals too

5ff155f

Fixed type error

8a85f7b

filipsekulic dismissed adonesky1’s stale review via 8a85f7b September 21, 2022 12:07

filipsekulic force-pushed the update-nft-approval-and-set-approval-for-all-header branch from b66f487 to 8a85f7b Compare September 21, 2022 12:07

filipsekulic requested a review from adonesky1 September 21, 2022 13:04

brad-decker approved these changes Sep 21, 2022

View reviewed changes

adonesky1 approved these changes Sep 21, 2022

View reviewed changes

adonesky1 merged commit 2ba6e68 into develop Sep 21, 2022

adonesky1 deleted the update-nft-approval-and-set-approval-for-all-header branch September 21, 2022 14:15

github-actions Bot locked and limited conversation to collaborators Sep 21, 2022

Conversation

filipsekulic commented Aug 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation

More Information

Screenshots/Screencaps

Before

After

Manual Testing Steps

Uh oh!

github-actions Bot commented Aug 30, 2022

Uh oh!

mirjanaKukic commented Aug 31, 2022

Uh oh!

metamaskbot commented Sep 8, 2022

highlights:

Uh oh!

mirjanaKukic commented Sep 9, 2022

Uh oh!

adonesky1 commented Sep 9, 2022

Uh oh!

adonesky1 left a comment

Choose a reason for hiding this comment

Uh oh!

metamaskbot commented Sep 12, 2022

highlights:

Uh oh!

filipsekulic commented Sep 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

metamaskbot commented Sep 12, 2022

highlights:

Uh oh!

adonesky1 commented Sep 15, 2022

Uh oh!

filipsekulic commented Sep 15, 2022

Uh oh!

metamaskbot commented Sep 15, 2022

highlights:

Uh oh!

adonesky1 left a comment

Choose a reason for hiding this comment

Uh oh!

metamaskbot commented Sep 21, 2022

highlights:

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

filipsekulic commented Aug 30, 2022 •

edited

Loading

filipsekulic commented Sep 12, 2022 •

edited

Loading