Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix hideZeroBalance setting for non evm asset list #30816

Merged
merged 5 commits into from
Mar 7, 2025

Conversation

sahar-fehri
Copy link
Contributor

@sahar-fehri sahar-fehri commented Mar 6, 2025

Description

PR to respect the hideZeroBalance setting in the asset list for non-evm tokens.

Open in GitHub Codespaces

Related issues

Fixes: #30623

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Before

After

Screen.Recording.2025-03-06.at.13.46.30.mov

Pre-merge author checklist

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.

Copy link
Contributor

github-actions bot commented Mar 6, 2025

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.

@sahar-fehri sahar-fehri marked this pull request as ready for review March 6, 2025 12:47
@@ -73,3 +74,103 @@ describe('getAssetsRates', () => {
expect(() => getAssetsRates(invalidState)).toThrow();
});
});

describe('getMultiChainAssets', () => {
Copy link
Contributor

@Prithpal-Sooriya Prithpal-Sooriya Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit (can be done in a separate PR) - can we add some "equality" check to help reduce selectors from returning new data and causing re-renders. E.g.

it('returns the same data if state does not change', () => {
  const result1 = getMultiChainAssets(...)
  const result2 = getMultiChainAssets(...)
  expect(result1 === result2).toBe(true)
})

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@metamaskbot
Copy link
Collaborator

Builds ready [29b3073]
Page Load Metrics (1624 ± 52 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint50119061513336161
domContentLoaded14811880160010450
load14891904162410852
domInteractive258634157
backgroundConnect96023168
firstReactRender1466282110
getState45014168
initialActions0452105
loadScripts1094140311989244
setupStore712811
uiStartup16992163184212459

@metamaskbot
Copy link
Collaborator

Builds ready [2f3180d]
Page Load Metrics (1625 ± 56 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint14441963162912057
domContentLoaded14321876159510450
load14461971162511756
domInteractive2286442110
backgroundConnect12135382914
firstReactRender1472312311
getState521942
initialActions01000
loadScripts1021143311919244
setupStore85417168
uiStartup16612197183912560

@metamaskbot
Copy link
Collaborator

Builds ready [3c5880e]
Page Load Metrics (1982 ± 108 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint43423841878390187
domContentLoaded167823741947225108
load168324221982226108
domInteractive28163604421
backgroundConnect981332110
firstReactRender1674382110
getState56126209
initialActions01000
loadScripts12151817146919493
setupStore85918168
uiStartup195627882248257123
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 111 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@sahar-fehri sahar-fehri added this pull request to the merge queue Mar 7, 2025
Merged via the queue into main with commit fb3f6ad Mar 7, 2025
75 checks passed
@sahar-fehri sahar-fehri deleted the fix/fix-asset-list-for-non-evm branch March 7, 2025 18:37
@github-actions github-actions bot locked and limited conversation to collaborators Mar 7, 2025
@metamaskbot metamaskbot added the release-12.15.0 Issue or pull request that will be included in release 12.15.0 label Mar 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.15.0 Issue or pull request that will be included in release 12.15.0 team-assets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: 0 balance asset that an account previously owned is still displayed in the assets list
4 participants