metamask-extension icon indicating copy to clipboard operation
metamask-extension copied to clipboard

Replace deprecated Chip component with Tag from the component-library

Open georgewrmarshall opened this issue 1 year ago • 11 comments

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 avatar Aug 16 '23 17:08 georgewrmarshall

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

MaxwellDG avatar Aug 31 '23 02:08 MaxwellDG

Hey @MaxwellDG, thats great! In doing a search for the <Chip in the code base the most effective intance replacement would be here https://github.com/MetaMask/metamask-extension/blob/92f04eb6e81366f3419fb20d3e07592786974c0c/ui/components/ui/new-network-info/new-network-info.js#L92-L115 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

georgewrmarshall avatar Sep 01 '23 16:09 georgewrmarshall

@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

MaxwellDG avatar Sep 02 '23 17:09 MaxwellDG

Sounds good thanks @MaxwellDG!

georgewrmarshall avatar Sep 07 '23 00:09 georgewrmarshall

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

mkos11 avatar Dec 19 '23 07:12 mkos11

@georgewrmarshall Is it still open?

nickpismenkov avatar May 07 '24 02:05 nickpismenkov

Hey @npismenkov, yes still open. I believe the last deprecated Chip component to be replaced is in https://github.com/MetaMask/metamask-extension/blob/e775193ecde4b9da33d80350da06fb0ecedb1a80/ui/pages/institutional/confirm-add-custodian-token/confirm-add-custodian-token.js#L138-L146

georgewrmarshall avatar May 07 '24 14:05 georgewrmarshall

@georgewrmarshall Could you assign it on me?

nickpismenkov avatar May 07 '24 16:05 nickpismenkov

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!

georgewrmarshall avatar May 07 '24 16:05 georgewrmarshall

@georgewrmarshall I created PR - https://github.com/MetaMask/metamask-extension/pull/24477. Sorry if something is wrong. This is my first metamask PR

nickpismenkov avatar May 10 '24 05:05 nickpismenkov

@georgewrmarshall I've failing e2e tests in this PR - https://github.com/MetaMask/metamask-extension/pull/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 avatar May 11 '24 07:05 nickpismenkov

@georgewrmarshall ?

nickpismenkov avatar May 15 '24 01:05 nickpismenkov