fix: remove scroll-to-bottom requirement in redesigned transaction confirmations #27910

vinistevam · 2024-10-16T16:07:27Z

Description

This PR addresses the removal of the scroll-to-bottom requirement in the redesigned transaction confirmation screens. It eliminates the need for users to scroll to the bottom in order to enable the confirm button, streamlining the confirmation process. The scroll-to-bottom arrow is also removed, ensuring a smoother user experience without unnecessary interaction barriers.

Related issues

Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3495

Manual testing steps

Go to test dapp
Have 2+ transaction insights snaps installed
Click Create Token
See the confirm button disabled until you scroll to bottom

Screenshots/Recordings

deploy.webm

Before

After

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 · 2024-10-16T16:07:39Z

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.

sonarcloud · 2024-10-16T16:19:46Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

metamaskbot · 2024-10-17T06:27:13Z

Builds ready [7f55b70]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
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, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1780 ± 96 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	333	2372	1718	377	181
		domContentLoaded	1460	2304	1750	202	97
		load	1472	2317	1780	199	96
		domInteractive	15	176	52	36	17
		backgroundConnect	6	91	31	25	12
		firstReactRender	46	208	95	43	21
		getState	4	67	20	23	11
		initialActions	0	1	0	0	0
		loadScripts	1045	1876	1324	190	91
		setupStore	10	95	32	30	15
		uiStartup	1656	2522	2012	219	105

Bundle size diffs

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

OGPoyraz · 2024-10-17T11:49:31Z

ui/pages/confirmations/components/confirm/scroll-to-bottom/scroll-to-bottom.tsx

+      setIsScrollToBottomCompleted(true);
+      return;
+    }
+
    setIsScrollToBottomCompleted(!isScrollable || hasScrolledToBottom);
  }, [isScrollable, hasScrolledToBottom]);


Should this hook to have isTransactionRedesign dependency?

Great catch, done.

OGPoyraz · 2024-10-17T11:52:12Z

ui/pages/confirmations/components/confirm/scroll-to-bottom/scroll-to-bottom.tsx

@@ -49,6 +51,13 @@ const ScrollToBottom = ({ children }: ContentProps) => {
    offsetPxFromBottom: 0,
  });

+  const isTransactionRedesign = REDESIGN_USER_TRANSACTION_TYPES.includes(


Maybe a dumb question, this should definitely check if current confirmation in REDESIGN_USER_TRANSACTION_TYPES but also if user opted-in for redesigned transactions?

I assumed ScrollToBottom is used only for redesigned confirmations. I will double-check.

That make sense if that's the case

In this case we probably want to use REDESIGN_DEV_TRANSACTION_TYPES instead, which is a superset of REDESIGN_USER_TRANSACTION_TYPES and includes any types not yet visible to the user but already in active development

metamaskbot · 2024-10-20T14:03:38Z

Builds ready [e53536f]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
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, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1912 ± 189 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	623	3514	1844	477	229
		domContentLoaded	1618	2635	1830	222	106
		load	1650	3526	1912	393	189
		domInteractive	16	103	47	24	11
		backgroundConnect	11	899	86	188	90
		firstReactRender	52	290	95	55	26
		getState	4	70	23	20	10
		initialActions	0	1	0	0	0
		loadScripts	1171	1745	1329	135	65
		setupStore	10	93	38	25	12
		uiStartup	1794	3717	2155	405	194

Bundle size diffs

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

vinistevam added the team-confirmations Push issues to confirmations team label Oct 16, 2024

vinistevam changed the title ~~fix: remove scroll to bottom when confirmation is transaction redesigned~~ fix: remove scroll-to-bottom requirement in redesigned transaction confirmations Oct 17, 2024

vinistevam marked this pull request as ready for review October 17, 2024 07:28

vinistevam requested review from a team as code owners October 17, 2024 07:28

OGPoyraz reviewed Oct 17, 2024

View reviewed changes

OGPoyraz previously approved these changes Oct 17, 2024

View reviewed changes

vinistevam dismissed OGPoyraz’s stale review via 34fe44c October 20, 2024 13:12

vinistevam requested review from a team and kumavis as code owners October 20, 2024 13:12

vinistevam force-pushed the fix/remove-scroll-to-bottom-transaction branch from 34fe44c to 42027be Compare October 20, 2024 13:13

vinistevam added 3 commits October 20, 2024 14:13

fix: remove scroll to bottom when confirmation is transaction redesigned

815c322

fix: e2e for transaction redesign

016fa9c

fix: add missing dependencies

b344e29

vinistevam force-pushed the fix/remove-scroll-to-bottom-transaction branch from 42027be to b344e29 Compare October 20, 2024 13:14

apply suggestions

e53536f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: remove scroll-to-bottom requirement in redesigned transaction confirmations #27910

fix: remove scroll-to-bottom requirement in redesigned transaction confirmations #27910

vinistevam commented Oct 16, 2024 •

edited

Loading

github-actions bot commented Oct 16, 2024

sonarcloud bot commented Oct 16, 2024

metamaskbot commented Oct 17, 2024

OGPoyraz Oct 17, 2024

vinistevam Oct 17, 2024

OGPoyraz Oct 17, 2024 •

edited

Loading

vinistevam Oct 17, 2024

OGPoyraz Oct 17, 2024

pedronfigueiredo Oct 17, 2024

metamaskbot commented Oct 20, 2024

fix: remove scroll-to-bottom requirement in redesigned transaction confirmations #27910

Are you sure you want to change the base?

fix: remove scroll-to-bottom requirement in redesigned transaction confirmations #27910

Conversation

vinistevam commented Oct 16, 2024 • edited Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Oct 16, 2024

sonarcloud bot commented Oct 16, 2024

Quality Gate passed

metamaskbot commented Oct 17, 2024

OGPoyraz Oct 17, 2024

Choose a reason for hiding this comment

vinistevam Oct 17, 2024

Choose a reason for hiding this comment

OGPoyraz Oct 17, 2024 • edited Loading

Choose a reason for hiding this comment

vinistevam Oct 17, 2024

Choose a reason for hiding this comment

OGPoyraz Oct 17, 2024

Choose a reason for hiding this comment

pedronfigueiredo Oct 17, 2024

Choose a reason for hiding this comment

metamaskbot commented Oct 20, 2024

vinistevam commented Oct 16, 2024 •

edited

Loading

OGPoyraz Oct 17, 2024 •

edited

Loading