Update Text to use TS version of Box #19572

georgewrmarshall · 2023-06-12T20:11:15Z

Explanation

Currently the Text component still uses the JS version of the Box component and blocks TypeScript migration. This PR updates the Text component to use the TS version of the Box component. I've also added a few Before/After screenshots in the code comments to ensure no visual regression

Fixes Update Text component to use TS version of Box #19569

Screenshots/Screencaps

Before

Showing original Text component in storybook to show no visual regressions

before.mov

After

Showing updated Text component in storybook using the TS version of the Box component. Showing no visual regressions and all stories and controls are working as expected. I have made some small updates to more easily find the replace Typography with Text section and linked the issue

after.mov

Manual Testing Steps

I've added a few screenshots in the code comments to show no visual regressions between develop and the updates in this PR

To check storybook

Go to the latest build of storybook in this PR
Search Text in the search panel
Check the controls, stories and docs are working as expected.

Pre-merge author checklist

I've clearly explained:
- What problem this PR is solving
- How this problem was solved
- How reviewers can test my changes
Sufficient automated test coverage has been added

Pre-merge reviewer checklist

Manual testing (e.g. pull and build branch, run in browser, test code being changed)
PR is linked to the appropriate GitHub issue
IF this PR fixes a bug in the release milestone, add this PR to the release milestone

If further QA is required (e.g. new feature, complex testing steps, large refactor), add the Extension QA Board label.

In this case, a QA Engineer approval will be be required.

github-actions · 2023-06-12T20:11:30Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

ui/components/component-library/avatar-base/avatar-base.tsx

ui/components/component-library/avatar-base/avatar-base.types.ts

georgewrmarshall · 2023-06-12T20:14:59Z

ui/components/component-library/banner-base/banner-base.js

@@ -91,15 +91,15 @@ BannerBase.propTypes = {
  /**
   * Additional props to pass to the `Text` component used for the `title` text
   */
-  titleProps: PropTypes.shape(Text.PropTypes),
+  titleProps: PropTypes.object,


Unfortunately using Text.propTypes breaks so need to update proptypes to use generic object while during our migration process

Is there a task captured for returning to PropTypes?

Not a particular issues addressing the propTypes but is it possible to use types from a TypeScript component with PropTypes. The issue that would fix this in future ins the migration of the component to TypeScript

georgewrmarshall · 2023-06-12T20:15:11Z

ui/components/component-library/text/text.scss

@@ -33,6 +33,7 @@ $text-variants: (
  color: var(--color-text-default);
  font-family: var(--font-family-sans);

+  &:is(strong),


Updating so it's based on CSS not a prop

georgewrmarshall · 2023-06-12T20:35:54Z

ui/components/app/beta-header/__snapshots__/beta-header.test.js.snap

@@ -6,7 +6,7 @@ exports[`Beta Header should match snapshot 1`] = `
    class="box beta-header box--padding-2 box--display-flex box--flex-direction-row box--align-items-center box--width-full box--background-color-warning-default"
  >
    <h6
-      class="box mm-text beta-header__message mm-text--body-sm box--flex-direction-row box--color-warning-inverse"
+      class="mm-box mm-text beta-header__message mm-text--body-sm mm-box--color-warning-inverse"
    >
      <span>



georgewrmarshall · 2023-06-13T21:02:45Z

ui/components/app/custom-spending-cap/__snapshots__/custom-spending-cap.test.js.snap

          >
            <span>

              This allows the third party to spend 
              <h6
-                class="box mm-text custom-spending-cap__input-value-and-token-name mm-text--body-sm-bold box--flex-direction-row box--color-text-default"
+                class="mm-box mm-text custom-spending-cap__input-value-and-token-name mm-text--body-sm-bold"
              >
                10



georgewrmarshall · 2023-06-13T21:04:13Z

ui/components/app/ledger-instruction-field/__snapshots__/ledger-instruction-field.test.js.snap

            >
              <span>
                <button
-                  class="box mm-text mm-button-base mm-button-link mm-button-link--size-auto mm-text--body-md mm-text--text-align-left box--display-inline-flex box--flex-direction-row box--justify-content-center box--align-items-center box--color-primary-default box--background-color-transparent"
+                  class="mm-box mm-text mm-button-base mm-button-link mm-button-link--size-auto mm-text--body-md mm-text--text-align-left mm-box--padding-right-0 mm-box--padding-left-0 mm-box--display-inline-flex mm-box--justify-content-center mm-box--align-items-center mm-box--color-primary-default mm-box--background-color-transparent"
                >
                  Go to full screen to connect your Ledger.
                </button>


georgewrmarshall · 2023-06-13T21:05:22Z

ui/components/app/modals/customize-nonce/__snapshots__/customize-nonce.test.js.snap

@@ -54,7 +54,7 @@ exports[`Customize Nonce should match snapshot 1`] = `
          >
            <h6
              boxprops="[object Object]"
-              class="box mm-text mm-text--body-sm mm-text--font-weight-bold box--flex-direction-row box--color-text-default"
+              class="mm-box mm-text mm-text--body-sm mm-text--font-weight-bold"
            >
              Edit nonce
            </h6>


georgewrmarshall · 2023-06-13T21:06:31Z

ui/components/app/modals/eth-sign-modal/__snapshots__/eth-sign-modal.test.js.snap

      >
        Cancel
      </button>
      <button
-        class="box mm-text mm-button-base mm-button-base--size-lg mm-button-base--disabled mm-button-base--block mm-button-primary mm-button-primary--disabled mm-text--body-md box--padding-right-4 box--padding-left-4 box--display-inline-flex box--flex-direction-row box--justify-content-center box--align-items-center box--color-primary-inverse box--background-color-primary-default box--rounded-pill"
+        class="mm-box mm-text mm-button-base mm-button-base--size-lg mm-button-base--disabled mm-button-base--block mm-button-primary mm-button-primary--disabled mm-text--body-md mm-box--padding-right-4 mm-box--padding-left-4 mm-box--display-inline-flex mm-box--justify-content-center mm-box--align-items-center mm-box--color-primary-inverse mm-box--background-color-primary-default mm-box--rounded-pill"
        disabled=""
      >
        Continue


georgewrmarshall · 2023-06-13T21:07:38Z

...ents/app/modals/export-private-key-modal/__snapshots__/export-private-key-modal.test.js.snap

          type="secondary"
        >
          Cancel
        </button>
        <button
-          class="box mm-text mm-button-base mm-button-base--size-lg mm-button-base--disabled mm-button-primary mm-button-primary--disabled mm-text--body-md box--padding-right-4 box--padding-left-4 box--display-inline-flex box--flex-direction-row box--justify-content-center box--align-items-center box--width-1/2 box--color-primary-inverse box--background-color-primary-default box--rounded-pill"
+          class="mm-box mm-text mm-button-base mm-button-base--size-lg mm-button-base--disabled mm-button-primary mm-button-primary--disabled mm-text--body-md mm-box--padding-right-4 mm-box--padding-left-4 mm-box--display-inline-flex mm-box--justify-content-center mm-box--align-items-center mm-box--width-1/2 mm-box--color-primary-inverse mm-box--background-color-primary-default mm-box--rounded-pill"
          disabled=""
          type="primary"
        >


georgewrmarshall · 2023-06-13T21:08:47Z

ui/components/app/token-cell/__snapshots__/token-cell.test.js.snap

          />
        </div>
        <p
-          class="box mm-text mm-text--body-md box--flex-direction-row box--color-text-alternative"
+          class="mm-box mm-text mm-text--body-md mm-box--color-text-alternative"
          data-testid="multichain-token-list-item-value"
        >
          5.000


ui/components/component-library/text/README.mdx

georgewrmarshall · 2023-06-13T21:59:17Z

ui/components/component-library/text/text.scss

 .mm-text {
  // Set default styles
-  color: var(--color-text-default);


Now that the CSS specificity of the Box has been fixed i.e the order of stylesheets has been updated. We should set the default by the utility prop so it this style doesn't override it

georgewrmarshall · 2023-06-13T22:03:38Z

ui/components/component-library/text/text.tsx

+        dir={textDirection}
+        ref={ref}
+        color={TextColor.textDefault}
+        {...props} // TODO: Need to fix Box types to allow for spreading props


Still dealing with a TypeScript issue that complains about spreading props. This doesn't effect the functionality of the component and doesn't block the build so I'll create a ticket to look into it. Need some more time to look into it. I think as this doesn't effect anything else we move forward to unblock migration and I'll look into it in a separate issue.

Hmmm why isn't it blocking the build 🤔 type errors should

Is it possible your editor's typescript process was just... slow on reading updates? I don't see the error on my end when I view this file

garrettbear · 2023-06-14T21:42:50Z

ui/components/component-library/help-text/help-text.js

-   * HelpText also accepts all Text and Box props
+   * HelpText also accepts all Box props
   */
-  ...Text.propTypes,
+  ...Box.propTypes,


This seems fine and not that someone should be changing the variant but what happens if they do? Would the variant prop error?

Hmm yes I think this should cause a temporary prop error while this component is still in JS. There is an open contributor PR to migrate it though so once this is merged I can assist with that.

georgewrmarshall mentioned this pull request Jun 12, 2023

Update Text to use TS version of Box #19570

Closed

8 tasks

georgewrmarshall commented Jun 12, 2023

View reviewed changes

ui/components/component-library/avatar-base/avatar-base.tsx Show resolved Hide resolved

georgewrmarshall commented Jun 12, 2023

View reviewed changes

ui/components/component-library/avatar-base/avatar-base.types.ts Show resolved Hide resolved

georgewrmarshall commented Jun 12, 2023

View reviewed changes

georgewrmarshall self-assigned this Jun 12, 2023

georgewrmarshall added the team-design-system All issues relating to design system in Extension label Jun 12, 2023

georgewrmarshall commented Jun 12, 2023

View reviewed changes

georgewrmarshall mentioned this pull request Jun 12, 2023

Part of #19125: Migrate component to TS: Tag #19186

Closed

8 tasks

georgewrmarshall force-pushed the fix/19569/update-text-with-ts-box branch from f99f22f to 66a9dc5 Compare June 13, 2023 15:14

georgewrmarshall added the DO-NOT-MERGE Pull requests that should not be merged label Jun 13, 2023

georgewrmarshall force-pushed the fix/19569/update-text-with-ts-box branch from 66a9dc5 to 0339946 Compare June 13, 2023 20:56

georgewrmarshall commented Jun 13, 2023

View reviewed changes

georgewrmarshall force-pushed the fix/19569/update-text-with-ts-box branch 2 times, most recently from 762ccd7 to c348964 Compare June 13, 2023 21:46

georgewrmarshall commented Jun 13, 2023

View reviewed changes

ui/components/component-library/text/README.mdx Outdated Show resolved Hide resolved

georgewrmarshall commented Jun 13, 2023

View reviewed changes

georgewrmarshall force-pushed the fix/19569/update-text-with-ts-box branch from c348964 to 4f7c1e1 Compare June 13, 2023 22:15

georgewrmarshall mentioned this pull request Jun 13, 2023

Replace deprecated Box component with new Box component #19526

Open

georgewrmarshall force-pushed the fix/19569/update-text-with-ts-box branch from 4f7c1e1 to db05689 Compare June 14, 2023 21:37

garrettbear reviewed Jun 14, 2023

View reviewed changes

georgewrmarshall force-pushed the fix/19569/update-text-with-ts-box branch 2 times, most recently from 727b6bd to 837b5d2 Compare June 15, 2023 16:03