Part of #17670: Replace Typography with Text component #17959

ayushj1910 · 2023-03-02T10:40:11Z

Explanation

This pull request has the changes which are required in issue #17670. It changes all the Typography components with Text components.

Files updated through this PR :

Screenshots/Screencaps

Before

``

confirmation-warning-modol.js

create-new-vault.js

cusotm-spending-tooltip-cap.js

After

confirmation-warning-modol.js

create-new-vault.js

cusotm-spending-tooltip-cap.js

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-03-02T10:40:26Z

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.

ayushj1910 · 2023-03-02T10:44:40Z

Hey @georgewrmarshall and @digiwand ,
Can you please confirm if these changes are correct ?
Also, please check the screenshots if this is what needs to be added in the explanation part.
Thanks!

georgewrmarshall

Hey @ayushj1910, Code looks good and screenshots are great! Just need to fix linting and other failing tests

ayushj1910 · 2023-03-02T20:30:06Z

Hey @georgewrmarshall,
Can you please tell me a way to fix the linting issues and other failing tests. I tried running 'yarn lint' but it failed giving an error "Spawn ERROR - ENAMETOOLONG"

digiwand · 2023-03-03T13:06:16Z

Hi @ayushj1910, have you tried yarn lint:fix?

ayushj1910 · 2023-03-03T13:44:53Z

Hi @ayushj1910, have you tried yarn lint:fix?

Hey @digiwand ,
Yes I tried this as well. It gives the same error.

digiwand · 2023-03-03T14:53:27Z

Hey @ayushj1910,

I think there is something wrong with the setup on your local machine. I have not encountered that error before. Maybe try

> rm -rf node_modules
> yarn setup

The command appears to work on my end. The other tests may be failing because some of the file paths are not resolving.

I have the lint fixes on my local. I can add them to this PR if you'd like

ayushj1910 · 2023-03-06T13:11:48Z

Hey @ayushj1910,

I think there is something wrong with the setup on your local machine. I have not encountered that error before. Maybe try
> rm -rf node_modules
> yarn setup
The command appears to work on my end. The other tests may be failing because some of the file paths are not resolving.

I have the lint fixes on my local. I can add them to this PR if you'd like

Hey @digiwand ,
I tried but it doesn't work. It would be very helpful if you add the lint fix to this PR. Thanks!
Once these 4 files are approved, I will start working on rest of the files as well.

georgewrmarshall · 2023-03-06T22:49:50Z

ui/components/app/confirmation-warning-modal/confirmation-warning-modal.js

              {t('addEthereumChainWarningModalListPointThree')}
-            </Typography>
+            </Text>
          </li>
        </ul>
      </Box>


georgewrmarshall · 2023-03-06T22:51:04Z

ui/components/app/create-new-vault/create-new-vault.js

@@ -135,7 +135,7 @@ export default function CreateNewVault({
            className="create-new-vault__terms-label"
            htmlFor="create-new-vault__terms-checkbox"
          >
-            <Typography as="span">{termsOfUse}</Typography>
+            <Text as="span">{termsOfUse}</Text>
          </label>
        </div>
      ) : null}


georgewrmarshall · 2023-03-06T22:56:58Z

...container-content/confirm-page-container-summary/confirm-page-container-summary.component.js

              title={title}
            >
              {titleComponent || title}
-            </Typography>
+            </Text>
          ) : null}
        </div>
        {hideSubtitle ? null : (


georgewrmarshall · 2023-03-06T22:59:21Z

Hey @ayushj1910, I've fixed the import and linting errors as well as added a storybook file for ConfirmPageContainerSummary hopefully that fixes the build

ayushj1910 · 2023-03-08T12:40:58Z

Hey @ayushj1910, I've fixed the import and linting errors as well as added a storybook file for ConfirmPageContainerSummary hopefully that fixes the build

Hey @georgewrmarshall ,
Thank you for fixing the issues. I will start working on the rest of the files now.
Also, please add these 4 files to the Issue description so that there is no clash.

georgewrmarshall · 2023-03-08T20:10:19Z

Hey @ayushj1910, I would suggest creating another PR for more changes so we can make improvements iteratively and prevent stale PRs. I'm going force push back to where there are no merge conflicts. Once that passes we can merge this PR. I would suggested branching off of develop and cherry picking your custom-spending-cap-tooltip.js changes

codecov · 2023-03-08T20:42:54Z

Codecov Report

Merging #17959 (d2ecd04) into develop (423e085) will increase coverage by 0.06%.
The diff coverage is 56.56%.

@@             Coverage Diff             @@
##           develop   #17959      +/-   ##
===========================================
+ Coverage    63.86%   63.91%   +0.06%     
===========================================
  Files          908      908              
  Lines        35390    35398       +8     
  Branches      8966     8975       +9     
===========================================
+ Hits         22599    22624      +25     
+ Misses       12791    12774      -17

Impacted Files	Coverage Δ
app/scripts/controllers/preferences.js	`77.16% <ø> (-2.73%)`	⬇️
app/scripts/migrations/index.js	`100.00% <ø> (ø)`
shared/constants/metametrics.js	`100.00% <ø> (ø)`
shared/constants/network.ts	`100.00% <ø> (ø)`
...mation-warning-modal/confirmation-warning-modal.js	`50.00% <ø> (ø)`
...omponents/app/create-new-vault/create-new-vault.js	`41.18% <ø> (ø)`
...delete-network/confirm-delete-network.container.js	`66.67% <0.00%> (ø)`
ui/ducks/app/app.ts	`62.28% <0.00%> (+1.60%)`	⬆️
ui/ducks/metamask/metamask.js	`76.19% <ø> (ø)`
...rm-transaction/confirm-token-transaction-switch.js	`5.26% <0.00%> (-3.07%)`	⬇️
... and 27 more

... and 5 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

ayushj1910 · 2023-03-15T14:25:11Z

Hey @georgewrmarshall ,
Is there any work pending for this PR?

georgewrmarshall

LGTM! I can't see any reason for the code coverage to be failing on this PR? Would recommend merging after 2 approvals

ayushj1910 requested a review from a team as a code owner March 2, 2023 10:40

ayushj1910 requested a review from digiwand March 2, 2023 10:40

georgewrmarshall reviewed Mar 2, 2023

View reviewed changes

georgewrmarshall reviewed Mar 6, 2023

View reviewed changes

georgewrmarshall force-pushed the Fix-#17670 branch from 3901cb1 to 6c127e8 Compare March 6, 2023 22:55

georgewrmarshall reviewed Mar 6, 2023

View reviewed changes

georgewrmarshall force-pushed the Fix-#17670 branch from bd23f1c to 66fbdfd Compare March 8, 2023 20:16

georgewrmarshall force-pushed the Fix-#17670 branch from 66fbdfd to f1d616c Compare March 8, 2023 23:40

ayushj1910 and others added 7 commits March 9, 2023 18:07

Updated confirm-page-container-summary.component.js

a98d590

Updated snap-insight.js

71dd90b

Updated confirmation-warning-modal.js

8552c76

Updated create-new-vault.js

a58cd3d

Fixing import errors, linting errors and adding a story

e1d45c7

Updating storybook path

82476d0

Updating snapshot

d2ecd04

georgewrmarshall force-pushed the Fix-#17670 branch from f1d616c to d2ecd04 Compare March 10, 2023 02:07

georgewrmarshall self-assigned this Mar 10, 2023

georgewrmarshall approved these changes Mar 15, 2023

View reviewed changes

garrettbear approved these changes Mar 15, 2023

View reviewed changes

georgewrmarshall merged commit 1c613a4 into MetaMask:develop Mar 16, 2023

github-actions bot locked and limited conversation to collaborators Mar 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Part of #17670: Replace Typography with Text component #17959

Part of #17670: Replace Typography with Text component #17959

ayushj1910 commented Mar 2, 2023 •

edited

Loading

github-actions bot commented Mar 2, 2023

ayushj1910 commented Mar 2, 2023

georgewrmarshall left a comment •

edited

Loading

ayushj1910 commented Mar 2, 2023

digiwand commented Mar 3, 2023

ayushj1910 commented Mar 3, 2023

digiwand commented Mar 3, 2023 •

edited

Loading

ayushj1910 commented Mar 6, 2023

georgewrmarshall Mar 6, 2023

georgewrmarshall Mar 6, 2023

georgewrmarshall Mar 6, 2023

georgewrmarshall commented Mar 6, 2023

ayushj1910 commented Mar 8, 2023

georgewrmarshall commented Mar 8, 2023 •

edited

Loading

codecov bot commented Mar 8, 2023 •

edited

Loading

ayushj1910 commented Mar 15, 2023

georgewrmarshall left a comment

Part of #17670: Replace Typography with Text component #17959

Part of #17670: Replace Typography with Text component #17959

Conversation

ayushj1910 commented Mar 2, 2023 • edited Loading

Explanation

Screenshots/Screencaps

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Mar 2, 2023

ayushj1910 commented Mar 2, 2023

georgewrmarshall left a comment • edited Loading

Choose a reason for hiding this comment

ayushj1910 commented Mar 2, 2023

digiwand commented Mar 3, 2023

ayushj1910 commented Mar 3, 2023

digiwand commented Mar 3, 2023 • edited Loading

ayushj1910 commented Mar 6, 2023

georgewrmarshall Mar 6, 2023

Choose a reason for hiding this comment

georgewrmarshall Mar 6, 2023

Choose a reason for hiding this comment

georgewrmarshall Mar 6, 2023

Choose a reason for hiding this comment

georgewrmarshall commented Mar 6, 2023

ayushj1910 commented Mar 8, 2023

georgewrmarshall commented Mar 8, 2023 • edited Loading

codecov bot commented Mar 8, 2023 • edited Loading

Codecov Report

ayushj1910 commented Mar 15, 2023

georgewrmarshall left a comment

Choose a reason for hiding this comment

ayushj1910 commented Mar 2, 2023 •

edited

Loading

georgewrmarshall left a comment •

edited

Loading

digiwand commented Mar 3, 2023 •

edited

Loading

georgewrmarshall commented Mar 8, 2023 •

edited

Loading

codecov bot commented Mar 8, 2023 •

edited

Loading