From 181634e9aecc8bf4c2d7dc4d908309a582c4cba0 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Wed, 16 Oct 2024 12:02:47 +0100 Subject: [PATCH] Address PR review --- .../confirm/info/approve/approve.tsx | 8 +-- .../base-transaction-info.tsx | 8 +-- .../set-approval-for-all-info.tsx | 8 +-- .../advanced-details.test.tsx.snap | 23 +++----- .../advanced-details.test.tsx | 45 ++++++++++++-- .../advanced-details/advanced-details.tsx | 15 ++++- .../name-or-address-display.test.tsx.snap | 59 ------------------- .../transaction-flow-section.test.tsx.snap | 53 +++++++++++++++++ .../name-or-address-display.test.tsx | 26 -------- .../name-or-address-display.tsx | 59 ------------------- .../token-transfer/token-details-section.tsx | 7 +-- .../token-transfer/token-transfer.stories.tsx | 18 +++++- .../info/token-transfer/token-transfer.tsx | 8 +-- .../transaction-flow-section.test.tsx | 48 +++++++++++++++ .../transaction-flow-section.tsx | 18 ++++-- 15 files changed, 196 insertions(+), 207 deletions(-) delete mode 100644 ui/pages/confirmations/components/confirm/info/token-transfer/__snapshots__/name-or-address-display.test.tsx.snap create mode 100644 ui/pages/confirmations/components/confirm/info/token-transfer/__snapshots__/transaction-flow-section.test.tsx.snap delete mode 100644 ui/pages/confirmations/components/confirm/info/token-transfer/name-or-address-display.test.tsx delete mode 100644 ui/pages/confirmations/components/confirm/info/token-transfer/name-or-address-display.tsx create mode 100644 ui/pages/confirmations/components/confirm/info/token-transfer/transaction-flow-section.test.tsx diff --git a/ui/pages/confirmations/components/confirm/info/approve/approve.tsx b/ui/pages/confirmations/components/confirm/info/approve/approve.tsx index eabf8639ccfb..fed03a75e17e 100644 --- a/ui/pages/confirmations/components/confirm/info/approve/approve.tsx +++ b/ui/pages/confirmations/components/confirm/info/approve/approve.tsx @@ -3,10 +3,8 @@ import { TransactionType, } from '@metamask/transaction-controller'; import React, { useState } from 'react'; -import { useSelector } from 'react-redux'; import { useConfirmContext } from '../../../../context/confirm'; import { useAssetDetails } from '../../../../hooks/useAssetDetails'; -import { selectConfirmationAdvancedDetailsOpen } from '../../../../selectors/preferences'; import { AdvancedDetails } from '../shared/advanced-details/advanced-details'; import { ConfirmLoader } from '../shared/confirm-loader/confirm-loader'; import { GasFeesSection } from '../shared/gas-fees-section/gas-fees-section'; @@ -24,10 +22,6 @@ const ApproveInfo = () => { currentConfirmation: TransactionMeta; }; - const showAdvancedDetails = useSelector( - selectConfirmationAdvancedDetailsOpen, - ); - const { isNFT } = useIsNFT(transactionMeta); const [isOpenEditSpendingCapModal, setIsOpenEditSpendingCapModal] = @@ -70,7 +64,7 @@ const ApproveInfo = () => { /> )} - {showAdvancedDetails && } + { const { currentConfirmation: transactionMeta } = useConfirmContext(); - const showAdvancedDetails = useSelector( - selectConfirmationAdvancedDetailsOpen, - ); - if (!transactionMeta?.txParams) { return null; } @@ -33,7 +27,7 @@ const BaseTransactionInfo = () => { - {showAdvancedDetails && } + ); }; diff --git a/ui/pages/confirmations/components/confirm/info/set-approval-for-all-info/set-approval-for-all-info.tsx b/ui/pages/confirmations/components/confirm/info/set-approval-for-all-info/set-approval-for-all-info.tsx index 92df913783a1..6902a6da9b1f 100644 --- a/ui/pages/confirmations/components/confirm/info/set-approval-for-all-info/set-approval-for-all-info.tsx +++ b/ui/pages/confirmations/components/confirm/info/set-approval-for-all-info/set-approval-for-all-info.tsx @@ -1,8 +1,6 @@ import { TransactionMeta } from '@metamask/transaction-controller'; import React from 'react'; -import { useSelector } from 'react-redux'; import { useConfirmContext } from '../../../../context/confirm'; -import { selectConfirmationAdvancedDetailsOpen } from '../../../../selectors/preferences'; import { ApproveDetails } from '../approve/approve-details/approve-details'; import { useDecodedTransactionData } from '../hooks/useDecodedTransactionData'; import { AdvancedDetails } from '../shared/advanced-details/advanced-details'; @@ -16,10 +14,6 @@ const SetApprovalForAllInfo = () => { const { currentConfirmation: transactionMeta } = useConfirmContext(); - const showAdvancedDetails = useSelector( - selectConfirmationAdvancedDetailsOpen, - ); - const decodedResponse = useDecodedTransactionData(); const { value, pending } = decodedResponse; @@ -45,7 +39,7 @@ const SetApprovalForAllInfo = () => { )} - {showAdvancedDetails && } + ); }; diff --git a/ui/pages/confirmations/components/confirm/info/shared/advanced-details/__snapshots__/advanced-details.test.tsx.snap b/ui/pages/confirmations/components/confirm/info/shared/advanced-details/__snapshots__/advanced-details.test.tsx.snap index f66db615defe..3a93bae1e26d 100644 --- a/ui/pages/confirmations/components/confirm/info/shared/advanced-details/__snapshots__/advanced-details.test.tsx.snap +++ b/ui/pages/confirmations/components/confirm/info/shared/advanced-details/__snapshots__/advanced-details.test.tsx.snap @@ -1,6 +1,8 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[` does not render component for advanced transaction details 1`] = ` +exports[` does not render component when the state property is false 1`] = `
`; + +exports[` renders component when the prop override is passed 1`] = `
does not render component for advanced transaction

does not render component for advanced transaction
`; -exports[` renders component for advanced transaction details 1`] = ` +exports[` renders component when the state property is true 1`] = `
renders component for advanced transaction details

renders component for advanced transaction details class="mm-box mm-text mm-text--body-md mm-box--color-inherit" style="white-space: pre-wrap;" > - 12 + undefined

-
diff --git a/ui/pages/confirmations/components/confirm/info/shared/advanced-details/advanced-details.test.tsx b/ui/pages/confirmations/components/confirm/info/shared/advanced-details/advanced-details.test.tsx index b965e2015895..60512441e77d 100644 --- a/ui/pages/confirmations/components/confirm/info/shared/advanced-details/advanced-details.test.tsx +++ b/ui/pages/confirmations/components/confirm/info/shared/advanced-details/advanced-details.test.tsx @@ -9,8 +9,18 @@ import { AdvancedDetails } from './advanced-details'; describe('', () => { const middleware = [thunk]; - it('does not render component for advanced transaction details', () => { - const state = mockState; + it('does not render component when the state property is false', () => { + const state = { + ...mockState, + metamask: { + ...mockState.metamask, + preferences: { + ...mockState.metamask.preferences, + showConfirmationAdvancedDetails: false, + }, + }, + }; + const mockStore = configureMockStore(middleware)(state); const { container } = renderWithConfirmContextProvider( , @@ -20,16 +30,18 @@ describe('', () => { expect(container).toMatchSnapshot(); }); - it('renders component for advanced transaction details', () => { + it('renders component when the state property is true', () => { const state = { ...mockState, metamask: { ...mockState.metamask, - useNonceField: true, - nextNonce: 1, - customNonceValue: '12', + preferences: { + ...mockState.metamask.preferences, + showConfirmationAdvancedDetails: true, + }, }, }; + const mockStore = configureMockStore(middleware)(state); const { container } = renderWithConfirmContextProvider( , @@ -38,4 +50,25 @@ describe('', () => { expect(container).toMatchSnapshot(); }); + + it('renders component when the prop override is passed', () => { + const state = { + ...mockState, + metamask: { + ...mockState.metamask, + preferences: { + ...mockState.metamask.preferences, + showConfirmationAdvancedDetails: false, + }, + }, + }; + + const mockStore = configureMockStore(middleware)(state); + const { container } = renderWithConfirmContextProvider( + , + mockStore, + ); + + expect(container).toMatchSnapshot(); + }); }); diff --git a/ui/pages/confirmations/components/confirm/info/shared/advanced-details/advanced-details.tsx b/ui/pages/confirmations/components/confirm/info/shared/advanced-details/advanced-details.tsx index 7e0cee721bb8..ebb0f69d75c1 100644 --- a/ui/pages/confirmations/components/confirm/info/shared/advanced-details/advanced-details.tsx +++ b/ui/pages/confirmations/components/confirm/info/shared/advanced-details/advanced-details.tsx @@ -16,6 +16,7 @@ import { showModal, updateCustomNonce, } from '../../../../../../../store/actions'; +import { selectConfirmationAdvancedDetailsOpen } from '../../../../../selectors/preferences'; import { TransactionData } from '../transaction-data/transaction-data'; const NonceDetails = () => { @@ -65,7 +66,19 @@ const NonceDetails = () => { ); }; -export const AdvancedDetails: React.FC = () => { +export const AdvancedDetails = ({ + overrideVisibility = false, +}: { + overrideVisibility?: boolean; +}) => { + const showAdvancedDetails = useSelector( + selectConfirmationAdvancedDetailsOpen, + ); + + if (!overrideVisibility && !showAdvancedDetails) { + return null; + } + return ( <> diff --git a/ui/pages/confirmations/components/confirm/info/token-transfer/__snapshots__/name-or-address-display.test.tsx.snap b/ui/pages/confirmations/components/confirm/info/token-transfer/__snapshots__/name-or-address-display.test.tsx.snap deleted file mode 100644 index 4aa8fad70e26..000000000000 --- a/ui/pages/confirmations/components/confirm/info/token-transfer/__snapshots__/name-or-address-display.test.tsx.snap +++ /dev/null @@ -1,59 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`NameOrAddressDisplay renders correctly 1`] = ` -
-
- -

- 0x388C8...19297 -

-
-
-`; diff --git a/ui/pages/confirmations/components/confirm/info/token-transfer/__snapshots__/transaction-flow-section.test.tsx.snap b/ui/pages/confirmations/components/confirm/info/token-transfer/__snapshots__/transaction-flow-section.test.tsx.snap new file mode 100644 index 000000000000..23cddb2b59b2 --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/token-transfer/__snapshots__/transaction-flow-section.test.tsx.snap @@ -0,0 +1,53 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` renders correctly 1`] = ` +
+
+
+
+
+ +

+ 0x2e0D7...5d09B +

+
+
+ +
+
+ +

+ 0x6B175...71d0F +

+
+
+
+
+
+`; diff --git a/ui/pages/confirmations/components/confirm/info/token-transfer/name-or-address-display.test.tsx b/ui/pages/confirmations/components/confirm/info/token-transfer/name-or-address-display.test.tsx deleted file mode 100644 index d6726d758e0d..000000000000 --- a/ui/pages/confirmations/components/confirm/info/token-transfer/name-or-address-display.test.tsx +++ /dev/null @@ -1,26 +0,0 @@ -import React from 'react'; -import configureMockStore from 'redux-mock-store'; -import { getMockTokenTransferConfirmState } from '../../../../../../../test/data/confirmations/helper'; -import { renderWithConfirmContextProvider } from '../../../../../../../test/lib/confirmations/render-helpers'; -import { NameOrAddressDisplay } from './name-or-address-display'; - -jest.mock( - '../../../../../../components/app/alert-system/contexts/alertMetricsContext', - () => ({ - useAlertMetrics: jest.fn(() => ({ - trackAlertMetrics: jest.fn(), - })), - }), -); - -describe('NameOrAddressDisplay', () => { - it('renders correctly', () => { - const state = getMockTokenTransferConfirmState({}); - const mockStore = configureMockStore([])(state); - const { container } = renderWithConfirmContextProvider( - , - mockStore, - ); - expect(container).toMatchSnapshot(); - }); -}); diff --git a/ui/pages/confirmations/components/confirm/info/token-transfer/name-or-address-display.tsx b/ui/pages/confirmations/components/confirm/info/token-transfer/name-or-address-display.tsx deleted file mode 100644 index 6b8b7de3a86d..000000000000 --- a/ui/pages/confirmations/components/confirm/info/token-transfer/name-or-address-display.tsx +++ /dev/null @@ -1,59 +0,0 @@ -import { NameType } from '@metamask/name-controller'; -import React, { useState } from 'react'; -import { useSelector } from 'react-redux'; -import { useFallbackDisplayName } from '../../../../../../components/app/confirm/info/row/hook'; -import NicknamePopovers from '../../../../../../components/app/modals/nickname-popovers'; -import Name from '../../../../../../components/app/name'; -import { - AvatarAccount, - AvatarAccountSize, - Box, - Text, -} from '../../../../../../components/component-library'; -import { - AlignItems, - BorderColor, - Display, - FlexDirection, - TextColor, -} from '../../../../../../helpers/constants/design-system'; -import { getPetnamesEnabled } from '../../../../../../selectors'; - -export const NameOrAddressDisplay = ({ address }: { address: string }) => { - const isPetNamesEnabled = useSelector(getPetnamesEnabled); - const { displayName, hexAddress } = useFallbackDisplayName(address); - const [isNicknamePopoverShown, setIsNicknamePopoverShown] = useState(false); - const handleDisplayNameClick = () => setIsNicknamePopoverShown(true); - const onCloseHandler = () => setIsNicknamePopoverShown(false); - - if (isPetNamesEnabled) { - return ; - } - - return ( - <> - - - - {displayName} - - - {isNicknamePopoverShown ? ( - - ) : null} - - ); -}; diff --git a/ui/pages/confirmations/components/confirm/info/token-transfer/token-details-section.tsx b/ui/pages/confirmations/components/confirm/info/token-transfer/token-details-section.tsx index a5b6726bd019..48a5f2dad74c 100644 --- a/ui/pages/confirmations/components/confirm/info/token-transfer/token-details-section.tsx +++ b/ui/pages/confirmations/components/confirm/info/token-transfer/token-details-section.tsx @@ -23,10 +23,7 @@ import { TextVariant, } from '../../../../../../helpers/constants/design-system'; import { useI18nContext } from '../../../../../../hooks/useI18nContext'; -import { - getCurrentChainId, - getNetworkConfigurationsByChainId, -} from '../../../../../../selectors'; +import { getNetworkConfigurationsByChainId } from '../../../../../../selectors'; import { useConfirmContext } from '../../../../context/confirm'; export const TokenDetailsSection = () => { @@ -34,7 +31,7 @@ export const TokenDetailsSection = () => { const { currentConfirmation: transactionMeta } = useConfirmContext(); - const chainId = useSelector(getCurrentChainId); + const { chainId } = transactionMeta; const networkConfigurations = useSelector(getNetworkConfigurationsByChainId); const networkName = networkConfigurations[chainId].name; diff --git a/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.stories.tsx b/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.stories.tsx index 384a8f161e9b..1cb5f3b40ab2 100644 --- a/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.stories.tsx +++ b/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.stories.tsx @@ -1,6 +1,13 @@ import React from 'react'; import { Provider } from 'react-redux'; import { getMockTokenTransferConfirmState } from '../../../../../../../test/data/confirmations/helper'; +import { Box } from '../../../../../../components/component-library'; +import { + AlignItems, + Display, + FlexDirection, + JustifyContent, +} from '../../../../../../helpers/constants/design-system'; import configureStore from '../../../../../../store/store'; import { ConfirmContextProvider } from '../../../../context/confirm'; import TokenTransferInfo from './token-transfer'; @@ -13,7 +20,16 @@ const Story = { decorators: [ (story: () => any) => ( - {story()} + + + {story()} + + ), ], diff --git a/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.tsx b/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.tsx index 0f0649659467..9c0dfe81f536 100644 --- a/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.tsx +++ b/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.tsx @@ -1,6 +1,4 @@ import React from 'react'; -import { useSelector } from 'react-redux'; -import { selectConfirmationAdvancedDetailsOpen } from '../../../../selectors/preferences'; import { AdvancedDetails } from '../shared/advanced-details/advanced-details'; import { GasFeesSection } from '../shared/gas-fees-section/gas-fees-section'; import SendHeading from '../shared/send-heading/send-heading'; @@ -8,17 +6,13 @@ import { TokenDetailsSection } from './token-details-section'; import { TransactionFlowSection } from './transaction-flow-section'; const TokenTransferInfo = () => { - const showAdvancedDetails = useSelector( - selectConfirmationAdvancedDetailsOpen, - ); - return ( <> - {showAdvancedDetails && } + ); }; diff --git a/ui/pages/confirmations/components/confirm/info/token-transfer/transaction-flow-section.test.tsx b/ui/pages/confirmations/components/confirm/info/token-transfer/transaction-flow-section.test.tsx new file mode 100644 index 000000000000..c23d3645abd3 --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/token-transfer/transaction-flow-section.test.tsx @@ -0,0 +1,48 @@ +import { TransactionType } from '@metamask/transaction-controller'; +import React from 'react'; +import configureMockStore from 'redux-mock-store'; +import { getMockTokenTransferConfirmState } from '../../../../../../../test/data/confirmations/helper'; +import { renderWithConfirmContextProvider } from '../../../../../../../test/lib/confirmations/render-helpers'; +import { useDecodedTransactionData } from '../hooks/useDecodedTransactionData'; +import { TransactionFlowSection } from './transaction-flow-section'; + +jest.mock('../hooks/useDecodedTransactionData', () => ({ + ...jest.requireActual('../hooks/useDecodedTransactionData'), + useDecodedTransactionData: jest.fn(), +})); + +describe('', () => { + const useDecodedTransactionDataMock = jest.fn().mockImplementation(() => ({ + pending: false, + value: { + data: [ + { + name: TransactionType.tokenMethodTransfer, + params: [ + { + name: 'dst', + type: 'address', + value: '0x6B175474E89094C44Da98b954EedeAC495271d0F', + }, + { name: 'wad', type: 'uint256', value: 0 }, + ], + }, + ], + source: 'Sourcify', + }, + })); + + (useDecodedTransactionData as jest.Mock).mockImplementation( + useDecodedTransactionDataMock, + ); + + it('renders correctly', () => { + const state = getMockTokenTransferConfirmState({}); + const mockStore = configureMockStore([])(state); + const { container } = renderWithConfirmContextProvider( + , + mockStore, + ); + expect(container).toMatchSnapshot(); + }); +}); diff --git a/ui/pages/confirmations/components/confirm/info/token-transfer/transaction-flow-section.tsx b/ui/pages/confirmations/components/confirm/info/token-transfer/transaction-flow-section.tsx index cd195d2c4e38..de0e928c10f8 100644 --- a/ui/pages/confirmations/components/confirm/info/token-transfer/transaction-flow-section.tsx +++ b/ui/pages/confirmations/components/confirm/info/token-transfer/transaction-flow-section.tsx @@ -1,6 +1,8 @@ +import { NameType } from '@metamask/name-controller'; import { TransactionMeta } from '@metamask/transaction-controller'; import React from 'react'; import { ConfirmInfoSection } from '../../../../../../components/app/confirm/info/row/section'; +import Name from '../../../../../../components/app/name'; import { Box, Icon, @@ -17,7 +19,6 @@ import { import { useConfirmContext } from '../../../../context/confirm'; import { useDecodedTransactionData } from '../hooks/useDecodedTransactionData'; import { ConfirmLoader } from '../shared/confirm-loader/confirm-loader'; -import { NameOrAddressDisplay } from './name-or-address-display'; export const TransactionFlowSection = () => { const { currentConfirmation: transactionMeta } = @@ -25,9 +26,9 @@ export const TransactionFlowSection = () => { const { value, pending } = useDecodedTransactionData(); - const recipientAddress = - value?.data[0].params.find((param) => param.type === 'address')?.value || - '0x0000000000000000000000000000000000000000'; + const recipientAddress = value?.data[0].params.find( + (param) => param.type === 'address', + )?.value; if (pending) { return ; @@ -42,13 +43,18 @@ export const TransactionFlowSection = () => { alignItems={AlignItems.center} padding={3} > - + - + {recipientAddress && ( + + )} );