fix: Signature Redesign Disabled confirm button with missing ScrollToBottom button with 16px or less scroll buffer #24489

digiwand · 2024-05-10T21:28:29Z

Description

Removes 16px buffer
Allow optional scroll buffer in useScrollRequired hook

Related issues

Fixes: #24485

Manual testing steps

Create sign request from OpenSea login
Observe scroll behavior

Screenshots/Recordings

Before

Screen.Recording.2024-05-10.at.12.30.06.mov

After

CleanShot.2024-05-14.at.06.21.08.mp4

Pre-merge author checklist

I’ve followed MetaMask 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-05-10T21:28:42Z

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 · 2024-05-10T21:59:42Z

Builds ready [2a284d0]

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, 2, 3, 4, 5, 6, 7, 8
- content-script: 0
- ui: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1218 ± 612 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	71	142	96	21	10
		domContentLoaded	9	66	19	15	7
		load	58	2998	1218	1274	612
		domInteractive	9	66	19	15	7

Bundle size diffs

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

digiwand · 2024-05-10T23:07:41Z

ui/hooks/useScrollRequired.js

-export const useScrollRequired = (dependencies = []) => {
+export const useScrollRequired = (
+  dependencies = [],
+  { offsetPxFromBottom = 16 } = {},


Leaning towards defaulting to 0. We can handle this in a separate PR

…h-16px-or-less-scroll-buffer

metamaskbot · 2024-05-14T05:37:54Z

Builds ready [879f8ef]

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, 2, 3, 4, 5, 6, 7, 8
- content-script: 0
- ui: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (787 ± 602 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	71	118	87	15	7
		domContentLoaded	9	31	13	5	3
		load	57	3234	787	1253	602
		domInteractive	9	31	13	5	3

Bundle size diffs

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

metamaskbot · 2024-05-14T05:38:17Z

Builds ready [879f8ef]

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, 2, 3, 4, 5, 6, 7, 8
- content-script: 0
- ui: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (787 ± 602 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	71	118	87	15	7
		domContentLoaded	9	31	13	5	3
		load	57	3234	787	1253	602
		domInteractive	9	31	13	5	3

Bundle size diffs

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

metamaskbot · 2024-05-14T13:44:52Z

Builds ready [eec6c25]

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, 2, 3, 4, 5, 6, 7, 8
- content-script: 0
- ui: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (778 ± 594 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	64	149	90	25	12
		domContentLoaded	9	54	18	13	6
		load	52	3189	778	1238	594
		domInteractive	9	54	18	13	6

Bundle size diffs

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

codecov · 2024-05-14T13:45:04Z

Codecov Report

Attention: Patch coverage is 83.33333% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 67.48%. Comparing base (cfcbd1e) to head (eec6c25).
Report is 3 commits behind head on develop.

Files	Patch %	Lines
ui/hooks/useScrollRequired.js	80.00%	1 Missing ⚠️
...ents/confirm/scroll-to-bottom/scroll-to-bottom.tsx	85.71%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           develop   #24489   +/-   ##
========================================
  Coverage    67.48%   67.48%           
========================================
  Files         1288     1288           
  Lines        50153    50158    +5     
  Branches     13023    13025    +2     
========================================
+ Hits         33842    33846    +4     
- Misses       16311    16312    +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

digiwand added 2 commits May 10, 2024 16:35

fix: confirm redesign ScrollToBottom reset logic

fe1f282

fix: disabled Confirm with no shown scrollToBottom

2a284d0

digiwand commented May 10, 2024

View reviewed changes

digiwand changed the title ~~Fix confirm redesign disabled confirm with 16px or less scroll buffer~~ fix: Signature Redesign Disabled confirm button with missing ScrollToBottom button with 16px or less scroll buffer May 11, 2024

digiwand added the team-confirmations Push issues to confirmations team label May 13, 2024

digiwand added 2 commits May 13, 2024 21:56

fix: revert ScrollToBottom setHasScrolledToBottom to fix logic

5ce3228

test: add ScrollToBottom setHasScrolledToBottom test

879f8ef

digiwand marked this pull request as ready for review May 14, 2024 05:15

digiwand requested review from a team as code owners May 14, 2024 05:15

Merge branch 'develop' into fix-confirm-redesign-disabled-confirm-wit…

eec6c25

…h-16px-or-less-scroll-buffer

OGPoyraz approved these changes May 14, 2024

View reviewed changes

vinistevam approved these changes May 14, 2024

View reviewed changes

digiwand merged commit 0e775fd into develop May 14, 2024
75 checks passed

digiwand deleted the fix-confirm-redesign-disabled-confirm-with-16px-or-less-scroll-buffer branch May 14, 2024 13:50

github-actions bot locked and limited conversation to collaborators May 14, 2024

metamaskbot added the release-11.18.0 label May 14, 2024

digiwand added release-11.17.0 and removed release-11.18.0 labels May 15, 2024

gauthierpetetin added the release-12.0.0 label Jun 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Signature Redesign Disabled confirm button with missing ScrollToBottom button with 16px or less scroll buffer #24489

fix: Signature Redesign Disabled confirm button with missing ScrollToBottom button with 16px or less scroll buffer #24489

digiwand commented May 10, 2024 •

edited

github-actions bot commented May 10, 2024

metamaskbot commented May 10, 2024

digiwand May 10, 2024

metamaskbot commented May 14, 2024

metamaskbot commented May 14, 2024

metamaskbot commented May 14, 2024

codecov bot commented May 14, 2024

fix: Signature Redesign Disabled confirm button with missing ScrollToBottom button with 16px or less scroll buffer #24489

fix: Signature Redesign Disabled confirm button with missing ScrollToBottom button with 16px or less scroll buffer #24489

Conversation

digiwand commented May 10, 2024 • edited

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented May 10, 2024

metamaskbot commented May 10, 2024

digiwand May 10, 2024

Choose a reason for hiding this comment

metamaskbot commented May 14, 2024

metamaskbot commented May 14, 2024

metamaskbot commented May 14, 2024

codecov bot commented May 14, 2024

Codecov Report

digiwand commented May 10, 2024 •

edited