Replace deprecated mixins `@include H1` - `@include H9` in SCSS with Text component #20496

georgewrmarshall · 2023-08-17T03:59:02Z

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 the Text 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

Remove instances of the deprecated mixins from the CSS and replace the JSX element with Text component (ui/components/component-library/text/text.tsx) or appropriate component from the component-library.
There will also be CSS that can be removed in favor of the style utility props that are available on the Text component. For example, display: flex can be replaced with display={Display.Flex} prop on the Text component. Check out all the available style utility props in the Text docs in storybook.
The Text component has a variant prop that can be used to set the font size and weight. Use the TextVariant enum (ui/helpers/constants/design-system.ts) to set the appropriate variant.
There are 9 deprecated mixins that need to be replaced. The following is a mapping of the mixin to the component that should be used to replace it:

@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

Instances of deprecated mixins @include H1 - @include H9 are completely replaced with the Text component or appropriate component from the component-library
Each Pull Request (PR) should include no more than 3 files
The code changes should pass Jest tests, e2e 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

The text was updated successfully, but these errors were encountered:

subhajit20 · 2023-08-30T16:45:03Z

metamask-extension/ui/pages/unlock-page/index.scss

Lines 37 to 38 in 982a1b2

    
           &__title { 
        
             @include H2;

@georgewrmarshall Do I have to put <Text variant={TextVariant.bodyXs}> component in replace with the above @include H8 in this file index.scss file ?
Is it what this issue is all about ?

georgewrmarshall · 2023-08-30T18:27:21Z

Hey @subhajit20, this issue is to replace the jsx associated with the mixin with the Text component from the component-library so on this line

metamask-extension/ui/pages/unlock-page/unlock-page.component.js

Line 179 in 656b787

<h1 className="unlock-page__title">{t('welcomeBack')}</h1>

<h1 className="unlock-page__title">{t('welcomeBack')}</h1>

Would be replaced with

import { TextVariant, TextColor } from '../../helpers/constants/design-system';
import { Text } from '../../components/ui/component-library';

<Text as="h1" variant={TextVariant.displayMd} color={TextColor.textAlternative} marginTop={1}>{t('welcomeBack')}</Text>

Then the scss can be removed here

  &__title {
    @include H2;

    margin-top: 5px;
    font-weight: 800;
    color: var(--color-text-alternative);
  }

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.

subhajit20 · 2023-08-31T01:31:53Z

@georgewrmarshall in every single page that has h1-h9 tags will be replaced with this <Text /> component. right?
One more question, Which one should I follow for setting up locally?

Building Locally or
Development builds

subhajit20 · 2023-08-31T06:50:47Z

@georgewrmarshall hey this is being shown while running yarn start.
in which port application up and running ?

and this page is opened

subhajit20 · 2023-09-02T18:44:25Z

@georgewrmarshall Hey can you help me out with this test issue? I have replaced the H1 tag with Text component on the below file.

FAIL ui/pages/unlock-page/unlock-page.test.js (8.608 s) ● Unlock Page › should match snapshot

expect(received).toMatchSnapshot()

Snapshot name: `Unlock Page should match snapshot 1`

- Snapshot  - 1
+ Received  + 1

@@ -14,11 +14,11 @@
          >
            <svg />
          </div>
        </div>
        <h1
-         class="unlock-page__title"
+         class="mm-box mm-text mm-text--display-md mm-box--margin-top-1 mm-box--color-text-alternative"
        >
          Welcome back!
        </h1>
        <div>
          The decentralized web awaits

  35 |     const { container } = renderWithProvider(<UnlockPage />, mockStore);
  36 |
> 37 |     expect(container).toMatchSnapshot();
     |                       ^
  38 |   });

0xClint · 2023-09-02T19:10:30Z

Hey @georgewrmarshall ,
I just open a PR by replacing deprecated minxins H1,H2,.. of few components. Please check whether they are right or not so that to commit more changes.

PR #20700

subhajit20 · 2023-09-09T05:59:30Z

@georgewrmarshall Hey, made a pr.
Can you please check and let me know for any further modifications.

georgewrmarshall added good first issue Good for newcomers team-design-system All issues relating to design system in Extension labels Aug 17, 2023

georgewrmarshall changed the title ~~Remove deprecated mixins @include H1 - @include H9 in CSS and replace with Text component~~ Replcae deprecated mixins @include H1 - @include H9 in CSS with Text component Aug 17, 2023

georgewrmarshall changed the title ~~Replcae deprecated mixins @include H1 - @include H9 in CSS with Text component~~ Replcae deprecated mixins @include H1 - @include H9 in SCSS with Text component Aug 17, 2023

georgewrmarshall changed the title ~~Replcae deprecated mixins @include H1 - @include H9 in SCSS with Text component~~ Replace deprecated mixins @include H1 - @include H9 in SCSS with Text component Aug 30, 2023

georgewrmarshall mentioned this issue Sep 6, 2023

Replace deprecated mixins H1-H9 in SCSS with Text component of Few Co… #20700

Closed

8 tasks

subhajit20 mentioned this issue Sep 9, 2023

deprecated mixins @include H1 in SCSS replaced with Text component #20805

Closed

8 tasks

metamaskbot added the INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. label May 31, 2024

Devin-Apps mentioned this issue Jun 10, 2024

Part of #17670: Replace Typography with Text component in actionable-message and icon stories Devin-Applications/metamask-extension-devin#44

Closed

7 tasks

georgewrmarshall closed this as completed in #25228 Jun 24, 2024

georgewrmarshall closed this as completed in a445529 Jun 24, 2024

This was referenced Jun 27, 2024

refactor: replace deprecated mixins with text component in transaction-confirmed #25551

Open

refactor: replace deprecated mixins with text component in loading-swaps-quotes #25553

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace deprecated mixins `@include H1` - `@include H9` in SCSS with Text component #20496

Replace deprecated mixins `@include H1` - `@include H9` in SCSS with Text component #20496

georgewrmarshall commented Aug 17, 2023 •

edited

Loading

subhajit20 commented Aug 30, 2023 •

edited

Loading

georgewrmarshall commented Aug 30, 2023 •

edited

Loading

subhajit20 commented Aug 31, 2023 •

edited

Loading

subhajit20 commented Aug 31, 2023 •

edited

Loading

subhajit20 commented Sep 2, 2023 •

edited

Loading

0xClint commented Sep 2, 2023 •

edited

Loading

subhajit20 commented Sep 9, 2023 •

edited

Loading

Replace deprecated mixins @include H1 - @include H9 in SCSS with Text component #20496

Replace deprecated mixins @include H1 - @include H9 in SCSS with Text component #20496

Comments

georgewrmarshall commented Aug 17, 2023 • edited Loading

Description

Technical Details

Acceptance Criteria

Difficulty: Intermediate

subhajit20 commented Aug 30, 2023 • edited Loading

georgewrmarshall commented Aug 30, 2023 • edited Loading

subhajit20 commented Aug 31, 2023 • edited Loading

subhajit20 commented Aug 31, 2023 • edited Loading

subhajit20 commented Sep 2, 2023 • edited Loading

0xClint commented Sep 2, 2023 • edited Loading

subhajit20 commented Sep 9, 2023 • edited Loading

Replace deprecated mixins `@include H1` - `@include H9` in SCSS with Text component #20496

Replace deprecated mixins `@include H1` - `@include H9` in SCSS with Text component #20496

georgewrmarshall commented Aug 17, 2023 •

edited

Loading

subhajit20 commented Aug 30, 2023 •

edited

Loading

georgewrmarshall commented Aug 30, 2023 •

edited

Loading

subhajit20 commented Aug 31, 2023 •

edited

Loading

subhajit20 commented Aug 31, 2023 •

edited

Loading

subhajit20 commented Sep 2, 2023 •

edited

Loading

0xClint commented Sep 2, 2023 •

edited

Loading

subhajit20 commented Sep 9, 2023 •

edited

Loading