From 4e256177c5bef72fb49ddfa45dfd5f7cc82171ed Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 24 Oct 2024 10:22:53 +0100 Subject: [PATCH 1/2] fix: Fall back to token list for the token symbol (#28003) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28003?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/27970 ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** ### **Before** ### **After** Screenshot 2024-10-22 at 11 19 10 ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] 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](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). 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. --- ...-image.test.ts => useTokenDetails.test.ts} | 32 +++++++++++++------ ...{use-token-image.ts => useTokenDetails.ts} | 13 ++++++-- .../info/shared/send-heading/send-heading.tsx | 13 ++++---- 3 files changed, 38 insertions(+), 20 deletions(-) rename ui/pages/confirmations/components/confirm/info/hooks/{use-token-image.test.ts => useTokenDetails.test.ts} (73%) rename ui/pages/confirmations/components/confirm/info/hooks/{use-token-image.ts => useTokenDetails.ts} (65%) diff --git a/ui/pages/confirmations/components/confirm/info/hooks/use-token-image.test.ts b/ui/pages/confirmations/components/confirm/info/hooks/useTokenDetails.test.ts similarity index 73% rename from ui/pages/confirmations/components/confirm/info/hooks/use-token-image.test.ts rename to ui/pages/confirmations/components/confirm/info/hooks/useTokenDetails.test.ts index 23e4cc3c1bda..efdf2b66ac56 100644 --- a/ui/pages/confirmations/components/confirm/info/hooks/use-token-image.test.ts +++ b/ui/pages/confirmations/components/confirm/info/hooks/useTokenDetails.test.ts @@ -2,9 +2,9 @@ import { TransactionMeta } from '@metamask/transaction-controller'; import { genUnapprovedTokenTransferConfirmation } from '../../../../../../../test/data/confirmations/token-transfer'; import mockState from '../../../../../../../test/data/mock-state.json'; import { renderHookWithProvider } from '../../../../../../../test/lib/render-helpers'; -import { useTokenImage } from './use-token-image'; +import { useTokenDetails } from './useTokenDetails'; -describe('useTokenImage', () => { +describe('useTokenDetails', () => { it('returns iconUrl from selected token if it exists', () => { const transactionMeta = genUnapprovedTokenTransferConfirmation( {}, @@ -19,11 +19,14 @@ describe('useTokenImage', () => { }; const { result } = renderHookWithProvider( - () => useTokenImage(transactionMeta, TEST_SELECTED_TOKEN), + () => useTokenDetails(transactionMeta, TEST_SELECTED_TOKEN), mockState, ); - expect(result.current).toEqual({ tokenImage: 'iconUrl' }); + expect(result.current).toEqual({ + tokenImage: 'iconUrl', + tokenSymbol: 'symbol', + }); }); it('returns selected token image if no iconUrl is included', () => { @@ -39,11 +42,14 @@ describe('useTokenImage', () => { }; const { result } = renderHookWithProvider( - () => useTokenImage(transactionMeta, TEST_SELECTED_TOKEN), + () => useTokenDetails(transactionMeta, TEST_SELECTED_TOKEN), mockState, ); - expect(result.current).toEqual({ tokenImage: 'image' }); + expect(result.current).toEqual({ + tokenImage: 'image', + tokenSymbol: 'symbol', + }); }); it('returns token list icon url if no image is included in the token', () => { @@ -58,7 +64,7 @@ describe('useTokenImage', () => { }; const { result } = renderHookWithProvider( - () => useTokenImage(transactionMeta, TEST_SELECTED_TOKEN), + () => useTokenDetails(transactionMeta, TEST_SELECTED_TOKEN), { ...mockState, metamask: { @@ -72,7 +78,10 @@ describe('useTokenImage', () => { }, ); - expect(result.current).toEqual({ tokenImage: 'tokenListIconUrl' }); + expect(result.current).toEqual({ + tokenImage: 'tokenListIconUrl', + tokenSymbol: 'symbol', + }); }); it('returns undefined if no image is found', () => { @@ -87,10 +96,13 @@ describe('useTokenImage', () => { }; const { result } = renderHookWithProvider( - () => useTokenImage(transactionMeta, TEST_SELECTED_TOKEN), + () => useTokenDetails(transactionMeta, TEST_SELECTED_TOKEN), mockState, ); - expect(result.current).toEqual({ tokenImage: undefined }); + expect(result.current).toEqual({ + tokenImage: undefined, + tokenSymbol: 'symbol', + }); }); }); diff --git a/ui/pages/confirmations/components/confirm/info/hooks/use-token-image.ts b/ui/pages/confirmations/components/confirm/info/hooks/useTokenDetails.ts similarity index 65% rename from ui/pages/confirmations/components/confirm/info/hooks/use-token-image.ts rename to ui/pages/confirmations/components/confirm/info/hooks/useTokenDetails.ts index 5817d08028ab..be9578496205 100644 --- a/ui/pages/confirmations/components/confirm/info/hooks/use-token-image.ts +++ b/ui/pages/confirmations/components/confirm/info/hooks/useTokenDetails.ts @@ -1,20 +1,27 @@ import { TokenListMap } from '@metamask/assets-controllers'; import { TransactionMeta } from '@metamask/transaction-controller'; import { useSelector } from 'react-redux'; +import { useI18nContext } from '../../../../../../hooks/useI18nContext'; import { getTokenList } from '../../../../../../selectors'; import { SelectedToken } from '../shared/selected-token'; -export const useTokenImage = ( +export const useTokenDetails = ( transactionMeta: TransactionMeta, selectedToken: SelectedToken, ) => { + const t = useI18nContext(); + const tokenList = useSelector(getTokenList) as TokenListMap; - // TODO: Add support for NFT images in one of the following tasks const tokenImage = selectedToken?.iconUrl || selectedToken?.image || tokenList[transactionMeta?.txParams?.to as string]?.iconUrl; - return { tokenImage }; + const tokenSymbol = + selectedToken?.symbol || + tokenList[transactionMeta?.txParams?.to as string]?.symbol || + t('unknown'); + + return { tokenImage, tokenSymbol }; }; diff --git a/ui/pages/confirmations/components/confirm/info/shared/send-heading/send-heading.tsx b/ui/pages/confirmations/components/confirm/info/shared/send-heading/send-heading.tsx index 2806c33936c0..40c571d4bc75 100644 --- a/ui/pages/confirmations/components/confirm/info/shared/send-heading/send-heading.tsx +++ b/ui/pages/confirmations/components/confirm/info/shared/send-heading/send-heading.tsx @@ -16,22 +16,23 @@ import { TextColor, TextVariant, } from '../../../../../../../helpers/constants/design-system'; -import { useI18nContext } from '../../../../../../../hooks/useI18nContext'; import { getWatchedToken } from '../../../../../../../selectors'; import { MultichainState } from '../../../../../../../selectors/multichain'; import { useConfirmContext } from '../../../../../context/confirm'; -import { useTokenImage } from '../../hooks/use-token-image'; +import { useTokenDetails } from '../../hooks/useTokenDetails'; import { useTokenValues } from '../../hooks/use-token-values'; import { ConfirmLoader } from '../confirm-loader/confirm-loader'; const SendHeading = () => { - const t = useI18nContext(); const { currentConfirmation: transactionMeta } = useConfirmContext(); const selectedToken = useSelector((state: MultichainState) => getWatchedToken(transactionMeta)(state), ); - const { tokenImage } = useTokenImage(transactionMeta, selectedToken); + const { tokenImage, tokenSymbol } = useTokenDetails( + transactionMeta, + selectedToken, + ); const { decodedTransferValue, fiatDisplayValue, pending } = useTokenValues(transactionMeta); @@ -57,9 +58,7 @@ const SendHeading = () => { variant={TextVariant.headingLg} color={TextColor.inherit} marginTop={3} - >{`${decodedTransferValue || ''} ${ - selectedToken?.symbol || t('unknown') - }`} + >{`${decodedTransferValue || ''} ${tokenSymbol}`} {fiatDisplayValue && ( {fiatDisplayValue} From 2ae92364cca791bde80072a62da7c45dd809a20a Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 24 Oct 2024 10:23:26 +0100 Subject: [PATCH 2/2] fix: Support dynamic native token name on gas component (#28048) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Uses the multinetwork ticker. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28048?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/28001 ## **Manual testing steps** See original ticket linked above. ## **Screenshots/Recordings** ### **Before** ### **After** Screenshot 2024-10-23 at 16 16 19 ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] 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](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). 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. --- .../transactions/contract-deployment.test.tsx | 4 ++-- .../transactions/contract-interaction.test.tsx | 4 ++-- .../confirm/info/hooks/useFeeCalculations.test.ts | 4 ++-- .../confirm/info/hooks/useFeeCalculations.ts | 12 ++++++++---- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/test/integration/confirmations/transactions/contract-deployment.test.tsx b/test/integration/confirmations/transactions/contract-deployment.test.tsx index ecef04f30861..c2625e06e3e7 100644 --- a/test/integration/confirmations/transactions/contract-deployment.test.tsx +++ b/test/integration/confirmations/transactions/contract-deployment.test.tsx @@ -283,7 +283,7 @@ describe('Contract Deployment Confirmation', () => { expect(editGasFeesRow).toHaveTextContent(tEn('networkFee') as string); const firstGasField = within(editGasFeesRow).getByTestId('first-gas-field'); - expect(firstGasField).toHaveTextContent('0.0001 ETH'); + expect(firstGasField).toHaveTextContent('0.0001 SepoliaETH'); const editGasFeeNativeCurrency = within(editGasFeesRow).getByTestId('native-currency'); expect(editGasFeeNativeCurrency).toHaveTextContent('$0.47'); @@ -371,7 +371,7 @@ describe('Contract Deployment Confirmation', () => { const maxFee = screen.getByTestId('gas-fee-details-max-fee'); expect(gasFeesSection).toContainElement(maxFee); expect(maxFee).toHaveTextContent(tEn('maxFee') as string); - expect(maxFee).toHaveTextContent('0.0023 ETH'); + expect(maxFee).toHaveTextContent('0.0023 SepoliaETH'); expect(maxFee).toHaveTextContent('$7.72'); const nonceSection = screen.getByTestId('advanced-details-nonce-section'); diff --git a/test/integration/confirmations/transactions/contract-interaction.test.tsx b/test/integration/confirmations/transactions/contract-interaction.test.tsx index 1102cb21c67d..b77e48f1d660 100644 --- a/test/integration/confirmations/transactions/contract-interaction.test.tsx +++ b/test/integration/confirmations/transactions/contract-interaction.test.tsx @@ -301,7 +301,7 @@ describe('Contract Interaction Confirmation', () => { expect(editGasFeesRow).toHaveTextContent(tEn('networkFee') as string); const firstGasField = within(editGasFeesRow).getByTestId('first-gas-field'); - expect(firstGasField).toHaveTextContent('0.0001 ETH'); + expect(firstGasField).toHaveTextContent('0.0001 SepoliaETH'); const editGasFeeNativeCurrency = within(editGasFeesRow).getByTestId('native-currency'); expect(editGasFeeNativeCurrency).toHaveTextContent('$0.47'); @@ -402,7 +402,7 @@ describe('Contract Interaction Confirmation', () => { const maxFee = screen.getByTestId('gas-fee-details-max-fee'); expect(gasFeesSection).toContainElement(maxFee); expect(maxFee).toHaveTextContent(tEn('maxFee') as string); - expect(maxFee).toHaveTextContent('0.0023 ETH'); + expect(maxFee).toHaveTextContent('0.0023 SepoliaETH'); expect(maxFee).toHaveTextContent('$7.72'); const nonceSection = screen.getByTestId('advanced-details-nonce-section'); diff --git a/ui/pages/confirmations/components/confirm/info/hooks/useFeeCalculations.test.ts b/ui/pages/confirmations/components/confirm/info/hooks/useFeeCalculations.test.ts index 911cdb20118c..17c8ab8dd8f6 100644 --- a/ui/pages/confirmations/components/confirm/info/hooks/useFeeCalculations.test.ts +++ b/ui/pages/confirmations/components/confirm/info/hooks/useFeeCalculations.test.ts @@ -22,13 +22,13 @@ describe('useFeeCalculations', () => { expect(result.current).toMatchInlineSnapshot(` { "estimatedFeeFiat": "$0.00", - "estimatedFeeNative": "0 WEI", + "estimatedFeeNative": "0 ETH", "l1FeeFiat": "", "l1FeeNative": "", "l2FeeFiat": "", "l2FeeNative": "", "maxFeeFiat": "$0.00", - "maxFeeNative": "0 WEI", + "maxFeeNative": "0 ETH", } `); }); diff --git a/ui/pages/confirmations/components/confirm/info/hooks/useFeeCalculations.ts b/ui/pages/confirmations/components/confirm/info/hooks/useFeeCalculations.ts index 587d70c9c9ef..70bd2c0e3af2 100644 --- a/ui/pages/confirmations/components/confirm/info/hooks/useFeeCalculations.ts +++ b/ui/pages/confirmations/components/confirm/info/hooks/useFeeCalculations.ts @@ -8,7 +8,6 @@ import { addHexes, decGWEIToHexWEI, decimalToHex, - getEthConversionFromWeiHex, getValueFromWeiHex, multiplyHexes, } from '../../../../../../../shared/modules/conversion.utils'; @@ -17,6 +16,7 @@ import { getConversionRate } from '../../../../../../ducks/metamask/metamask'; import { useFiatFormatter } from '../../../../../../hooks/useFiatFormatter'; import { useGasFeeEstimates } from '../../../../../../hooks/useGasFeeEstimates'; import { getCurrentCurrency } from '../../../../../../selectors'; +import { getMultichainNetwork } from '../../../../../../selectors/multichain'; import { HEX_ZERO } from '../shared/constants'; import { useEIP1559TxFees } from './useEIP1559TxFees'; import { useSupportsEIP1559 } from './useSupportsEIP1559'; @@ -33,14 +33,18 @@ export function useFeeCalculations(transactionMeta: TransactionMeta) { const conversionRate = useSelector(getConversionRate); const fiatFormatter = useFiatFormatter(); + const multichainNetwork = useSelector(getMultichainNetwork); + const ticker = multichainNetwork?.network?.ticker; + const getFeesFromHex = useCallback( (hexFee: string) => { - const nativeCurrencyFee = - getEthConversionFromWeiHex({ + const nativeCurrencyFee = `${ + getValueFromWeiHex({ value: hexFee, fromCurrency: EtherDenomination.GWEI, numberOfDecimals: 4, - }) || `0 ${EtherDenomination.ETH}`; + }) || 0 + } ${ticker}`; const currentCurrencyFee = fiatFormatter( Number(