Skip to content

refactor: Consolidate Token screen asset selectors#13985

Merged
gambinish merged 37 commits into
mainfrom
chore/consolidate-token-list-loops
Mar 17, 2025
Merged

refactor: Consolidate Token screen asset selectors#13985
gambinish merged 37 commits into
mainfrom
chore/consolidate-token-list-loops

Conversation

@gambinish

@gambinish gambinish commented Mar 12, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Description

From Wallet Screen → Tokens component, we are looping through the tokens from state several times in memory. This is not efficient. This PR introduces some improvements to the Tokens component, specifically:

  1. Breaks out several in memory loops to memoized selectors
  2. Creates selectors for native and staked asset, rather than building them manually in React
  3. Removes a few isPortfolioViewEnabled checks. Confirmed that we can begin to remove these.

Next step will be to refactor calculateFiatBalances

Related issues

Fixes: #13995

Manual testing steps

  1. App should remain functional without regressions
  2. Please check that token list renders as expected with balances. Network filter, sort, and token imports should still work as before.
  3. Hide zero balance setting should be respected. Keep in mind that if the "current network" is selected in the tokenNetworkFilter, that we are still showing zero balance tokens for native and staked assets.
  4. Testnet balances should not render when "Popular Networks" filter is selected.

Screenshots/Recordings

Screen.Recording.2025-03-13.at.8.40.58.PM.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.

@github-actions

github-actions Bot commented Mar 12, 2025

Copy link
Copy Markdown
Contributor

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

github-actions Bot commented Mar 13, 2025 •
edited by metamaskbot
Loading

Copy link
Copy Markdown
Contributor

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 4609dc5
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/d107d9b3-8238-4e9f-bf1b-5e8379337d2b

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

@github-actions

github-actions Bot commented Mar 14, 2025 •
edited by metamaskbot
Loading

Copy link
Copy Markdown
Contributor

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: a394c96
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/9e01ec77-0e86-4920-92e1-a8986604b979

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

@github-actions

github-actions Bot commented Mar 14, 2025 •
edited by metamaskbot
Loading

Copy link
Copy Markdown
Contributor

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 6561e3f
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/86f172a3-3dd0-45d9-bed0-5273d20f47ca

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

infiniteflower
infiniteflower previously approved these changes Mar 14, 2025
View reviewed changes
@github-actions

github-actions Bot commented Mar 14, 2025 •
edited by metamaskbot
Loading

Copy link
Copy Markdown
Contributor

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 242af20
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/9238dfb1-c89e-4490-8455-d49686654596

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 gambinish dismissed stale reviews from infiniteflower and salimtb via e0f98fc March 14, 2025 19:16
nickewansmith
nickewansmith approved these changes Mar 14, 2025
View reviewed changes

@nickewansmith nickewansmith left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Staking changes lgtm

OGPoyraz
OGPoyraz approved these changes Mar 17, 2025
View reviewed changes

@OGPoyraz OGPoyraz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Confirmation changes LGTM

@github-actions

github-actions Bot commented Mar 17, 2025 •
edited by metamaskbot
Loading

Copy link
Copy Markdown
Contributor

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 133892e
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/5260ec4d-6e9a-4a39-9710-7fc2f1af3db6

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

@github-actions

github-actions Bot commented Mar 17, 2025 •
edited by metamaskbot
Loading

Copy link
Copy Markdown
Contributor

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: c7717d4
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/59f76a0c-0856-4bd3-b08f-53e0f41713c8

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

@sonarqubecloud

sonarqubecloud Bot commented Mar 17, 2025

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
2 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

bergarces
bergarces reviewed Mar 17, 2025
View reviewed changes
Comment thread app/components/UI/Tokens/index.tsx
@github-project-automation github-project-automation Bot moved this from Needs dev review to Review finalised - Ready to be merged in PR review queue Mar 17, 2025
@gambinish gambinish added this pull request to the merge queue Mar 17, 2025
Merged via the queue into main with commit 40610f9 Mar 17, 2025
@gambinish gambinish deleted the chore/consolidate-token-list-loops branch March 17, 2025 22:25
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 17, 2025
@metamaskbot metamaskbot added the release-7.44.0 Issue or pull request that will be included in release 7.44.0 label Mar 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@bergarces bergarces bergarces approved these changes

@OGPoyraz OGPoyraz OGPoyraz approved these changes

@tommasini tommasini Awaiting requested review from tommasini

@owencraston owencraston Awaiting requested review from owencraston

@sahar-fehri sahar-fehri Awaiting requested review from sahar-fehri

@Prithpal-Sooriya Prithpal-Sooriya Awaiting requested review from Prithpal-Sooriya

@salimtb salimtb Awaiting requested review from salimtb

@infiniteflower infiniteflower Awaiting requested review from infiniteflower

+1 more reviewer

@nickewansmith nickewansmith nickewansmith approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

release-7.44.0 Issue or pull request that will be included in release 7.44.0 solana-beta team-assets

Projects

PR review queue
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Refactor]: Consolidate token list loops into memoized selector

10 participants

@gambinish @codecov-commenter @salimtb @bergarces @nickewansmith @OGPoyraz @Prithpal-Sooriya @infiniteflower @sahar-fehri @metamaskbot