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

[MC 0.5][FEATURE] Token list with network logo and token name #6020

Merged
merged 38 commits into from
Apr 5, 2023

Conversation

tommasini
Copy link
Contributor

Description
This pull request aims to improve the token list, showing the name of the tokens and the network logo, it removes the default '$0.00' of token balances and it's switching them to a skeleton loader.
It was developed unit tests to the components as well.

This PR also have performance improvement on how we fetch the tokens balances.

Technical Details

This PR for core repo aims to update the Tokens name:
https://github.com/MetaMask/core/pull/1127/files#diff-7ada7b099f6bcc6f3d4acd71f94b604636abd22cc556a76dbe7b3e610b1d9233

Draft created for performance improve
MetaMask/core#1128

Test Steps (All of them are recorded in the Screenshots/Recording section)

  • Detect tokens
  • Switch account
  • Switch network
  • Kill and reopen the app
  • Show 0 balance main token wallet
  • Navigate to buy token
  • Hide token (DAI Stable coin) on mainnet
  • Import automatically the token (DAI Stable Coin) hidden by address
    • Address: 0x6B175474E89094C44Da98b954EedeAC495271d0F
  • Import manually the token hidden with custom token import
    • Address: 0x6B175474E89094C44Da98b954EedeAC495271d0F
  • Navigate to the asset page

Non-recorded but tested

  • Add token after the swap
  • add token with coin gecko API

Screenshots/Recordings
Full behaviour on light mode simulator iPhone 14 pro (iOS16.2)

Test Cases
User imports tokens via token autocomplete across supported networks: https://recordit.co/fRCLO6haNT
User imports tokens by contract address: https://recordit.co/kntnmF0N5n
User imports token on network A then switches to network B: https://recordit.co/nEedgiJEfp
User imports token via asset watcher: https://recordit.co/iXi6IXEwpH
User performs a swap via a dapp then imports token: http://recordit.co/iXtKpUAe2f
(On the next both recording, there is only the part of the feature but I updated the app from a past version)
User updates the app to this feature with token detection off: https://recordit.co/SGyPxQ21Bl
User updates the app to this feature with token detection already enabled: https://recordit.co/G2iRCJQEEo
User imports an account to wallet then removes that imported account: https://recordit.co/iQEaWdhIPT

To be improved

  • When the token list it's big, randomly times the endpoint fails (error 429) and we need to refresh the wallet view to update the token balances, we need a callback for this situation, any ideas are welcome

Issue

Progresses #

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

@tommasini tommasini requested a review from a team as a code owner March 22, 2023 18:38
@github-actions
Copy link
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.

@tommasini tommasini added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Mar 22, 2023
Co-authored-by: Samuel Salas <samuel.salas.reyes@gmail.com>
@SamuelSalas
Copy link
Member

Detox and Webdriver.IO Test passed

Copy link
Contributor

@sethkfman sethkfman left a comment

Choose a reason for hiding this comment

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

LGTM

@sethkfman sethkfman added needs-qa Any New Features that needs a full manual QA prior to being added to a release. and removed needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Mar 22, 2023
Copy link
Member

@cortisiko cortisiko left a comment

Choose a reason for hiding this comment

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

🌮 🌮

@cortisiko cortisiko added QA Passed A successful QA run through has been done and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Mar 31, 2023
@sethkfman sethkfman added the release-6.4.0 PR for release 6.4.0 label Apr 3, 2023
Copy link
Contributor

@Cal-L Cal-L left a comment

Choose a reason for hiding this comment

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

LGTM

@Cal-L Cal-L merged commit bf83865 into main Apr 5, 2023
@Cal-L Cal-L deleted the feature/new-649-wallet-token-list-update branch April 5, 2023 01:05
@github-actions github-actions bot locked and limited conversation to collaborators Apr 5, 2023
@gauthierpetetin gauthierpetetin changed the title [FEATURE][MC] Token list with network logo and token name [MC 0.5][FEATURE] Token list with network logo and token name Apr 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
QA Passed A successful QA run through has been done release-6.4.0 PR for release 6.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants