georgewrmarshall commented 1 year ago

Description

In an effort to reduce duplication and replace our old design system components with new components it would be great to replace all <Typography /> with <Text /> components. This is a massive undertaking by itself and creating a single PR would be too large. Instead this issue could be broken up into multiple PRs.

Technical Details

Swap all Typography components with Text equivalent(make sure to check the component apis as they are slightly different)
boxProps should be migrated appropriately e.g. boxProps={{marginTop: 1}} => marginTop={1}
Some TextVariant have bold font weight options which can be used instead of fontWeight={FONT_WEIGHT.BOLD} => variant={TextVariant.bodyMdBold}
Check the Typography => Text component map below for the correct variant prop values

`Typography` => `Text` component map

<Typography variant={TypographyVariant.H1}> => <Text variant={TextVariant.displayMd}> <Typography variant={TypographyVariant.H2}> => <Text variant={TextVariant.headingLg}> <Typography variant={TypographyVariant.H3}> => <Text variant={TextVariant.headingMd}> <Typography variant={TypographyVariant.H4}> => <Text variant={TextVariant.headingSm}> <Typography variant={TypographyVariant.H5}> => <Text variant={TextVariant.bodyMd}> <Typography variant={TypographyVariant.H6}> => <Text variant={TextVariant.bodySm}> <Typography variant={TypographyVariant.Paragraph}> => <Text variant={TextVariant.bodyMd} <Typography variant={TypographyVariant.H7}> => <Text variant={TextVariant.bodySm}> <Typography variant={TypographyVariant.H8}> => <Text variant={TextVariant.bodyXs}> <Typography variant={TypographyVariant.H9}> => <Text variant={TextVariant.bodyXs}>

Acceptance Criteria

All Typography components have been replaced with their Text equivalent
PR contains a minimum of 1 file and a maximum of 3 files
Add Before / After screenshots to your PR to ensure no visual regressions
Reference this issue in the description of your PR
All CI tests are passing jest, lint, e2e etc

PRs that don't meet the acceptance criteria may be closed.

References

Text component documentation

georgewrmarshall commented 1 year ago

Please take note that the current Text documentation will be out of date until https://github.com/MetaMask/metamask-extension/pull/17674 has been merged 🙏

itsAfnanAlam commented 1 year ago

Hi, can I work on this?

georgewrmarshall commented 1 year ago

Hi @itsAfnanAlam, you're welcome to submit a PR toward this issue please title it Part of #17670: Replace Typography with Text component

itsAfnanAlam commented 1 year ago

@georgewrmarshall, working on it. Thanks!

Aathirajan commented 1 year ago

hi @georgewrmarshall is it up for grabs? if yes can I?

itsAfnanAlam commented 1 year ago

@Aathirajan I am working on it.

Aathirajan commented 1 year ago

Great! Glad you mentioned it.

georgewrmarshall commented 1 year ago

Hey @itsAfnanAlam and @Aathirajan, this issue is intended to be broken up in to multiple PRs for easy review. I'm listing files engineers would like to, or are working on in the description under "Files being currently converted". You could both comment on the files you would like to convert and I'll add them to the list. See example PR here https://github.com/MetaMask/metamask-extension/pull/17753

sammaji commented 1 year ago

@georgewrmarshall can you point out some files that need change first and we'll look into them? Here's some that I found:

\ui\components\app\add-network\add-network.js
\ui\components\app\advanced-gas-fee-popover\advanced-gas-fee-defaults\advanced-gas-fee-defaults.js
\ui\components\app\advanced-gas-fee-popover\advanced-gas-fee-gas-limit\advanced-gas-fee-gas-limit.js
\ui\components\app\approve-content-card\approve-content-card.js

We could work on these first.

georgewrmarshall commented 1 year ago

Hey @samyabrata-maji, some of those are being worked on see "Files currently being converted" at the bottom section in this issue.

\ui\components\app\add-network\add-network.js in-progress 🚫
\ui\components\app\advanced-gas-fee-popover\advanced-gas-fee-defaults\advanced-gas-fee-defaults.js available to work on ✅
\ui\components\app\advanced-gas-fee-popover\advanced-gas-fee-gas-limit\advanced-gas-fee-gas-limit.js available to work on ✅
\ui\components\app\approve-content-card\approve-content-card.js in-progress 🚫

I'll add your name to the list for the available files in that list thanks!

spiritanand commented 1 year ago

Hey @georgewrmarshall are all the files being worked by other contributors? Could I contribute in any way?

sammaji commented 1 year ago

Hey @georgewrmarshall are all the files being worked by other contributors? Could I contribute in any way? There are other files that require changes.

@spiritanand Just make sure to let us know which ones you would be working on 👍

spiritanand commented 1 year ago

@samyabrata-maji I did not understand. Are the ones in the ss available for me to work on?

Yomna-J commented 1 year ago

@georgewrmarshall A quick update: I've been working on the following and changes have been made. However, I'm unable to add screenshots as "This conversation has been locked and limited to collaborators."

ui/components/app/add-network/add-network.js @Yomna-J Part of #17670: Replace Typography with Text component #17681

ui/components/app/confirm-page-container/confirm-page-container.component.js @Yomna-J Part of #17670: Replace Typography with Text component #17681

sammaji commented 1 year ago

@samyabrata-maji I did not understand. Are the ones in the ss available for me to work on?

Some of them are available. There are other files too that are not in the ss.

georgewrmarshall commented 1 year ago

Hey @spiritanand, you can do a search for <Typography in your code editor. Excluding the Typography folder itself(ui/components/ui/typography) There are 119 files you can choose from that need to be converted. So far the bottom section "Files currently being converted" at the bottom of this issue description is what is being worked on. So you can comment here on what file you would like to work on and I will add it to the list under your name.

Search <Typography
Pick a file that has not been taken and IS NOT in this list

Comment on this issue the file that you'd like to convert and I will add it to the description under your name

SawanKumarBhagat commented 1 year ago

Hey! @georgewrmarshall I worked on these files and made a pr #17766 can you check them please? metamask-extension\ui\components\app\asset-list\asset-list.js metamask-extension\ui\components\app\beta-header\index.js metamask-extension\ui\components\app\cancel-speedup-popover\cancel-speedup-popover.js metamask-extension\ui\components\app\confirm-page-container\confirm-page-container.component.js metamask-extension\ui\components\app\confirm-page-container\confirm-page-container-content\confirm-page-container-content.component.js

ayushj1910 commented 1 year ago

Hey @georgewrmarshall , I would like to work on this issue.

georgewrmarshall commented 1 year ago

Hey @ayushj1910, That's great! Please read the issue and comment thread for details 🙏

moheet333 commented 1 year ago

Hey @georgewrmarshall , I am new to open source where do i start working on this issue?

georgewrmarshall commented 1 year ago

Hey @moheet333, a great start would be

Get the extension running on your local machine by reading through the contributor docs
Carefully read through this issue
See example PR https://github.com/MetaMask/metamask-extension/pull/17753

spiritanand commented 1 year ago

Hey @georgewrmarshall, I will get started with the file /ui/pages/confirm-approve/confirm-approve-content/confirm-approve-content.component.js and ui/pages/home/home.component.js. Please assign them to me.

georgewrmarshall commented 1 year ago

Hey @spiritanand, updated description and looking forward to your PRs. Thanks!

spiritanand commented 1 year ago

Happy Holi 🎉 @georgewrmarshall

I have created a PR for the two files confirm-approve-content.component.js and home.component.js. Please have a look 🤞

spiritanand commented 1 year ago

Hey @georgewrmarshall, Please add welcome.js and edit-gas-display.component.js under my tab. I will be working on them this weekend.

Also, should I create a new PR for them or add them in the same PR I created?

georgewrmarshall commented 1 year ago

Hey @spiritanand, please create a new PR it helps get approved updates in faster as well as prevent stale PRs which could be in danger of having merge conflicts if those files are update during the time the PR is open. Smaller PRs are almost always best! Will add those file next to your name. Cheers! One suggestion is to tackle components with .stories.js files so we can easily review your changes in isolation. Oh both have storybook files so that's great 💯

sambhavgupta0705 commented 1 year ago

Hey @georgewrmarshall is there any file through which I can contribute?

spiritanand commented 1 year ago

Hey @georgewrmarshall is there any file through which I can contribute?

@sambhavgupta0705 please read the above thread, there are many files you can contribute to by searching for <Typography> component in your fork.

For reference, see the already merged PR ⇒ #17753

All the best.

sambhavgupta0705 commented 1 year ago

Hey @georgewrmarshall is there any file through which I can contribute?

@sambhavgupta0705 please read the above thread, there are many files you can contribute to by searching for <Typography> component in your fork.

For reference, see the already merged PR ⇒ #17753

All the best.

Thanks @spiritanand

sscode commented 1 year ago

Just submitted my first PR for file - ui/components/app/create-new-vault/create-new-vault.js

VedanthB commented 1 year ago

Hey @georgewrmarshall I have raised a PR that changes ui/components/app/nft-details/nft-details.js.

Also can I change other files and raise more PRs ?

HemanthPaila commented 1 year ago

Hi there,

I want to contribute to this project and take on this issue as my first task. Could you please assign it to me? I am excited to work towards resolving it and will keep the team updated on my progress.

Thanks!

Charliehexx commented 1 year ago

Hey @georgewrmarshall I have raised a pull request #18382 that changes ui/components/app/custom-spending-cap/custom-spending-cap.js. Could you please review it and give feedback if any changes required.

Ananyamadhu08 commented 1 year ago

Hey @georgewrmarshall I have raised a PR that changes ui/components/app/detected-token/detected-token-address/detected-token-address.js

dswilson4 commented 1 year ago

Hello @georgewrmarshall , I opened to make a change to the import-token component in the swaps page ui\pages\swaps\import-token\import-token.js

PrgrmrHarshShukla commented 1 year ago

Hello @georgewrmarshall , I "Replaced Typography with Text component" in the detected-token-aggregators.js component in ui/components/app/detected-token/detected-token-aggregators/detected-token-aggregators.js

Please mention any mistake(s) I have made as this is my first Open Source Contribution.

PrgrmrHarshShukla commented 1 year ago

Hello @georgewrmarshall , I "Replaced Typography with Text component" in

ui\components\app\beta-header\index.js ui\components\app\advanced-gas-fee-popover\advanced-gas-fee-gas-limit\advanced-gas-fee-gas-limit.js ui\components\app\advanced-gas-fee-popover\advanced-gas-fee-defaults\advanced-gas-fee-defaults.js

and raised (my FIRST) Pull Request for the same. (https://github.com/MetaMask/metamask-extension/pull/18609)

PrgrmrHarshShukla commented 1 year ago

Hello @georgewrmarshall , I have worked on the file ui\components\app\signature-request\signature-request-data\signature-request-data.js to replace Typography with Text component.

Please review my PR #18612

Regards, Harsh

PrgrmrHarshShukla commented 1 year ago

Hello @georgewrmarshall ,

I have worked on the file ui\components\app\edit-gas-fee-popover\network-statistics\network-statistics.js to replace Typography with Text component.

Please review my PR #18644

PrgrmrHarshShukla commented 1 year ago

Hello @georgewrmarshall ,

I have worked on this file to replace Typography with Text component.

ui\components\app\custom-spending-cap\custom-spending-cap-tooltip.js

Please review my PR #18686

PrgrmrHarshShukla commented 1 year ago

Hello @georgewrmarshall , @brad-decker ,

I have worked on some other linting issues in the file

ui\components\app\custom-spending-cap\custom-spending-cap-tooltip.js

Kindly review the PR #18687

Regards, Harsh

PrgrmrHarshShukla commented 1 year ago

Hey @georgewrmarshall ,

In the PR #18736 , I have replaced Typography with Text component AND replaced design system typography consts with enums in the file:

ui\components\app\detected-token\detected-token-aggregators\detected-token-aggregators.js

Please review it.

PrgrmrHarshShukla commented 1 year ago

Hey @georgewrmarshall ,

In the PR #18738 , I have replaced Typography with Text component in the file:

ui\components\app\detected-token\detected-token-ignored-popover\detected-token-ignored-popover.js

Please review it.

PrgrmrHarshShukla commented 1 year ago

Hey @georgewrmarshall ,@brad-decker ,

In the PR #18741 ,I have replaced Typography with Text component AND replaced design system typography consts with enums in the file:

ui\pages\token-details\token-details-page.js

Kindly review it.

PrgrmrHarshShukla commented 1 year ago

Hey @georgewrmarshall ,@brad-decker ,

In the PR #18742 ,I have replaced Typography with Text component AND replaced design system typography consts with enums in the file:

ui\pages\token-allowance\token-allowance.js

Kindly review it.

PrgrmrHarshShukla commented 1 year ago

Hey @georgewrmarshall , @brad-decker ,

In the PR #18743 ,I have replaced Typography with Text component AND replaced design system typography consts with enums in the file:

ui\pages\swaps\smart-transaction-status\smart-transaction-status.js

Kindly review it.

PrgrmrHarshShukla commented 1 year ago

Hey @georgewrmarshall , @brad-decker ,

In the PRs

18745

18746

18750

18752

18753

18754

18755

,I have replaced Typography with Text component in the files:

ui\pages\swaps\slippage-buttons\slippage-buttons.js
ui\pages\swaps\fee-card\fee-card.js
ui\pages\swaps\build-quote\build-quote.js
ui\pages\swaps\awaiting-signatures\awaiting-signatures.js
ui\pages\settings\settings-tab\settings-tab.component.js
ui\pages\settings\networks-tab\networks-list\networks-list.js
ui\pages\settings\flask\view-snap\view-snap.js respectively.

Please review the PRs.

PrgrmrHarshShukla commented 1 year ago

Hi there @georgewrmarshall , @NidhiKJha , @brad-decker ,

In the PR #18771 , I have worked on the issue #17670 and #18714 for the file: ui\components\app\nft-details\nft-details.js

Please review it.

dhruvv173 commented 1 year ago

@georgewrmarshall @NidhiKJha @brad-decker Hi, I'd like to work on this issue, are there any files left to work on? Thanks!

dhruvv173 commented 1 year ago

Hey @georgewrmarshall Can I work on it?

MetaMask / metamask-extension

Replace deprecated Typography with Text component #17670

Description

Technical Details

`Typography` => `Text` component map

Acceptance Criteria

References

18745

18746

18750

18752

18753

18754

18755

MetaMask / metamask-extension

Replace deprecated Typography with Text component #17670

Description

Technical Details

Typography => Text component map

Acceptance Criteria

References

18745

18746

18750

18752

18753

18754

18755

`Typography` => `Text` component map