From 9ecdb80250a8b9b283879ac2511fa39c071d0b71 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 1 Aug 2024 14:52:18 +0800 Subject: [PATCH 01/19] feat: add account-list-menu account type filter --- .../account-list-menu/account-list-menu.js | 29 +++- ...enu.test.js => account-list-menu.test.tsx} | 162 ++++++++++-------- 2 files changed, 115 insertions(+), 76 deletions(-) rename ui/components/multichain/account-list-menu/{account-list-menu.test.js => account-list-menu.test.tsx} (81%) diff --git a/ui/components/multichain/account-list-menu/account-list-menu.js b/ui/components/multichain/account-list-menu/account-list-menu.js index 67098bc97e41..f276437649af 100644 --- a/ui/components/multichain/account-list-menu/account-list-menu.js +++ b/ui/components/multichain/account-list-menu/account-list-menu.js @@ -1,4 +1,4 @@ -import React, { useContext, useState } from 'react'; +import React, { useContext, useState, useMemo } from 'react'; import PropTypes from 'prop-types'; import { useHistory } from 'react-router-dom'; import Fuse from 'fuse.js'; @@ -154,10 +154,16 @@ export const AccountListMenu = ({ onClose, showAccountCreation = true, accountListItemProps = {}, + excludeAccountTypes = [], }) => { const t = useI18nContext(); const trackEvent = useContext(MetaMetricsContext); const accounts = useSelector(getMetaMaskAccountsOrdered); + const filteredAccounts = useMemo( + () => + accounts.filter((account) => !excludeAccountTypes.includes(account.type)), + [accounts, excludeAccountTypes], + ); const selectedAccount = useSelector(getSelectedInternalAccount); const connectedSites = useSelector(getConnectedSubjectsForAllAddresses); const currentTabOrigin = useSelector(getOriginOfCurrentTab); @@ -165,6 +171,13 @@ export const AccountListMenu = ({ const dispatch = useDispatch(); const hiddenAddresses = useSelector(getHiddenAccountsList); const updatedAccountsList = useSelector(getUpdatedAndSortedAccounts); + const filteredUpdatedAccountList = useMemo( + () => + updatedAccountsList.filter( + (account) => !excludeAccountTypes.includes(account.type), + ), + [updatedAccountsList, excludeAccountTypes], + ); ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) const addSnapAccountEnabled = useSelector(getIsAddSnapAccountEnabled); ///: END:ONLY_INCLUDE_IF @@ -194,9 +207,9 @@ export const AccountListMenu = ({ const [searchQuery, setSearchQuery] = useState(''); const [actionMode, setActionMode] = useState(ACTION_MODES.LIST); - let searchResults = updatedAccountsList; + let searchResults = filteredUpdatedAccountList; if (searchQuery) { - const fuse = new Fuse(accounts, { + const fuse = new Fuse(filteredAccounts, { threshold: 0.2, location: 0, distance: 100, @@ -204,10 +217,10 @@ export const AccountListMenu = ({ minMatchCharLength: 1, keys: ['metadata.name', 'address'], }); - fuse.setCollection(accounts); + fuse.setCollection(filteredAccounts); searchResults = fuse.search(searchQuery); } - searchResults = mergeAccounts(searchResults, accounts); + searchResults = mergeAccounts(searchResults, filteredAccounts); const title = getActionTitle(t, actionMode); @@ -454,7 +467,7 @@ export const AccountListMenu = ({ {actionMode === ACTION_MODES.LIST ? ( <> {/* Search box */} - {accounts.length > 1 ? ( + {filteredAccounts.length > 1 ? ( ({ useHistory: jest.fn(() => []), })); -const render = (props = { onClose: () => jest.fn() }) => { - const store = configureStore({ +const render = ( + state = {}, + props: { + onClose: () => void; + excludeAccountTypes: string[]; + } = { + onClose: () => jest.fn(), + excludeAccountTypes: [], + }, +) => { + const defaultState = { ...mockState, metamask: { ...mockState.metamask, @@ -76,7 +86,8 @@ const render = (props = { onClose: () => jest.fn() }) => { unconnectedAccount: { state: 'OPEN', }, - }); + }; + const store = configureStore(state ?? defaultState); return renderWithProvider(, store); }; @@ -87,6 +98,7 @@ describe('AccountListMenu', () => { jest .spyOn(reactRouterDom, 'useHistory') .mockImplementation() + // @ts-ignore mocking histroy return .mockReturnValue({ push: historyPushMock }); }); @@ -103,13 +115,13 @@ describe('AccountListMenu', () => { }); it('displays accounts for list and filters by search', () => { - render(); + const { container } = render(); const listItems = document.querySelectorAll( '.multichain-account-list-item', ); expect(listItems).toHaveLength(6); - const searchBox = document.querySelector('input[type=search]'); + const searchBox = document.querySelector('input[type=search]') as Element; fireEvent.change(searchBox, { target: { value: 'Le' }, }); @@ -123,7 +135,7 @@ describe('AccountListMenu', () => { it('displays the "no accounts" message when search finds nothing', () => { const { getByTestId } = render(); - const searchBox = document.querySelector('input[type=search]'); + const searchBox = document.querySelector('input[type=search]') as Element; fireEvent.change(searchBox, { target: { value: 'adslfkjlx' }, }); @@ -217,11 +229,11 @@ describe('AccountListMenu', () => { }); it('add / Import / Hardware button functions as it should', () => { - const { getByText } = render(); + const { getByText, getAllByTestId, getByRole, getByLabelText } = render(); // Ensure the button is displaying - const button = document.querySelectorAll( - '[data-testid="multichain-account-menu-popover-action-button"]', + const button = getAllByTestId( + 'multichain-account-menu-popover-action-button', ); expect(button).toHaveLength(1); @@ -230,13 +242,13 @@ describe('AccountListMenu', () => { expect(getByText('Add a new Ethereum account')).toBeInTheDocument(); expect(getByText('Import account')).toBeInTheDocument(); expect(getByText('Add hardware wallet')).toBeInTheDocument(); - const header = document.querySelector('header'); + const header = document.querySelector('header') as Element; expect(header.innerHTML).toContain('Add account'); expect( document.querySelector('button[aria-label="Close"]'), ).toBeInTheDocument(); - const backButton = document.querySelector('button[aria-label="Back"]'); + const backButton = getByLabelText('Back'); expect(backButton).toBeInTheDocument(); backButton.click(); @@ -244,11 +256,9 @@ describe('AccountListMenu', () => { }); it('shows the account creation UI when Add Account is clicked', () => { - const { getByText, getByPlaceholderText } = render(); + const { getByText, getByPlaceholderText, getByTestId } = render(); - const button = document.querySelector( - '[data-testid="multichain-account-menu-popover-action-button"]', - ); + const button = getByTestId('multichain-account-menu-popover-action-button'); button.click(); fireEvent.click(getByText('Add a new Ethereum account')); @@ -263,11 +273,9 @@ describe('AccountListMenu', () => { }); it('shows the account import UI when Import Account is clicked', () => { - const { getByText, getByPlaceholderText } = render(); + const { getByText, getByPlaceholderText, getByTestId } = render(); - const button = document.querySelector( - '[data-testid="multichain-account-menu-popover-action-button"]', - ); + const button = getByTestId('multichain-account-menu-popover-action-button'); button.click(); fireEvent.click(getByText('Import account')); @@ -279,11 +287,9 @@ describe('AccountListMenu', () => { }); it('navigates to hardware wallet connection screen when clicked', () => { - const { getByText } = render(); + const { getByText, getByTestId } = render(); - const button = document.querySelector( - '[data-testid="multichain-account-menu-popover-action-button"]', - ); + const button = getByTestId('multichain-account-menu-popover-action-button'); button.click(); fireEvent.click(getByText('Add hardware wallet')); @@ -292,7 +298,10 @@ describe('AccountListMenu', () => { ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) describe('addSnapAccountButton', () => { - const renderWithState = (state, props = { onClose: mockOnClose }) => { + const renderWithState = ( + state: Record, + props = { onClose: mockOnClose }, + ) => { const store = configureStore({ ...mockState, ...{ @@ -338,9 +347,11 @@ describe('AccountListMenu', () => { }; it("doesn't render the add snap account button if it's disabled", async () => { - const { getByText } = renderWithState({ addSnapAccountEnabled: false }); - const button = document.querySelector( - '[data-testid="multichain-account-menu-popover-action-button"]', + const { getByText, getByTestId } = renderWithState({ + addSnapAccountEnabled: false, + }); + const button = getByTestId( + 'multichain-account-menu-popover-action-button', ); button.click(); expect(() => getByText(messages.settingAddSnapAccount.message)).toThrow( @@ -349,10 +360,13 @@ describe('AccountListMenu', () => { }); it('renders the "Add account Snap" button if it\'s enabled', async () => { + // @ts-ignore mocking platform global.platform = { openTab: jest.fn() }; - const { getByText } = renderWithState({ addSnapAccountEnabled: true }); - const button = document.querySelector( - '[data-testid="multichain-account-menu-popover-action-button"]', + const { getByText, getByTestId } = renderWithState({ + addSnapAccountEnabled: true, + }); + const button = getByTestId( + 'multichain-account-menu-popover-action-button', ); button.click(); const addSnapAccountButton = getByText( @@ -368,13 +382,16 @@ describe('AccountListMenu', () => { it('opens the Snaps registry in a new tab', async () => { // Set up mock state + // @ts-ignore mocking platform global.platform = { openTab: jest.fn() }; - const { getByText } = renderWithState({ addSnapAccountEnabled: true }); + const { getByText, getByTestId } = renderWithState({ + addSnapAccountEnabled: true, + }); mockGetEnvironmentType.mockReturnValueOnce('fullscreen'); // Open account picker - const button = document.querySelector( - '[data-testid="multichain-account-menu-popover-action-button"]', + const button = getByTestId( + 'multichain-account-menu-popover-action-button', ); button.click(); @@ -450,45 +467,13 @@ describe('AccountListMenu', () => { const listItems = document.querySelectorAll( '.multichain-account-list-item', ); - const tag = listItems[0].querySelector('.mm-tag'); + const tag = listItems[0].querySelector('.mm-tag') as Element; expect(tag.textContent).toBe('Snaps (Beta)'); }); it('displays the correct label for named snap accounts', () => { - const mockStore = configureStore({ - activeTab: { - title: 'Eth Sign Tests', - origin: 'https://remix.ethereum.org', - protocol: 'https:', - url: 'https://remix.ethereum.org/', - }, + render({ metamask: { - ...mockState.metamask, - permissionHistory: { - 'https://test.dapp': { - eth_accounts: { - accounts: { - '0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc': 1596681857076, - }, - }, - }, - }, - subjects: { - 'https://test.dapp': { - permissions: { - eth_accounts: { - caveats: [ - { - type: 'restrictReturnedAccounts', - value: ['0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc'], - }, - ], - invoker: 'https://test.dapp', - parentCapability: 'eth_accounts', - }, - }, - }, - }, internalAccounts: { accounts: { ...mockState.metamask.internalAccounts.accounts, @@ -512,12 +497,49 @@ describe('AccountListMenu', () => { }, }, }); - renderWithProvider(, mockStore); const listItems = document.querySelectorAll( '.multichain-account-list-item', ); - const tag = listItems[0].querySelector('.mm-tag'); + const tag = listItems[0].querySelector('.mm-tag') as Element; expect(tag.textContent).toBe('Test Snap Name (Beta)'); }); ///: END:ONLY_INCLUDE_IF + + it('prop `excludeAccountTypes` filters accounts', () => { + const mockAccount = createMockInternalAccount(); + const mockBtcAccount = createMockInternalAccount({ + name: 'Bitcoin Account', + type: BtcAccountType.P2wpkh, + address: 'bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq', + }); + const { queryByText } = render( + { + ...mockState, + metamask: { + ...mockState.metamask, + internalAccounts: { + accounts: { + [mockAccount.id]: mockAccount, + [mockBtcAccount.id]: mockBtcAccount, + }, + selectedAccount: mockAccount.id, + }, + keyrings: [ + { + type: 'HD Key Tree', + accounts: [mockAccount.address], + }, + { + type: 'Snap Keyring', + accounts: [mockBtcAccount.address], + }, + ], + }, + }, + { onClose: jest.fn(), excludeAccountTypes: [BtcAccountType.P2wpkh] }, + ); + + expect(queryByText(mockAccount.metadata.name)).toBeInTheDocument(); + expect(queryByText(mockBtcAccount.metadata.name)).not.toBeInTheDocument(); + }); }); From 53abca1e9222ab63e87917e58034373b48bf68d5 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 1 Aug 2024 14:52:47 +0800 Subject: [PATCH 02/19] fix: exclude btc account types in account picker of send --- .../send/components/account-picker.test.tsx | 66 +++++++++++++++++-- .../pages/send/components/account-picker.tsx | 2 + 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/ui/components/multichain/pages/send/components/account-picker.test.tsx b/ui/components/multichain/pages/send/components/account-picker.test.tsx index 550397882ed0..35cb89650d5e 100644 --- a/ui/components/multichain/pages/send/components/account-picker.test.tsx +++ b/ui/components/multichain/pages/send/components/account-picker.test.tsx @@ -1,17 +1,28 @@ import React from 'react'; import configureMockStore from 'redux-mock-store'; import thunk from 'redux-thunk'; - +import { BtcAccountType } from '@metamask/keyring-api'; import mockState from '../../../../../../test/data/mock-state.json'; import { fireEvent, renderWithProvider } from '../../../../../../test/jest'; import { SEND_STAGES } from '../../../../../ducks/send'; -import { INITIAL_SEND_STATE_FOR_EXISTING_DRAFT } from '../../../../../../test/jest/mocks'; +import { + INITIAL_SEND_STATE_FOR_EXISTING_DRAFT, + createMockInternalAccount, +} from '../../../../../../test/jest/mocks'; +import { CombinedBackgroundAndReduxState } from '../../../../../store/store'; +import { shortenAddress } from '../../../../../helpers/utils/util'; +import { normalizeSafeAddress } from '../../../../../../app/scripts/lib/multichain/address'; import { SendPageAccountPicker } from '.'; -const render = (props = {}, sendStage = SEND_STAGES.ADD_RECIPIENT) => { +const render = ( + state: Partial = {}, + props = {}, + sendStage = SEND_STAGES.ADD_RECIPIENT, +) => { const middleware = [thunk]; const store = configureMockStore(middleware)({ ...mockState, + ...state, send: { ...INITIAL_SEND_STATE_FOR_EXISTING_DRAFT, stage: sendStage, @@ -20,6 +31,7 @@ const render = (props = {}, sendStage = SEND_STAGES.ADD_RECIPIENT) => { history: { mostRecentOverviewPage: 'activity' }, metamask: { ...mockState.metamask, + ...state.metamask, permissionHistory: { 'https://test.dapp': { eth_accounts: { @@ -65,7 +77,7 @@ describe('SendPageAccountPicker', () => { it('renders as disabled when editing a send', () => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore - const { getByTestId } = render({}, SEND_STAGES.EDIT); + const { getByTestId } = render({}, {}, SEND_STAGES.EDIT); expect(getByTestId('send-page-account-picker')).toBeDisabled(); }); @@ -80,4 +92,50 @@ describe('SendPageAccountPicker', () => { ).toBeInTheDocument(); }); }); + + describe('Multichain', () => { + it('cannot select BTC accounts', async () => { + const mockAccount = createMockInternalAccount(); + const mockBtcAccount = createMockInternalAccount({ + name: 'Bitcoin Account', + type: BtcAccountType.P2wpkh, + address: 'bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq', + }); + const { queryByText, queryAllByTestId, getByTestId } = render({ + metamask: { + internalAccounts: { + accounts: { + [mockAccount.id]: mockAccount, + [mockBtcAccount.id]: mockBtcAccount, + }, + selectedAccount: mockAccount.id, + }, + keyrings: [ + { + type: 'HD Key Tree', + accounts: [mockAccount.address], + }, + { + type: 'Snap Keyring', + accounts: [mockBtcAccount.address], + }, + ], + }, + } as CombinedBackgroundAndReduxState); + + expect(queryByText(mockAccount.metadata.name)).toBeInTheDocument(); + + const accountPicker = getByTestId('send-page-account-picker'); + fireEvent.click(accountPicker); + + const addressesInAccountList = queryAllByTestId('account-list-address'); + expect(addressesInAccountList).toHaveLength(1); + expect(addressesInAccountList[0].textContent).toContain( + shortenAddress(normalizeSafeAddress(mockAccount.address)), + ); + expect(addressesInAccountList[0].textContent).not.toContain( + shortenAddress(normalizeSafeAddress(mockBtcAccount.address)), + ); + }); + }); }); diff --git a/ui/components/multichain/pages/send/components/account-picker.tsx b/ui/components/multichain/pages/send/components/account-picker.tsx index c302c21af8cc..f925bf57d445 100644 --- a/ui/components/multichain/pages/send/components/account-picker.tsx +++ b/ui/components/multichain/pages/send/components/account-picker.tsx @@ -1,5 +1,6 @@ import React, { useContext, useState } from 'react'; import { useSelector } from 'react-redux'; +import { BtcAccountType } from '@metamask/keyring-api'; import { getSelectedInternalAccount } from '../../../../../selectors'; import { Label } from '../../../../component-library'; import { AccountPicker } from '../../../account-picker'; @@ -60,6 +61,7 @@ export const SendPageAccountPicker = () => { {showAccountPicker ? ( setShowAccountPicker(false)} /> From 2e2d5fc22621f998971f044664ecbbfb41fa7e4f Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 1 Aug 2024 14:53:09 +0800 Subject: [PATCH 03/19] fix: add keyrings type in TemporaryBackgroundState type --- ui/store/store.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/store/store.ts b/ui/store/store.ts index 14d899542aa1..a70d02864b2e 100644 --- a/ui/store/store.ts +++ b/ui/store/store.ts @@ -85,6 +85,7 @@ type TemporaryBackgroundState = { }; selectedAccount: string; }; + keyrings: { type: string; accounts: string[] }[]; }; type RootReducerReturnType = ReturnType; From f99e01391cc691045d2de8672a9f87819abab332 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 1 Aug 2024 14:54:42 +0800 Subject: [PATCH 04/19] fix: filter btc accounts from SendPageYourAccounts --- .../send/components/your-accounts.test.tsx | 50 ++++++++++++++++++- .../pages/send/components/your-accounts.tsx | 11 +++- 2 files changed, 58 insertions(+), 3 deletions(-) diff --git a/ui/components/multichain/pages/send/components/your-accounts.test.tsx b/ui/components/multichain/pages/send/components/your-accounts.test.tsx index 815cd9ceacae..3cc9d424f50b 100644 --- a/ui/components/multichain/pages/send/components/your-accounts.test.tsx +++ b/ui/components/multichain/pages/send/components/your-accounts.test.tsx @@ -1,7 +1,10 @@ import React from 'react'; +import { BtcAccountType } from '@metamask/keyring-api'; import configureStore from '../../../../../store/store'; import mockState from '../../../../../../test/data/mock-state.json'; import { fireEvent, renderWithProvider } from '../../../../../../test/jest'; +import { createMockInternalAccount } from '../../../../../../test/jest/mocks'; +import { MultichainNativeAssets } from '../../../../../../shared/constants/multichain/assets'; import { SendPageYourAccounts } from '.'; const mockUpdateRecipient = jest.fn(); @@ -14,11 +17,12 @@ jest.mock('../../../../../ducks/send', () => ({ updateRecipientUserInput: () => mockUpdateRecipientUserInput, })); -const render = (props = {}) => { +const render = (props = {}, state = {}) => { const store = configureStore({ ...mockState, metamask: { ...mockState.metamask, + ...state, permissionHistory: { 'https://test.dapp': { eth_accounts: { @@ -57,4 +61,48 @@ describe('SendPageYourAccounts', () => { } }); }); + + describe('Multichain', () => { + it('does not render BTC accounts', () => { + const mockAccount = createMockInternalAccount(); + const mockBtcAccount = createMockInternalAccount({ + address: 'bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq', + type: BtcAccountType.P2wpkh, + name: 'Btc Account', + }); + const { queryByText } = render( + {}, + { + internalAccounts: { + accounts: { + [mockAccount.id]: mockAccount, + [mockBtcAccount.id]: mockBtcAccount, + }, + selectedAccount: mockAccount.id, + }, + keyrings: [ + { + type: 'HD Key Tree', + accounts: [mockAccount.address], + }, + { + type: 'Snap Keyring', + accounts: [mockBtcAccount.address], + }, + ], + balances: { + [mockBtcAccount.id]: { + [MultichainNativeAssets.BITCOIN]: { + amount: '1.00000000', + unit: 'BTC', + }, + }, + }, + }, + ); + + expect(queryByText(mockAccount.metadata.name)).toBeInTheDocument(); + expect(queryByText(mockBtcAccount.metadata.name)).not.toBeInTheDocument(); + }); + }); }); diff --git a/ui/components/multichain/pages/send/components/your-accounts.tsx b/ui/components/multichain/pages/send/components/your-accounts.tsx index a881e8922634..33f44276150b 100644 --- a/ui/components/multichain/pages/send/components/your-accounts.tsx +++ b/ui/components/multichain/pages/send/components/your-accounts.tsx @@ -1,5 +1,7 @@ -import React, { useContext } from 'react'; +import React, { useContext, useMemo } from 'react'; import { useDispatch, useSelector } from 'react-redux'; +import { KeyringTypes } from '@metamask/keyring-controller'; +import { BtcAccountType, InternalAccount } from '@metamask/keyring-api'; import { getUpdatedAndSortedAccounts, getInternalAccounts, @@ -25,7 +27,12 @@ export const SendPageYourAccounts = () => { // Your Accounts const accounts = useSelector(getUpdatedAndSortedAccounts); const internalAccounts = useSelector(getInternalAccounts); - const mergedAccounts = mergeAccounts(accounts, internalAccounts); + const mergedAccounts: InternalAccount & + { keyring: KeyringTypes; label: string }[] = useMemo(() => { + return mergeAccounts(accounts, internalAccounts).filter( + (account: InternalAccount) => account.type !== BtcAccountType.P2wpkh, + ); + }, [accounts, internalAccounts]); return ( From 99731969f2e4052eda11d819a5502ca9e833a17c Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 1 Aug 2024 15:23:40 +0800 Subject: [PATCH 05/19] fix: test --- .../account-list-menu.test.tsx | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/ui/components/multichain/account-list-menu/account-list-menu.test.tsx b/ui/components/multichain/account-list-menu/account-list-menu.test.tsx index 5e25aff0cf2a..45b44d634342 100644 --- a/ui/components/multichain/account-list-menu/account-list-menu.test.tsx +++ b/ui/components/multichain/account-list-menu/account-list-menu.test.tsx @@ -10,8 +10,9 @@ import messages from '../../../../app/_locales/en/messages.json'; import { CONNECT_HARDWARE_ROUTE } from '../../../helpers/constants/routes'; ///: END:ONLY_INCLUDE_IF import { ETH_EOA_METHODS } from '../../../../shared/constants/eth-methods'; -import { AccountListMenu } from '.'; import { createMockInternalAccount } from '../../../../test/jest/mocks'; +import { AccountListMenu } from '.'; +import { merge } from 'lodash'; ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) const mockOnClose = jest.fn(); @@ -87,7 +88,7 @@ const render = ( state: 'OPEN', }, }; - const store = configureStore(state ?? defaultState); + const store = configureStore(merge(defaultState, state)); return renderWithProvider(, store); }; @@ -98,7 +99,7 @@ describe('AccountListMenu', () => { jest .spyOn(reactRouterDom, 'useHistory') .mockImplementation() - // @ts-ignore mocking histroy return + // @ts-expect-error mocking histroy return .mockReturnValue({ push: historyPushMock }); }); @@ -115,7 +116,7 @@ describe('AccountListMenu', () => { }); it('displays accounts for list and filters by search', () => { - const { container } = render(); + render(); const listItems = document.querySelectorAll( '.multichain-account-list-item', ); @@ -229,7 +230,7 @@ describe('AccountListMenu', () => { }); it('add / Import / Hardware button functions as it should', () => { - const { getByText, getAllByTestId, getByRole, getByLabelText } = render(); + const { getByText, getAllByTestId, getByLabelText } = render(); // Ensure the button is displaying const button = getAllByTestId( @@ -299,7 +300,7 @@ describe('AccountListMenu', () => { ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) describe('addSnapAccountButton', () => { const renderWithState = ( - state: Record, + state: { addSnapAccountEnabled: boolean }, props = { onClose: mockOnClose }, ) => { const store = configureStore({ @@ -360,7 +361,7 @@ describe('AccountListMenu', () => { }); it('renders the "Add account Snap" button if it\'s enabled', async () => { - // @ts-ignore mocking platform + // @ts-expect-error mocking platform global.platform = { openTab: jest.fn() }; const { getByText, getByTestId } = renderWithState({ addSnapAccountEnabled: true, @@ -382,7 +383,7 @@ describe('AccountListMenu', () => { it('opens the Snaps registry in a new tab', async () => { // Set up mock state - // @ts-ignore mocking platform + // @ts-expect-error mocking platform global.platform = { openTab: jest.fn() }; const { getByText, getByTestId } = renderWithState({ addSnapAccountEnabled: true, From 8ef26ee8e1df0f4451c665b9e8e996e18e4aa839 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 1 Aug 2024 15:34:14 +0800 Subject: [PATCH 06/19] fix: lint --- .../multichain/account-list-menu/account-list-menu.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/components/multichain/account-list-menu/account-list-menu.test.tsx b/ui/components/multichain/account-list-menu/account-list-menu.test.tsx index 45b44d634342..52c6884756f2 100644 --- a/ui/components/multichain/account-list-menu/account-list-menu.test.tsx +++ b/ui/components/multichain/account-list-menu/account-list-menu.test.tsx @@ -2,6 +2,7 @@ import React from 'react'; import reactRouterDom from 'react-router-dom'; import { BtcAccountType, EthAccountType } from '@metamask/keyring-api'; +import { merge } from 'lodash'; import { fireEvent, renderWithProvider, waitFor } from '../../../../test/jest'; import configureStore from '../../../store/store'; import mockState from '../../../../test/data/mock-state.json'; @@ -12,7 +13,6 @@ import { CONNECT_HARDWARE_ROUTE } from '../../../helpers/constants/routes'; import { ETH_EOA_METHODS } from '../../../../shared/constants/eth-methods'; import { createMockInternalAccount } from '../../../../test/jest/mocks'; import { AccountListMenu } from '.'; -import { merge } from 'lodash'; ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) const mockOnClose = jest.fn(); From 69091bead185b4134a30391522585309eb9b45a2 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 1 Aug 2024 21:13:23 +0800 Subject: [PATCH 07/19] fix: convert account-list-item to ts and change excludedAccountTypes to allowedAccountTypes --- .../account-list-menu.test.tsx | 98 ++++++++++++------ ...unt-list-menu.js => account-list-menu.tsx} | 99 +++++++++++++------ .../pages/send/components/account-picker.tsx | 2 - ui/selectors/permissions.js | 9 ++ ui/selectors/selectors.js | 6 ++ ui/selectors/selectors.type.ts | 28 ++++++ 6 files changed, 181 insertions(+), 61 deletions(-) rename ui/components/multichain/account-list-menu/{account-list-menu.js => account-list-menu.tsx} (88%) create mode 100644 ui/selectors/selectors.type.ts diff --git a/ui/components/multichain/account-list-menu/account-list-menu.test.tsx b/ui/components/multichain/account-list-menu/account-list-menu.test.tsx index 52c6884756f2..ad48d8213f39 100644 --- a/ui/components/multichain/account-list-menu/account-list-menu.test.tsx +++ b/ui/components/multichain/account-list-menu/account-list-menu.test.tsx @@ -1,7 +1,11 @@ /* eslint-disable jest/require-top-level-describe */ import React from 'react'; import reactRouterDom from 'react-router-dom'; -import { BtcAccountType, EthAccountType } from '@metamask/keyring-api'; +import { + BtcAccountType, + EthAccountType, + KeyringAccountType, +} from '@metamask/keyring-api'; import { merge } from 'lodash'; import { fireEvent, renderWithProvider, waitFor } from '../../../../test/jest'; import configureStore from '../../../store/store'; @@ -41,10 +45,10 @@ const render = ( state = {}, props: { onClose: () => void; - excludeAccountTypes: string[]; + allowedAccountTypes: KeyringAccountType[]; } = { onClose: () => jest.fn(), - excludeAccountTypes: [], + allowedAccountTypes: [EthAccountType.Eoa, EthAccountType.Erc4337], }, ) => { const defaultState = { @@ -506,41 +510,79 @@ describe('AccountListMenu', () => { }); ///: END:ONLY_INCLUDE_IF - it('prop `excludeAccountTypes` filters accounts', () => { + describe('prop `allowedAccountTypes`', () => { const mockAccount = createMockInternalAccount(); const mockBtcAccount = createMockInternalAccount({ name: 'Bitcoin Account', type: BtcAccountType.P2wpkh, address: 'bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq', }); - const { queryByText } = render( - { - ...mockState, - metamask: { - ...mockState.metamask, - internalAccounts: { - accounts: { - [mockAccount.id]: mockAccount, - [mockBtcAccount.id]: mockBtcAccount, + + it('allows only EthAccountTypes', () => { + const { queryByText } = render( + { + ...mockState, + metamask: { + ...mockState.metamask, + internalAccounts: { + accounts: { + [mockAccount.id]: mockAccount, + [mockBtcAccount.id]: mockBtcAccount, + }, + selectedAccount: mockAccount.id, }, - selectedAccount: mockAccount.id, + keyrings: [ + { + type: 'HD Key Tree', + accounts: [mockAccount.address], + }, + { + type: 'Snap Keyring', + accounts: [mockBtcAccount.address], + }, + ], }, - keyrings: [ - { - type: 'HD Key Tree', - accounts: [mockAccount.address], - }, - { - type: 'Snap Keyring', - accounts: [mockBtcAccount.address], + }, + { + onClose: jest.fn(), + allowedAccountTypes: [EthAccountType.Eoa, EthAccountType.Erc4337], + }, + ); + + expect(queryByText(mockAccount.metadata.name)).toBeInTheDocument(); + expect(queryByText(mockBtcAccount.metadata.name)).not.toBeInTheDocument(); + }); + + it('allows only BtcAccountType', () => { + const { queryByText } = render( + { + ...mockState, + metamask: { + ...mockState.metamask, + internalAccounts: { + accounts: { + [mockAccount.id]: mockAccount, + [mockBtcAccount.id]: mockBtcAccount, + }, + selectedAccount: mockAccount.id, }, - ], + keyrings: [ + { + type: 'HD Key Tree', + accounts: [mockAccount.address], + }, + { + type: 'Snap Keyring', + accounts: [mockBtcAccount.address], + }, + ], + }, }, - }, - { onClose: jest.fn(), excludeAccountTypes: [BtcAccountType.P2wpkh] }, - ); + { onClose: jest.fn(), allowedAccountTypes: [BtcAccountType.P2wpkh] }, + ); - expect(queryByText(mockAccount.metadata.name)).toBeInTheDocument(); - expect(queryByText(mockBtcAccount.metadata.name)).not.toBeInTheDocument(); + expect(queryByText(mockAccount.metadata.name)).not.toBeInTheDocument(); + expect(queryByText(mockBtcAccount.metadata.name)).toBeInTheDocument(); + }); }); }); diff --git a/ui/components/multichain/account-list-menu/account-list-menu.js b/ui/components/multichain/account-list-menu/account-list-menu.tsx similarity index 88% rename from ui/components/multichain/account-list-menu/account-list-menu.js rename to ui/components/multichain/account-list-menu/account-list-menu.tsx index f276437649af..bfcf250cde0c 100644 --- a/ui/components/multichain/account-list-menu/account-list-menu.js +++ b/ui/components/multichain/account-list-menu/account-list-menu.tsx @@ -4,7 +4,13 @@ import { useHistory } from 'react-router-dom'; import Fuse from 'fuse.js'; import { useDispatch, useSelector } from 'react-redux'; ///: BEGIN:ONLY_INCLUDE_IF(build-flask) -import { KeyringClient } from '@metamask/keyring-api'; +import { + EthAccountType, + InternalAccount, + KeyringAccountType, + KeyringClient, +} from '@metamask/keyring-api'; +import { Caip2ChainId } from '@metamask/snaps-utils'; import { BITCOIN_WALLET_NAME, BITCOIN_WALLET_SNAP_ID, @@ -14,9 +20,9 @@ import { import { Box, ButtonLink, + ButtonLinkSize, ButtonSecondary, ButtonSecondarySize, - ButtonVariant, IconName, Modal, ModalOverlay, @@ -78,6 +84,11 @@ import { } from '../../../selectors/accounts'; import { MultichainNetworks } from '../../../../shared/constants/multichain/networks'; ///: END:ONLY_INCLUDE_IF +import { + InternalAccountWithBalance, + AccountConnections, + MergedInternalAccount, +} from '../../../selectors/selectors.type'; import { HiddenAccountList } from './hidden-account-list'; const ACTION_MODES = { @@ -104,7 +115,10 @@ const ACTION_MODES = { * @param actionMode - An action mode. * @returns The title for this action mode. */ -export const getActionTitle = (t, actionMode) => { +export const getActionTitle = ( + t: (text: string) => string, + actionMode: string, +) => { switch (actionMode) { case ACTION_MODES.ADD: return t('addAccount'); @@ -130,7 +144,10 @@ export const getActionTitle = (t, actionMode) => { * @param internalAccounts - internal accounts * @returns merged accounts list with balances and internal account data */ -export const mergeAccounts = (accountsWithBalances, internalAccounts) => { +export const mergeAccounts = ( + accountsWithBalances: MergedInternalAccount[], + internalAccounts: InternalAccount[], +) => { return accountsWithBalances.map((account) => { const internalAccount = internalAccounts.find( (intAccount) => intAccount.address === account.address, @@ -150,19 +167,30 @@ export const mergeAccounts = (accountsWithBalances, internalAccounts) => { }); }; +type AccountListMenuProps = { + onClose: () => void; + showAccountCreation?: boolean; + accountListItemProps?: object; + allowedAccountTypes?: KeyringAccountType[]; +}; + export const AccountListMenu = ({ onClose, showAccountCreation = true, accountListItemProps = {}, - excludeAccountTypes = [], -}) => { + allowedAccountTypes = [EthAccountType.Eoa, EthAccountType.Erc4337], +}: AccountListMenuProps) => { const t = useI18nContext(); const trackEvent = useContext(MetaMetricsContext); - const accounts = useSelector(getMetaMaskAccountsOrdered); + const accounts: InternalAccountWithBalance[] = useSelector( + getMetaMaskAccountsOrdered, + ); const filteredAccounts = useMemo( () => - accounts.filter((account) => !excludeAccountTypes.includes(account.type)), - [accounts, excludeAccountTypes], + accounts.filter((account: InternalAccountWithBalance) => + allowedAccountTypes.includes(account.type), + ), + [accounts, allowedAccountTypes], ); const selectedAccount = useSelector(getSelectedInternalAccount); const connectedSites = useSelector(getConnectedSubjectsForAllAddresses); @@ -173,10 +201,10 @@ export const AccountListMenu = ({ const updatedAccountsList = useSelector(getUpdatedAndSortedAccounts); const filteredUpdatedAccountList = useMemo( () => - updatedAccountsList.filter( - (account) => !excludeAccountTypes.includes(account.type), + updatedAccountsList.filter((account) => + allowedAccountTypes.includes(account.type), ), - [updatedAccountsList, excludeAccountTypes], + [updatedAccountsList, allowedAccountTypes], ); ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) const addSnapAccountEnabled = useSelector(getIsAddSnapAccountEnabled); @@ -193,7 +221,7 @@ export const AccountListMenu = ({ hasCreatedBtcTestnetAccount, ); - const createBitcoinAccount = async (scope) => { + const createBitcoinAccount = async (scope: Caip2ChainId) => { // Client to create the account using the Bitcoin Snap const client = new KeyringClient(new BitcoinWalletSnapSender()); @@ -207,7 +235,7 @@ export const AccountListMenu = ({ const [searchQuery, setSearchQuery] = useState(''); const [actionMode, setActionMode] = useState(ACTION_MODES.LIST); - let searchResults = filteredUpdatedAccountList; + let searchResults: MergedInternalAccount[] = filteredUpdatedAccountList; if (searchQuery) { const fuse = new Fuse(filteredAccounts, { threshold: 0.2, @@ -222,9 +250,10 @@ export const AccountListMenu = ({ } searchResults = mergeAccounts(searchResults, filteredAccounts); - const title = getActionTitle(t, actionMode); + const title = getActionTitle(t as (text: string) => string, actionMode); - let onBack = null; + // eslint-disable-next-line no-empty-function + let onBack = () => {}; if (actionMode !== ACTION_MODES.LIST) { if (actionMode === ACTION_MODES.MENU) { onBack = () => setActionMode(ACTION_MODES.LIST); @@ -284,7 +313,7 @@ export const AccountListMenu = ({ { trackEvent({ @@ -308,7 +337,7 @@ export const AccountListMenu = ({ { trackEvent({ @@ -343,7 +372,7 @@ export const AccountListMenu = ({ { // The account creation + renaming is handled by the Snap account bridge, so @@ -364,7 +393,7 @@ export const AccountListMenu = ({ } { trackEvent({ @@ -383,7 +412,7 @@ export const AccountListMenu = ({ { onClose(); @@ -396,6 +425,8 @@ export const AccountListMenu = ({ }, }); if (getEnvironmentType() === ENVIRONMENT_TYPE_POPUP) { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore - this will be defined global.platform.openExtensionInBrowser( CONNECT_HARDWARE_ROUTE, ); @@ -412,7 +443,7 @@ export const AccountListMenu = ({ addSnapAccountEnabled ? ( { onClose(); @@ -425,7 +456,7 @@ export const AccountListMenu = ({ }, }); global.platform.openTab({ - url: process.env.ACCOUNT_SNAPS_DIRECTORY_URL, + url: process.env.ACCOUNT_SNAPS_DIRECTORY_URL as string, }); }} > @@ -439,7 +470,7 @@ export const AccountListMenu = ({ ///: BEGIN:ONLY_INCLUDE_IF(build-mmi) { onClose(); @@ -449,6 +480,8 @@ export const AccountListMenu = ({ MetaMetricsEventName.ConnectCustodialAccountClicked, }); if (getEnvironmentType() === ENVIRONMENT_TYPE_POPUP) { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore - this will be defined global.platform.openExtensionInBrowser( CUSTODY_ACCOUNT_ROUTE, ); @@ -479,12 +512,17 @@ export const AccountListMenu = ({ width={BlockSize.Full} placeholder={t('searchAccounts')} value={searchQuery} - onChange={(e) => setSearchQuery(e.target.value)} + onChange={(e: React.ChangeEvent) => + setSearchQuery(e.target.value) + } clearButtonOnClick={() => setSearchQuery('')} clearButtonProps={{ size: Size.SM, }} inputProps={{ autoFocus: true }} + // TODO: These props are required + endAccessory + className /> ) : null} @@ -501,9 +539,9 @@ export const AccountListMenu = ({ ) : null} {searchResults.map((account) => { - const connectedSite = connectedSites[account.address]?.find( - ({ origin }) => origin === currentTabOrigin, - ); + const connectedSite = (connectedSites as AccountConnections)[ + account.address + ]?.find(({ origin }) => origin === currentTabOrigin); const hideAccountListItem = searchQuery.length === 0 && account.hidden; @@ -564,7 +602,6 @@ export const AccountListMenu = ({ > setActionMode(ACTION_MODES.MENU)} @@ -595,7 +632,7 @@ AccountListMenu.propTypes = { */ accountListItemProps: PropTypes.object, /** - * Filters the account types to be excluded in the account list + * Filters the account types to be included in the account list */ - excludeAccountTypes: PropTypes.array, + allowedAccountTypes: PropTypes.array, }; diff --git a/ui/components/multichain/pages/send/components/account-picker.tsx b/ui/components/multichain/pages/send/components/account-picker.tsx index f925bf57d445..c302c21af8cc 100644 --- a/ui/components/multichain/pages/send/components/account-picker.tsx +++ b/ui/components/multichain/pages/send/components/account-picker.tsx @@ -1,6 +1,5 @@ import React, { useContext, useState } from 'react'; import { useSelector } from 'react-redux'; -import { BtcAccountType } from '@metamask/keyring-api'; import { getSelectedInternalAccount } from '../../../../../selectors'; import { Label } from '../../../../component-library'; import { AccountPicker } from '../../../account-picker'; @@ -61,7 +60,6 @@ export const SendPageAccountPicker = () => { {showAccountPicker ? ( setShowAccountPicker(false)} /> diff --git a/ui/selectors/permissions.js b/ui/selectors/permissions.js index 20956b1d7114..d8b53ebb6555 100644 --- a/ui/selectors/permissions.js +++ b/ui/selectors/permissions.js @@ -132,6 +132,15 @@ export function getConnectedSubjectsForSelectedAddress(state) { return connectedSubjects; } +/** + * @typedef {import('./selectors.type.ts').AccountConnections} AccountConnections + */ + +/** + * Retrieves the connected subjects for all addresses. + * + * @returns {AccountConnections} The connected subjects for all addresses. + */ export const getConnectedSubjectsForAllAddresses = createDeepEqualSelector( getPermissionSubjects, getSubjectMetadata, diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index ed1ba2cc4eae..30af2214156a 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -442,8 +442,14 @@ export function getMetaMaskCachedBalances(state) { return {}; } +/** + * @typedef {import('./selectors.type.ts').InternalAccountWithBalance} InternalAccountWithBalance + */ + /** * Get ordered (by keyrings) accounts with InternalAccount and balance + * + * @returns {InternalAccountWithBalance} An array of internal accounts with balance */ export const getMetaMaskAccountsOrdered = createSelector( getInternalAccountsSortedByKeyring, diff --git a/ui/selectors/selectors.type.ts b/ui/selectors/selectors.type.ts new file mode 100644 index 000000000000..155f7e74c93b --- /dev/null +++ b/ui/selectors/selectors.type.ts @@ -0,0 +1,28 @@ +import type { InternalAccount } from '@metamask/keyring-api'; +import { SubjectMetadata } from '@metamask/permission-controller'; +import { KeyringType } from '../hooks/metamask-notifications/useProfileSyncing'; + +export type InternalAccountWithBalance = InternalAccount & { + balance: string; +}; + +export type InternalAccountWithPinnedHiddenActive = + InternalAccountWithBalance & { + pinned: boolean; + hidden: boolean; + lastSelected: number; + active: number; + }; + +export type MergedInternalAccount = InternalAccountWithPinnedHiddenActive & { + keyring: KeyringType; + label: string | null; +}; + +export type AccountConnections = { + [address: string]: { + origin: string; + iconUrl?: string; + metadata: SubjectMetadata; + }[]; +}; From 903c52163070dbd5053c70f9b0f16ec551d351af Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 1 Aug 2024 22:01:19 +0800 Subject: [PATCH 08/19] fix: lint --- .../multichain/account-list-menu/account-list-menu.tsx | 4 +++- ui/components/multichain/pages/connections/connections.tsx | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/ui/components/multichain/account-list-menu/account-list-menu.tsx b/ui/components/multichain/account-list-menu/account-list-menu.tsx index bfcf250cde0c..e0bf477d722f 100644 --- a/ui/components/multichain/account-list-menu/account-list-menu.tsx +++ b/ui/components/multichain/account-list-menu/account-list-menu.tsx @@ -3,12 +3,14 @@ import PropTypes from 'prop-types'; import { useHistory } from 'react-router-dom'; import Fuse from 'fuse.js'; import { useDispatch, useSelector } from 'react-redux'; +// eslint-disable-next-line import/no-duplicates +import { EthAccountType } from '@metamask/keyring-api'; ///: BEGIN:ONLY_INCLUDE_IF(build-flask) import { - EthAccountType, InternalAccount, KeyringAccountType, KeyringClient, + // eslint-disable-next-line import/no-duplicates } from '@metamask/keyring-api'; import { Caip2ChainId } from '@metamask/snaps-utils'; import { diff --git a/ui/components/multichain/pages/connections/connections.tsx b/ui/components/multichain/pages/connections/connections.tsx index 6e75c222c59d..2d751da873e6 100644 --- a/ui/components/multichain/pages/connections/connections.tsx +++ b/ui/components/multichain/pages/connections/connections.tsx @@ -174,7 +174,7 @@ export const Connections = () => { index === mergedAccounts.reduce( ( - acc: string | number, + acc: number, cur: { metadata: { lastSelected: number } }, // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any From b36975fa3ba1dcc3ffc77309822be8fbc72d93ea Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 1 Aug 2024 22:44:56 +0800 Subject: [PATCH 09/19] fix: test --- .../multichain/account-list-menu/account-list-menu.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ui/components/multichain/account-list-menu/account-list-menu.tsx b/ui/components/multichain/account-list-menu/account-list-menu.tsx index e0bf477d722f..2e6519dfb73b 100644 --- a/ui/components/multichain/account-list-menu/account-list-menu.tsx +++ b/ui/components/multichain/account-list-menu/account-list-menu.tsx @@ -4,7 +4,7 @@ import { useHistory } from 'react-router-dom'; import Fuse from 'fuse.js'; import { useDispatch, useSelector } from 'react-redux'; // eslint-disable-next-line import/no-duplicates -import { EthAccountType } from '@metamask/keyring-api'; +import { BtcAccountType, EthAccountType } from '@metamask/keyring-api'; ///: BEGIN:ONLY_INCLUDE_IF(build-flask) import { InternalAccount, @@ -180,7 +180,11 @@ export const AccountListMenu = ({ onClose, showAccountCreation = true, accountListItemProps = {}, - allowedAccountTypes = [EthAccountType.Eoa, EthAccountType.Erc4337], + allowedAccountTypes = [ + EthAccountType.Eoa, + EthAccountType.Erc4337, + BtcAccountType.P2wpkh, + ], }: AccountListMenuProps) => { const t = useI18nContext(); const trackEvent = useContext(MetaMetricsContext); From f3a9f5a478444c9ff28dffbdb743c79f7fa6667e Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 1 Aug 2024 23:04:39 +0800 Subject: [PATCH 10/19] fix: account picker test --- .../multichain/pages/send/components/account-picker.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ui/components/multichain/pages/send/components/account-picker.tsx b/ui/components/multichain/pages/send/components/account-picker.tsx index c302c21af8cc..3fd56b654960 100644 --- a/ui/components/multichain/pages/send/components/account-picker.tsx +++ b/ui/components/multichain/pages/send/components/account-picker.tsx @@ -14,6 +14,7 @@ import { I18nContext } from '../../../../../contexts/i18n'; import { AccountListMenu } from '../../..'; import { SEND_STAGES, getSendStage } from '../../../../../ducks/send'; import { SendPageRow } from '.'; +import { EthAccountType } from '@metamask/keyring-api'; export const SendPageAccountPicker = () => { const t = useContext(I18nContext); @@ -62,6 +63,7 @@ export const SendPageAccountPicker = () => { accountListItemProps={{ showOptions: false }} showAccountCreation={false} onClose={() => setShowAccountPicker(false)} + allowedAccountTypes={[EthAccountType.Eoa, EthAccountType.Erc4337]} /> ) : null} From 6d640ed7773963afdd1d40bcb347a9bdd244f16e Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 1 Aug 2024 23:16:27 +0800 Subject: [PATCH 11/19] fix: lint --- .../multichain/pages/send/components/account-picker.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/components/multichain/pages/send/components/account-picker.tsx b/ui/components/multichain/pages/send/components/account-picker.tsx index 3fd56b654960..22c33f288a57 100644 --- a/ui/components/multichain/pages/send/components/account-picker.tsx +++ b/ui/components/multichain/pages/send/components/account-picker.tsx @@ -1,5 +1,6 @@ import React, { useContext, useState } from 'react'; import { useSelector } from 'react-redux'; +import { EthAccountType } from '@metamask/keyring-api'; import { getSelectedInternalAccount } from '../../../../../selectors'; import { Label } from '../../../../component-library'; import { AccountPicker } from '../../../account-picker'; @@ -14,7 +15,6 @@ import { I18nContext } from '../../../../../contexts/i18n'; import { AccountListMenu } from '../../..'; import { SEND_STAGES, getSendStage } from '../../../../../ducks/send'; import { SendPageRow } from '.'; -import { EthAccountType } from '@metamask/keyring-api'; export const SendPageAccountPicker = () => { const t = useContext(I18nContext); From a8e9a4ab0eaa86cb113ef7cc01e9f8adc3659a65 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 1 Aug 2024 23:27:01 +0800 Subject: [PATCH 12/19] Apply suggestions from code review Co-authored-by: Charly Chevalier --- .../account-list-menu/account-list-menu.test.tsx | 2 +- .../pages/send/components/account-picker.test.tsx | 10 ++++++---- .../multichain/pages/send/components/your-accounts.tsx | 3 +-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/ui/components/multichain/account-list-menu/account-list-menu.test.tsx b/ui/components/multichain/account-list-menu/account-list-menu.test.tsx index ad48d8213f39..6be0255574f8 100644 --- a/ui/components/multichain/account-list-menu/account-list-menu.test.tsx +++ b/ui/components/multichain/account-list-menu/account-list-menu.test.tsx @@ -103,7 +103,7 @@ describe('AccountListMenu', () => { jest .spyOn(reactRouterDom, 'useHistory') .mockImplementation() - // @ts-expect-error mocking histroy return + // @ts-expect-error mocking history return .mockReturnValue({ push: historyPushMock }); }); diff --git a/ui/components/multichain/pages/send/components/account-picker.test.tsx b/ui/components/multichain/pages/send/components/account-picker.test.tsx index 35cb89650d5e..5d48e5fe4fe0 100644 --- a/ui/components/multichain/pages/send/components/account-picker.test.tsx +++ b/ui/components/multichain/pages/send/components/account-picker.test.tsx @@ -128,12 +128,14 @@ describe('SendPageAccountPicker', () => { const accountPicker = getByTestId('send-page-account-picker'); fireEvent.click(accountPicker); - const addressesInAccountList = queryAllByTestId('account-list-address'); - expect(addressesInAccountList).toHaveLength(1); - expect(addressesInAccountList[0].textContent).toContain( + const accountListAddresses = queryAllByTestId('account-list-address'); + expect(accountListAddresses).toHaveLength(1); + + const accountListAddressesContent = accountListAddresses[0].textContent; + expect(accountListAddressesContent).toContain( shortenAddress(normalizeSafeAddress(mockAccount.address)), ); - expect(addressesInAccountList[0].textContent).not.toContain( + expect(accountListAddressesContent).not.toContain( shortenAddress(normalizeSafeAddress(mockBtcAccount.address)), ); }); diff --git a/ui/components/multichain/pages/send/components/your-accounts.tsx b/ui/components/multichain/pages/send/components/your-accounts.tsx index 33f44276150b..73eafd41f8a9 100644 --- a/ui/components/multichain/pages/send/components/your-accounts.tsx +++ b/ui/components/multichain/pages/send/components/your-accounts.tsx @@ -27,8 +27,7 @@ export const SendPageYourAccounts = () => { // Your Accounts const accounts = useSelector(getUpdatedAndSortedAccounts); const internalAccounts = useSelector(getInternalAccounts); - const mergedAccounts: InternalAccount & - { keyring: KeyringTypes; label: string }[] = useMemo(() => { + const mergedAccounts: MergedInternalAccount[] = useMemo(() => { return mergeAccounts(accounts, internalAccounts).filter( (account: InternalAccount) => account.type !== BtcAccountType.P2wpkh, ); From aba5745ad79e9b4a2e535a6b87ea64712b337bbf Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 1 Aug 2024 23:31:33 +0800 Subject: [PATCH 13/19] refactor: move mockstate to upper scope --- .../account-list-menu.test.tsx | 86 +++++++------------ 1 file changed, 31 insertions(+), 55 deletions(-) diff --git a/ui/components/multichain/account-list-menu/account-list-menu.test.tsx b/ui/components/multichain/account-list-menu/account-list-menu.test.tsx index 6be0255574f8..60e1ca8aa99f 100644 --- a/ui/components/multichain/account-list-menu/account-list-menu.test.tsx +++ b/ui/components/multichain/account-list-menu/account-list-menu.test.tsx @@ -517,69 +517,45 @@ describe('AccountListMenu', () => { type: BtcAccountType.P2wpkh, address: 'bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq', }); - - it('allows only EthAccountTypes', () => { - const { queryByText } = render( - { - ...mockState, - metamask: { - ...mockState.metamask, - internalAccounts: { - accounts: { - [mockAccount.id]: mockAccount, - [mockBtcAccount.id]: mockBtcAccount, - }, - selectedAccount: mockAccount.id, - }, - keyrings: [ - { - type: 'HD Key Tree', - accounts: [mockAccount.address], - }, - { - type: 'Snap Keyring', - accounts: [mockBtcAccount.address], - }, - ], + const defaultMockState = { + ...mockState, + metamask: { + ...mockState.metamask, + internalAccounts: { + accounts: { + [mockAccount.id]: mockAccount, + [mockBtcAccount.id]: mockBtcAccount, }, + selectedAccount: mockAccount.id, }, - { - onClose: jest.fn(), - allowedAccountTypes: [EthAccountType.Eoa, EthAccountType.Erc4337], - }, - ); + keyrings: [ + { + type: 'HD Key Tree', + accounts: [mockAccount.address], + }, + { + type: 'Snap Keyring', + accounts: [mockBtcAccount.address], + }, + ], + }, + }; + + it('allows only EthAccountTypes', () => { + const { queryByText } = render(defaultMockState, { + onClose: jest.fn(), + allowedAccountTypes: [EthAccountType.Eoa, EthAccountType.Erc4337], + }); expect(queryByText(mockAccount.metadata.name)).toBeInTheDocument(); expect(queryByText(mockBtcAccount.metadata.name)).not.toBeInTheDocument(); }); it('allows only BtcAccountType', () => { - const { queryByText } = render( - { - ...mockState, - metamask: { - ...mockState.metamask, - internalAccounts: { - accounts: { - [mockAccount.id]: mockAccount, - [mockBtcAccount.id]: mockBtcAccount, - }, - selectedAccount: mockAccount.id, - }, - keyrings: [ - { - type: 'HD Key Tree', - accounts: [mockAccount.address], - }, - { - type: 'Snap Keyring', - accounts: [mockBtcAccount.address], - }, - ], - }, - }, - { onClose: jest.fn(), allowedAccountTypes: [BtcAccountType.P2wpkh] }, - ); + const { queryByText } = render(defaultMockState, { + onClose: jest.fn(), + allowedAccountTypes: [BtcAccountType.P2wpkh], + }); expect(queryByText(mockAccount.metadata.name)).not.toBeInTheDocument(); expect(queryByText(mockBtcAccount.metadata.name)).toBeInTheDocument(); From 58f3043935886fc50eb32fde6222e0507d77f6d4 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 1 Aug 2024 23:33:41 +0800 Subject: [PATCH 14/19] refactor: use CaipChainId from @metamask/utils --- .../multichain/account-list-menu/account-list-menu.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/components/multichain/account-list-menu/account-list-menu.tsx b/ui/components/multichain/account-list-menu/account-list-menu.tsx index 2e6519dfb73b..222fd7ebd5e7 100644 --- a/ui/components/multichain/account-list-menu/account-list-menu.tsx +++ b/ui/components/multichain/account-list-menu/account-list-menu.tsx @@ -12,7 +12,7 @@ import { KeyringClient, // eslint-disable-next-line import/no-duplicates } from '@metamask/keyring-api'; -import { Caip2ChainId } from '@metamask/snaps-utils'; +import { CaipChainId } from '@metamask/utils'; import { BITCOIN_WALLET_NAME, BITCOIN_WALLET_SNAP_ID, @@ -227,7 +227,7 @@ export const AccountListMenu = ({ hasCreatedBtcTestnetAccount, ); - const createBitcoinAccount = async (scope: Caip2ChainId) => { + const createBitcoinAccount = async (scope: CaipChainId) => { // Client to create the account using the Bitcoin Snap const client = new KeyringClient(new BitcoinWalletSnapSender()); From eea8d69a90cb87d740f2bb51baf3d62df31e8108 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 1 Aug 2024 23:37:30 +0800 Subject: [PATCH 15/19] fix: update comment and refactor names --- .../account-list-menu/account-list-menu.tsx | 14 ++++++++------ ui/selectors/permissions.js | 2 +- ui/selectors/selectors.js | 2 +- .../{selectors.type.ts => selectors.types.ts} | 11 ++++++----- 4 files changed, 16 insertions(+), 13 deletions(-) rename ui/selectors/{selectors.type.ts => selectors.types.ts} (72%) diff --git a/ui/components/multichain/account-list-menu/account-list-menu.tsx b/ui/components/multichain/account-list-menu/account-list-menu.tsx index 222fd7ebd5e7..95a774853491 100644 --- a/ui/components/multichain/account-list-menu/account-list-menu.tsx +++ b/ui/components/multichain/account-list-menu/account-list-menu.tsx @@ -90,7 +90,7 @@ import { InternalAccountWithBalance, AccountConnections, MergedInternalAccount, -} from '../../../selectors/selectors.type'; +} from '../../../selectors/selectors.types'; import { HiddenAccountList } from './hidden-account-list'; const ACTION_MODES = { @@ -199,7 +199,9 @@ export const AccountListMenu = ({ [accounts, allowedAccountTypes], ); const selectedAccount = useSelector(getSelectedInternalAccount); - const connectedSites = useSelector(getConnectedSubjectsForAllAddresses); + const connectedSites = useSelector( + getConnectedSubjectsForAllAddresses, + ) as AccountConnections; const currentTabOrigin = useSelector(getOriginOfCurrentTab); const history = useHistory(); const dispatch = useDispatch(); @@ -526,7 +528,7 @@ export const AccountListMenu = ({ size: Size.SM, }} inputProps={{ autoFocus: true }} - // TODO: These props are required + // TODO: These props are required in the TextFieldSearch component. These should be optional endAccessory className /> @@ -545,9 +547,9 @@ export const AccountListMenu = ({ ) : null} {searchResults.map((account) => { - const connectedSite = (connectedSites as AccountConnections)[ - account.address - ]?.find(({ origin }) => origin === currentTabOrigin); + const connectedSite = connectedSites[account.address]?.find( + ({ origin }) => origin === currentTabOrigin, + ); const hideAccountListItem = searchQuery.length === 0 && account.hidden; diff --git a/ui/selectors/permissions.js b/ui/selectors/permissions.js index d8b53ebb6555..65f2acf37c4b 100644 --- a/ui/selectors/permissions.js +++ b/ui/selectors/permissions.js @@ -133,7 +133,7 @@ export function getConnectedSubjectsForSelectedAddress(state) { } /** - * @typedef {import('./selectors.type.ts').AccountConnections} AccountConnections + * @typedef {import('./selectors.types').AccountConnections} AccountConnections */ /** diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index 30af2214156a..caae2292d742 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -443,7 +443,7 @@ export function getMetaMaskCachedBalances(state) { } /** - * @typedef {import('./selectors.type.ts').InternalAccountWithBalance} InternalAccountWithBalance + * @typedef {import('./selectors.types').InternalAccountWithBalance} InternalAccountWithBalance */ /** diff --git a/ui/selectors/selectors.type.ts b/ui/selectors/selectors.types.ts similarity index 72% rename from ui/selectors/selectors.type.ts rename to ui/selectors/selectors.types.ts index 155f7e74c93b..18a6b2b16f1c 100644 --- a/ui/selectors/selectors.type.ts +++ b/ui/selectors/selectors.types.ts @@ -6,7 +6,7 @@ export type InternalAccountWithBalance = InternalAccount & { balance: string; }; -export type InternalAccountWithPinnedHiddenActive = +export type InternalAccountWithPinnedHiddenActiveLastSelected = InternalAccountWithBalance & { pinned: boolean; hidden: boolean; @@ -14,10 +14,11 @@ export type InternalAccountWithPinnedHiddenActive = active: number; }; -export type MergedInternalAccount = InternalAccountWithPinnedHiddenActive & { - keyring: KeyringType; - label: string | null; -}; +export type MergedInternalAccount = + InternalAccountWithPinnedHiddenActiveLastSelected & { + keyring: KeyringType; + label: string | null; + }; export type AccountConnections = { [address: string]: { From 019695ff381bb2d84a79c642a9a11b76e4416c5c Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 1 Aug 2024 23:57:51 +0800 Subject: [PATCH 16/19] refactor: allowedAccountTypes in SendPageYourAccoutns --- .../send/components/account-picker.test.tsx | 2 +- .../pages/send/components/your-accounts.tsx | 20 +++++++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/ui/components/multichain/pages/send/components/account-picker.test.tsx b/ui/components/multichain/pages/send/components/account-picker.test.tsx index 5d48e5fe4fe0..ea3e65ac0655 100644 --- a/ui/components/multichain/pages/send/components/account-picker.test.tsx +++ b/ui/components/multichain/pages/send/components/account-picker.test.tsx @@ -130,7 +130,7 @@ describe('SendPageAccountPicker', () => { const accountListAddresses = queryAllByTestId('account-list-address'); expect(accountListAddresses).toHaveLength(1); - + const accountListAddressesContent = accountListAddresses[0].textContent; expect(accountListAddressesContent).toContain( shortenAddress(normalizeSafeAddress(mockAccount.address)), diff --git a/ui/components/multichain/pages/send/components/your-accounts.tsx b/ui/components/multichain/pages/send/components/your-accounts.tsx index 73eafd41f8a9..7320fe2bca51 100644 --- a/ui/components/multichain/pages/send/components/your-accounts.tsx +++ b/ui/components/multichain/pages/send/components/your-accounts.tsx @@ -1,7 +1,10 @@ import React, { useContext, useMemo } from 'react'; import { useDispatch, useSelector } from 'react-redux'; -import { KeyringTypes } from '@metamask/keyring-controller'; -import { BtcAccountType, InternalAccount } from '@metamask/keyring-api'; +import { + EthAccountType, + InternalAccount, + KeyringAccountType, +} from '@metamask/keyring-api'; import { getUpdatedAndSortedAccounts, getInternalAccounts, @@ -18,9 +21,18 @@ import { MetaMetricsEventCategory, MetaMetricsEventName, } from '../../../../../../shared/constants/metametrics'; +import { MergedInternalAccount } from '../../../../../selectors/selectors.types'; import { SendPageRow } from '.'; -export const SendPageYourAccounts = () => { +type SendPageYourAccountsProps = { + allowedAccountTypes?: KeyringAccountType[]; +}; + +const defaultAllowedAccountTypes = [EthAccountType.Eoa, EthAccountType.Erc4337]; + +export const SendPageYourAccounts = ({ + allowedAccountTypes = defaultAllowedAccountTypes, +}: SendPageYourAccountsProps) => { const dispatch = useDispatch(); const trackEvent = useContext(MetaMetricsContext); @@ -29,7 +41,7 @@ export const SendPageYourAccounts = () => { const internalAccounts = useSelector(getInternalAccounts); const mergedAccounts: MergedInternalAccount[] = useMemo(() => { return mergeAccounts(accounts, internalAccounts).filter( - (account: InternalAccount) => account.type !== BtcAccountType.P2wpkh, + (account: InternalAccount) => allowedAccountTypes.includes(account.type), ); }, [accounts, internalAccounts]); From 3f2439d8d1ef5181e0deb69ecb41a8d588f998b6 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Fri, 2 Aug 2024 00:02:37 +0800 Subject: [PATCH 17/19] fix: remove ts ignore --- .../multichain/account-list-menu/account-list-menu.tsx | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/ui/components/multichain/account-list-menu/account-list-menu.tsx b/ui/components/multichain/account-list-menu/account-list-menu.tsx index 95a774853491..d11cc25968a7 100644 --- a/ui/components/multichain/account-list-menu/account-list-menu.tsx +++ b/ui/components/multichain/account-list-menu/account-list-menu.tsx @@ -433,9 +433,7 @@ export const AccountListMenu = ({ }, }); if (getEnvironmentType() === ENVIRONMENT_TYPE_POPUP) { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - this will be defined - global.platform.openExtensionInBrowser( + global.platform.openExtensionInBrowser?.( CONNECT_HARDWARE_ROUTE, ); } else { @@ -488,9 +486,7 @@ export const AccountListMenu = ({ MetaMetricsEventName.ConnectCustodialAccountClicked, }); if (getEnvironmentType() === ENVIRONMENT_TYPE_POPUP) { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - this will be defined - global.platform.openExtensionInBrowser( + global.platform.openExtensionInBrowser?.( CUSTODY_ACCOUNT_ROUTE, ); } else { From 5591de34af983e5f2cf36a78ab7b73da8d9e8c6d Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Fri, 2 Aug 2024 00:23:50 +0800 Subject: [PATCH 18/19] refactor: imports --- .../multichain/account-list-menu/account-list-menu.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/ui/components/multichain/account-list-menu/account-list-menu.tsx b/ui/components/multichain/account-list-menu/account-list-menu.tsx index d11cc25968a7..af928443a40e 100644 --- a/ui/components/multichain/account-list-menu/account-list-menu.tsx +++ b/ui/components/multichain/account-list-menu/account-list-menu.tsx @@ -3,14 +3,13 @@ import PropTypes from 'prop-types'; import { useHistory } from 'react-router-dom'; import Fuse from 'fuse.js'; import { useDispatch, useSelector } from 'react-redux'; -// eslint-disable-next-line import/no-duplicates -import { BtcAccountType, EthAccountType } from '@metamask/keyring-api'; -///: BEGIN:ONLY_INCLUDE_IF(build-flask) import { + BtcAccountType, + EthAccountType, + ///: BEGIN:ONLY_INCLUDE_IF(build-flask) InternalAccount, KeyringAccountType, KeyringClient, - // eslint-disable-next-line import/no-duplicates } from '@metamask/keyring-api'; import { CaipChainId } from '@metamask/utils'; import { From cf67b29c470bd141f07f89172fe10a51ef4fd5a9 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Fri, 2 Aug 2024 00:29:36 +0800 Subject: [PATCH 19/19] fix: update fence --- .../multichain/account-list-menu/account-list-menu.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ui/components/multichain/account-list-menu/account-list-menu.tsx b/ui/components/multichain/account-list-menu/account-list-menu.tsx index af928443a40e..2074d307f602 100644 --- a/ui/components/multichain/account-list-menu/account-list-menu.tsx +++ b/ui/components/multichain/account-list-menu/account-list-menu.tsx @@ -10,7 +10,9 @@ import { InternalAccount, KeyringAccountType, KeyringClient, + ///: END:ONLY_INCLUDE_IF } from '@metamask/keyring-api'; +///: BEGIN:ONLY_INCLUDE_IF(build-flask) import { CaipChainId } from '@metamask/utils'; import { BITCOIN_WALLET_NAME,