metamask-mobile
metamask-mobile copied to clipboard
feat: setting to show fiat values on testnets
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
- Switch to an eth testnet like sepolia, linea goerli, etc.
- Verify no fiat $ values show in the UI
- Toggle settings > advanced > show conversion on test networks = on
- 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.
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.
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 theRun Smoke E2E
label on the pull request
i just triggered the e2e tests on your PR ⬆️
@bergeron you have to check the failing e2e tests
Added unit and e2e tests
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.
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.
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.
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 theRun Smoke E2E
label on the pull request
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 theRun Smoke E2E
label on the pull request
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 theRun Smoke E2E
label on the pull request
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 theRun Smoke E2E
label on the pull request
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 theRun Smoke E2E
label on the pull request
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 theRun Smoke E2E
label on the pull request
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?
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();
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.
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.
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 theRun Smoke E2E
label on the pull request
Quality Gate passed
Issues
11 New issues
0 Accepted issues
Measures
0 Security Hotspots
50.0% Coverage on New Code
8.0% Duplication on New Code