chore: tab component refactor #30143

georgewrmarshall · 2025-02-05T19:42:34Z

Description

This PR adds prop objects to access the children components inside the Tabs and Tab components, along with test coverage. This allows for the Tabs and Tab component to be further customized. There are no visual updates, so these components should remain unchanged with no visual regressions.

Tabs

Spread props to the root Box component
Spread tabListProps to the child tab list ul Box component
Spread tabContentProps to the child tabs content Box component

Tab

Spread props to the root component
Spread textProps to the child Text component

Related issues

Fixes: #30138

Manual testing steps

To check no visual regressions

Pull this branch and run storybook yarn storybook
Navigate to the Tabs component
Check against main branch storybook for no visual regressions

To check prop objects run tests

Pull this branch
Run tests using yarn jest ui/components/ui/tabs
Verify tests run as expected and cover new prop object props

Screenshots/Recordings

Screen recordings show no visual regressions or changes to UI

Before

before.mov

After

after.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 [

github-actions · 2025-02-05T19:42:46Z

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.

ui/components/multichain/pages/send/__snapshots__/send.test.js.snap

georgewrmarshall · 2025-02-05T19:47:48Z

ui/components/ui/tabs/tab/tab.component.js

-  BLOCK_SIZES,
-  DISPLAY,
+  BlockSize,
+  Display,


Replacing deprecated constants for enums

georgewrmarshall · 2025-02-05T19:48:04Z

ui/components/ui/tabs/tab/tab.component.js

-import Box from '../../box';
-import { Text } from '../../../component-library';
+import { Text, Box } from '../../../component-library';


Replacing deprecated Box component with component-library version

ui/components/ui/tabs/tab/tab.component.js

georgewrmarshall · 2025-02-05T19:48:59Z

ui/components/ui/tabs/tab/tab.component.js

+  children: PropTypes.node, // required, but we are not rendering it explicitly
+  textProps: PropTypes.object, // props to spread to the Text component


Fixing spelling: explictly => explicitly

Adding PropType for textProps

georgewrmarshall · 2025-02-05T19:49:16Z

ui/components/ui/tabs/tab/tab.component.test.js

@@ -0,0 +1,100 @@
+import React from 'react';


Adding comprehensive unit test coverage for tab

georgewrmarshall · 2025-02-05T19:49:49Z

ui/components/ui/tabs/tabs.component.js

-import Box from '../box';
+import { Box } from '../../component-library';


Replacing deprecated Box component with component-library version

georgewrmarshall · 2025-02-05T19:49:59Z

ui/components/ui/tabs/tabs.component.js

 import {
  BackgroundColor,
-  DISPLAY,
+  Display,


Replacing deprecated consts with enum

georgewrmarshall · 2025-02-05T19:50:34Z

ui/components/ui/tabs/tabs.test.js

@@ -0,0 +1,117 @@
+import React from 'react';


Adding unit test coverage for tabs component

georgewrmarshall · 2025-02-05T20:20:21Z

ui/components/ui/tabs/tabs.component.js

+        {...tabListProps}
+        className={classnames(
+          'tabs__list',
+          tabsClassName,
+          tabListProps.className,
+        )}


Prevents all classNames being overridden if tabListProps contains a className

metamaskbot · 2025-02-05T20:57:34Z

Builds ready [b248779]

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 (1665 ± 38 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1529	1806	1666	76	36
		domContentLoaded	1513	1772	1640	77	37
		load	1526	1795	1665	79	38
		domInteractive	24	86	41	20	9
		backgroundConnect	9	76	27	18	9
		firstReactRender	15	99	38	27	13
		getState	5	53	15	15	7
		initialActions	0	1	0	0	0
		loadScripts	1070	1297	1197	64	31
		setupStore	8	90	28	27	13
		uiStartup	1752	2512	1939	179	86

Bundle size diffs

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

ui/components/multichain/account-overview/account-overview-tabs.tsx

ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal-tabs.tsx

georgewrmarshall · 2025-02-05T21:03:07Z

ui/components/multichain/pages/send/components/recipient.tsx

+        onTabClick={() => null}
+        tabsClassName=""
+        subHeader={null}


Typescript linting fix

ui/components/ui/tabs/tabs.d.ts

ui/components/ui/tabs/tabs.test.tsx

ui/pages/notifications/notifications.tsx

metamaskbot · 2025-02-05T21:49:25Z

Builds ready [e4b889b]

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 (1539 ± 37 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1427	1799	1543	77	37
		domContentLoaded	1416	1717	1516	70	34
		load	1429	1788	1539	76	37
		domInteractive	24	101	34	20	9
		backgroundConnect	10	78	28	20	9
		firstReactRender	15	74	32	21	10
		getState	5	43	8	8	4
		initialActions	0	1	0	0	0
		loadScripts	980	1244	1084	62	30
		setupStore	7	53	10	10	5
		uiStartup	1647	1990	1744	77	37

Bundle size diffs

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

…ents and adding testing coverage

ui/components/ui/tabs/tabs.component.js

georgewrmarshall · 2025-02-07T17:11:37Z

ui/pages/notifications/notifications.tsx

@@ -202,7 +202,7 @@ export default function Notifications() {
        {hasNotifySnaps && (
          <Tabs
            defaultActiveTabKey={activeTab}
-            onTabClick={(tab) => setActiveTab(tab)}
+            onTabClick={(tab: string) => setActiveTab(tab as TAB_KEYS)}


Fixes type error

…pt errors

georgewrmarshall · 2025-02-07T17:14:29Z

ui/components/ui/tabs/tabs.component.js

-  tabsClassName,
-  subHeader,
+  tabsClassName = '',
+  subHeader = null,
+  tabListProps = {},
+  tabContentProps = {},
+  ...props


Adding prop objects to access children components and setting defaults to prevent typescript errors where possible.

georgewrmarshall · 2025-02-07T17:18:56Z

ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal-tabs.tsx

@@ -32,6 +32,7 @@ export const AssetPickerModalTabs = ({
      <Tabs
        defaultActiveTabKey={defaultActiveTabKey}
        tabsClassName="modal-tab__tabs"
+        onTabClick={() => null}


Tried a few approaches to avoid setting onTabClick={() => null} here, including setting defaults in the JavaScript component, but couldn’t resolve the issue without refactoring the Tabs component to TypeScript. The @MetaMask/design-system team is working on Tabs this quarter, so this is a temporary fix until the design-system-react component is ready.

metamaskbot · 2025-02-07T17:49:38Z

Builds ready [7663a36]

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 (1704 ± 60 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1512	2003	1699	121	58
		domContentLoaded	1474	1968	1671	123	59
		load	1512	2011	1704	124	60
		domInteractive	21	89	46	23	11
		backgroundConnect	10	144	35	30	14
		firstReactRender	16	102	39	30	14
		getState	4	56	10	11	5
		initialActions	0	0	0	0	0
		loadScripts	1069	1465	1213	110	53
		setupStore	7	85	29	27	13
		uiStartup	1712	2478	1983	219	105

Bundle size diffs

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

georgewrmarshall self-assigned this Feb 5, 2025

georgewrmarshall force-pushed the fix/30138/tabs-refactor branch from 4ff32f5 to 8dfe92f Compare February 5, 2025 19:43

metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Feb 5, 2025