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

feat: setting to show fiat values on testnets

Open bergeron opened this issue 10 months ago • 18 comments

Description

Adds a setting toggle to show fiat $ conversions on testnets. It is off by default. It also has a bottom sheet friction informing users about the setting.

The selectConversionRate selector is modified to not return a fiat conversion rate when the setting is off. This allows the setting to affect in all parts of the UI that access the conversion rate.

Related issues

Translations PR: https://github.com/MetaMask/metamask-mobile/pull/9361

https://consensyssoftware.atlassian.net/jira/software/projects/MMASSETS/boards/640/backlog?assignee=63f30ca01223974bc049f942&selectedIssue=MMASSETS-190

Manual testing steps

  1. Switch to an eth testnet like sepolia, linea goerli, etc.
  2. Verify no fiat $ values show in the UI
  3. Toggle settings > advanced > show conversion on test networks = on
  4. Verify that fiat $ values appear

Screenshots/Recordings

Before

https://github.com/MetaMask/metamask-mobile/assets/3500406/1ce5b5f5-62c2-4a08-ad6b-8623c5818de7

After

https://github.com/MetaMask/metamask-mobile/assets/3500406/1c95bd2f-3a92-44d0-a9c5-6a1bda1a15ca

Pre-merge author checklist

  • [ ] I’ve followed MetaMask 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.

bergeron avatar Apr 04 '24 01:04 bergeron

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 Apr 04 '24 01:04 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 865eda87332781359dbbb0466237faa482005f49 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/420614b6-b75c-411c-abd1-15fb4ab9c2e8

[!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 Apr 16 '24 16:04 github-actions[bot]

i just triggered the e2e tests on your PR ⬆️

salimtb avatar Apr 16 '24 16:04 salimtb

@bergeron you have to check the failing e2e tests

salimtb avatar Apr 17 '24 08:04 salimtb

Added unit and e2e tests

bergeron avatar Apr 26 '24 04:04 bergeron

I think it would make sense to add a small advisory badge or label in the UI next to the fiat value on testnets that indicates that this is indeed "not real money" to avoid this feature being leveraged as a vector for scams.

It could show an abbreviated version of the information in the warning displayed on feature-enabling on tap.

legobeat avatar Apr 30 '24 20:04 legobeat

I think it would make sense to add a small advisory badge or label in the UI next to the fiat value on testnets that indicates that this is indeed "not real money"

I think this is challenging because there are so many places in the UI that show fiat values. But perhaps the main balance on the home page would be sufficient.

The existing behavior before introducing this setting is that we've always been showing fiat values on testnets, so it's becoming much safer to be off by default.

bergeron avatar May 01 '24 15:05 bergeron

I like the idea of badges to represent tokens with no monetary value but it should be a separate design effort so that we are intentional about where to put badges.

This current work already improves the security of our app. But the badge work will do even better.

alfeng6 avatar May 01 '24 15:05 alfeng6

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: e04889e58b9f92e8765f0c090554bbf77e7da3e2 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/893d3a98-5b0b-45d0-b61a-f0b75b89f9ba

[!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 May 01 '24 23:05 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: c0f0ba7305eb5c430e7298e78d1a9be4e9ced982 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/a7e96e5e-2969-48ba-a342-904c878e6b40

[!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 May 02 '24 15:05 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: ce6725d03eb7c0912f900a74d569aaf76c281d81 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/292a923a-b247-41a0-becf-e13f4476ff0e

[!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 May 10 '24 14:05 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: ba0b31c8515da71f23e07c79e53670f4d02bc94c Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/b7b5615d-0d68-4c98-91c1-0f628bc59381

[!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 May 14 '24 15:05 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 0d6599ac86b5c84e7595dbce091c8921207d5fd5 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/8f264562-5307-4612-aec5-719459418d8f

[!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 May 14 '24 16:05 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 44015d360743e66f10cd5150da06f8d57a183814 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/3a7e50c7-f6fc-4581-b5fc-1819c8987c03

[!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 May 14 '24 20:05 github-actions[bot]

The new e2e test is passing locally on android, so I'm not sure what's happening in bitrise.

Based on the bitrise screenshot, it's not even logging in to MM?

testFnFailure

But locally it passes just fine:

https://github.com/MetaMask/metamask-mobile/assets/3500406/aa7d656f-6d61-4559-93e2-fcf3a534d42d

Update: Resolved by adding await TestHelpers.reverseServerPort();

bergeron avatar May 14 '24 21:05 bergeron

Codecov Report

Attention: Patch coverage is 38.70968% with 19 lines in your changes are missing coverage. Please review.

Project coverage is 46.41%. Comparing base (220d717) to head (6aefe2b). Report is 20 commits behind head on main.

Files Patch % Lines
app/core/Engine.ts 0.00% 7 Missing :warning:
.../FiatOnTestnetsFriction/FiatOnTestnetsFriction.tsx 63.63% 4 Missing :warning:
...omponents/Views/Settings/AdvancedSettings/index.js 0.00% 4 Missing :warning:
...pp/components/UI/AssetOverview/Balance/Balance.tsx 0.00% 2 Missing :warning:
app/actions/settings/index.js 0.00% 1 Missing :warning:
app/reducers/settings/index.js 0.00% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9129      +/-   ##
==========================================
+ Coverage   46.27%   46.41%   +0.14%     
==========================================
  Files        1336     1342       +6     
  Lines       32582    32597      +15     
  Branches     3445     3486      +41     
==========================================
+ Hits        15076    15129      +53     
+ Misses      16594    16535      -59     
- Partials      912      933      +21     

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

codecov-commenter avatar May 14 '24 21:05 codecov-commenter

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: d1bc5d085207a33651abc67278fd6614d25454fc Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/f0eb5581-1789-4156-a505-fd88b9d27dab

[!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 May 14 '24 22:05 github-actions[bot]