Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { AvatarAccountType } from '../../../components/Avatars/Avatar';
import { Maskicon } from '@metamask/design-system-react-native';
import JazzIcon from 'react-native-jazzicon';
import { Image as RNImage } from 'react-native';
import { AccountCellIds } from '../../../../../e2e/selectors/MultichainAccounts/AccountCell.selectors';

// Configurable mock balance for selector
const mockBalance: { value: number; currency: string } = {
Expand All @@ -35,6 +34,18 @@ jest.mock('../../../../selectors/assets/balances', () => {
};
});

// Mock account selector to avoid deep store dependencies
jest.mock('../../../../selectors/multichainAccounts/accounts', () => {
const actual = jest.requireActual(
'../../../../selectors/multichainAccounts/accounts',
);
return {
...actual,
selectIconSeedAddressByAccountGroupId: () => () =>
'0x1234567890abcdef1234567890abcdef12345678',
};
});

// Mock navigation
const mockNavigate = jest.fn();

Expand Down Expand Up @@ -132,26 +143,23 @@ describe('AccountCell', () => {
});

it('renders Maskicon AvatarAccount when avatarAccountType is Maskicon', () => {
const { getByTestId } = renderAccountCell({
const { UNSAFE_getByType } = renderAccountCell({
avatarAccountType: AvatarAccountType.Maskicon,
});
const avatarContainer = getByTestId(AccountCellIds.AVATAR);
expect(() => avatarContainer.findByType(Maskicon)).not.toThrow();
expect(UNSAFE_getByType(Maskicon)).toBeTruthy();
});

it('renders JazzIcon AvatarAccount when avatarAccountType is JazzIcon', () => {
const { getByTestId } = renderAccountCell({
const { UNSAFE_getByType } = renderAccountCell({
avatarAccountType: AvatarAccountType.JazzIcon,
});
const avatarContainer = getByTestId(AccountCellIds.AVATAR);
expect(() => avatarContainer.findByType(JazzIcon)).not.toThrow();
expect(UNSAFE_getByType(JazzIcon)).toBeTruthy();
});

it('renders Blockies AvatarAccount when avatarAccountType is Blockies', () => {
const { getByTestId } = renderAccountCell({
const { UNSAFE_getByType } = renderAccountCell({
avatarAccountType: AvatarAccountType.Blockies,
});
const avatarContainer = getByTestId(AccountCellIds.AVATAR);
expect(() => avatarContainer.findByType(RNImage)).not.toThrow();
expect(UNSAFE_getByType(RNImage)).toBeTruthy();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,15 @@ interface AccountCellProps {
avatarAccountType: AvatarAccountType;
isSelected: boolean;
hideMenu?: boolean;
startAccessory?: React.ReactNode;
}

const AccountCell = ({
accountGroup,
avatarAccountType,
isSelected,
hideMenu = false,
startAccessory,
}: AccountCellProps) => {
const { styles } = useStyles(styleSheet, { isSelected });
const { navigate } = useNavigation();
Expand Down Expand Up @@ -79,6 +81,7 @@ const AccountCell = ({
alignItems={AlignItems.center}
testID={AccountCellIds.CONTAINER}
>
{startAccessory}
<View style={styles.avatarWrapper}>
<AvatarAccount
accountAddress={evmAddress}
Expand All @@ -98,7 +101,7 @@ const AccountCell = ({
>
{accountGroup.metadata.name}
</Text>
{isSelected && (
{!startAccessory && isSelected && (
<Icon
name={IconName.CheckBold}
size={IconSize.Md}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
createMockWallet,
} from '../../test-utils';
import { AvatarAccountType } from '../../../../components/Avatars/Avatar';
import { RootState } from '../../../../../reducers';

const mockNavigate = jest.fn();

Expand Down Expand Up @@ -58,21 +59,18 @@ describe('AccountListCell', () => {
onSelectAccount={mockOnSelectAccount}
/>,
{
state: {
...baseState,
settings: { avatarAccountType: 'Maskicon' },
},
state: baseState,
},
);
expect(getByText('Test Account')).toBeTruthy();
});

it('calls onSelectAccount when pressed', () => {
const mockOnSelectAccount = jest.fn();
const groups2 = [mockAccountGroup];
const wallet2 = createMockWallet('test-group', 'Test Wallet', groups2);
const internalAccounts2 = createMockInternalAccountsFromGroups(groups2);
const baseState2 = createMockState([wallet2], internalAccounts2);
const groups = [mockAccountGroup];
const wallet = createMockWallet('test-group', 'Test Wallet', groups);
const internalAccounts = createMockInternalAccountsFromGroups(groups);
const baseState = createMockState([wallet], internalAccounts);
const { getByText } = renderWithProvider(
<AccountListCell
accountGroup={mockAccountGroup}
Expand All @@ -81,10 +79,7 @@ describe('AccountListCell', () => {
onSelectAccount={mockOnSelectAccount}
/>,
{
state: {
...baseState2,
settings: { avatarAccountType: 'Maskicon' },
},
state: baseState,
},
);
// Given a rendered cell
Expand All @@ -93,4 +88,165 @@ describe('AccountListCell', () => {
fireEvent.press(getByText('Test Account'));
expect(mockOnSelectAccount).toHaveBeenCalledWith(mockAccountGroup);
});

describe('Checkbox functionality', () => {
let baseState: RootState;

beforeEach(() => {
const groups = [mockAccountGroup];
const wallet = createMockWallet('test-group', 'Test Wallet', groups);
const internalAccounts = createMockInternalAccountsFromGroups(groups);
baseState = createMockState([wallet], internalAccounts);
});

it('shows checkbox when showCheckbox prop is true', () => {
const mockOnSelectAccount = jest.fn();
const { getByTestId } = renderWithProvider(
<AccountListCell
accountGroup={mockAccountGroup}
isSelected={false}
onSelectAccount={mockOnSelectAccount}
showCheckbox
avatarAccountType={AvatarAccountType.Maskicon}
/>,
{ state: baseState },
);

expect(
getByTestId(`account-list-cell-checkbox-${mockAccountGroup.id}`),
).toBeTruthy();
});

it('hides checkbox when showCheckbox prop is false', () => {
const mockOnSelectAccount = jest.fn();
const { queryByTestId } = renderWithProvider(
<AccountListCell
accountGroup={mockAccountGroup}
isSelected={false}
onSelectAccount={mockOnSelectAccount}
showCheckbox={false}
avatarAccountType={AvatarAccountType.Maskicon}
/>,
{ state: baseState },
);

expect(
queryByTestId(`account-list-cell-checkbox-${mockAccountGroup.id}`),
).toBeFalsy();
});

it('hides checkbox by default when showCheckbox prop is not provided', () => {
const mockOnSelectAccount = jest.fn();
const { queryByTestId } = renderWithProvider(
<AccountListCell
accountGroup={mockAccountGroup}
isSelected={false}
onSelectAccount={mockOnSelectAccount}
avatarAccountType={AvatarAccountType.Maskicon}
/>,
{ state: baseState },
);

expect(
queryByTestId(`account-list-cell-checkbox-${mockAccountGroup.id}`),
).toBeFalsy();
});

it('renders checked checkbox when isSelected is true', () => {
const mockOnSelectAccount = jest.fn();
const { getByTestId } = renderWithProvider(
<AccountListCell
accountGroup={mockAccountGroup}
isSelected
onSelectAccount={mockOnSelectAccount}
showCheckbox
avatarAccountType={AvatarAccountType.Maskicon}
/>,
{ state: baseState },
);

expect(
getByTestId(`account-list-cell-checkbox-${mockAccountGroup.id}`),
).toBeTruthy();
expect(getByTestId('checkbox-icon-component')).toBeTruthy();
});

it('renders unchecked checkbox when isSelected is false', () => {
const mockOnSelectAccount = jest.fn();
const { getByTestId, queryByTestId } = renderWithProvider(
<AccountListCell
accountGroup={mockAccountGroup}
isSelected={false}
onSelectAccount={mockOnSelectAccount}
showCheckbox
avatarAccountType={AvatarAccountType.Maskicon}
/>,
{ state: baseState },
);

expect(
getByTestId(`account-list-cell-checkbox-${mockAccountGroup.id}`),
).toBeTruthy();
expect(queryByTestId('checkbox-icon-component')).toBeFalsy();
});

it('calls onSelectAccount when checkbox is pressed', () => {
const mockOnSelectAccount = jest.fn();
const { getByTestId } = renderWithProvider(
<AccountListCell
accountGroup={mockAccountGroup}
isSelected={false}
onSelectAccount={mockOnSelectAccount}
showCheckbox
avatarAccountType={AvatarAccountType.Maskicon}
/>,
{ state: baseState },
);

const checkboxElement = getByTestId(
`account-list-cell-checkbox-${mockAccountGroup.id}`,
);
fireEvent.press(checkboxElement);

expect(mockOnSelectAccount).toHaveBeenCalledWith(mockAccountGroup);
});

it('calls onSelectAccount when TouchableOpacity container is pressed with checkbox enabled', () => {
const mockOnSelectAccount = jest.fn();
const { getByText } = renderWithProvider(
<AccountListCell
accountGroup={mockAccountGroup}
isSelected={false}
onSelectAccount={mockOnSelectAccount}
showCheckbox
avatarAccountType={AvatarAccountType.Maskicon}
/>,
{ state: baseState },
);

fireEvent.press(getByText('Test Account'));

expect(mockOnSelectAccount).toHaveBeenCalledWith(mockAccountGroup);
});

it('renders AccountCell with correct props when checkbox is shown', () => {
const mockOnSelectAccount = jest.fn();
const { getByTestId, getByText } = renderWithProvider(
<AccountListCell
accountGroup={mockAccountGroup}
isSelected
onSelectAccount={mockOnSelectAccount}
showCheckbox
avatarAccountType={AvatarAccountType.Maskicon}
/>,
{ state: baseState },
);

expect(
getByTestId(`account-list-cell-checkbox-${mockAccountGroup.id}`),
).toBeTruthy();
expect(getByTestId('checkbox-icon-component')).toBeTruthy();
expect(getByText('Test Account')).toBeTruthy();
});
});
});
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
import React, { memo, useCallback } from 'react';
import { TouchableOpacity } from 'react-native';
import { TouchableOpacity, View } from 'react-native';

import { useStyles } from '../../../../hooks';
import AccountCell from '../../AccountCell';
import createStyles from '../MultichainAccountSelectorList.styles';
import { AccountListCellProps } from './AccountListCell.types';
import Checkbox from '../../../../components/Checkbox';

const AccountListCell = memo(
({
accountGroup,
avatarAccountType,
isSelected,
onSelectAccount,
showCheckbox = false,
}: AccountListCellProps) => {
const { styles } = useStyles(createStyles, {});

Expand All @@ -26,6 +28,13 @@ const AccountListCell = memo(
activeOpacity={0.7}
>
<AccountCell
startAccessory={
showCheckbox ? (
Copy link
Contributor

Choose a reason for hiding this comment

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

should we extract this into its own component?

<View testID={`account-list-cell-checkbox-${accountGroup.id}`}>
<Checkbox isChecked={isSelected} onPress={handlePress} />
</View>
) : undefined
}
Copy link

Choose a reason for hiding this comment

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

Bug: Double Event Triggering in Nested Pressables

When showCheckbox is true, pressing the Checkbox invokes onSelectAccount twice. This occurs because both the Checkbox and its parent TouchableOpacity have onPress={handlePress}, causing handlePress to fire once directly and again from event bubbling. This can lead to unexpected account selection behavior.

Fix in Cursor Fix in Web

accountGroup={accountGroup}
avatarAccountType={avatarAccountType}
isSelected={isSelected}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ export interface AccountListCellProps {
avatarAccountType: AvatarAccountType;
isSelected: boolean;
onSelectAccount: (accountGroup: AccountGroupObject) => void;
showCheckbox?: boolean;
}
Loading
Loading