Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replace deprecated Chip component with Tag from the component-library #20487

Open
georgewrmarshall opened this issue Aug 16, 2023 · 12 comments · May be fixed by #24477, #24593 or #24594
Open

Replace deprecated Chip component with Tag from the component-library #20487

georgewrmarshall opened this issue Aug 16, 2023 · 12 comments · May be fixed by #24477, #24593 or #24594
Assignees
@nickpismenkov
Labels
good first issue Good for newcomers team-design-system All issues relating to design system in Extension

Comments

@georgewrmarshall
Copy link
Contributor

Description

Currently, the extension is using an outdated Chip component, which needs to be replaced with the new Tag component.

This is a massive undertaking by itself and creating a single PR would be too large. Smaller PRs can be submitted against this issue to ensure easier review and gradual improvements.

Technical Details

  • Replace instances of Chip component (ui/components/ui/chip/chip.js) with Tag component (ui/components/component-library/tag/tag.tsx)
  • Component APIs are slightly different so ensure all props have been migrated appropriately

Acceptance Criteria

  • Instances of Chip component are completely replaced with the new Tag component
  • The component APIs are updated to reflect the changes in the new `Chip`` component and there is no functional changes or visual regression
  • Each Pull Request (PR) should include no more than 3 files
  • The code changes should pass Jest tests, linting, and Storybook without any errors.
  • The PR must include before and after screenshots of the UI to ensure there are no visual regressions.

If the acceptance criteria is not met, PRs may be closed.

Difficulty: Intermediate

Good first issue for: External contributors who are familiar with running the extension locally, have knowledge of React, component props, Jest tests, linting, and Storybook, and want to contribute to improving the cohesiveness of UI in the extension
@georgewrmarshall georgewrmarshall added good first issue Good for newcomers team-design-system All issues relating to design system in Extension labels Aug 16, 2023
@MaxwellDG
Copy link

I'd like to claim this one. Will submit a PR tomorrow evening

@georgewrmarshall
Copy link
Contributor Author

Hey @MaxwellDG, thats great! In doing a search for the <Chip in the code base the most effective intance replacement would be here

metamask-extension/ui/components/ui/new-network-info/new-network-info.js

Lines 92 to 115 in 92f04eb

<Chip
className="new-network-info__token-box"
backgroundColor={Color.backgroundAlternative}
maxContent={false}
label={
providerConfig.type === NETWORK_TYPES.RPC
? providerConfig.nickname ?? t('privateNetwork')
: t(getNetworkLabelKey(providerConfig.type))
}
labelProps={{
color: Color.textDefault,
}}
leftIcon={
primaryTokenImage ? (
<Identicon image={primaryTokenImage} diameter={14} />
) : (
<Icon
className="question"
name={IconName.Question}
color={Color.iconDefault}
/>
)
}
/>
I would suggest creating a storybook file for the NewNetworkInfo component so it can be reviewed and worked on in isolation. You can create a basic story here https://metamask.github.io/metamask-storybook/?path=/docs/getting-started-documentation-guidelines--docs#creating-a-story

@MaxwellDG
Copy link

@georgewrmarshall Great, I'll get on that asap. I decided to start with #20485 instead since it often included the Chip component inside of it. When that's completed I'll switch to this

@georgewrmarshall
Copy link
Contributor Author

Sounds good thanks @MaxwellDG!

@mkos11
Copy link

mkos11 commented Dec 19, 2023

I'd like to claim this one. Will submit a PR tomorrow evening @georgewrmarshall

@nickpismenkov
Copy link

@georgewrmarshall Is it still open?

@georgewrmarshall
Copy link
Contributor Author

georgewrmarshall commented May 7, 2024 •
edited

Hey @npismenkov, yes still open. I believe the last deprecated Chip component to be replaced is in

metamask-extension/ui/pages/institutional/confirm-add-custodian-token/confirm-add-custodian-token.js

Lines 138 to 146 in e775193

<Chip
borderColor={BorderColor.borderMuted}
label={connectRequest.origin}
maxContent={false}
leftIconUrl={custodian?.iconUrl}
labelProps={{
textAlign: TextAlign.Center,
}}
/>

@nickpismenkov
Copy link

@georgewrmarshall Could you assign it on me?

@georgewrmarshall
Copy link
Contributor Author

georgewrmarshall commented May 7, 2024 •
edited

Usually these issues are left without an assignee because they cover a large task but seeing as there is only one instance left that I can see need replacing I've assigned it to you @npismenkov. Looking forward to your PR!

@nickpismenkov nickpismenkov linked a pull request May 10, 2024 that will close this issue
Open
7 tasks
@nickpismenkov
Copy link

@georgewrmarshall I created PR - #24477. Sorry if something is wrong. This is my first metamask PR

@nickpismenkov
Copy link

@georgewrmarshall I've failing e2e tests in this PR - #24477. It seems like it's failed because of timeout. I just wanna retry this step but I don't know how. So, my question is - "How I can rerun tests manually?".

@nickpismenkov
Copy link

@georgewrmarshall ?

This was referenced May 18, 2024
Open
Open
@devin-ai-integration devin-ai-integration bot mentioned this issue May 31, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nickpismenkov nickpismenkov

Labels
good first issue Good for newcomers team-design-system All issues relating to design system in Extension
Projects
None yet
Milestone
No milestone
5 participants
@georgewrmarshall @nickpismenkov @MaxwellDG @mkos11 and others