fix: add percent change for 24h on asset list #14839

sahar-fehri · 2025-04-24T10:42:55Z

Description

Adds asset list 24h price change

Related issues

Fixes:

Manual testing steps

Switch to a Solana account
You should be able to see the 24h percent change on the asset list and aggregated balance

Screenshots/Recordings

Before

After

Pre-merge author checklist

I’ve followed MetaMask Contributor Docs and MetaMask Mobile 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 · 2025-04-24T10:43:10Z

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.

github-actions · 2025-04-24T10:44:18Z

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 5427bd4
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/4adb105c-b3a3-46ad-bc01-5a2724c65d2a

Note

You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

Check the documentation if you have any doubts on how to understand the failure on bitrise

sahar-fehri · 2025-04-24T13:46:11Z

app/components/hooks/useMultichainBalances/useMultichainBalances.ts

-      return !isTestNet(chainId) && isEvmAccountType(account.type);
-      ///: END:ONLY_INCLUDE_IF
-
-      // Note: This code marked as unreachable however when the above block gets removed after code fencing this return becomes necessary


Removed this because now we also display 24h change for non EVM

github-actions · 2025-04-24T18:02:39Z

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: b0b6244
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/6f7bca76-0d19-406a-9c76-81692f2a561a

Note

You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

Check the documentation if you have any doubts on how to understand the failure on bitrise

gambinish · 2025-04-24T18:28:30Z

Noticing a couple things:

The percent change on the main token list is different than the AssetOverview screen. Is this expected?

Screen.Recording.2025-04-24.at.11.22.08.AM.mov

For USDC, I'm noticing that it's -0.00%. This seems like an odd display value. On AssetOverview we are showing a different value of +0.00%. Which is correct? Neither seems logical, and I wonder what the intended behavior should be. Maybe we can use formatWithThreshold to display -<0.01% in red when the difference is below the smallest display value (say -0.00001%). We could also do something similar if the trend is upwards, and display +<0.01% in green.

gambinish

Approving since my comments aren't technically regressions, and this is the existing behavior.

We should probably revisit this in the future.

sonarqubecloud · 2025-04-24T19:50:22Z

Quality Gate passed

Issues
6 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

sahar-fehri · 2025-04-24T21:30:58Z

TODO: update title with cp-7.46.0

The base branch was changed.

owencraston · 2025-04-25T01:19:24Z

In testing this seemed to work fairly well but I am noticing a few things. Namely the fact that the percentage change is different on the token list vs the asset overview. I tested with eth accounts and noticed this is the case as well.

owencraston · 2025-04-25T01:31:12Z

app/component-library/components-temp/Price/AggregatedPercentage/NonEvmAggregatedPercentage.tsx

+    nonEvmChainId,
+  );
+
+  const nonEvmFiatBalancesArray = Object.entries(


Both this and nonEvmAccountBalance should be memoized. The Object.entries will eagerly create a new object on every render which can trigger more re renders. Instead we can warp these two in a useMemo.

owencraston · 2025-04-25T01:31:54Z

app/component-library/components-temp/Price/AggregatedPercentage/NonEvmAggregatedPercentage.tsx

+  const nonEvmChainId = useSelector(selectSelectedNonEvmNetworkChainId);
+
+  const nonEvmAccountBalance = getMultichainNetworkAggregatedBalance(
+    account as unknown as InternalAccount,


I think the casting is unnecessary here. If not we should explicit mark the return type of this selector to be InternalAccount.

I can make the return type explicit in the selector;
Although i think i might need to still cast because the return type is InternalAccount | undefined

owencraston · 2025-04-25T01:34:02Z

app/component-library/components-temp/Price/AggregatedPercentage/NonEvmAggregatedPercentage.tsx

+  const amountChange = totalNonEvmBalance - totalNonEvmBalance1dAgo;
+
+  const percentageChange =
+    ((totalNonEvmBalance - totalNonEvmBalance1dAgo) / totalNonEvmBalance1dAgo) *


should we make sure that the totalNonEvmBalance1dAgo is greater than 0 in order to avoid a divide by 0 issue? I believe if the value is 0 we will see infinity. Is this a valid case or am I being paranoid?

Also can we just reuse the amountChange var from above?

Good point; i can add a check but i think that to be in that case everything would have to be zero
if
const totalNonEvmBalance1dAgo = getNonEvmTotalFiat1dAgo();
is zero then totalNonEvmBalance is also zero in which case we return null

if (!totalNonEvmBalance) { return null; }

owencraston · 2025-04-25T01:34:33Z

app/component-library/components-temp/Price/AggregatedPercentage/NonEvmAggregatedPercentage.tsx

+  );
+};
+
+export default NonEvmAggregatedPercentage;


lets wrap this whole thing in a React.memo

owencraston · 2025-04-25T01:35:26Z

app/components/UI/AssetOverview/AssetOverview.tsx

@@ -255,7 +278,10 @@ const AssetOverview: React.FC<AssetOverviewProps> = ({
      )),
    [handleSelectTimePeriod, timePeriod],
  );
-  const itemAddress = safeToChecksumAddress(asset.address);
+
+  const itemAddress = isEvmSelected


In see we are doing lots of this across the app. Perhaps we can turn this into a util.

I haven't touched this component i think you see this change now because the branch i was branched off of was merged; good point though can add a TODO seems like it would not be a small refactor ^^

owencraston · 2025-04-25T01:36:54Z

app/components/UI/AssetOverview/TokenDetails/TokenDetails.tsx

+  );
+
+  ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps)
+  const allMultichainAssetsRates = useSelector(selectMultichainAssetsRates);


Lets make sure this selector is a deep equal selector. I did not verify that it is/is not. Just wanted to mention it.

Will update this, i also think this was not originally modifed in this PR but worth improving 🙏

owencraston · 2025-04-25T01:39:55Z

app/component-library/components-temp/Price/AggregatedPercentage/NonEvmAggregatedPercentage.tsx

+    fiatAmount: Number(amount),
+  }));
+
+  const getNonEvmTotalFiat1dAgo = () => {


Can we also memoize this?

owencraston · 2025-04-25T01:41:55Z

app/component-library/components-temp/Price/AggregatedPercentage/NonEvmAggregatedPercentage.tsx

+    <View style={styles.wrapper}>
+      <SensitiveText
+        isHidden={privacyMode}
+        length="10"


Is the length always going to be 10?

owencraston · 2025-04-25T01:42:36Z

app/component-library/components-temp/Price/AggregatedPercentage/NonEvmAggregatedPercentage.tsx

+    percentageTextColor = TextColor.Alternative;
+  }
+
+  const formattedPercentage = isValidAmount(percentageChange)


we should move these into utils.

sahar-fehri · 2025-04-25T13:29:58Z

closing in favor of #14892

sahar-fehri · 2025-04-25T14:18:58Z

hings. Namely the fact that the p

This is happening also in extension; for evms too; both values are computed differently;
One mainly from price-api (historical prices endpoint) (the one on the asset details)
While the percentage on the aggregated balance view is computed based on the user's balance and the market data we get from price api.

gambinish and others added 24 commits April 14, 2025 09:30

feat: init

766c563

chore: Add Solana MarketDetails

a52f4c7

chore: Add price chart and 24 hour diff

4597d04

chore: Cleanup

16ab115

chore: Merge main

b4ac7d5

Merge branch 'main' into feat/solana-token-details-2

8e819ce

chore: Merge main, address conflicts

8996f86

chore: Integrate non-evm historical prices into price chart

2844d0f

chore: Uncomment code

2ed94ce

fix: Update market details

35cb919

feat: Add non evm market data and token details

e6fc961

chore: Update asset price calculation to account for non evm asset types

bc27757

fix: Cleanup

9575438

chore: Merge main

d079304

fix: Cleanup and memoize callbacks

73e8397

fix: Cleanup, lint, tweak unit tests

bc51d06

fix: lint:tsc

1725579

fix: Lint

a9a34a7

fix: Update incorrect unit tests

5102a40

fix: Update AssetOverview test

3df6ffe

fix: Price tests

cdb9327

fix: Remove unused useTokenHistoricalPricesV3 hook

9969265

chore: codefencing

283eb7b

fix: add percent change for 24h on asset list

5427bd4

metamaskbot added the team-assets label Apr 24, 2025

sahar-fehri added Run Smoke E2E Triggers smoke e2e on Bitrise no-changelog Indicates no external facing user changes, therefore no changelog documentation needed labels Apr 24, 2025

fix: fix type

fffb8e7

sahar-fehri commented Apr 24, 2025

View reviewed changes

fix: fix test

b0b6244

sahar-fehri added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Apr 24, 2025

sahar-fehri marked this pull request as ready for review April 24, 2025 17:17

sahar-fehri requested review from a team as code owners April 24, 2025 17:17

gambinish previously approved these changes Apr 24, 2025

View reviewed changes

sahar-fehri added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Apr 24, 2025

Base automatically changed from feat/solana-token-details-2 to main April 24, 2025 22:53

owencraston reviewed Apr 25, 2025

View reviewed changes

sahar-fehri closed this Apr 25, 2025

github-actions bot locked and limited conversation to collaborators Apr 25, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: add percent change for 24h on asset list #14839

fix: add percent change for 24h on asset list #14839

sahar-fehri commented Apr 24, 2025 •

edited

Loading

github-actions bot commented Apr 24, 2025

github-actions bot commented Apr 24, 2025 •

edited by metamaskbot

Loading

sahar-fehri Apr 24, 2025

github-actions bot commented Apr 24, 2025 •

edited by metamaskbot

Loading

gambinish commented Apr 24, 2025

gambinish left a comment

sonarqubecloud bot commented Apr 24, 2025

sahar-fehri commented Apr 24, 2025

owencraston commented Apr 25, 2025

owencraston Apr 25, 2025

owencraston Apr 25, 2025

sahar-fehri Apr 25, 2025

owencraston Apr 25, 2025

owencraston Apr 25, 2025

sahar-fehri Apr 25, 2025

owencraston Apr 25, 2025

owencraston Apr 25, 2025 •

edited

Loading

sahar-fehri Apr 25, 2025

owencraston Apr 25, 2025

sahar-fehri Apr 25, 2025

owencraston Apr 25, 2025

owencraston Apr 25, 2025

owencraston Apr 25, 2025

sahar-fehri commented Apr 25, 2025

sahar-fehri commented Apr 25, 2025

fix: add percent change for 24h on asset list #14839

fix: add percent change for 24h on asset list #14839

Conversation

sahar-fehri commented Apr 24, 2025 • edited Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Apr 24, 2025

github-actions bot commented Apr 24, 2025 • edited by metamaskbot Loading

Bitrise

Choose a reason for hiding this comment

github-actions bot commented Apr 24, 2025 • edited by metamaskbot Loading

Bitrise

gambinish commented Apr 24, 2025

gambinish left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Apr 24, 2025

Quality Gate passed

sahar-fehri commented Apr 24, 2025

owencraston commented Apr 25, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

owencraston Apr 25, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sahar-fehri commented Apr 25, 2025

sahar-fehri commented Apr 25, 2025

sahar-fehri commented Apr 24, 2025 •

edited

Loading

github-actions bot commented Apr 24, 2025 •

edited by metamaskbot

Loading

github-actions bot commented Apr 24, 2025 •

edited by metamaskbot

Loading

owencraston Apr 25, 2025 •

edited

Loading