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

fix: fix fiat values in testnet visibility #26273

Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
ad576c0
Fix fiat values in testnet visibility
OGPoyraz Aug 1, 2024
f75cce6
Fix lint
OGPoyraz Aug 1, 2024
6bb9ace
Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…
OGPoyraz Sep 3, 2024
3c74845
Fix suggestions
OGPoyraz Sep 3, 2024
3042a8f
remove unnecessary useCurrencyRateCheck
OGPoyraz Sep 3, 2024
76c1beb
Remove comment
OGPoyraz Sep 3, 2024
4665294
Add localhost to testnets
OGPoyraz Sep 3, 2024
775f822
revert
OGPoyraz Sep 3, 2024
2ddf53b
Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…
OGPoyraz Sep 3, 2024
81854ef
Fix e2e
OGPoyraz Sep 3, 2024
bc2cce1
Fix unit tests
OGPoyraz Sep 3, 2024
7eb149b
Fix tests
OGPoyraz Sep 3, 2024
bf2e3d7
Update snapshot
OGPoyraz Sep 3, 2024
c4e530a
Fix e2e tests
OGPoyraz Sep 3, 2024
114e26f
Remove localization e2e test
OGPoyraz Sep 3, 2024
e2713a5
Fix e2e tests
OGPoyraz Sep 4, 2024
4b274ec
Remove delay
OGPoyraz Sep 4, 2024
35543ad
Fix unit tests
OGPoyraz Sep 4, 2024
dde672c
Fix e2e tests
OGPoyraz Sep 4, 2024
81dcfaf
Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…
OGPoyraz Sep 5, 2024
5e49bc8
Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…
legobeat Sep 6, 2024
abab1f5
Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…
OGPoyraz Sep 6, 2024
6aa5d5b
Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…
OGPoyraz Sep 9, 2024
a22e2e0
Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…
OGPoyraz Sep 10, 2024
e72d35e
Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…
OGPoyraz Sep 10, 2024
0fbc9df
Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…
OGPoyraz Sep 11, 2024
9c67f8c
Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…
OGPoyraz Sep 11, 2024
9e18129
Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…
OGPoyraz Sep 11, 2024
bad0757
Fix e2e test
OGPoyraz Sep 11, 2024
625217c
Fix mmi e2e test
OGPoyraz Sep 16, 2024
b3f44ac
Merge branch 'develop' into 24945-bug-fiat-values-continue-to-show-de…
OGPoyraz Sep 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions shared/constants/network.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ export const CHAIN_IDS = {
SEI: '0x531',
BERACHAIN: '0x138d5',
METACHAIN_ONE: '0x1b6e6',
ARBITRUM_SEPOLIA: '0x66eee',
NEAR: '0x18d',
NEAR_TESTNET: '0x18e',
} as const;
Expand Down Expand Up @@ -1034,4 +1035,5 @@ export const TEST_NETWORK_IDS = [
CHAIN_IDS.SEPOLIA,
CHAIN_IDS.LINEA_GOERLI,
CHAIN_IDS.LINEA_SEPOLIA,
CHAIN_IDS.ARBITRUM_SEPOLIA,
];
51 changes: 0 additions & 51 deletions test/e2e/tests/settings/account-token-list.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,55 +37,4 @@ describe('Settings', function () {
},
);
});

it('Should match the value of token list item and account list item for fiat conversion', async function () {
await withFixtures(
{
fixtures: new FixtureBuilder().build(),
ganacheOptions: defaultGanacheOptions,
title: this.test.fullTitle(),
},
async ({ driver, ganacheServer }) => {
await logInWithBalanceValidation(driver, ganacheServer);

await driver.clickElement(
'[data-testid="account-options-menu-button"]',
);
await driver.clickElement({ text: 'Settings', tag: 'div' });
await driver.clickElement({
text: 'General',
tag: 'div',
});
await driver.clickElement({ text: 'Fiat', tag: 'label' });
// We now need to enable "Show fiat on testnet" if we are using testnets (and since our custom
Copy link
Member Author

@OGPoyraz OGPoyraz Sep 3, 2024

Choose a reason for hiding this comment

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

This is incorrect, 0x539 (the network used for e2e tests) is not testnet and not in the testnet list (See here https://github.com/MetaMask/metamask-extension/blob/24945-bug-fiat-values-continue-to-show-despite-show-conversion-on-test-networks-advanced-settings-being-disabled/shared/constants/network.ts#L1033)

0x539 is also not custom network (See getIsCustomNetwork function here https://github.com/MetaMask/metamask-extension/blob/24945-bug-fiat-values-continue-to-show-despite-show-conversion-on-test-networks-advanced-settings-being-disabled/ui/selectors/selectors.js#L2372)

Since this PR's aim is to correct previous getShouldShowFiat selector, this means that this e2e test must toggle useCurrencyRateCheck to make it work, and it's done via withConversionRate Enabled/Disabled fixture.

// network during test is using a testnet chain ID, it will be considered as a test network)
await driver.clickElement({
text: 'Advanced',
tag: 'div',
});
await driver.clickElement('.show-fiat-on-testnets-toggle');
// Looks like when enabling the "Show fiat on testnet" it takes some time to re-update the
// overview screen, so just wait a bit here:
await driver.delay(1000);

await driver.clickElement(
'.settings-page__header__title-container__close-button',
);
await driver.clickElement(
'[data-testid="account-overview__asset-tab"]',
);

const tokenListAmount = await driver.findElement(
'.eth-overview__primary-container',
);
assert.equal(await tokenListAmount.getText(), '$42,500.00\nUSD');
await driver.clickElement('[data-testid="account-menu-icon"]');
const accountTokenValue = await driver.waitForSelector(
'.multichain-account-list-item .multichain-account-list-item__asset',
);

assert.equal(await accountTokenValue.getText(), '$42,500.00USD');
},
);
});
});
69 changes: 0 additions & 69 deletions ui/hooks/useHideFiatForTestnet.test.ts

This file was deleted.

17 changes: 0 additions & 17 deletions ui/hooks/useHideFiatForTestnet.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,6 @@ exports[`ConfirmGasDisplay should match snapshot 1`] = `
class="mm-box mm-text mm-text--inherit mm-box--color-primary-default"
/>
</button>
<div
class="mm-box currency-display-component mm-box--padding-inline-start-1 mm-box--display-flex mm-box--flex-wrap-wrap mm-box--align-items-center"
title="0.001197"
>
<span
class="mm-box mm-text currency-display-component__text mm-text--body-md-bold mm-text--ellipsis mm-box--color-text-default"
>
0.001197
</span>
</div>
</div>
</h6>
<h6
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
Text,
} from '../../../../components/component-library';
import TransactionDetailItem from '../transaction-detail-item/transaction-detail-item.component';
import { getPreferences } from '../../../../selectors';
import { getPreferences, getShouldShowFiat } from '../../../../selectors';
import { useI18nContext } from '../../../../hooks/useI18nContext';
import LoadingHeartBeat from '../../../../components/ui/loading-heartbeat';
import UserPreferencedCurrencyDisplay from '../../../../components/app/user-preferenced-currency-display/user-preferenced-currency-display.component';
Expand All @@ -34,6 +34,7 @@ export default function FeeDetailsComponent({
}) {
const layer1GasFee = txData?.layer1GasFee ?? null;
const [expandFeeDetails, setExpandFeeDetails] = useState(false);
const shouldShowFiat = useSelector(getShouldShowFiat);

const { useNativeCurrencyAsPrimaryCurrency } = useSelector(getPreferences);

Expand All @@ -50,20 +51,22 @@ export default function FeeDetailsComponent({
return (
<div className="confirm-page-container-content__total-value">
<LoadingHeartBeat estimateUsed={txData?.userFeeLevel} />
<UserPreferencedCurrencyDisplay
type={SECONDARY}
key="total-detail-text"
value={value}
suffixProps={{
color: TextColor.textAlternative,
variant: TextVariant.bodySmBold,
}}
textProps={{
color: TextColor.textAlternative,
variant: TextVariant.bodySmBold,
}}
hideLabel={Boolean(useNativeCurrencyAsPrimaryCurrency)}
/>
{shouldShowFiat && (
<UserPreferencedCurrencyDisplay
type={SECONDARY}
key="total-detail-text"
value={value}
suffixProps={{
color: TextColor.textAlternative,
variant: TextVariant.bodySmBold,
}}
textProps={{
color: TextColor.textAlternative,
variant: TextVariant.bodySmBold,
}}
hideLabel={Boolean(useNativeCurrencyAsPrimaryCurrency)}
/>
)}
</div>
);
},
Expand All @@ -75,20 +78,22 @@ export default function FeeDetailsComponent({
return (
<Box className="confirm-page-container-content__total-value">
<LoadingHeartBeat estimateUsed={txData?.userFeeLevel} />
<UserPreferencedCurrencyDisplay
type={PRIMARY}
key="total-detail-value"
value={value}
suffixProps={{
color: TextColor.textAlternative,
variant: TextVariant.bodySm,
}}
textProps={{
color: TextColor.textAlternative,
variant: TextVariant.bodySm,
}}
hideLabel={!useNativeCurrencyAsPrimaryCurrency}
/>
{shouldShowFiat && (
<UserPreferencedCurrencyDisplay
type={PRIMARY}
key="total-detail-value"
value={value}
suffixProps={{
color: TextColor.textAlternative,
variant: TextVariant.bodySm,
}}
textProps={{
color: TextColor.textAlternative,
variant: TextVariant.bodySm,
}}
hideLabel={!useNativeCurrencyAsPrimaryCurrency}
/>
)}
</Box>
);
},
Expand Down Expand Up @@ -168,18 +173,15 @@ export default function FeeDetailsComponent({
{t('layer1Fees')}
</Text>
}
detailText={
useCurrencyRateCheck && renderTotalDetailText(layer1GasFee)
Copy link
Member Author

Choose a reason for hiding this comment

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

useCurrencyRateCheck is already checked in getShouldShowFiat selector hence am removing this.

}
detailText={shouldShowFiat && renderTotalDetailText(layer1GasFee)}
detailTotal={renderTotalDetailValue(layer1GasFee)}
/>
)}
{!hasLayer1GasFee && (
<TransactionDetailItem
detailTitle={t('total')}
detailText={
useCurrencyRateCheck &&
renderTotalDetailText(getTransactionFeeTotal)
shouldShowFiat && renderTotalDetailText(getTransactionFeeTotal)
}
detailTotal={renderTotalDetailValue(getTransactionFeeTotal)}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import { PRIMARY, SECONDARY } from '../../../../helpers/constants/common';
import { PriorityLevels } from '../../../../../shared/constants/gas';
import {
getPreferences,
getShouldShowFiat,
getTxData,
getUseCurrencyRateCheck,
transactionFeeSelector,
} from '../../../../selectors';
import { getCurrentDraftTransaction } from '../../../../ducks/send';
Expand All @@ -44,12 +44,14 @@ const GasDetailsItem = ({
userAcknowledgedGasMissing = false,
}) => {
const t = useI18nContext();
const shouldShowFiat = useSelector(getShouldShowFiat);

const txData = useSelector(getTxData);
const { layer1GasFee } = txData;

const draftTransaction = useSelector(getCurrentDraftTransaction);
const transactionData = useDraftTransactionWithTxParams();

const {
hexMinimumTransactionFee: draftHexMinimumTransactionFee,
hexMaximumTransactionFee: draftHexMaximumTransactionFee,
Expand All @@ -67,8 +69,6 @@ const GasDetailsItem = ({
} = useGasFeeContext();

const { useNativeCurrencyAsPrimaryCurrency } = useSelector(getPreferences);

const useCurrencyRateCheck = useSelector(getUseCurrencyRateCheck);
const getTransactionFeeTotal = useMemo(() => {
if (layer1GasFee) {
return sumHexes(hexMinimumTransactionFee, layer1GasFee);
Expand Down Expand Up @@ -137,7 +137,7 @@ const GasDetailsItem = ({
<EditGasFeeIcon
userAcknowledgedGasMissing={userAcknowledgedGasMissing}
/>
{useCurrencyRateCheck && (
{shouldShowFiat && (
<UserPreferencedCurrencyDisplay
paddingInlineStart={1}
suffixProps={{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ const mockStateWithShowingFiatOnTestnets = merge({}, mockStateWithTestnet, {
preferences: {
showFiatInTestnets: true,
},
useCurrencyRateCheck: true,
currencyRates: {
SepoliaETH: {
conversionRate: 1,
},
},
},
});
const mockStoreWithShowingFiatOnTestnets = configureStore()(
Expand All @@ -32,6 +38,7 @@ const mockStateWithHidingFiatOnTestnets = merge({}, mockStateWithTestnet, {
preferences: {
showFiatInTestnets: false,
},
useCurrencyRateCheck: false,
},
});
const mockStoreWithHidingFiatOnTestnets = configureStore()(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import { useSelector } from 'react-redux';
import {
TextAlign,
TextColor,
Expand All @@ -9,7 +10,7 @@ import { Text } from '../../../../components/component-library';
import { SizeNumber } from '../../../../components/component-library/box/box.types';
import Tooltip from '../../../../components/ui/tooltip';
import { useFiatFormatter } from '../../../../hooks/useFiatFormatter';
import { useHideFiatForTestnet } from '../../../../hooks/useHideFiatForTestnet';
import { getShouldShowFiat } from '../../../../selectors';
import { FIAT_UNAVAILABLE, FiatAmount } from './types';

const textStyle = {
Expand Down Expand Up @@ -41,10 +42,10 @@ export const IndividualFiatDisplay: React.FC<{
fiatAmount: FiatAmount;
shorten?: boolean;
}> = ({ fiatAmount, shorten = false }) => {
const hideFiatForTestnet = useHideFiatForTestnet();
const shouldShowFiat = useSelector(getShouldShowFiat);
const fiatFormatter = useFiatFormatter();

if (hideFiatForTestnet) {
if (!shouldShowFiat) {
return null;
}

Expand Down Expand Up @@ -76,12 +77,12 @@ export const IndividualFiatDisplay: React.FC<{
export const TotalFiatDisplay: React.FC<{
fiatAmounts: FiatAmount[];
}> = ({ fiatAmounts }) => {
const hideFiatForTestnet = useHideFiatForTestnet();
const shouldShowFiat = useSelector(getShouldShowFiat);
const t = useI18nContext();
const fiatFormatter = useFiatFormatter();
const totalFiat = calculateTotalFiat(fiatAmounts);

if (hideFiatForTestnet) {
if (!shouldShowFiat) {
return null;
}

Expand Down
Loading
Loading