Added a 'Verify contract details' link to SetApprovalForAll confirmation screens #15756

adnansahovic · 2022-09-07T09:55:02Z

Explanation

Added a 'Verify contract details' link to SetApprovalForAll and NFT Allowance confirmation screens. Clicking on this link will take you to new contract detail modal. On the contract detail modal the NFT contract is the NFT contract that the user is granting allowance for, and the Contract requesting access is the address that being granted access to the NFTs (we currently show it as "Granted to" under permission request).

More Information

Fixes Add a 'Verify contract details' link to SetApprovalForAll confirmation screens #15697

Screenshots/Screencaps

Screen.Recording.2022-10-14.at.01.34.56.mov

Manual Testing Steps

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 "Set Approval For All" or "Approve"

github-actions · 2022-09-07T09:55:15Z

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.

metamaskbot · 2022-09-08T09:19:14Z

Builds ready [86812d5]

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 (1759 ± 49 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	97	164	121	16	8
		domContentLoaded	1597	1966	1743	92	44
		load	1620	2068	1759	102	49
		domInteractive	1597	1966	1743	92	44

highlights:

storybook

metamaskbot · 2022-09-12T08:06:46Z

Builds ready [fea6d49]

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 (1860 ± 55 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	98	1901	293	526	253
		domContentLoaded	1625	2082	1838	116	56
		load	1693	2107	1860	115	55
		domInteractive	1625	2082	1838	116	56

highlights:

storybook

mirjanaKukic · 2022-09-13T12:02:59Z

Verified by QA

metamaskbot · 2022-09-13T13:02:38Z

Builds ready [d5f4f2d]

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 (1921 ± 60 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	103	176	127	18	9
		domContentLoaded	1672	2130	1894	123	59
		load	1672	2200	1921	126	60
		domInteractive	1672	2130	1894	123	59

highlights:

storybook

adonesky1 · 2022-09-13T14:59:31Z

Does this mean we're no longer showing the address we're granting approval to on this screen?

adnansahovic · 2022-09-14T09:36:17Z

Does this mean we're no longer showing the address we're granting approval to on this screen?

hi @adonesky1, If this is what you meant, the setApprovalForAll confirmation screen doesn't show the address by default, but you can see it by clicking on View full transaction details ("Granted to:"), also clicking Verify contract details.

adonesky1 · 2022-09-15T22:27:00Z

hi @adonesky1, If this is what you meant, the setApprovalForAll confirmation screen doesn't show the address by default, but you can see it by clicking on View full transaction details ("Granted to:"), also clicking Verify contract details.

Ah after watching the video I see that the address hasn't changed there.

Looks good, only thing I would change is the placement. Seems like the address component should go above the "Verify Contract Details" button, since it should be right under the "You are allowing the follow account to access your funds".

adnansahovic · 2022-09-16T09:36:20Z

hi @adonesky1, If this is what you meant, the setApprovalForAll confirmation screen doesn't show the address by default, but you can see it by clicking on View full transaction details ("Granted to:"), also clicking Verify contract details.

Ah after watching the video I see that the address hasn't changed there.

Looks good, only thing I would change is the placement. Seems like the address component should go above the "Verify Contract Details" button, since it should be right under the "You are allowing the follow account to access your funds".

That address component will be deleted in this ticket

metamaskbot · 2022-10-14T00:12:22Z

Builds ready [105d4e0]

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, 7
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
- 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 (2475 ± 90 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	95	344	132	57	28
		domContentLoaded	2158	2871	2453	193	93
		load	2158	2872	2475	188	90
		domInteractive	2158	2871	2453	193	93

highlights:

storybook

metamaskbot · 2022-10-14T08:46:27Z

Builds ready [1853d18]

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, 7
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
- 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 (2230 ± 57 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	85	144	106	14	7
		domContentLoaded	1970	2432	2217	126	61
		load	1970	2432	2230	119	57
		domInteractive	1970	2432	2217	126	61

highlights:

storybook

mirjanaKukic · 2022-10-17T08:40:30Z

Verified by QA

metamaskbot · 2022-10-17T09:17:48Z

Builds ready [09c474e]

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, 7
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
- 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 (2068 ± 44 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	83	321	105	50	24
		domContentLoaded	1841	2211	2061	92	44
		load	1841	2212	2068	91	44
		domInteractive	1841	2211	2061	92	44

highlights:

storybook

metamaskbot · 2022-10-17T11:35:10Z

Builds ready [17ce341]

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, 7
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
- 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 (2344 ± 71 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	87	2537	346	701	336
		domContentLoaded	2169	2733	2327	136	65
		load	2169	2821	2344	147	71
		domInteractive	2169	2733	2327	136	65

highlights:

storybook

danjm · 2022-10-20T01:15:43Z

ui/components/app/modals/contract-details-modal/contract-details-modal.js

+  let nftCollectionNameExist;
+  let nftCollectionImageExist;
+
+  Object.values(collections).forEach((nftCollections) => {


I commented on some similar code in another PR here: #15844 (comment)

If we need the same code here, it might be best to create a reusable component for rendering nft collection images

Hi @danjm , You can see changes here

…ion screens added one larger button in footer verify locales modified contract-details-modal fixed contract details modal modified open in block explorer modified contract details modal deleted gotItModal in locales modified collection image modified nft collection image

metamaskbot · 2022-10-26T08:57:39Z

Builds ready [14a43cd]

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, 7
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
- 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 (2514 ± 159 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	99	2615	371	660	317
		domContentLoaded	1857	3173	2472	344	165
		load	1883	3173	2514	332	159
		domInteractive	1857	3173	2472	344	165

highlights:

storybook

adnansahovic self-assigned this Sep 7, 2022

adnansahovic force-pushed the verify-contract-details-SetApprovalForAll branch 2 times, most recently from ca276f9 to fea6d49 Compare September 12, 2022 07:12

adnansahovic force-pushed the verify-contract-details-SetApprovalForAll branch from fea6d49 to 148bb8a Compare September 13, 2022 09:44

adnansahovic force-pushed the verify-contract-details-SetApprovalForAll branch from 148bb8a to d5f4f2d Compare September 13, 2022 12:05

adnansahovic marked this pull request as ready for review September 13, 2022 13:05

adnansahovic requested a review from a team as a code owner September 13, 2022 13:05

adnansahovic requested a review from segun September 13, 2022 13:05

adnansahovic marked this pull request as draft September 15, 2022 12:17

adnansahovic force-pushed the verify-contract-details-SetApprovalForAll branch 2 times, most recently from 5eaeeb3 to fc5edea Compare September 15, 2022 18:29

adnansahovic force-pushed the verify-contract-details-SetApprovalForAll branch from fc5edea to 20927e7 Compare October 13, 2022 23:04

adnansahovic force-pushed the verify-contract-details-SetApprovalForAll branch 2 times, most recently from 628c778 to 09c474e Compare October 15, 2022 12:02

adnansahovic force-pushed the verify-contract-details-SetApprovalForAll branch from 09c474e to 4438e5f Compare October 17, 2022 09:54

adnansahovic marked this pull request as ready for review October 17, 2022 11:15

danjm reviewed Oct 20, 2022

View reviewed changes

adnansahovic added 3 commits October 26, 2022 09:51

modified contract title and contract requesting access

f528355

modified nft collection image

14a43cd

adnansahovic force-pushed the verify-contract-details-SetApprovalForAll branch from 17ce341 to 14a43cd Compare October 26, 2022 07:56

VSaric mentioned this pull request Oct 27, 2022

Updated secondary copy and removed the address component from SetAppovalForAll and NFT Approve screens #16292

Merged

danjm approved these changes Nov 3, 2022

View reviewed changes

brad-decker approved these changes Nov 14, 2022

View reviewed changes

brad-decker merged commit bf1db00 into develop Nov 14, 2022

brad-decker deleted the verify-contract-details-SetApprovalForAll branch November 14, 2022 19:28

github-actions bot locked and limited conversation to collaborators Nov 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added a 'Verify contract details' link to SetApprovalForAll confirmation screens #15756

Added a 'Verify contract details' link to SetApprovalForAll confirmation screens #15756

adnansahovic commented Sep 7, 2022 •

edited

Loading

github-actions bot commented Sep 7, 2022

metamaskbot commented Sep 8, 2022

metamaskbot commented Sep 12, 2022

mirjanaKukic commented Sep 13, 2022

metamaskbot commented Sep 13, 2022

adonesky1 commented Sep 13, 2022

adnansahovic commented Sep 14, 2022

adonesky1 commented Sep 15, 2022

adnansahovic commented Sep 16, 2022

metamaskbot commented Oct 14, 2022

metamaskbot commented Oct 14, 2022

mirjanaKukic commented Oct 17, 2022

metamaskbot commented Oct 17, 2022

metamaskbot commented Oct 17, 2022

danjm Oct 20, 2022

adnansahovic Oct 26, 2022

metamaskbot commented Oct 26, 2022

Added a 'Verify contract details' link to SetApprovalForAll confirmation screens #15756

Added a 'Verify contract details' link to SetApprovalForAll confirmation screens #15756

Conversation

adnansahovic commented Sep 7, 2022 • edited Loading

Explanation

More Information

Screenshots/Screencaps

Manual Testing Steps

github-actions bot commented Sep 7, 2022

metamaskbot commented Sep 8, 2022

highlights:

metamaskbot commented Sep 12, 2022

highlights:

mirjanaKukic commented Sep 13, 2022

metamaskbot commented Sep 13, 2022

highlights:

adonesky1 commented Sep 13, 2022

adnansahovic commented Sep 14, 2022

adonesky1 commented Sep 15, 2022

adnansahovic commented Sep 16, 2022

metamaskbot commented Oct 14, 2022

highlights:

metamaskbot commented Oct 14, 2022

highlights:

mirjanaKukic commented Oct 17, 2022

metamaskbot commented Oct 17, 2022

highlights:

metamaskbot commented Oct 17, 2022

highlights:

danjm Oct 20, 2022

Choose a reason for hiding this comment

adnansahovic Oct 26, 2022

Choose a reason for hiding this comment

metamaskbot commented Oct 26, 2022

highlights:

adnansahovic commented Sep 7, 2022 •

edited

Loading