feat: design changes in signature paged message section #28038

jpuri · 2024-10-23T12:02:28Z

Description

Make message section in signature collapsible and add copy option.

Related issues

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

Manual testing steps

Go to test dapp
Submit signatures and check message section

Screenshots/Recordings

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-23T12:02: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-10-24T11:17:40Z

Builds ready [c2aef7b]

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 (2103 ± 86 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1864	2811	2123	214	103
		domContentLoaded	1853	2477	2072	163	78
		load	1862	2602	2103	179	86
		domInteractive	30	110	55	22	11
		backgroundConnect	9	136	33	31	15
		firstReactRender	54	209	116	35	17
		getState	5	178	31	41	20
		initialActions	0	1	0	0	0
		loadScripts	1388	1902	1568	146	70
		setupStore	12	74	35	24	12
		uiStartup	2128	3235	2366	241	116

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

background: 0 Bytes (0.00%)
ui: 695 Bytes (0.01%)
common: 22 Bytes (0.00%)

matthewwalsh0 · 2024-10-24T12:40:00Z

ui/components/app/confirm/info/row/row.tsx

@@ -40,6 +40,7 @@ export type ConfirmInfoRowProps = {
  copyEnabled?: boolean;
  copyText?: string;
  'data-testid'?: string;
+  collapsible?: boolean;


Can we not re-use the existing ui/components/app/confirm/info/row/expandable-row.tsx?

The designs are very different from expandable row.

matthewwalsh0 · 2024-10-24T12:41:45Z

ui/components/app/confirm/info/row/row.tsx

+          alignItems={AlignItems.flexStart}
+          color={color}
+        >
+          <Box display={Display.Flex} alignItems={AlignItems.center}>


Assuming this behaviour is different to the expandable row, while it's nice to add configuration to the core row, this adds a lot of logic and complexity, so would it be better to create an alternate component that potentially inherits the same properties of row? Or a wrapper component?

Or at the very least, should we break this very large schema up with local components?

Changes are required in info row requires to support collapsible row, it is hard to only wrap into row and make it collapsible.

In that case, could we split this for readability into some local components (in this file) such as Container, CopyIcon, Tooltip, Content, CollapsibleIcon etc?

Yep, let me create a followup PR to do that, and avoid in this PR to change the scope here.

metamaskbot · 2024-10-24T15:10:22Z

Builds ready [ac11fc3]

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 (2038 ± 77 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1777	2408	2028	161	77
		domContentLoaded	1764	2368	1982	163	78
		load	1774	2416	2038	159	77
		domInteractive	19	110	53	22	10
		backgroundConnect	11	98	54	28	13
		firstReactRender	61	286	110	53	26
		getState	7	64	29	23	11
		initialActions	0	1	0	0	0
		loadScripts	1260	1701	1455	127	61
		setupStore	13	106	38	27	13
		uiStartup	1974	2824	2304	205	99

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

background: 0 Bytes (0.00%)
ui: 1.16 KiB (0.02%)
common: 22 Bytes (0.00%)

georgewrmarshall

LGTM! 🚀

Checked collapse icon svg and that it renders in storybook correctly ✅
Checked collapse icon exists in Figma icon library for 1:1 match ✅
Did not check any other part of the code ❌

georgewrmarshall · 2024-10-24T15:41:43Z

app/images/icons/collapse.svg

@@ -0,0 +1 @@
+<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 512 512"><path d="m422 64c5 0 9 2 13 5 7 8 7 19 0 26l-107 107 66 0c11 0 19 7 19 18 0 12-8 19-19 19l-110 0c-2 0-6 0-8-2-2 0-3-2-5-4-2-1-4-3-4-5-2-2-2-6-2-8l0-110c0-11 8-19 19-19 11 0 18 8 18 19l0 66 107-107c4-3 7-5 13-5z m-194 193l-110 0c-11 0-19 8-19 19 0 11 8 18 19 18l66 0-107 107c-7 7-7 19 0 26 8 7 19 7 26 0l107-107 0 66c0 11 7 19 18 19 11 0 19-8 19-19l0-110c0-2 0-6-2-8 0-1-2-3-4-5-2-2-3-4-5-4-2-2-6-2-8-2z"/></svg>


Just pushed a commit to optimize the svg here. 44ec97d

Before

After

Checked in storybook

georgewrmarshall · 2024-10-24T15:47:21Z

ui/components/app/confirm/info/row/copy-icon.tsx

+        right: 0,
+        top: 2,
+        ...style,
+      }}
      onClick={handleClick}


non-blocking: Is there any reason we aren't using the ButtonIcon component here? The current implementation has poor accessibility as it renders a span instead of a button, and it lacks an aria-label, making it inaccessible for screen reader users.

cc @matthewwalsh0

I agree button icon is good idea. Let me create separate PR for this as it would need lot of snapshot updates in confirmation pages.

metamaskbot · 2024-10-24T16:19:34Z

Builds ready [44ec97d]

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 (1961 ± 101 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	354	2442	1892	412	198
		domContentLoaded	1553	2431	1930	197	94
		load	1563	2443	1961	209	101
		domInteractive	23	166	61	34	16
		backgroundConnect	9	158	32	33	16
		firstReactRender	47	304	98	53	25
		getState	4	86	29	29	14
		initialActions	0	1	0	0	0
		loadScripts	1130	1954	1453	178	86
		setupStore	10	83	29	21	10
		uiStartup	1710	2751	2212	278	133

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

background: 0 Bytes (0.00%)
ui: 1.16 KiB (0.02%)
common: 22 Bytes (0.00%)

jpuri added 2 commits October 23, 2024 17:28

Design changes in signature paged message section

8a24413

merge

c68d6fb

jpuri added the team-confirmations Push issues to confirmations team label Oct 23, 2024

jpuri requested review from a team as code owners October 23, 2024 12:02

jpuri added 7 commits October 23, 2024 17:41

update

1181b8d

update

a699c9e

update

73f7b71

update

7a59656

update

94000e3

update

f2ed1c6

update

4af935c

jpuri requested a review from a team as a code owner October 23, 2024 15:13

jpuri added 5 commits October 23, 2024 21:12

update

b834e33

update

a9f7d46

update

440a8d3

update

c95f82d

update

c2aef7b

matthewwalsh0 requested changes Oct 24, 2024

View reviewed changes

jpuri added 3 commits October 24, 2024 18:49

update

b6b4d59

update

8d31dad

update

f389d57

matthewwalsh0 previously approved these changes Oct 24, 2024

View reviewed changes

update

cd2c82f

jpuri dismissed matthewwalsh0’s stale review via cd2c82f October 24, 2024 14:05

update

ac11fc3

jpuri requested a review from matthewwalsh0 October 24, 2024 14:54

matthewwalsh0 previously approved these changes Oct 24, 2024

View reviewed changes

chore: optimizing svg for collapse icon svg

44ec97d

georgewrmarshall dismissed matthewwalsh0’s stale review via 44ec97d October 24, 2024 15:40

georgewrmarshall requested a review from matthewwalsh0 October 24, 2024 15:41

georgewrmarshall approved these changes Oct 24, 2024

View reviewed changes

georgewrmarshall reviewed Oct 24, 2024

View reviewed changes

matthewwalsh0 approved these changes Oct 24, 2024

View reviewed changes

jpuri enabled auto-merge October 24, 2024 15:58

jpuri added this pull request to the merge queue Oct 24, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 24, 2024

jpuri added this pull request to the merge queue Oct 24, 2024

jpuri mentioned this pull request Oct 24, 2024

chore: Using button icon component for clikable icons #28082

Merged

7 tasks

Merged via the queue into develop with commit 1e55692 Oct 24, 2024
76 checks passed

jpuri deleted the sig_message_changes branch October 24, 2024 17:20

github-actions bot locked and limited conversation to collaborators Oct 24, 2024

metamaskbot added the release-12.7.0 Issue or pull request that will be included in release 12.7.0 label Oct 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: design changes in signature paged message section #28038

feat: design changes in signature paged message section #28038

jpuri commented Oct 23, 2024

github-actions bot commented Oct 23, 2024

metamaskbot commented Oct 24, 2024

matthewwalsh0 Oct 24, 2024

jpuri Oct 24, 2024

matthewwalsh0 Oct 24, 2024 •

edited

Loading

jpuri Oct 24, 2024

matthewwalsh0 Oct 24, 2024

jpuri Oct 24, 2024

metamaskbot commented Oct 24, 2024

georgewrmarshall left a comment •

edited

Loading

georgewrmarshall Oct 24, 2024 •

edited

Loading

georgewrmarshall Oct 24, 2024 •

edited

Loading

jpuri Oct 24, 2024

metamaskbot commented Oct 24, 2024

		@@ -0,0 +1 @@
		<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 512 512"><path d="m422 64c5 0 9 2 13 5 7 8 7 19 0 26l-107 107 66 0c11 0 19 7 19 18 0 12-8 19-19 19l-110 0c-2 0-6 0-8-2-2 0-3-2-5-4-2-1-4-3-4-5-2-2-2-6-2-8l0-110c0-11 8-19 19-19 11 0 18 8 18 19l0 66 107-107c4-3 7-5 13-5z m-194 193l-110 0c-11 0-19 8-19 19 0 11 8 18 19 18l66 0-107 107c-7 7-7 19 0 26 8 7 19 7 26 0l107-107 0 66c0 11 7 19 18 19 11 0 19-8 19-19l0-110c0-2 0-6-2-8 0-1-2-3-4-5-2-2-3-4-5-4-2-2-6-2-8-2z"/></svg>

feat: design changes in signature paged message section #28038

feat: design changes in signature paged message section #28038

Conversation

jpuri commented Oct 23, 2024

Description

Related issues

Manual testing steps

Screenshots/Recordings

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Oct 23, 2024

metamaskbot commented Oct 24, 2024

matthewwalsh0 Oct 24, 2024

Choose a reason for hiding this comment

jpuri Oct 24, 2024

Choose a reason for hiding this comment

matthewwalsh0 Oct 24, 2024 • edited Loading

Choose a reason for hiding this comment

jpuri Oct 24, 2024

Choose a reason for hiding this comment

matthewwalsh0 Oct 24, 2024

Choose a reason for hiding this comment

jpuri Oct 24, 2024

Choose a reason for hiding this comment

metamaskbot commented Oct 24, 2024

georgewrmarshall left a comment • edited Loading

Choose a reason for hiding this comment

georgewrmarshall Oct 24, 2024 • edited Loading

Choose a reason for hiding this comment

Before

After

georgewrmarshall Oct 24, 2024 • edited Loading

Choose a reason for hiding this comment

jpuri Oct 24, 2024

Choose a reason for hiding this comment

metamaskbot commented Oct 24, 2024

matthewwalsh0 Oct 24, 2024 •

edited

Loading

georgewrmarshall left a comment •

edited

Loading

georgewrmarshall Oct 24, 2024 •

edited

Loading

georgewrmarshall Oct 24, 2024 •

edited

Loading