Skip to content

Commit

Permalink
fix: privacy mode is enabled in account selector by params (#12235)
Browse files Browse the repository at this point in the history
## **Description**

Current Issue: The `AccountSelectorList` subscribed to the `privacyMode`
selector at a component level. It had the unintended consequence of
hiding the balance when choosing an account during the send flow.

Solution: Passing it as a param to allow fine tuned control over hidden
balances in the `AccountSelector` component

## **Related issues**

Follow Fix:
[#3240](MetaMask/MetaMask-planning#3420)

## **Manual testing steps**

1. Click on the privacy mode button on the home screen
2. Click on the account selector to verify your balance is hidden
3. Click on the send flow and attempt to switch accounts - confirm that
your balance isn't hidden here while pivacy mode is enabled

## **Screenshots/Recordings**

| Before  | After  |
|:---:|:---:|

|![before](https://github.com/user-attachments/assets/ee0c13bb-c636-4a1a-9b87-64f1da16eca4)|![after](https://github.com/user-attachments/assets/3606b2e9-d853-4a84-8177-b7437487ae21)|


### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
  • Loading branch information
vinnyhoward authored Nov 11, 2024
1 parent 12c3e59 commit 61ab41a
Show file tree
Hide file tree
Showing 9 changed files with 846 additions and 12 deletions.
51 changes: 48 additions & 3 deletions app/components/UI/AccountSelectorList/AccountSelector.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
} from '../../../util/test/accountsControllerTestUtils';
import { mockNetworkState } from '../../../util/test/network';
import { CHAIN_IDS } from '@metamask/transaction-controller';
import { AccountSelectorListProps } from './AccountSelectorList.types';

const BUSINESS_ACCOUNT = '0xC4955C0d639D99699Bfd7Ec54d9FaFEe40e4D272';
const PERSONAL_ACCOUNT = '0xd018538C87232FF95acbCe4870629b75640a78E7';
Expand Down Expand Up @@ -82,8 +83,9 @@ const initialState = {

const onSelectAccount = jest.fn();
const onRemoveImportedAccount = jest.fn();

const AccountSelectorListUseAccounts = () => {
const AccountSelectorListUseAccounts: React.FC<AccountSelectorListProps> = ({
privacyMode = false,
}) => {
const { accounts, ensByAccountAddress } = useAccounts();
return (
<AccountSelectorList
Expand All @@ -92,6 +94,7 @@ const AccountSelectorListUseAccounts = () => {
accounts={accounts}
ensByAccountAddress={ensByAccountAddress}
isRemoveAccountEnabled
privacyMode={privacyMode}
/>
);
};
Expand All @@ -118,7 +121,7 @@ const renderComponent = (
// eslint-disable-next-line @typescript-eslint/no-explicit-any
state: any = {},
AccountSelectorListTest = AccountSelectorListUseAccounts,
) => renderWithProvider(<AccountSelectorListTest />, { state });
) => renderWithProvider(<AccountSelectorListTest {...state} />, { state });

describe('AccountSelectorList', () => {
beforeEach(() => {
Expand Down Expand Up @@ -238,4 +241,46 @@ describe('AccountSelectorList', () => {
expect(snapTag).toBeDefined();
});
});
it('Text is not hidden when privacy mode is off', async () => {
const state = {
...initialState,
privacyMode: false,
};

const { queryByTestId } = renderComponent(state);

await waitFor(() => {
const businessAccountItem = queryByTestId(
`${AccountListViewSelectorsIDs.ACCOUNT_BALANCE_BY_ADDRESS_TEST_ID}-${BUSINESS_ACCOUNT}`,
);

expect(within(businessAccountItem).getByText(regex.eth(1))).toBeDefined();
expect(
within(businessAccountItem).getByText(regex.usd(3200)),
).toBeDefined();

expect(within(businessAccountItem).queryByText('••••••')).toBeNull();
});
});
it('Text is hidden when privacy mode is on', async () => {
const state = {
...initialState,
privacyMode: true,
};

const { queryByTestId } = renderComponent(state);

await waitFor(() => {
const businessAccountItem = queryByTestId(
`${AccountListViewSelectorsIDs.ACCOUNT_BALANCE_BY_ADDRESS_TEST_ID}-${BUSINESS_ACCOUNT}`,
);

expect(within(businessAccountItem).queryByText(regex.eth(1))).toBeNull();
expect(
within(businessAccountItem).queryByText(regex.usd(3200)),
).toBeNull();

expect(within(businessAccountItem).getByText('••••••')).toBeDefined();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import Cell, {
} from '../../../component-library/components/Cells/Cell';
import { InternalAccount } from '@metamask/keyring-api';
import { useStyles } from '../../../component-library/hooks';
import { selectPrivacyMode } from '../../../selectors/preferencesController';
import { TextColor } from '../../../component-library/components/Texts/Text';
import SensitiveText, {
SensitiveTextLength,
Expand Down Expand Up @@ -52,6 +51,7 @@ const AccountSelectorList = ({
isSelectionDisabled,
isRemoveAccountEnabled = false,
isAutoScrollEnabled = true,
privacyMode = false,
...props
}: AccountSelectorListProps) => {
const { navigate } = useNavigation();
Expand All @@ -72,7 +72,6 @@ const AccountSelectorList = ({
);

const internalAccounts = useSelector(selectInternalAccounts);
const privacyMode = useSelector(selectPrivacyMode);
const getKeyExtractor = ({ address }: Account) => address;

const renderAccountBalances = useCallback(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,8 @@ export interface AccountSelectorListProps
* Optional boolean to enable removing accounts.
*/
isRemoveAccountEnabled?: boolean;
/**
* Optional boolean to indicate if privacy mode is enabled.
*/
privacyMode?: boolean;
}
63 changes: 59 additions & 4 deletions app/components/UI/WalletAccount/WalletAccount.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ const mockInitialState: DeepPartial<RootState> = {
engine: {
backgroundState: {
...backgroundState,
PreferencesController: {
privacyMode: false,
},
AccountsController: MOCK_ACCOUNTS_CONTROLLER_STATE,
NetworkController: {
...mockNetworkState({
Expand Down Expand Up @@ -101,14 +104,21 @@ jest.mock('../../../util/ENSUtils', () => ({
}),
}));

const mockSelector = jest
.fn()
.mockImplementation((callback) => callback(mockInitialState));

jest.mock('react-redux', () => ({
...jest.requireActual('react-redux'),
useSelector: jest
.fn()
.mockImplementation((callback) => callback(mockInitialState)),
// eslint-disable-next-line @typescript-eslint/no-explicit-any
useSelector: (selector: any) => mockSelector(selector),
}));

describe('WalletAccount', () => {
beforeEach(() => {
mockSelector.mockImplementation((callback) => callback(mockInitialState));
});

it('renders correctly', () => {
const { toJSON } = renderWithProvider(<WalletAccount />, {
state: mockInitialState,
Expand All @@ -132,7 +142,9 @@ describe('WalletAccount', () => {

fireEvent.press(getByTestId(WalletViewSelectorsIDs.ACCOUNT_ICON));
expect(mockNavigate).toHaveBeenCalledWith(
...createAccountSelectorNavDetails({}),
...createAccountSelectorNavDetails({
privacyMode: false,
}),
);
});
it('displays the correct account name', () => {
Expand Down Expand Up @@ -164,4 +176,47 @@ describe('WalletAccount', () => {
expect(getByText(customAccountName)).toBeDefined();
});
});

it('should navigate to account selector with privacy mode disabled', () => {
const { getByTestId } = renderWithProvider(<WalletAccount />, {
state: mockInitialState,
});

fireEvent.press(getByTestId(WalletViewSelectorsIDs.ACCOUNT_ICON));
expect(mockNavigate).toHaveBeenCalledWith(
...createAccountSelectorNavDetails({
privacyMode: false,
}),
);
});

it('should navigate to account selector with privacy mode enabled', () => {
const stateWithPrivacyMode = {
...mockInitialState,
engine: {
...mockInitialState.engine,
backgroundState: {
...mockInitialState.engine?.backgroundState,
PreferencesController: {
privacyMode: true,
},
},
},
};

mockSelector.mockImplementation((callback) =>
callback(stateWithPrivacyMode),
);

const { getByTestId } = renderWithProvider(<WalletAccount />, {
state: stateWithPrivacyMode,
});

fireEvent.press(getByTestId(WalletViewSelectorsIDs.ACCOUNT_ICON));
expect(mockNavigate).toHaveBeenCalledWith(
...createAccountSelectorNavDetails({
privacyMode: true,
}),
);
});
});
8 changes: 7 additions & 1 deletion app/components/UI/WalletAccount/WalletAccount.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { useNavigation } from '@react-navigation/native';
import { View } from 'react-native';

// External dependencies
import { selectPrivacyMode } from '../../../selectors/preferencesController';
import { IconName } from '../../../component-library/components/Icons/Icon';
import PickerAccount from '../../../component-library/components/Pickers/PickerAccount';
import { AvatarAccountType } from '../../../component-library/components/Avatars/Avatar/variants/AvatarAccount';
Expand Down Expand Up @@ -34,6 +35,7 @@ const WalletAccount = ({ style }: WalletAccountProps, ref: React.Ref<any>) => {
const yourAccountRef = useRef(null);
const accountActionsRef = useRef(null);
const selectedAccount = useSelector(selectSelectedInternalAccount);
const privacyMode = useSelector(selectPrivacyMode);
const { ensName } = useEnsNameByAddress(selectedAccount?.address);
const defaultName = selectedAccount?.metadata?.name;
const accountName = useMemo(
Expand Down Expand Up @@ -78,7 +80,11 @@ const WalletAccount = ({ style }: WalletAccountProps, ref: React.Ref<any>) => {
accountName={accountName}
accountAvatarType={accountAvatarType}
onPress={() => {
navigate(...createAccountSelectorNavDetails({}));
navigate(
...createAccountSelectorNavDetails({
privacyMode,
}),
);
}}
accountTypeLabel={
getLabelTextByAddress(selectedAccount?.address) || undefined
Expand Down
Loading

0 comments on commit 61ab41a

Please sign in to comment.