feat: Fix plan 835 Hid request dialog keep on prompting up during pagination. #30384

dawnseeker8 · 2025-02-18T10:40:31Z

This PR is relative to solve the plan 835

This PR including following change:

in actions.js file, change connectHardware method signature to has extra loadHid boolean parameter to see whether we need to load HID request prompt up.
change connect-hardware/index.js to and pass loadHid = true parameter value to above actions.
change connect-hardware/account-list.js and all pagination feature relative functions to pass loadHid = false to disable HID request popup apear.

Description

Related issues

Fixes: https://github.com/MetaMask/accounts-planning/issues/835

Manual testing steps

This test requires a Ledger hardware device with more than 5 accounts:

Add hardware wallet
Connect to Ledger device
Verify 5 Ledger accounts are displayed
Click Next
Verify the HID popup is not displayed each time the user clicks Next button

Screenshots/Recordings

Before

The following popup is displayed after each "Next" action

After

The popup is not displayed repeatedly

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.

github-actions · 2025-02-18T10:40:44Z

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 · 2025-02-18T11:17:00Z

Builds ready [2dcab12]

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
- content-script: 0
- offscreen: 0

Page Load Metrics (1832 ± 91 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1478	2222	1837	193	93
		domContentLoaded	1472	2198	1797	188	90
		load	1477	2216	1832	189	91
		domInteractive	28	143	55	33	16
		backgroundConnect	9	200	39	43	20
		firstReactRender	15	78	37	25	12
		getState	4	68	20	20	10
		initialActions	0	1	0	0	0
		loadScripts	1066	1628	1319	159	76
		setupStore	8	58	24	20	10
		uiStartup	1744	2660	2089	222	106

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: 17 Bytes (0.00%)
common: 5 Bytes (0.00%)

metamaskbot · 2025-02-19T01:41:31Z

Builds ready [cf89481]

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
- content-script: 0
- offscreen: 0

Page Load Metrics (1654 ± 42 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	457	1858	1591	277	133
		domContentLoaded	1468	1847	1630	90	43
		load	1520	1857	1654	88	42
		domInteractive	25	78	38	18	9
		backgroundConnect	9	71	28	21	10
		firstReactRender	14	75	40	25	12
		getState	5	56	11	14	7
		initialActions	0	0	0	0	0
		loadScripts	1005	1337	1178	86	41
		setupStore	6	60	14	15	7
		uiStartup	1718	2087	1886	99	48

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: 17 Bytes (0.00%)
common: 5 Bytes (0.00%)

metamaskbot · 2025-03-06T13:16:45Z

Builds ready [d5a8a67]

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, 8
- 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
- content-script: 0
- offscreen: 0

Page Load Metrics (1954 ± 101 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	285	2584	1785	538	258
		domContentLoaded	1627	2501	1920	217	104
		load	1639	2507	1954	211	101
		domInteractive	24	139	50	31	15
		backgroundConnect	12	84	43	25	12
		firstReactRender	17	86	52	24	12
		getState	6	53	14	11	5
		initialActions	0	0	0	0	0
		loadScripts	1217	1893	1436	177	85
		setupStore	8	70	17	16	8
		uiStartup	1881	2829	2227	236	113

gantunesr · 2025-03-06T13:29:44Z

ui/pages/create-account/connect-hardware/index.js

@@ -185,9 +185,9 @@ class ConnectHardwareForm extends Component {
    }, SECOND * 5);
  }

-  getPage = (device, page, hdPath) => {
+  getPage = (device, page, hdPath, loadHid) => {


lets improve the unit tests to cover the cases of loadHid to be true or false and avoid regressions

gantunesr · 2025-03-06T13:31:28Z

ui/store/actions.ts

@@ -599,6 +599,7 @@ export function connectHardware(
  deviceName: HardwareDeviceNames,
  page: string,
  hdPath: string,
+  loadHid: boolean,


Same thing here regarding connectHardware, we should update the unit test to reflect this cases too

… all possible cases.

metamaskbot · 2025-03-07T14:34:44Z

Builds ready [1359e03]

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, 8
- 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
- content-script: 0
- offscreen: 0

Page Load Metrics (2033 ± 127 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	247	2764	1761	629	302
		domContentLoaded	1603	2715	1997	268	129
		load	1667	2741	2033	264	127
		domInteractive	22	127	54	30	14
		backgroundConnect	9	84	43	22	10
		firstReactRender	15	77	40	21	10
		getState	8	62	22	17	8
		initialActions	0	1	0	0	0
		loadScripts	1196	2122	1516	232	111
		setupStore	9	56	20	15	7
		uiStartup	1836	3146	2312	310	149

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: 17 Bytes (0.00%)
common: 5 Bytes (0.00%)

feat: First draft fix for HID request keep promping up.

506915a

metamaskbot added the team-hardware-wallets label Feb 18, 2025

dawnseeker8 added the hardware-ledger label Feb 18, 2025

Merge branch 'main' into feat/plan-836-ledger-hid-prompt-up-issue

2dcab12

dawnseeker8 changed the title ~~feat: First draft fix for HID request keep promping up.~~ feat: Fix plan 835 Hid request dialog keep on prompting up during pagination. Feb 18, 2025

dawnseeker8 marked this pull request as ready for review February 19, 2025 01:06

Merge branch 'main' into feat/plan-836-ledger-hid-prompt-up-issue

cf89481

dawnseeker8 added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Feb 21, 2025

Merge branch 'main' into feat/plan-836-ledger-hid-prompt-up-issue

d5a8a67

gantunesr reviewed Mar 6, 2025

View reviewed changes

fix: add unit tests to test connectHardware, mock HID connection with…

1359e03

… all possible cases.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Fix plan 835 Hid request dialog keep on prompting up during pagination. #30384

feat: Fix plan 835 Hid request dialog keep on prompting up during pagination. #30384

dawnseeker8 commented Feb 18, 2025 •

edited by Montoya

Loading

github-actions bot commented Feb 18, 2025

metamaskbot commented Feb 18, 2025

metamaskbot commented Feb 19, 2025

metamaskbot commented Mar 6, 2025

gantunesr Mar 6, 2025

gantunesr Mar 6, 2025

metamaskbot commented Mar 7, 2025

feat: Fix plan 835 Hid request dialog keep on prompting up during pagination. #30384

Are you sure you want to change the base?

feat: Fix plan 835 Hid request dialog keep on prompting up during pagination. #30384

Conversation

dawnseeker8 commented Feb 18, 2025 • edited by Montoya Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Feb 18, 2025

metamaskbot commented Feb 18, 2025

metamaskbot commented Feb 19, 2025

metamaskbot commented Mar 6, 2025

gantunesr Mar 6, 2025

Choose a reason for hiding this comment

gantunesr Mar 6, 2025

Choose a reason for hiding this comment

metamaskbot commented Mar 7, 2025

dawnseeker8 commented Feb 18, 2025 •

edited by Montoya

Loading