fix: hide testnet amounts if user opt out in the settings #25167

OGPoyraz · 2024-06-10T10:17:56Z

Description

This PR aims to hide testnet fiat values if user opt out in the settings.

Related issues

Fixes: Hide amount in simulations for testnets if it's opted out by user #25169

Manual testing steps

Change current network to any testnet
Go to setting, turn on "Show conversion on test networks"
Try simple send - see fiat values displayed in simulations
Go to setting, turn off "Show conversion on test networks"
Try simple send - see no fiat values displayed in simulations
Change current network to mainnet or any other network which is not testnet
Try simple send - see fiat values displayed in simulations

Screenshots/Recordings

Before

After

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension 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.

shared/constants/network.ts

metamaskbot · 2024-06-10T11:23:05Z

Builds ready [cefe856]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (272 ± 287 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	77	405	115	69	33
		domContentLoaded	10	93	19	18	9
		load	47	2155	272	598	287
		domInteractive	10	93	18	18	9

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: 1 KiB (0.01%)
common: 90 Bytes (0.00%)

codecov · 2024-06-10T11:23:16Z

Codecov Report

Attention: Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 64.89%. Comparing base (533a483) to head (9852b71).
Report is 13 commits behind head on develop.

Files	Patch %	Lines
...ons/components/simulation-details/fiat-display.tsx	83.33%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #25167      +/-   ##
===========================================
+ Coverage    64.88%   64.89%   +0.01%     
===========================================
  Files         1388     1389       +1     
  Lines        55091    55103      +12     
  Branches     14471    14475       +4     
===========================================
+ Hits         35743    35754      +11     
- Misses       19348    19349       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ui/pages/confirmations/components/simulation-details/fiat-display.tsx

ui/pages/confirmations/components/simulation-details/fiat-display.test.tsx

ui/hooks/useHideFiatForTestnet.ts

shared/constants/network.ts

OGPoyraz · 2024-06-11T12:06:43Z

ui/pages/confirmations/components/simulation-details/fiat-display.test.tsx

+const mockStateWithTestnet = merge({}, mockState, {
+  metamask: {
+    providerConfig: {
+      chainId: CHAIN_IDS.SEPOLIA,


If we are going with this approach, I believe it's crucial to set some state props like this for the branch of these tests.

Even if mockState is set as using Goerli chain id which is fine for this test, I believe we should still set the stage for only our test. This is the reason of I am creating mockStateWithTestnet

I think extending the default state in the unit tests is a standard pattern.

We also could have defined a CHAIN_ID_MOCK constant using the mockState but this is great also.

…ets-if-its-opted-out-by-user

metamaskbot · 2024-06-12T09:44:14Z

Builds ready [588c8d1]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (55 ± 6 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	69	116	86	13	6
		domContentLoaded	9	16	12	2	1
		load	43	91	55	12	6
		domInteractive	9	16	12	2	1

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: 1 KiB (0.01%)
common: 90 Bytes (0.00%)

matthewwalsh0 · 2024-06-20T16:08:01Z

ui/hooks/useHideFiatForTestnet.ts

+ */
+export const useHideFiatForTestnet = (providedChainId?: Hex): boolean => {
+  const showFiatInTestnets = useSelector(getShowFiatInTestnets);
+  const chainId = providedChainId ?? useSelector(getCurrentChainId);


We always have to call useSelector even if we don't use the value, since it's a hook.

matthewwalsh0 · 2024-06-20T16:09:02Z

ui/pages/confirmations/components/simulation-details/fiat-display.test.tsx

+const mockStateWithTestnet = merge({}, mockState, {
+  metamask: {
+    providerConfig: {
+      chainId: CHAIN_IDS.SEPOLIA,


I think extending the default state in the unit tests is a standard pattern.

We also could have defined a CHAIN_ID_MOCK constant using the mockState but this is great also.

…ets-if-its-opted-out-by-user

metamaskbot · 2024-06-21T09:12:34Z

Builds ready [9852b71]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (133 ± 165 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	70	110	85	11	5
		domContentLoaded	9	24	12	4	2
		load	41	1631	133	344	165
		domInteractive	9	24	12	4	2

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: 1.01 KiB (0.01%)
common: 90 Bytes (0.00%)

Hide testnet amounts if user optout

58e9d2f

OGPoyraz requested review from a team as code owners June 10, 2024 10:17

OGPoyraz linked an issue Jun 10, 2024 that may be closed by this pull request

Hide amount in simulations for testnets if it's opted out by user MetaMask/metamask-mobile#9906

Closed

OGPoyraz added the team-confirmations Push issues to confirmations team label Jun 10, 2024

OGPoyraz changed the title ~~Hide testnet amounts if user opt out in the settings~~ fix: hide testnet amounts if user opt out in the settings Jun 10, 2024

add function description

6e9d892

OGPoyraz commented Jun 10, 2024

View reviewed changes

shared/constants/network.ts Show resolved Hide resolved

OGPoyraz removed a link to an issue Jun 10, 2024

Hide amount in simulations for testnets if it's opted out by user MetaMask/metamask-mobile#9906

Closed

Fix lint

cefe856

OGPoyraz linked an issue Jun 10, 2024 that may be closed by this pull request

Hide amount in simulations for testnets if it's opted out by user #25169

Closed

jpuri previously approved these changes Jun 10, 2024

View reviewed changes

matthewwalsh0 reviewed Jun 11, 2024

View reviewed changes

OGPoyraz added 2 commits June 11, 2024 13:13

Relocate early return

c6c0ddb

Change testing approach

0741134

OGPoyraz dismissed jpuri’s stale review via 0741134 June 11, 2024 12:00

OGPoyraz commented Jun 11, 2024

View reviewed changes

OGPoyraz added 2 commits June 11, 2024 17:59

Merge branch 'develop' into 9906-hide-amount-in-simulations-for-testn…

7e94399

…ets-if-its-opted-out-by-user

Merge branch 'develop' into 9906-hide-amount-in-simulations-for-testn…

588c8d1

…ets-if-its-opted-out-by-user

matthewwalsh0 reviewed Jun 20, 2024

View reviewed changes

OGPoyraz added 2 commits June 21, 2024 10:05

Fix hook order

9bd1f17

Merge branch 'develop' into 9906-hide-amount-in-simulations-for-testn…

9852b71

…ets-if-its-opted-out-by-user

jpuri approved these changes Jun 24, 2024

View reviewed changes

vinistevam approved these changes Jun 25, 2024

View reviewed changes

OGPoyraz merged commit 52a65bc into develop Jun 25, 2024
74 checks passed

OGPoyraz deleted the 9906-hide-amount-in-simulations-for-testnets-if-its-opted-out-by-user branch June 25, 2024 13:06

github-actions bot locked and limited conversation to collaborators Jun 25, 2024

metamaskbot added the release-12.1.0 Issue or pull request that will be included in release 12.1.0 label Jun 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: hide testnet amounts if user opt out in the settings #25167

fix: hide testnet amounts if user opt out in the settings #25167

OGPoyraz commented Jun 10, 2024 •

edited by legobeat

Loading

metamaskbot commented Jun 10, 2024

codecov bot commented Jun 10, 2024 •

edited

Loading

OGPoyraz Jun 11, 2024

matthewwalsh0 Jun 20, 2024

metamaskbot commented Jun 12, 2024

matthewwalsh0 Jun 20, 2024

OGPoyraz Jun 21, 2024

matthewwalsh0 Jun 20, 2024

metamaskbot commented Jun 21, 2024

fix: hide testnet amounts if user opt out in the settings #25167

fix: hide testnet amounts if user opt out in the settings #25167

Conversation

OGPoyraz commented Jun 10, 2024 • edited by legobeat Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

metamaskbot commented Jun 10, 2024

codecov bot commented Jun 10, 2024 • edited Loading

Codecov Report

OGPoyraz Jun 11, 2024

Choose a reason for hiding this comment

matthewwalsh0 Jun 20, 2024

Choose a reason for hiding this comment

metamaskbot commented Jun 12, 2024

matthewwalsh0 Jun 20, 2024

Choose a reason for hiding this comment

OGPoyraz Jun 21, 2024

Choose a reason for hiding this comment

matthewwalsh0 Jun 20, 2024

Choose a reason for hiding this comment

metamaskbot commented Jun 21, 2024

OGPoyraz commented Jun 10, 2024 •

edited by legobeat

Loading

codecov bot commented Jun 10, 2024 •

edited

Loading