-
Notifications
You must be signed in to change notification settings - Fork 5k
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 mixins @include H1
- @include H9
in SCSS with Text component
#20496
Comments
@include H1
- @include H9
in CSS and replace with Text component@include H1
- @include H9
in CSS with Text component
@include H1
- @include H9
in CSS with Text component@include H1
- @include H9
in SCSS with Text component
metamask-extension/ui/pages/unlock-page/index.scss Lines 37 to 38 in 982a1b2
@georgewrmarshall Do I have to put |
@include H1
- @include H9
in SCSS with Text component@include H1
- @include H9
in SCSS with Text component
Hey @subhajit20, this issue is to replace the jsx associated with the mixin with the
Would be replaced with
Then the scss can be removed here
This updates the deprecated styles and reduces the amount of CSS that is loaded improving the consistency and the performance of the extension. I would like to point out that this is a very important page so it will need a lot of scrutinizing and review. Which means adding before/after screenshots which will require the extension to be running on your local In fact I think any UI updates to the unlock screen page are so important that it would require some marketing mentions that we updated it. I would suggest leaving the styles for this one. |
@georgewrmarshall in every single page that has |
@georgewrmarshall hey this is being shown while running yarn start. |
@georgewrmarshall Hey can you help me out with this test issue? I have replaced the
|
Hey @georgewrmarshall , PR #20700 |
@georgewrmarshall Hey, made a pr. |
…e.component.js (#25227) ## **Description** This pull request replaces the deprecated mixins in the `unlock-page.component.js` and `index.scss` files with the `Text` component. The changes include updating the `unlock-page__title` class to use the `Text` component with appropriate properties and removing the deprecated mixin instance from the SCSS file. Devin Run Link: https://preview.devin.ai/devin/de079f9a40fd45adb09783a36409256c [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25227?quickstart=1) ## **Related issues** Partially Fixes: #20496 ## **Manual testing steps** 1. Go to the latest build of storybook in this PR 2. Verify that the "UnlockPage" component renders correctly with the updated `Text` component 3. Ensure that the `unlock-page__title` class is replaced with `[data-testid="unlock-page-title"]` in the `test/e2e/tests/settings/auto-lock.spec.js` file ## **Screenshots/Recordings** ### **Before** ![](https://api.devin.ai/attachments/991317c8-2109-4a41-bf0c-5d3edd8add50/980098aa-0372-4728-9f89-f8eff532a8f2.png) ### **After** ![](https://api.devin.ai/attachments/49521e22-bbd6-4016-81a2-839816a3008f/0361fee1-e517-4e0e-8959-f22c6b2e1fac.png) ## **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 completed the PR template to the best of my ability - [X] I’ve included tests if applicable - [X] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [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)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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. --------- Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: George Marshall <[email protected]>
…ccount.component.js (#25262) ## **Description** Replaced deprecated mixins with Text component in `selected-account.component.js`. The change aims to modernize the codebase by using the Text component from the design system, ensuring consistency and maintainability. Devin Run Link: https://preview.devin.ai/devin/6dcddd7b3ee2456ca004b34d033b0d82 [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25262?quickstart=1) ## **Related issues** Partially Fixes: #20496 ## **Manual testing steps** 1. Go to the latest build of storybook in this PR 2. Verify the "SelectedAccount" component displays correctly with the updated Text component. ## **Screenshots/Recordings** ### **Before** ![](https://api.devin.ai/attachments/ad6f5c71-8714-4f0d-9675-d36481abbafa/before_changes_selected-account.component.png) ### **After** <img width="1653" alt="Screenshot 2024-06-12 at 21 55 36" src="https://github.com/MetaMask/metamask-extension/assets/168687171/3e05ef54-8272-4c48-8637-ddedcf2f3830"> ## **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 completed the PR template to the best of my ability - [X] I’ve included tests if applicable - [X] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [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)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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. --------- Co-authored-by: George Marshall <[email protected]> Co-authored-by: Shreyasi Mandal <[email protected]>
…n-confirmed (#25551) ## **Description** This pull request replaces deprecated mixins with the Text component in the `transaction-confirmed.component.js` file. The padding values in the Text components have been updated to 4. Devin Run Link: https://staging.itsdev.in/devin/0bf228de1dc64981adb90e7ac712d1e7 [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25551?quickstart=1) ## **Related issues** Partially Fixes: #20496 ## **Manual testing steps** 1. Go to the latest build of storybook in this PR 2. Manually check if `TransactionConfirmed` is getting rendered correctly ## **Screenshots/Recordings** ### **Before** ![](https://api.devin.ai/attachments/8e7c9a3e-1b46-4835-8539-0f17be6f72cf/before_changes_transaction-confirmed.png) ### **After** ![](https://api.devin.ai/attachments/8e7c9a3e-1b46-4835-8539-0f17be6f72cf/after_changes_transaction-confirmed.png) ## **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 completed the PR template to the best of my ability - [X] I’ve included tests if applicable - [X] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [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)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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. --------- Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: George Marshall <[email protected]>
…ew (#25637) ## **Description** This pull request replaces deprecated mixins @include H1 - @include H9 in the SCSS file with the Text component in the `qr-code-view` component. The changes include updating the SCSS file to remove the deprecated mixins and modifying the JavaScript file to use the Text component with appropriate properties. Devin Run Link : https://preview.devin.ai/devin/8158744e0716415fb64b023769e4ffab [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25637?quickstart=1) ## **Related issues** Partially Fixes: #20496 ## **Manual testing steps** 1. Go to the latest build of storybook in this PR 2. Verify that the QrCodeView component renders correctly without any visual issues 3. Ensure that the Text component is used instead of the deprecated mixins ## **Screenshots/Recordings** ### **Before** ![Before Changes](https://api.devin.ai/attachments/9a8904f8-6bff-4d54-90c8-2b0522e7f345/79852720-e3a7-4bb5-b62a-38679aca0b28.png) ### **After** ![After Changes](https://api.devin.ai/attachments/9a8904f8-6bff-4d54-90c8-2b0522e7f345/79852720-e3a7-4bb5-b62a-38679aca0b28.png) ## **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 completed the PR template to the best of my ability - [X] I’ve included tests if applicable - [X] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [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)). Not required for external contributors. ## **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. [This Devin run](https://preview.devin.ai/devin/8158744e0716415fb64b023769e4ffab) was requested by Devin. --------- Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: George Marshall <[email protected]>
…n-confirmed (#25551) ## **Description** This pull request replaces deprecated mixins with the Text component in the `transaction-confirmed.component.js` file. The padding values in the Text components have been updated to 4. Devin Run Link: https://staging.itsdev.in/devin/0bf228de1dc64981adb90e7ac712d1e7 [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25551?quickstart=1) ## **Related issues** Partially Fixes: #20496 ## **Manual testing steps** 1. Go to the latest build of storybook in this PR 2. Manually check if `TransactionConfirmed` is getting rendered correctly ## **Screenshots/Recordings** ### **Before** ![](https://api.devin.ai/attachments/8e7c9a3e-1b46-4835-8539-0f17be6f72cf/before_changes_transaction-confirmed.png) ### **After** ![](https://api.devin.ai/attachments/8e7c9a3e-1b46-4835-8539-0f17be6f72cf/after_changes_transaction-confirmed.png) ## **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 completed the PR template to the best of my ability - [X] I’ve included tests if applicable - [X] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [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)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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. --------- Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: George Marshall <[email protected]>
…aps-quotes (#25553) ## **Description** This pull request replaces deprecated mixins with the Text component in the `loading-swaps-quotes` file. The changes include updating the SCSS and JS files to use the Text component and related enums, as well as deleting specific SCSS instances as instructed by the user. Devin Run Link: https://staging.itsdev.in/devin/edffa827e63141c8a3214c25ecf4a760 [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25553?quickstart=1) ## **Related issues** Partially Fixes: #20496 ## **Manual testing steps** 1. Go to the latest build of storybook in this PR 2. Manually check if `LoadingSwapQuotes` is getting rendered properly ## **Screenshots/Recordings** ### **Before** ![](https://api.devin.ai/attachments/957044a7-ae7a-442e-ad5c-b5ca03c5d6f2/before_changes_loading-swaps-quotes.png) ### **After** ![](https://api.devin.ai/attachments/f3b372f6-d4f7-4e40-a485-8c642a28b1e7/4e483d0c-1c7c-467c-9c0a-f945b6341919.png) ## **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 completed the PR template to the best of my ability - [X] I’ve included tests if applicable - [X] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [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)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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. --------- Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: George Marshall <[email protected]>
…uttons (#25638) ## **Description** This pull request replaces deprecated mixins @include H1 - @include H9 in SCSS with the Text component in the "MetaMask/metamask-extension" repository. The changes are made in the `ui/pages/swaps/slippage-buttons/index.scss` and `slippage-buttons.js` files. 1. What is the reason for the change? - The deprecated mixins @include H1 - @include H9 need to be replaced with the Text component to ensure consistency and maintainability in the codebase. 2. What is the improvement/solution? - The deprecated mixins are replaced with the Text component, and the corresponding CSS properties are mapped to the Text component props. Devin Run Link : https://preview.devin.ai/devin/78aa98c3ee2b43c685a605db902abe1a Requested by: Devin [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25638?quickstart=1) ## **Related issues** Partially Fixes: #20496 ## **Manual testing steps** 1. Go to the latest build of storybook in this PR 2. Verify that the SlippageButtons component renders correctly with the updated Text components. 3. Ensure that the styles and functionality of the component remain consistent with the previous implementation. ## **Screenshots/Recordings** ### **Before** ![Before Changes](https://api.devin.ai/attachments/5d79c749-95ee-4131-92f6-8920e7ee6cc1/bbf6edb1-2de5-4a8b-ae0a-6e6681408f49.png) ### **After** ![After Changes](https://api.devin.ai/attachments/6e6a22ac-a050-4bff-858a-a15752ce5d2d/after_changes_slippage-buttons.png) ## **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 completed the PR template to the best of my ability. - [X] I’ve included tests if applicable. - [X] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable. - [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)). Not required for external contributors. ## **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. --------- Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: George Marshall <[email protected]> Co-authored-by: Garrett Bear <[email protected]>
Missing release label release-12.2.0 on issue. Adding release label release-12.2.0 on issue, as issue is linked to PR #25227 which has this release label. |
Missing release label release-12.2.0 on issue. Adding release label release-12.2.0 on issue, as issue is linked to PR #25262 which has this release label. |
Missing release label release-12.2.0 on issue. Adding release label release-12.2.0 on issue, as issue is linked to PR #25228 which has this release label. |
Description
Currently, the extension is using outdated mixins
@include H1
-@include H9
. This CSS should be removed and the JSX associated with these styles replaced with theText
component. This will reduce the amount of CSS in the extension and improve the cohesiveness of the UI.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
Text
component (ui/components/component-library/text/text.tsx
) or appropriate component from the component-library.Text
component. For example,display: flex
can be replaced withdisplay={Display.Flex}
prop on theText
component. Check out all the available style utility props in the Text docs in storybook.Text
component has avariant
prop that can be used to set the font size and weight. Use theTextVariant
enum (ui/helpers/constants/design-system.ts
) to set the appropriate variant.@include H1
=><Text variant={TextVariant.displayMd}>
@include H2
=><Text variant={TextVariant.displayLg}>
@include H3
=><Text variant={TextVariant.headingMd}>
@include H4
=><Text variant={TextVariant.headingSm}>
@include H5
=><Text variant={TextVariant.bodyMd}>
@include H6
=><Text variant={TextVariant.bodyMd}>
@include H7
=><Text variant={TextVariant.bodySm}>
@include H8
=><Text variant={TextVariant.bodyXs}>
@include H9
=><Text variant={TextVariant.bodyXs}>
Acceptance Criteria
@include H1
-@include H9
are completely replaced with theText
component or appropriate component from the component-libraryIf 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: