feat: `PortfolioView` #28593

gambinish · 2024-11-21T06:25:48Z

Description

This consolidates the changes from a series of 3 Multichain Asset List PRs that built on each other:

Product code (feature branch): feat: Multichain AssetList #28386
Unit tests: fix: Multichain AssetList unit tests #28451
e2e tests: fix: Multichain AssetList e2e tests #28524

We created separate branches for rapid iteration and isolated testing. The code is now cleaner and stable enough for review and merge into develop, gated by the PORTFOLIO_VIEW feature flag.

We will introduce another PR to remove this feature flag when we are ready to ship it.

Related issues

Fixes: https://github.com/orgs/MetaMask/projects/85/views/35?pane=issue&itemId=82217837

Manual testing steps

PORTFOLIO_VIEW=1 yarn webpack --watch

View tokens across all networks in one unified list.
Filter tokens by selected network
Crosschain navigation:
- Token detail pages update to display data from the appropriate network.
- Send/Swap actions automatically adjust the selected network for user convenience.
- Ensure that network switch is functional, and sends/swaps happen on correct chain.

Known caveats:

POL native token market data not populating. Will be addressed here: fix: market data for native tokens with non zero addresses #28584 and fix: price fetching for native tokens with non zero addresses core#4952
Native token swapping on different network than selected network swaps incorrect token: Fix #28534 - Show correct swap value from asset details #28587
Multichain token detection experimental draft: feat: multichain token detection #28380

Screenshots/Recordings

Screen.Recording.2024-11-20.at.11.19.12.AM.mov

Screen.Recording.2024-11-20.at.10.20.44.PM.mov

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.

…-balances' of github.com:MetaMask/metamask-extension into e2e-tests-token-list-aggregated-balance

…bject Model (#28383) ## **Description** - Migrate e2e tests `test/e2e/tests/tokens/nft/auto-detect-nft.spec.js` to TS and Page Object Model, to reduce flakiness. - Migrate e2e tests `test/e2e/tests/tokens/nft/import-nft.spec.js` to TS and Page Object Model, to reduce flakiness. - Create page classe functions for nft - Deprecate/remove old functions in `helper.js` [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27155?quickstart=1) ## **Related issues** Fixes: #28388 ## **Manual testing steps** Check code readability, make sure tests pass. ## **Screenshots/Recordings**  ### **Before**  ### **After**  ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension 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.

## **Description** There is a race condition in our testing setup, which causes that the expected fixtures state, not being there when we start the test. This has been surfaced in [this branch](#28277), where the account tracker multi polling is being added. The problem is that if we don't have the AccountTracker in state when the `resetState` function is called (at the beginning of wallet loading) the balance will remain loading until we refresh the wallet. Edit: performing the load state early in the test setup fixes the issue. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28421?quickstart=1) ## **Related issues** Fixes: MetaMask/MetaMask-planning#3627 ## **Manual testing steps** 1. Check all tests continue to pass 2. Check this changes fix this branch ENS test https://github.com/MetaMask/metamask-extension/pull/28402/files#diff-1acb7898d60977530c97169551d22dbe477a4e3aeb74f1f14bf2eea0b4d75d35 . Alternatively, see videos below with before and after behaviours ## **Screenshots/Recordings**  ### **Before** https://github.com/user-attachments/assets/8f50ec04-cf96-478e-9c3c-dce54254a628 ### **After** https://github.com/user-attachments/assets/0f109b1a-9289-48d9-b337-d51890c9d448 ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension 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.

## **Description**  This PR reintroduces the happy path send flow tests for BTC ## **Related issues** Fixes: https://github.com/MetaMask/accounts-planning/issues/668 ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings**  ### **Before**  ### **After**  ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] 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](https://jsdoc.app/) format if applicable - [ ] 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: Charly Chevalier <charly.chevalier@consensys.net>

…#28403)  ## **Description** Previously, some gas fee parameters defaulted to dApp suggested fee values. This was problematic, because these values are always defined, even when the user changed to a custom priority. To fix the issue, we now only show the fees contained in txParams, trusting they match the user selected priority level in the gas fees modal.  [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28403?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** See screen recording below. ## **Screenshots/Recordings**  ### **Before**  https://github.com/user-attachments/assets/3874d3b6-ad96-4fd9-9f40-361b38a0253b ### **After**  ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] 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](https://jsdoc.app/) format if applicable - [ ] 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 reverts commit aa3063f.

…n--prep-updates

…epath

…-balances' into e2e-tests-token-list-aggregated-balance

metamaskbot · 2024-11-21T06:56:37Z

Builds ready [1a8cb68]

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, 6, 7
- common: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1843 ± 101 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	283	2308	1755	392	188
		domContentLoaded	1529	2226	1805	202	97
		load	1546	2311	1843	210	101
		domInteractive	13	99	34	18	9
		backgroundConnect	9	99	43	25	12
		firstReactRender	53	135	99	23	11
		getState	5	57	20	18	8
		initialActions	0	1	0	0	0
		loadScripts	1096	1722	1317	180	86
		setupStore	6	61	13	14	7
		uiStartup	1734	2566	2047	228	109

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: 8.02 KiB (0.10%)
common: 1.85 KiB (0.02%)

salimtb · 2024-11-21T13:24:42Z

hey @gambinish
1 bug found , token detail page redirect to the wrong block explorer ( see the video below )

portfolio.mov

metamaskbot · 2024-11-21T15:15:02Z

Builds ready [a8de3b2]

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, 6, 7
- common: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1980 ± 122 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	291	2251	1774	485	233
		domContentLoaded	1621	2922	1944	259	124
		load	1670	2949	1980	254	122
		domInteractive	17	67	35	12	6
		backgroundConnect	11	98	43	28	13
		firstReactRender	61	306	159	80	38
		getState	5	87	27	28	14
		initialActions	0	1	0	0	0
		loadScripts	1162	2192	1422	205	99
		setupStore	6	61	11	12	6
		uiStartup	1832	3739	2321	407	195

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: 8.02 KiB (0.10%)
common: 1.85 KiB (0.02%)

ui/pages/asset/components/token-asset.tsx

salimtb · 2024-11-21T17:13:57Z

LGTM ✅

metamaskbot · 2024-11-21T17:53:51Z

Builds ready [bc6c4df]

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, 6, 7
- common: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (2006 ± 57 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1792	2332	2003	121	58
		domContentLoaded	1777	2207	1959	108	52
		load	1783	2288	2006	119	57
		domInteractive	17	219	54	53	25
		backgroundConnect	9	137	47	35	17
		firstReactRender	67	199	121	28	14
		getState	5	66	21	20	10
		initialActions	0	1	0	0	0
		loadScripts	1269	1626	1437	99	48
		setupStore	6	62	12	12	6
		uiStartup	2003	2476	2242	129	62

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: 8.22 KiB (0.11%)
common: 1.85 KiB (0.02%)

ccharly · 2024-11-27T13:30:08Z

ui/components/app/wallet-overview/coin-buttons.tsx

@@ -352,13 +375,29 @@ const CoinButtons = ({
      }
      ///: END:ONLY_INCLUDE_IF
      default: {
+        trackEvent(


I think this is wrong. We already have this event above. Probably a bad conflict resolution? 😅

We are probably sending this event twice now.

gambinish and others added 30 commits November 12, 2024 18:21

fix: Tweak unit test

40b36f5

fix: Tweak unit test

b24e25a

chore: Add low risk selectors to prepare for multichain asset list

18aa7e8

fix: Add unit testable utility in preperation for unified asset list

e127f07

fix: Revert changes to locales, can't do this in advance

7307904

fix: Update test id in custom-token-add-approve.spec.js

b5da71e

chore: Remove console.log

e939574

fix: Update token-sort.spec.ts testid

c71bee0

fix: Update other instances of testid Import

f9fa8da

Merge branch 'feat/mmassets-432_network-filter-extension--integration…

d0f53f2

…-balances' of github.com:MetaMask/metamask-extension into e2e-tests-token-list-aggregated-balance

test fixes, bump, lavamoat fix

65c25b1

Fix tests

678ba8c

chore: Add new unit test for findAssetByAddress util

1a1737a

fix: Update nullish search bug in chainSupprotsPricing

41adb56

Test fix

1237c16

Fix test

841aa63

fix: Lint error in util.test.ts

aa3063f

Revert "fix: Lint error in util.test.ts"

0055289

This reverts commit aa3063f.

Merge branch 'develop' into feat/mmassets-432_network-filter-extensio…

c006bb6

…n--prep-updates

fix: @metamask/assets-controllers should be imported before local fil…

25c27ea

…epath

chore: Merge main, address conflicts

ce6f520

Merge branch 'feat/mmassets-432_network-filter-extension--integration…

de3a412

…-balances' into e2e-tests-token-list-aggregated-balance

chore: Merge in prep updates to reduces file size diff

6ceb087

Merge branch 'feat/mmassets-432_network-filter-extension--integration…

ae1ea97

…-balances' into e2e-tests-token-list-aggregated-balance

fix: Remove duplicate import entry for getCurrentChainId

dcf39fd

Merge branch 'feat/mmassets-432_network-filter-extension--integration…

d215c45

…-balances' into e2e-tests-token-list-aggregated-balance

gambinish marked this pull request as ready for review November 21, 2024 06:56

gambinish requested review from a team as code owners November 21, 2024 06:56

sahar-fehri added 2 commits November 21, 2024 15:29

Merge branch 'develop' into feat/portfolio-view

a8de3b2

fix: fix blockexplorer redirection in token details page

a64e2ff

salimtb reviewed Nov 21, 2024

View reviewed changes

ui/pages/asset/components/token-asset.tsx Outdated Show resolved Hide resolved

gambinish added 3 commits November 21, 2024 08:21

fix: block explorer link

3612ccb

fix: Block explorer bug, redirect if no explorer present

874c372

fix: Cleanup

660ed55

salimtb previously approved these changes Nov 21, 2024

View reviewed changes

chore: Merge main, address conflicts

bc6c4df

gambinish dismissed salimtb’s stale review via bc6c4df November 21, 2024 17:17

salimtb approved these changes Nov 21, 2024

View reviewed changes

NidhiKJha approved these changes Nov 21, 2024

View reviewed changes

gambinish added this pull request to the merge queue Nov 21, 2024

This was referenced Nov 21, 2024

feat: Multichain AssetList #28386

Closed

fix: Multichain AssetList unit tests #28451

Closed

fix: Multichain AssetList e2e tests #28524

Closed

feat: multichain token detection #28380

Merged

Merged via the queue into develop with commit a04b34b Nov 21, 2024
75 checks passed

gambinish deleted the feat/portfolio-view branch November 21, 2024 18:14

github-actions bot locked and limited conversation to collaborators Nov 21, 2024

metamaskbot added the release-12.9.0 Issue or pull request that will be included in release 12.9.0 label Nov 21, 2024

ccharly reviewed Nov 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: `PortfolioView` #28593

feat: `PortfolioView` #28593

gambinish commented Nov 21, 2024 •

edited

Loading

metamaskbot commented Nov 21, 2024

salimtb commented Nov 21, 2024

metamaskbot commented Nov 21, 2024

salimtb commented Nov 21, 2024

metamaskbot commented Nov 21, 2024

ccharly Nov 27, 2024

feat: PortfolioView #28593

feat: PortfolioView #28593

Conversation

gambinish commented Nov 21, 2024 • edited Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Pre-merge author checklist

Pre-merge reviewer checklist

metamaskbot commented Nov 21, 2024

salimtb commented Nov 21, 2024

metamaskbot commented Nov 21, 2024

salimtb commented Nov 21, 2024

metamaskbot commented Nov 21, 2024

ccharly Nov 27, 2024

Choose a reason for hiding this comment

feat: `PortfolioView` #28593

feat: `PortfolioView` #28593

gambinish commented Nov 21, 2024 •

edited

Loading