metamask-mobile icon indicating copy to clipboard operation
metamask-mobile copied to clipboard

Published 20 hours ago • verified publisher icon MetaMask

feat: Sort/Import Tokens in Mobile

Open gambinish opened this issue 1 year ago • 17 comments
trafficstars

Description

Implements Token Sorting on mobile. This is the mobile implementation of the same feature on extension: https://github.com/MetaMask/metamask-extension/pull/27184

Note that this currently includes a patch of the PreferencesController. There is a corresponding PR in core to add this to a formal controller release: https://github.com/MetaMask/core/pull/4747

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/MMASSETS-357

Manual testing steps

Go to Portfolio Screen Sort should allow sorting by declining fiat balance by default. Users can then toggle and sort alphabetically or by declining fiat balance.

Import button should function the same as the import token link in footer

Screenshots/Recordings

https://github.com/user-attachments/assets/f9a9cf52-ef23-4705-a7db-ae5ecdc232f7

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.

gambinish avatar Oct 04 '24 05:10 gambinish

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[bot] avatar Oct 04 '24 05:10 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 469270a40c2ef4751d65d0a1e6eaf2c4f7ebdb6e Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/83984131-4359-4325-994c-3acef36b35d8

[!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[bot] avatar Oct 08 '24 16:10 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 82eb1a29efd6a780551325da23422a53d859de4f Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/99bce5c6-c156-41dd-a52d-7c5f85e00089

[!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[bot] avatar Oct 09 '24 19:10 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 25a72383a4957711a46b60fa4349d16b704c5f48 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/98b97cd0-68f6-4ab7-b42e-45a4a51d9bba

[!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[bot] avatar Oct 10 '24 21:10 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: ff32cc440de54e643b998bb2f70113d0dbf4d829 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/de7696d0-bd9c-4a30-9f6f-eda60d1e8216

[!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[bot] avatar Oct 10 '24 22:10 github-actions[bot]

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: bd61eff01e4a192c73c563116d5c73abc579d401 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/95fd3614-2df8-435f-b405-fa4086a44cfb

[!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

github-actions[bot] avatar Oct 11 '24 00:10 github-actions[bot]

https://bitrise.io/ Bitrise

🔄🔄🔄 pr_smoke_e2e_pipeline started on Bitrise...🔄🔄🔄

Commit hash: f28b6fe1d5b7fcb94a410e5838cf819353829b22 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/dbcce302-a12c-409d-9c1b-469092611f0e

[!NOTE]

  • This comment will auto-update when build completes
  • 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

github-actions[bot] avatar Oct 11 '24 00:10 github-actions[bot]

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: e747848385d677b53e60c73848c7ffcc287580b9 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/d91cc51f-e2c5-46f6-a325-87b4f41a0121

[!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

github-actions[bot] avatar Oct 11 '24 01:10 github-actions[bot]

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 68338a33e24ef7b5d41690d9ba505de0720e51d1 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/e133eb6c-97d8-496c-b1be-2989429a9d83

[!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

github-actions[bot] avatar Oct 15 '24 19:10 github-actions[bot]

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: ff95c727eebd94dffba483b2e3b1b098a23422df Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/436fb50e-438f-4522-8232-49b870488bc3

[!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

github-actions[bot] avatar Oct 15 '24 22:10 github-actions[bot]

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 1d929ce35fc6b3f84577b220a499f1c301b56872 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/e4142e7a-e9b7-4ebd-a8c5-ef55e483929c

[!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

github-actions[bot] avatar Oct 15 '24 23:10 github-actions[bot]

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 63bb1a25cbb568834eb76a46ce8a565b60d2389f Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/d2b32f4a-e044-4677-bef5-023583c77a36

[!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

github-actions[bot] avatar Oct 16 '24 18:10 github-actions[bot]

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: bd9ad0d3837f70306ff14337fa9d4c8aa27ac3e6 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/1ed7f7fe-b4ad-47c2-bdfd-197dca482d54

[!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

github-actions[bot] avatar Oct 16 '24 19:10 github-actions[bot]

Codecov Report

Attention: Patch coverage is 69.44444% with 11 lines in your changes missing coverage. Please review.

Please upload report for BASE (feat/mmassets_357-sort-import-tokens-mobile--reorg-portfolio-balance@96436be). Learn more about missing BASE report.

Files with missing lines Patch % Lines
app/components/UI/Tokens/index.tsx 67.74% 9 Missing and 1 partial :warning:
app/actions/settings/index.js 0.00% 1 Missing :warning:
Additional details and impacted files 
@@                                           Coverage Diff                                           @@
##             feat/mmassets_357-sort-import-tokens-mobile--reorg-portfolio-balance   #11618   +/-   ##
=======================================================================================================
  Coverage                                                                        ?   54.34%           
=======================================================================================================
  Files                                                                           ?     1714           
  Lines                                                                           ?    39129           
  Branches                                                                        ?     4849           
=======================================================================================================
  Hits                                                                            ?    21263           
  Misses                                                                          ?    16377           
  Partials                                                                        ?     1489

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Oct 16 '24 19:10 codecov-commenter

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: c0a28bc54f3313361f354d476a3a2aaa4be324f2 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/bd6b1bfc-c223-488f-bf26-f9862f02be34

[!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

github-actions[bot] avatar Oct 17 '24 19:10 github-actions[bot]

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 8cb67f7749afe0ca55fd5196a98f38607df80d08 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/9613b40a-de89-4934-8d59-3a964693ecfb

[!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

github-actions[bot] avatar Oct 17 '24 21:10 github-actions[bot]

I think im still seeing this issue; i remember it was fixed in a previous PR? Screenshot 2024-10-22 at 09 58 01

sahar-fehri avatar Oct 22 '24 07:10 sahar-fehri

i just tested the feature , looks great , lgtm ✅

salimtb avatar Oct 22 '24 12:10 salimtb

I ve seen this sonar issue https://sonarcloud.io/project/issues?sinceLeakPeriod=true&issueStatuses=OPEN%2CCONFIRMED&pullRequest=11618&id=metamask-mobile&open=AZJ4jkkCJ8jX2B2IGuL8

I think its related to this signature being deprecated trackEvent( event: IMetaMetricsEvent, properties?: CombinedProperties, saveDataRecording?: boolean, ): void;

I have seen a thread about this suggesting to use MetricsEventBuilder and non-deprecated trackEvent instead: trackEvent(event: ITrackingEvent, saveDataRecording?: boolean): void;

sahar-fehri avatar Oct 22 '24 15:10 sahar-fehri

https://bitrise.io/ Bitrise

🔄🔄🔄 pr_smoke_e2e_pipeline started on Bitrise...🔄🔄🔄

Commit hash: f3765518766724a649551a5faee1b2c332ac5f2b Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/cfccaf70-e0ea-4a4c-98b8-639a6e2e7a9d

[!NOTE]

  • This comment will auto-update when build completes
  • 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

github-actions[bot] avatar Oct 22 '24 16:10 github-actions[bot]

I think im still seeing this issue; i remember it was fixed in a previous PR?

@sahar-fehri have you pulled the latest? This is what I see:

Screenshot 2024-10-22 at 9 03 46 AM

gambinish avatar Oct 22 '24 16:10 gambinish

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: e106631db4bb17f3f6817121aef9b0adafcddc89 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/70a5ec82-c3c2-4de4-87c4-e14df4c4abb2

[!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

github-actions[bot] avatar Oct 22 '24 16:10 github-actions[bot]

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarCloud

sonarqubecloud[bot] avatar Oct 22 '24 16:10 sonarqubecloud[bot]

LGTM. Only nit - The term Declining Balance along with the red highlight makes it seem like a destructive action, which is not since sorting is for display only. To keep it neutral and consistent with the other Alphabetically, I'd recommend using the word Balance followed by high to low and remove the red highlight.

This is a good callout. I should have cut a new recording since this was addressed:

https://github.com/user-attachments/assets/fcce6700-ffc3-4966-8801-b2f7f1251630

As an aside, I have a follow up PR to update this to a BottomSheet component anyway, so should be pretty short-lived

gambinish avatar Oct 22 '24 19:10 gambinish