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 @@ -44,6 +44,7 @@ import {
FEEDBACK_CONFIG,
} from '../../constants/perpsConfig';
import { selectPerpsFeedbackEnabledFlag } from '../../selectors/featureFlags';
import { selectPrivacyMode } from '../../../../../selectors/preferencesController';
import PerpsMarketBalanceActions from '../../components/PerpsMarketBalanceActions';
import PerpsCard from '../../components/PerpsCard';
import PerpsWatchlistMarkets from '../../components/PerpsWatchlistMarkets/PerpsWatchlistMarkets';
Expand Down Expand Up @@ -80,6 +81,7 @@ const PerpsHomeView = () => {

// Feature flag for feedback button
const isFeedbackEnabled = useSelector(selectPerpsFeedbackEnabledFlag);
const privacyMode = useSelector(selectPrivacyMode);

// Use centralized navigation hook
const perpsNavigation = usePerpsNavigation();
Expand Down Expand Up @@ -428,9 +430,9 @@ const PerpsHomeView = () => {
{/* Positions Section */}
<PerpsHomeSection
title={strings('perps.home.positions')}
subtitle={positionsSubtitle}
subtitle={privacyMode ? undefined : positionsSubtitle}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Subtitle color still leaks PnL direction in privacy mode

Low Severity

When privacy mode is on, subtitle is correctly set to undefined so it won't render, but subtitleColor is still passed with the directional color (TextColor.Success or TextColor.Error). Currently this is safe because PerpsHomeSection guards with {subtitle && (...)}. However, every other component in this PR explicitly forces TextColor.Default when privacy mode is on (e.g., PerpsPositionsView, PerpsPositionCard, PerpsCard), making this the only place that omits the color-neutralization pattern described in the PR's "Color leaking fix."

Fix in Cursor Fix in Web

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It doesn't as you can see on the screenshot

subtitleColor={positionsSubtitleColor}
subtitleSuffix={positionsSubtitleSuffix}
subtitleSuffix={privacyMode ? undefined : positionsSubtitleSuffix}
subtitleTestID={PerpsHomeViewSelectorsIDs.POSITIONS_PNL_VALUE}
isLoading={isLoading.positions}
isEmpty={positions.length === 0}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from '@testing-library/react-native';
import { useNavigation, useFocusEffect } from '@react-navigation/native';
import PerpsPositionsView from './PerpsPositionsView';
import { selectPrivacyMode } from '../../../../../selectors/preferencesController';
import {
usePerpsTrading,
usePerpsTPSLUpdate,
Expand Down Expand Up @@ -195,7 +196,7 @@ describe('PerpsPositionsView', () => {
isInitialLoading: false,
});

// Default eligibility mock
// Default selector mocks: eligible, privacy mode off
const { useSelector } = jest.requireMock('react-redux');
const mockSelectPerpsEligibility = jest.requireMock(
'../../selectors/perpsController',
Expand All @@ -204,6 +205,9 @@ describe('PerpsPositionsView', () => {
if (selector === mockSelectPerpsEligibility) {
return true;
}
if (selector === selectPrivacyMode) {
return false;
}
return undefined;
});

Expand Down Expand Up @@ -566,4 +570,84 @@ describe('PerpsPositionsView', () => {
expect(mockLoadPositions).toHaveBeenCalledWith({ isRefresh: true });
});
});

describe('Privacy Mode', () => {
const DOTS_SHORT = '•'.repeat(6); // SensitiveTextLength.Short

const enablePrivacyMode = () => {
const { useSelector } = jest.requireMock('react-redux');
const mockSelectPerpsEligibility = jest.requireMock(
'../../selectors/perpsController',
).selectPerpsEligibility;
useSelector.mockImplementation((selector: unknown) => {
if (selector === mockSelectPerpsEligibility) return true;
if (selector === selectPrivacyMode) return true;
return undefined;
});
};

it('hides account balance values when privacy mode is enabled', async () => {
// Arrange
enablePrivacyMode();

// Act
render(<PerpsPositionsView />);

// Assert - actual balance numbers not shown, dots shown in their place
await waitFor(() => {
expect(screen.queryByText('$10,000')).toBeNull();
expect(screen.queryByText('$4,700')).toBeNull();
expect(screen.queryByText('$5,300')).toBeNull();
const hiddenValues = screen.getAllByText(DOTS_SHORT);
expect(hiddenValues.length).toBeGreaterThanOrEqual(3);
});
});

it('hides total unrealized PnL when privacy mode is enabled', async () => {
// Arrange
enablePrivacyMode();

// Act
render(<PerpsPositionsView />);

// Assert - PnL value hidden
await waitFor(() => {
expect(screen.queryByText(/\+\$75\.50/)).toBeNull();
const hiddenValues = screen.getAllByText(DOTS_SHORT);
expect(hiddenValues.length).toBeGreaterThanOrEqual(4);
});
});

it('keeps account summary labels visible when privacy mode is enabled', async () => {
// Arrange
enablePrivacyMode();

// Act
render(<PerpsPositionsView />);

// Assert - labels (not values) remain visible
await waitFor(() => {
expect(screen.getByText('Total balance')).toBeOnTheScreen();
expect(screen.getByText('Available balance')).toBeOnTheScreen();
expect(screen.getByText('Margin used')).toBeOnTheScreen();
expect(screen.getByText('Total unrealized P&L')).toBeOnTheScreen();
});
});

it('shows account balance values when privacy mode is disabled', async () => {
// Arrange - privacy mode off (default from beforeEach)

// Act
render(<PerpsPositionsView />);

// Assert
await waitFor(() => {
expect(screen.getByText('$10,000')).toBeOnTheScreen();
expect(screen.getByText('$4,700')).toBeOnTheScreen();
expect(screen.getByText('$5,300')).toBeOnTheScreen();
expect(screen.getByText(/\+\$75\.50/)).toBeOnTheScreen();
expect(screen.queryByText(DOTS_SHORT)).toBeNull();
});
});
});
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useNavigation } from '@react-navigation/native';
import React, { useMemo } from 'react';
import { ScrollView, View } from 'react-native';
import { useSelector } from 'react-redux';
import { strings } from '../../../../../../locales/i18n';
import ButtonIcon, {
ButtonIconSizes,
Expand All @@ -13,6 +14,10 @@
TextColor,
TextVariant,
} from '../../../../../component-library/components/Texts/Text';
import SensitiveText, {
SensitiveTextLength,
} from '../../../../../component-library/components/Texts/SensitiveText';
import { selectPrivacyMode } from '../../../../../selectors/preferencesController';
import { useStyles } from '../../../../../component-library/hooks';
import PerpsPositionCard from '../../components/PerpsPositionCard';
import { PERPS_CONSTANTS } from '@metamask/perps-controller';
Expand All @@ -32,6 +37,7 @@
const PerpsPositionsView: React.FC = () => {
const { styles } = useStyles(createStyles, {});
const navigation = useNavigation();
const privacyMode = useSelector(selectPrivacyMode);

const { account } = usePerpsLiveAccount();

Expand Down Expand Up @@ -154,55 +160,76 @@
<Text variant={TextVariant.BodySM} color={TextColor.Muted}>
{strings('perps.position.account.total_balance')}
</Text>
<Text variant={TextVariant.BodySMMedium} color={TextColor.Default}>
<SensitiveText
variant={TextVariant.BodySMMedium}
color={TextColor.Default}
isHidden={privacyMode}
length={SensitiveTextLength.Short}
>
{account?.totalBalance !== undefined &&
account?.totalBalance !== null
? formatPerpsFiat(account.totalBalance, {
ranges: PRICE_RANGES_MINIMAL_VIEW,
})
: PERPS_CONSTANTS.FallbackDataDisplay}
</Text>
</SensitiveText>
</View>

<View style={styles.summaryRow}>
<Text variant={TextVariant.BodySM} color={TextColor.Muted}>
{strings('perps.position.account.available_balance')}
</Text>
<Text variant={TextVariant.BodySMMedium} color={TextColor.Default}>
<SensitiveText
variant={TextVariant.BodySMMedium}
color={TextColor.Default}
isHidden={privacyMode}
length={SensitiveTextLength.Short}
>
{account?.availableBalance !== undefined &&
account?.availableBalance !== null
? formatPerpsFiat(account.availableBalance, {
ranges: PRICE_RANGES_MINIMAL_VIEW,
})
: PERPS_CONSTANTS.FallbackDataDisplay}
</Text>
</SensitiveText>
</View>

<View style={styles.summaryRow}>
<Text variant={TextVariant.BodySM} color={TextColor.Muted}>
{strings('perps.position.account.margin_used')}
</Text>
<Text variant={TextVariant.BodySMMedium} color={TextColor.Default}>
<SensitiveText
variant={TextVariant.BodySMMedium}
color={TextColor.Default}
isHidden={privacyMode}
length={SensitiveTextLength.Short}
>
{account?.marginUsed !== undefined && account?.marginUsed !== null
? formatPerpsFiat(account.marginUsed, {
ranges: PRICE_RANGES_MINIMAL_VIEW,
})
: PERPS_CONSTANTS.FallbackDataDisplay}
</Text>
</SensitiveText>
</View>

<View style={styles.summaryRow}>
<Text variant={TextVariant.BodySM} color={TextColor.Muted}>
{strings('perps.position.account.total_unrealized_pnl')}
</Text>
<Text
<SensitiveText
variant={TextVariant.BodySMMedium}
color={
totalUnrealizedPnl >= 0 ? TextColor.Success : TextColor.Error
privacyMode
? TextColor.Default
: totalUnrealizedPnl >= 0
? TextColor.Success
: TextColor.Error

Check warning on line 226 in app/components/UI/Perps/Views/PerpsPositionsView/PerpsPositionsView.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Extract this nested ternary operation into an independent statement.

See more on https://sonarcloud.io/project/issues?id=metamask-mobile&issues=AZzDgs6zqXgmdKQa6Xe6&open=AZzDgs6zqXgmdKQa6Xe6&pullRequest=27128
}
isHidden={privacyMode}
length={SensitiveTextLength.Short}
>
{formatPnl(totalUnrealizedPnl)}
</Text>
</SensitiveText>
</View>
</View>
{renderContent()}
Expand Down
87 changes: 87 additions & 0 deletions app/components/UI/Perps/components/PerpsCard/PerpsCard.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import { render, fireEvent } from '@testing-library/react-native';
import { useSelector } from 'react-redux';
import PerpsCard from './PerpsCard';
import Routes from '../../../../../constants/navigation/Routes';
import { usePerpsMarkets } from '../../hooks/usePerpsMarkets';
Expand All @@ -20,6 +21,15 @@ jest.mock('@react-navigation/native', () => {
};
});

jest.mock('react-redux', () => ({
...jest.requireActual('react-redux'),
useSelector: jest.fn(() => false), // Default: privacy mode off
}));

jest.mock('../../hooks/usePerpsEventTracking', () => ({
usePerpsEventTracking: jest.fn(() => ({ track: jest.fn() })),
}));

jest.mock('../../../../../component-library/hooks', () => ({
useStyles: () => ({
styles: {
Expand Down Expand Up @@ -50,6 +60,8 @@ describe('PerpsCard', () => {

beforeEach(() => {
jest.clearAllMocks();
// Default: privacy mode off
(useSelector as jest.Mock).mockReturnValue(false);
// Set up default mock return value
mockUsePerpsMarkets.mockReturnValue({
markets: [
Expand Down Expand Up @@ -368,4 +380,79 @@ describe('PerpsCard', () => {
expect(queryByText('perps.order.market_price')).toBeNull();
});
});

describe('Privacy Mode', () => {
const DOTS_SHORT = '•'.repeat(6); // SensitiveTextLength.Short

it('hides position value and PnL label when privacy mode is enabled', () => {
// Arrange
(useSelector as jest.Mock).mockReturnValue(true);
const positivePosition = {
...mockPosition,
positionValue: '4350.00',
unrealizedPnl: '150.00',
returnOnEquity: '10.3',
};

// Act
const { queryByText, getAllByText } = render(
<PerpsCard position={positivePosition} testID="test-card" />,
);

// Assert - right-side financial values replaced with dots
expect(queryByText('$4,350')).toBeNull();
expect(queryByText(/\+\$150/)).toBeNull();
const hiddenElements = getAllByText(DOTS_SHORT);
expect(hiddenElements.length).toBeGreaterThanOrEqual(2);
});

it('shows position value and PnL label when privacy mode is disabled', () => {
// Arrange
(useSelector as jest.Mock).mockReturnValue(false);
const positivePosition = {
...mockPosition,
unrealizedPnl: '100.50',
returnOnEquity: '0.05',
};

// Act
const { getByText, queryByText } = render(
<PerpsCard position={positivePosition} testID="test-card" />,
);

// Assert - actual values visible, no hiding dots
expect(getByText('+$100.50 (+5.0%)')).toBeOnTheScreen();
expect(queryByText(DOTS_SHORT)).toBeNull();
Comment thread
cursor[bot] marked this conversation as resolved.
});

it('hides order price value but not the order type label when privacy mode is enabled', () => {
// Arrange
(useSelector as jest.Mock).mockReturnValue(true);

// Act
const { queryByText, getAllByText, getByText } = render(
<PerpsCard order={mockOrder} testID="test-card" />,
);

// Assert - price value is hidden, but non-financial order type label is not
expect(queryByText('$3,000')).toBeNull();
const hiddenElements = getAllByText(DOTS_SHORT);
expect(hiddenElements.length).toBeGreaterThanOrEqual(1);
expect(getByText('perps.order.limit_price')).toBeOnTheScreen();
});

it('does not hide non-financial labels (symbol, direction) when privacy mode is enabled', () => {
// Arrange
(useSelector as jest.Mock).mockReturnValue(true);

// Act
const { getByText } = render(
<PerpsCard position={mockPosition} testID="test-card" />,
);

// Assert - left-side non-financial content is unaffected
expect(getByText('ETH 3x long')).toBeOnTheScreen();
expect(getByText('1.5 ETH')).toBeOnTheScreen();
});
});
});
Loading
Loading