-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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.
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
Replace deprecated Chip component with Tag from the component-library #20487
Replace deprecated Chip component with Tag from the component-library #20487
Comments
I'd like to claim this one. Will submit a PR tomorrow evening |
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
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 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 |
Sounds good thanks @MaxwellDG! |
I'd like to claim this one. Will submit a PR tomorrow evening @georgewrmarshall |
@georgewrmarshall Is it still open? |
Hey @npismenkov, yes still open. I believe the last deprecated metamask-extension/ui/pages/institutional/confirm-add-custodian-token/confirm-add-custodian-token.js Lines 138 to 146 in e775193
|
@georgewrmarshall Could you assign it on me? |
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 I created PR - #24477. Sorry if something is wrong. This is my first metamask PR |
@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?". |
Description
Currently, the extension is using an outdated
Chip
component, which needs to be replaced with the newTag
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
Chip
component (ui/components/ui/chip/chip.js
) withTag
component (ui/components/component-library/tag/tag.tsx
)Acceptance Criteria
Chip
component are completely replaced with the newTag
componentIf 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
The text was updated successfully, but these errors were encountered: