From 8646a76227d24ed8742ef5b076c172c4ba944c39 Mon Sep 17 00:00:00 2001 From: George Marshall Date: Fri, 13 Oct 2023 01:46:57 -0700 Subject: [PATCH] Picker network updates (#21301) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** In the [new designs for Amon Hen](https://github.com/MetaMask/metamask-extension/pull/21220/files#diff-afc7617dc1347d665d4f7a058e9ea8ac686f5ad520874a293f6e27e7c50f9568) the network picker(`PickerNetwork`) has been moved to inside of the tabs and needs to be full width. The current `PickerNetwork` requires some style updates to allow for this change This PR updates the `PickerNetwork` component styles and props to allow for this updated design. It uses the pattern of inversion of control to allow for the label wrapper component to be accessed through a prop called `labelProps`. It also updates the unit tests to include coverage for this change as well as updates the documentation and stories to provide examples. ## **Manual testing steps** 1. Go to the storybook build of this PR 2. Search `PickerNetwork` 3. See that there are no visual regressions 4. Check the updated documentation and Width story ## **Screenshots/Recordings** ### **Before** https://github.com/MetaMask/metamask-extension/assets/8112138/7ca4f2ea-63fb-4e12-a3ab-34e8890bbbfc ### **After** https://github.com/MetaMask/metamask-extension/assets/8112138/22036bca-7ebd-418d-8fae-c15f055bab0c Other instances of `PickerNetwork` without visual regressions Screenshot 2023-10-10 at 10 29 55 PMScreenshot 2023-10-10 at 7 54 53 PM Showing new-network-info modal with some deprecated components replaced and flex box added Screenshot 2023-10-11 at 3 12 43 PM Screenshot 2023-10-10 at 9 53 33 PM Screenshot 2023-10-10 at 10 29 24 PM Test coverage 100% Screenshot 2023-10-10 at 6 41 48 PM ## **Related issues** https://github.com/MetaMask/metamask-extension/pull/21220/files#diff-afc7617dc1347d665d4f7a058e9ea8ac686f5ad520874a293f6e27e7c50f9568 ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained: - [x] What problem this PR is solving. - [x] How this problem was solved. - [x] How reviewers can test my changes. - [ ] I’ve indicated what issue this PR is linked to: Fixes #??? - [x] I’ve included tests if applicable. - [x] I’ve documented any added code. - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). - [ ] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [ ] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **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. --- test/e2e/tests/add-custom-network.spec.js | 2 +- test/e2e/tests/custom-rpc-history.spec.js | 2 +- test/e2e/tests/switch-custom-network.spec.js | 2 +- .../picker-network/README.mdx | 31 ++++++++++++++--- .../picker-network.test.tsx.snap | 6 ++-- .../picker-network/picker-network.scss | 1 - .../picker-network/picker-network.stories.tsx | 33 +++++++++++++++---- .../picker-network/picker-network.test.tsx | 13 ++++++++ .../picker-network/picker-network.tsx | 4 ++- .../picker-network/picker-network.types.ts | 5 +++ .../__snapshots__/app-header.test.js.snap | 6 ++-- .../ui/new-network-info/new-network-info.js | 10 ++++-- .../new-network-info.stories.js | 10 ++++++ 13 files changed, 99 insertions(+), 26 deletions(-) create mode 100644 ui/components/ui/new-network-info/new-network-info.stories.js diff --git a/test/e2e/tests/add-custom-network.spec.js b/test/e2e/tests/add-custom-network.spec.js index 82233d09b34b..c4c3c3c1a31c 100644 --- a/test/e2e/tests/add-custom-network.spec.js +++ b/test/e2e/tests/add-custom-network.spec.js @@ -458,7 +458,7 @@ describe('Custom network', function () { }); // verify network switched const networkDisplayed = await driver.findElement({ - tag: 'p', + tag: 'span', text: 'Arbitrum One', }); assert.equal( diff --git a/test/e2e/tests/custom-rpc-history.spec.js b/test/e2e/tests/custom-rpc-history.spec.js index 46bde39df39e..93709a98817b 100644 --- a/test/e2e/tests/custom-rpc-history.spec.js +++ b/test/e2e/tests/custom-rpc-history.spec.js @@ -70,7 +70,7 @@ describe('Stores custom RPC history', function () { '.networks-tab__add-network-form-footer .btn-primary', ); - await driver.findElement({ text: networkName, tag: 'p' }); + await driver.findElement({ text: networkName, tag: 'span' }); }, ); }); diff --git a/test/e2e/tests/switch-custom-network.spec.js b/test/e2e/tests/switch-custom-network.spec.js index 67c3ad1fc5d8..ca40232d13a4 100644 --- a/test/e2e/tests/switch-custom-network.spec.js +++ b/test/e2e/tests/switch-custom-network.spec.js @@ -88,7 +88,7 @@ describe('Switch ethereum chain', function () { await driver.switchToWindow(extension); const currentNetworkName = await driver.findElement({ - tag: 'p', + tag: 'span', text: 'Localhost 8546', }); diff --git a/ui/components/component-library/picker-network/README.mdx b/ui/components/component-library/picker-network/README.mdx index 2e31576f8f8b..74481d251a95 100644 --- a/ui/components/component-library/picker-network/README.mdx +++ b/ui/components/component-library/picker-network/README.mdx @@ -16,7 +16,7 @@ The `PickerNetwork` is used for the action of changing a network. ### Label -Use the `label` prop for the text content of the `PickerNetwork` component. For long labels set a `max-width` using a `className` and the text will truncate showing an ellipsis. +Use the `label` prop for the text content of the `PickerNetwork` component. For long labels set a `max-width` using a `className` and the text will truncate showing an ellipsis. If the `src` prop is not set, the `label` prop will be used to generate fallback initial for `AvatarNetwork`. @@ -24,6 +24,7 @@ Use the `label` prop for the text content of the `PickerNetwork` component. For ```jsx import { PickerNetwork } from '../../ui/component-library'; + @@ -32,7 +33,7 @@ import { PickerNetwork } from '../../ui/component-library'; ### Src -Use the `src` prop with an image url to render the `AvatarNetwork`. Use the `avatarNetworkProps` to pass additional props to the `AvatarNetwork` component. +Use the `src` prop with an image url to render the `AvatarNetwork`. Use the `avatarNetworkProps` to pass additional props to the `AvatarNetwork` component. If the `src` prop is not set, the `label` prop will be used to generate fallback initial for `AvatarNetwork`. @@ -40,7 +41,27 @@ Use the `src` prop with an image url to render the `AvatarNetwork`. Use the `ava ```jsx import { PickerNetwork } from '../../ui/component-library'; - - - + + + +``` + +### Width + +The width of the `PickerNetwork` is set to auto by default. Use the style utility `width` prop with the `BlockSize` enum to set the width of the `PickerNetwork` component. + + + + + +```jsx +import { PickerNetwork } from '../../ui/component-library'; +import { BlockSize } from '../../../helpers/constants/design-system'; + +; +; ``` diff --git a/ui/components/component-library/picker-network/__snapshots__/picker-network.test.tsx.snap b/ui/components/component-library/picker-network/__snapshots__/picker-network.test.tsx.snap index 2b2226998a18..0827e30f349a 100644 --- a/ui/components/component-library/picker-network/__snapshots__/picker-network.test.tsx.snap +++ b/ui/components/component-library/picker-network/__snapshots__/picker-network.test.tsx.snap @@ -11,13 +11,13 @@ exports[`PickerNetwork should render the label inside the PickerNetwork 1`] = ` > I -

Imported -

+ diff --git a/ui/components/component-library/picker-network/picker-network.scss b/ui/components/component-library/picker-network/picker-network.scss index 1e2e58cf083e..72fba5d389d6 100644 --- a/ui/components/component-library/picker-network/picker-network.scss +++ b/ui/components/component-library/picker-network/picker-network.scss @@ -1,7 +1,6 @@ .mm-picker-network { --picker-network-height: 32px; - max-width: fit-content; height: var(--picker-network-height); &:active { diff --git a/ui/components/component-library/picker-network/picker-network.stories.tsx b/ui/components/component-library/picker-network/picker-network.stories.tsx index be2629223525..91536945bc1c 100644 --- a/ui/components/component-library/picker-network/picker-network.stories.tsx +++ b/ui/components/component-library/picker-network/picker-network.stories.tsx @@ -3,6 +3,7 @@ import { StoryFn, Meta } from '@storybook/react'; import { Display, FlexDirection, + BlockSize, } from '../../../helpers/constants/design-system'; import { Box } from '..'; @@ -31,15 +32,24 @@ export default { }, } as Meta; -export const DefaultStory = (args) => ; +const Template = (args) => ; + +export const DefaultStory = Template.bind({}); +DefaultStory.storyName = 'Default'; export const Label: StoryFn = (args) => ( - - - - + + + + + @@ -47,7 +57,11 @@ export const Label: StoryFn = (args) => ( ); export const Src: StoryFn = (args) => ( - + = (args) => ( ); -DefaultStory.storyName = 'Default'; +export const Width: StoryFn = (args) => ( + <> + + + +); diff --git a/ui/components/component-library/picker-network/picker-network.test.tsx b/ui/components/component-library/picker-network/picker-network.test.tsx index f0a5da9d4d2c..a942276c4ab0 100644 --- a/ui/components/component-library/picker-network/picker-network.test.tsx +++ b/ui/components/component-library/picker-network/picker-network.test.tsx @@ -65,4 +65,17 @@ describe('PickerNetwork', () => { ); expect(getByTestId('picker-network')).toHaveClass('test-class'); }); + it('should render with labelProps', () => { + const { getByTestId } = render( + , + ); + expect(getByTestId('picker-network-label')).toHaveClass('test-class'); + }); }); diff --git a/ui/components/component-library/picker-network/picker-network.tsx b/ui/components/component-library/picker-network/picker-network.tsx index 282269b00b8d..fd2a2a78b8bc 100644 --- a/ui/components/component-library/picker-network/picker-network.tsx +++ b/ui/components/component-library/picker-network/picker-network.tsx @@ -30,6 +30,7 @@ export const PickerNetwork: PickerNetworkComponent = React.forwardRef( avatarNetworkProps, iconProps, label, + labelProps, src, ...props }: PickerNetworkProps, @@ -56,7 +57,7 @@ export const PickerNetwork: PickerNetworkComponent = React.forwardRef( size={AvatarNetworkSize.Xs} {...avatarNetworkProps} /> - + {label} diff --git a/ui/components/component-library/picker-network/picker-network.types.ts b/ui/components/component-library/picker-network/picker-network.types.ts index 48d97be43c9d..4946f38ad4f4 100644 --- a/ui/components/component-library/picker-network/picker-network.types.ts +++ b/ui/components/component-library/picker-network/picker-network.types.ts @@ -4,6 +4,7 @@ import type { } from '../box'; import { IconProps } from '../icon/icon.types'; import { AvatarNetworkProps } from '../avatar-network/avatar-network.types'; +import { TextProps } from '../text'; export interface PickerNetworkStyleUtilityProps extends StyleUtilityProps { /** @@ -26,6 +27,10 @@ export interface PickerNetworkStyleUtilityProps extends StyleUtilityProps { * The text content of the PickerNetwork component */ label: string; + /** + * Additional props to pass to the label wrapper Text component + */ + labelProps?: TextProps<'span'>; } export type PickerNetworkProps = diff --git a/ui/components/multichain/app-header/__snapshots__/app-header.test.js.snap b/ui/components/multichain/app-header/__snapshots__/app-header.test.js.snap index 67d265f76e1e..f27c1db843a3 100644 --- a/ui/components/multichain/app-header/__snapshots__/app-header.test.js.snap +++ b/ui/components/multichain/app-header/__snapshots__/app-header.test.js.snap @@ -216,13 +216,13 @@ exports[`App Header should match snapshot 1`] = ` > C -

Chain 5 -

+
diff --git a/ui/components/ui/new-network-info/new-network-info.js b/ui/components/ui/new-network-info/new-network-info.js index fac443c44c00..ab58bbced971 100644 --- a/ui/components/ui/new-network-info/new-network-info.js +++ b/ui/components/ui/new-network-info/new-network-info.js @@ -9,6 +9,7 @@ import { AlignItems, Color, Display, + FlexDirection, FontWeight, TextAlign, TextVariant, @@ -16,8 +17,7 @@ import { import { IMPORT_TOKEN_ROUTE } from '../../../helpers/constants/routes'; import { getCurrentNetwork, getUseTokenDetection } from '../../../selectors'; import { setFirstTimeUsedNetwork } from '../../../store/actions'; -import { PickerNetwork, Text } from '../../component-library'; -import Box from '../box'; +import { PickerNetwork, Text, Box } from '../../component-library'; import Button from '../button'; import Popover from '../popover'; @@ -78,7 +78,11 @@ export default function NewNetworkInfo() { } > - + ; + +DefaultStory.storyName = 'Default';