chore: Update `asset-list` to Typescript, remove PropTypes #26952

gambinish · 2024-09-05T21:04:24Z

Description

I'm working on Token sorting, and since I was in the area, I took the opportunity to update the components I was touching to Typescript. This PR handles those changes, and I'll work on the actual sorting logic in a separate branch that I will merge with this one.

Related issues

First step of: https://consensyssoftware.atlassian.net/browse/MMASSETS-356

Manual testing steps

Nothing should be different visually or functionally. ~~I left a comment on a suspicious snapshot test where I can't explain the diff, other than the original snapshot not being accurate based on the mocks~~ 👈 no longer relevant.

Screenshots/Recordings

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
I've completed the PR template to the best of my ability
I’ve included tests if applicable
I’ve documented my code using JSDoc format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). 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.

github-actions · 2024-09-05T21:04:37Z

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/app/assets/token-cell/__snapshots__/token-cell.test.js.snap

…nsion

…ub.com:MetaMask/metamask-extension into feat/mmassets-356_sort-import-tokens-extension

…nsion

…ub.com:MetaMask/metamask-extension into feat/mmassets-356_sort-import-tokens-extension

…nsion

…ub.com:MetaMask/metamask-extension into feat/mmassets-356_sort-import-tokens-extension

codecov · 2024-09-12T22:16:10Z

Codecov Report

Attention: Patch coverage is 86.95652% with 3 lines in your changes missing coverage. Please review.

Project coverage is 69.96%. Comparing base (9459903) to head (aa566e4).

Files with missing lines	Patch %	Lines
ui/components/app/assets/asset-list/asset-list.tsx	83.33%	1 Missing ⚠️
ui/components/app/assets/token-list/token-list.tsx	50.00%	1 Missing ⚠️
...nts/multichain/token-list-item/token-list-item.tsx	92.31%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           develop   #26952   +/-   ##
========================================
  Coverage    69.95%   69.96%           
========================================
  Files         1441     1441           
  Lines        50099    50102    +3     
  Branches     14007    14012    +5     
========================================
+ Hits         35046    35049    +3     
  Misses       15053    15053

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

metamaskbot · 2024-09-12T22:26:18Z

Builds ready [5c5b22e]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1802 ± 92 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	280	2318	1656	474	227
		domContentLoaded	1560	2302	1772	179	86
		load	1569	2317	1802	191	92
		domInteractive	12	184	40	35	17

Bundle size diffs

background: 0 Bytes (0.00%)
ui: -1.27 KiB (-0.02%)
common: 0 Bytes (0.00%)

test/e2e/tests/tokens/custom-token-send-transfer.spec.js

metamaskbot · 2024-09-13T22:12:43Z

Builds ready [6fd9aa1]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1974 ± 93 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	240	2502	1899	431	207
		domContentLoaded	1677	2421	1955	195	93
		load	1719	2429	1974	194	93
		domInteractive	14	84	38	16	8

Bundle size diffs

background: 0 Bytes (0.00%)
ui: -1.3 KiB (-0.02%)
common: 0 Bytes (0.00%)

sahar-fehri · 2024-09-16T13:32:12Z

LGTM! Thnx! 🚀

…nsion

darkwing · 2024-09-16T13:38:09Z

ui/components/app/assets/asset-list/asset-list.tsx

@@ -150,6 +169,7 @@ const AssetList = ({ onClickAsset, showTokensLinks }) => {
      {detectedTokens.length > 0 &&
        !isTokenDetectionInactiveOnNonMainnetSupportedNetwork && (
          <DetectedTokensBanner
+            className=""


Should we make className not required if passing it an empty string is the solution?

Will need to converted detected-token-banner.js to Typescript as well in order to do this properly. Do you think it's worth it to also handle this refactor in this PR?

ui/components/multichain/token-list-item/token-list-item.test.tsx

…ub.com:MetaMask/metamask-extension into feat/mmassets-356_sort-import-tokens-extension

sonarqubecloud · 2024-09-16T14:23:23Z

Quality Gate passed

Issues
9 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

metamaskbot · 2024-09-16T14:31:26Z

Builds ready [822caf4]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1476 ± 54 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1342	1731	1480	118	57
		domContentLoaded	1335	1681	1462	108	52
		load	1343	1731	1476	113	54
		domInteractive	13	61	32	15	7

Bundle size diffs

background: 0 Bytes (0.00%)
ui: -1.3 KiB (-0.02%)
common: 0 Bytes (0.00%)

NidhiKJha

LGTM

metamaskbot · 2024-09-16T15:06:54Z

Builds ready [aa566e4]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1776 ± 78 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	236	2179	1646	486	234
		domContentLoaded	1529	2111	1747	151	72
		load	1537	2128	1776	162	78
		domInteractive	16	77	37	17	8

Bundle size diffs

background: 0 Bytes (0.00%)
ui: -1.3 KiB (-0.02%)
common: 0 Bytes (0.00%)

gambinish added 3 commits September 5, 2024 12:23

Convert js files to ts

da7a49d

Update to non-deprecated modal

ede7e00

Update snapshots

7d8aee9

github-actions bot added the team-assets label Sep 5, 2024

gambinish changed the title ~~Feat/mmassets 356 sort import tokens extension~~ chore: Update asset-list to Typescript, remove PropTypes Sep 5, 2024

gambinish commented Sep 5, 2024

View reviewed changes

ui/components/app/assets/token-cell/__snapshots__/token-cell.test.js.snap Outdated Show resolved Hide resolved

gambinish and others added 3 commits September 5, 2024 14:30

Use types instead of interfaces

24026bc

Prefer object destructuring

b787923

Merge branch 'develop' into feat/mmassets-356_sort-import-tokens-exte…

168b32e

…nsion

gambinish requested review from bergeron, sahar-fehri and salimtb September 6, 2024 15:29

gambinish and others added 13 commits September 6, 2024 13:00

Merge branch 'develop' into feat/mmassets-356_sort-import-tokens-exte…

3052a92

…nsion

Merge branch 'develop' into feat/mmassets-356_sort-import-tokens-exte…

a902a3f

…nsion

Merge branch 'feat/mmassets-356_sort-import-tokens-extension' of gith…

094fecb

…ub.com:MetaMask/metamask-extension into feat/mmassets-356_sort-import-tokens-extension

Merge branch 'develop' into feat/mmassets-356_sort-import-tokens-exte…

e348b10

…nsion

Merge branch 'develop' into feat/mmassets-356_sort-import-tokens-exte…

5055394

…nsion

Merge branch 'develop' into feat/mmassets-356_sort-import-tokens-exte…

d424655

…nsion

Merge branch 'feat/mmassets-356_sort-import-tokens-extension' of gith…

3a7c669

…ub.com:MetaMask/metamask-extension into feat/mmassets-356_sort-import-tokens-extension

Merge branch 'develop' into feat/mmassets-356_sort-import-tokens-exte…

4a45990

…nsion

Merge branch 'feat/mmassets-356_sort-import-tokens-extension' of gith…

43d3c3b

…ub.com:MetaMask/metamask-extension into feat/mmassets-356_sort-import-tokens-extension

Use tsignore for string variable in token-cell

0f7b5ac

Remove unecessary ts-ignores, and unskip unit tests

41f528a

Merge main, address conflicts in asset-list.tsx

6dd32e4

Update snapshots

5c5b22e

hjetpoluru reviewed Sep 13, 2024

View reviewed changes

test/e2e/tests/tokens/custom-token-send-transfer.spec.js Outdated Show resolved Hide resolved

Remove foo

aeee661

sahar-fehri previously approved these changes Sep 16, 2024

View reviewed changes

Merge branch 'develop' into feat/mmassets-356_sort-import-tokens-exte…

822caf4

…nsion

darkwing reviewed Sep 16, 2024

View reviewed changes

NidhiKJha previously approved these changes Sep 16, 2024

View reviewed changes

NidhiKJha reviewed Sep 16, 2024

View reviewed changes

ui/components/multichain/token-list-item/token-list-item.test.tsx Outdated Show resolved Hide resolved

salimtb previously approved these changes Sep 16, 2024

View reviewed changes

gambinish added 2 commits September 16, 2024 07:20

Address nit in token-list-item test to make more dry

6cdd04b

Merge branch 'feat/mmassets-356_sort-import-tokens-extension' of gith…

aa566e4

…ub.com:MetaMask/metamask-extension into feat/mmassets-356_sort-import-tokens-extension

gambinish dismissed stale reviews from salimtb, NidhiKJha, sahar-fehri, and bergeron via aa566e4 September 16, 2024 14:20

gambinish requested review from NidhiKJha, darkwing, bergeron, salimtb and sahar-fehri September 16, 2024 14:50

salimtb approved these changes Sep 16, 2024

View reviewed changes

NidhiKJha approved these changes Sep 16, 2024

View reviewed changes

gambinish merged commit 8550afc into develop Sep 16, 2024
78 checks passed

gambinish deleted the feat/mmassets-356_sort-import-tokens-extension branch September 16, 2024 15:07

github-actions bot locked and limited conversation to collaborators Sep 16, 2024

metamaskbot added the release-12.6.0 Issue or pull request that will be included in release 12.6.0 label Sep 16, 2024

metamaskbot added release-12.5.0 Issue or pull request that will be included in release 12.5.0 and removed release-12.6.0 Issue or pull request that will be included in release 12.6.0 labels Sep 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Update `asset-list` to Typescript, remove PropTypes #26952

chore: Update `asset-list` to Typescript, remove PropTypes #26952

gambinish commented Sep 5, 2024 •

edited

Loading

github-actions bot commented Sep 5, 2024

codecov bot commented Sep 12, 2024 •

edited

Loading

metamaskbot commented Sep 12, 2024

metamaskbot commented Sep 13, 2024

sahar-fehri commented Sep 16, 2024

darkwing Sep 16, 2024

gambinish Sep 16, 2024

sonarqubecloud bot commented Sep 16, 2024

metamaskbot commented Sep 16, 2024

NidhiKJha left a comment

metamaskbot commented Sep 16, 2024

chore: Update asset-list to Typescript, remove PropTypes #26952

chore: Update asset-list to Typescript, remove PropTypes #26952

Conversation

gambinish commented Sep 5, 2024 • edited Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Sep 5, 2024

codecov bot commented Sep 12, 2024 • edited Loading

Codecov Report

metamaskbot commented Sep 12, 2024

metamaskbot commented Sep 13, 2024

sahar-fehri commented Sep 16, 2024

darkwing Sep 16, 2024

Choose a reason for hiding this comment

gambinish Sep 16, 2024

Choose a reason for hiding this comment

sonarqubecloud bot commented Sep 16, 2024

Quality Gate passed

metamaskbot commented Sep 16, 2024

NidhiKJha left a comment

Choose a reason for hiding this comment

metamaskbot commented Sep 16, 2024

chore: Update `asset-list` to Typescript, remove PropTypes #26952

chore: Update `asset-list` to Typescript, remove PropTypes #26952

gambinish commented Sep 5, 2024 •

edited

Loading

codecov bot commented Sep 12, 2024 •

edited

Loading