fix: updated permissions header to be consistent #29880

NidhiKJha · 2025-01-23T17:17:50Z

This PR is to update the title of the Dapp in all the connection steps

Related issues

Fixes: #29785

Manual testing steps

Connect MM to a Dapp
Check the correct Dapp Url is shown through all screens while connecting

Screenshots/Recordings

Before

404338198-60256b44-59b6-47ec-9c0f-aeeec0ef2b45.mov

After

Dapp Connection

Screen.Recording.2025-01-23.at.5.13.23.PM.mov

Snap Connection

Screen.Recording.2025-01-23.at.5.14.44.PM.mov

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
I've completed the PR template to the best of my ability
I’ve included tests if applicable
I’ve documented my code using JSDoc format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

ui/pages/permissions-connect/permissions-connect.component.js

hmalik88 · 2025-01-23T18:10:34Z

This breaks currently intended functionality for snaps installation. In your recording, the header changes to the solflare wallet as the origin making the connection request, it should remain the snap.metamask.io origin until the installation screen.

NidhiKJha · 2025-01-23T18:17:39Z

This breaks currently intended functionality for snaps installation. In your recording, the header changes to the solflare wallet as the origin making the connection request, it should remain the snap.metamask.io origin until the installation screen.

Aah! I see. the npm title shouldn't be there. In this case, should I revert the props for snaps?

metamaskbot · 2025-01-23T18:38:09Z

Builds ready [29c534c]

builds: chrome, firefox
builds (flask): chrome, firefox
builds (MMI): chrome
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Bundle Size Stats
mv2: E2e Actions Stats
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, 10, 11, 12, 13
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- content-script: 0
- offscreen: 0

Page Load Metrics (1803 ± 78 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1619	2262	1803	173	83
		domContentLoaded	1596	2070	1764	143	69
		load	1622	2149	1803	163	78
		domInteractive	25	132	56	33	16
		backgroundConnect	9	251	44	52	25
		firstReactRender	16	111	39	29	14
		getState	4	57	18	18	9
		initialActions	0	1	0	0	0
		loadScripts	1143	1553	1311	129	62
		setupStore	6	83	27	27	13
		uiStartup	1839	2914	2105	250	120

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 388 Bytes (0.00%)
common: 0 Bytes (0.00%)

NidhiKJha · 2025-01-23T18:45:58Z

Aah! I see. the npm title shouldn't be there. In this case, should I revert the props for snaps?

I updated the prop value to use siteMetaData, I think it should resolve the issue.

Screen.Recording.2025-01-23.at.6.45.32.PM.mov

metamaskbot · 2025-01-23T19:17:40Z

Builds ready [60d93e4]

builds: chrome, firefox
builds (flask): chrome, firefox
builds (MMI): chrome
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Bundle Size Stats
mv2: E2e Actions Stats
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, 10, 11, 12, 13
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- content-script: 0
- offscreen: 0

Page Load Metrics (1601 ± 52 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1439	1900	1610	108	52
		domContentLoaded	1413	1881	1583	107	51
		load	1422	1904	1601	108	52
		domInteractive	23	74	35	16	8
		backgroundConnect	8	56	22	14	7
		firstReactRender	15	100	37	26	13
		getState	4	44	12	11	5
		initialActions	0	0	0	0	0
		loadScripts	1007	1396	1157	97	46
		setupStore	6	70	14	19	9
		uiStartup	1575	2454	1836	196	94

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 620 Bytes (0.01%)
common: 0 Bytes (0.00%)

…etadata

metamaskbot · 2025-01-24T09:33:58Z

Builds ready [66b1fcd]

builds: chrome, firefox
builds (flask): chrome, firefox
builds (MMI): chrome
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Bundle Size Stats
mv2: E2e Actions Stats
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, 10, 11, 12, 13
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- content-script: 0
- offscreen: 0

Page Load Metrics (1774 ± 111 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	195	2227	1691	418	201
		domContentLoaded	1427	2162	1751	236	113
		load	1438	2177	1774	231	111
		domInteractive	24	195	49	42	20
		backgroundConnect	10	55	20	14	7
		firstReactRender	15	92	34	24	11
		getState	4	59	19	20	10
		initialActions	0	0	0	0	0
		loadScripts	1015	1637	1305	207	99
		setupStore	6	57	14	17	8
		uiStartup	1657	2648	2046	276	132

Bundle size diffs [🚀 Bundle size reduced!]

background: 1.04 KiB (0.02%)
ui: 43 Bytes (0.00%)
common: -40.26 KiB (-0.44%)

ui/pages/permissions-connect/permissions-connect.component.js

metamaskbot · 2025-01-24T10:46:58Z

Builds ready [c3272cc]

builds: chrome, firefox
builds (flask): chrome, firefox
builds (MMI): chrome
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Bundle Size Stats
mv2: E2e Actions Stats
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, 10, 11, 12, 13
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- content-script: 0
- offscreen: 0

Page Load Metrics (1827 ± 77 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1493	2116	1828	157	75
		domContentLoaded	1487	2094	1797	152	73
		load	1491	2120	1827	161	77
		domInteractive	19	97	41	21	10
		backgroundConnect	7	63	34	15	7
		firstReactRender	17	101	39	25	12
		getState	5	69	29	23	11
		initialActions	0	1	0	0	0
		loadScripts	1068	1548	1314	137	66
		setupStore	6	51	15	14	7
		uiStartup	1730	2688	2118	220	106

Bundle size diffs [🚀 Bundle size reduced!]

background: 1.04 KiB (0.02%)
ui: -98 Bytes (-0.00%)
common: -40.26 KiB (-0.44%)

ui/pages/permissions-connect/permissions-connect.component.js

metamaskbot · 2025-01-24T12:54:43Z

Builds ready [4f47a96]

builds: chrome, firefox
builds (flask): chrome, firefox
builds (MMI): chrome
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Bundle Size Stats
mv2: E2e Actions Stats
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, 10, 11, 12, 13
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- content-script: 0
- offscreen: 0

Page Load Metrics (1934 ± 98 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	314	2341	1851	409	196
		domContentLoaded	1600	2284	1903	208	100
		load	1605	2339	1934	204	98
		domInteractive	28	108	50	24	11
		backgroundConnect	7	65	30	20	10
		firstReactRender	16	101	42	27	13
		getState	4	83	18	20	10
		initialActions	0	1	0	0	0
		loadScripts	1167	1751	1415	170	81
		setupStore	6	76	22	23	11
		uiStartup	1847	3089	2278	291	140

Bundle size diffs [🚀 Bundle size reduced!]

background: 1.04 KiB (0.02%)
ui: 86 Bytes (0.00%)
common: -40.26 KiB (-0.44%)

darkwing · 2025-02-14T21:00:51Z

1️⃣ What was the issue?

. The origin was not defined on the permissions page for snaps and Dapps and so, it was showing unknown origin in the connection screen

2️⃣ Why did it happen and why wasn’t it caught earlier?

. It happened because targetSubjectMetadata was not cached properly. This issue already existed in prod. It happened in a previous release and was introduced in this PR

3️⃣ What actions are we taking to prevent this in the future?

.

4️⃣ Any related tickets and ETAs for follow-up actions?

.

NidhiKJha requested a review from a team as a code owner January 23, 2025 17:17

metamaskbot added team-wallet-ux INVALID-PR-TEMPLATE PR's body doesn't match template labels Jan 23, 2025

vinnyhoward previously approved these changes Jan 23, 2025

View reviewed changes

NidhiKJha dismissed vinnyhoward’s stale review via 0dc265a January 23, 2025 17:41

NidhiKJha changed the title ~~fix: updated permissions-connect-header to remove the title unknown origin…~~ fix: updated permissions header to be consistent Jan 23, 2025

NidhiKJha requested a review from vinnyhoward January 23, 2025 17:49

vinnyhoward previously approved these changes Jan 23, 2025

View reviewed changes

NidhiKJha enabled auto-merge January 23, 2025 18:05

FrederikBolding reviewed Jan 23, 2025

View reviewed changes

ui/pages/permissions-connect/permissions-connect.component.js Outdated Show resolved Hide resolved

NidhiKJha dismissed vinnyhoward’s stale review via 60d93e4 January 23, 2025 18:43

vinnyhoward previously approved these changes Jan 23, 2025

View reviewed changes

updated permissions connect to use cached origin and target subject m…

66b1fcd

…etadata

NidhiKJha dismissed vinnyhoward’s stale review via 66b1fcd January 24, 2025 08:55

NidhiKJha force-pushed the permissions-connect-header branch from 60d93e4 to 66b1fcd Compare January 24, 2025 08:55

FrederikBolding reviewed Jan 24, 2025

View reviewed changes

ui/pages/permissions-connect/permissions-connect.component.js Outdated Show resolved Hide resolved

remove unnecessary logic

c3272cc

FrederikBolding reviewed Jan 24, 2025

View reviewed changes

ui/pages/permissions-connect/permissions-connect.component.js Show resolved Hide resolved

FrederikBolding reviewed Jan 24, 2025

View reviewed changes

ui/pages/permissions-connect/permissions-connect.component.js Outdated Show resolved Hide resolved

update state in componentDidUpdate

56c81e2

FrederikBolding reviewed Jan 24, 2025

View reviewed changes

ui/pages/permissions-connect/permissions-connect.component.js Show resolved Hide resolved

NidhiKJha added 2 commits January 24, 2025 12:11

lint fix

bb6d241

added comment

4f47a96

FrederikBolding approved these changes Jan 24, 2025

View reviewed changes

NidhiKJha requested review from danjm and vinnyhoward January 24, 2025 12:28

jonybur approved these changes Jan 24, 2025

View reviewed changes

salimtb approved these changes Jan 24, 2025

View reviewed changes

NidhiKJha added this pull request to the merge queue Jan 24, 2025

danjm approved these changes Jan 24, 2025

View reviewed changes

auto-merge was automatically disabled January 24, 2025 13:26
Pull Request is not mergeable

Merged via the queue into main with commit 2263ce4 Jan 24, 2025
69 checks passed

NidhiKJha deleted the permissions-connect-header branch January 24, 2025 13:35

github-actions bot locked and limited conversation to collaborators Jan 24, 2025

metamaskbot added the release-12.12.0 Issue or pull request that will be included in release 12.12.0 label Jan 24, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: updated permissions header to be consistent #29880

fix: updated permissions header to be consistent #29880

NidhiKJha commented Jan 23, 2025 •

edited

Loading

hmalik88 commented Jan 23, 2025 •

edited

Loading

NidhiKJha commented Jan 23, 2025

metamaskbot commented Jan 23, 2025

NidhiKJha commented Jan 23, 2025

metamaskbot commented Jan 23, 2025

metamaskbot commented Jan 24, 2025

metamaskbot commented Jan 24, 2025

metamaskbot commented Jan 24, 2025

darkwing commented Feb 14, 2025 •

edited by NidhiKJha

Loading

fix: updated permissions header to be consistent #29880

fix: updated permissions header to be consistent #29880

Conversation

NidhiKJha commented Jan 23, 2025 • edited Loading

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Dapp Connection

Snap Connection

Pre-merge author checklist

Pre-merge reviewer checklist

hmalik88 commented Jan 23, 2025 • edited Loading

NidhiKJha commented Jan 23, 2025

metamaskbot commented Jan 23, 2025

NidhiKJha commented Jan 23, 2025

metamaskbot commented Jan 23, 2025

metamaskbot commented Jan 24, 2025

metamaskbot commented Jan 24, 2025

metamaskbot commented Jan 24, 2025

darkwing commented Feb 14, 2025 • edited by NidhiKJha Loading

1️⃣ What was the issue?

2️⃣ Why did it happen and why wasn’t it caught earlier?

3️⃣ What actions are we taking to prevent this in the future?

4️⃣ Any related tickets and ETAs for follow-up actions?

NidhiKJha commented Jan 23, 2025 •

edited

Loading

hmalik88 commented Jan 23, 2025 •

edited

Loading

darkwing commented Feb 14, 2025 •

edited by NidhiKJha

Loading