Skip to content

Commit 348c47e

Browse files
wachuneiwachunei
authored
fix(homepage): revert perps position row to PerpsCard component (#27206)
## **Description** Replace `PerpsPositionCard` (compact position variant with TP/SL and ROE) with the simpler `PerpsCard` component for rendering position rows in the homepage Perps section, matching the pre-homepage-sections tab UI behavior. Changes: - Replace `PositionCardItem` (memoized `PerpsPositionCard` wrapper) with `PerpsCard` for position rows - Remove `positionDisplayKey` helper function (only needed for the custom memo comparator) - Remove TP/SL loading state logic (`anyPositionHasTpSl`, `tpSlSettled`, `tpSlReady`) which was only relevant for `PerpsPositionCard`'s compact position variant - Update `PerpsCard` mock in tests to handle both `position` and `order` props - Remove `positionDisplayKey` unit tests and TP/SL-specific tests ## **Changelog** CHANGELOG entry: null ## **Related issues** Refs: [TMCU-523](https://consensyssoftware.atlassian.net/browse/TMCU-523) ## **Manual testing steps** ```gherkin Feature: Perps position row on homepage Scenario: user views open perps positions on the homepage Given the user has open perps positions And the homepage Perps section is visible When user views the position rows Then the rows display the old/simple PerpsCard format (icon, symbol/leverage + size, value + PnL) And the rows do not show TP/SL info or ROE Scenario: user taps a position row Given the user has open perps positions on the homepage When user taps a position row Then the app navigates to the market details for that position ``` ## **Screenshots/Recordings** ### **Before** <img width="300" alt="perps_row_before" src="https://github.com/user-attachments/assets/3538094a-8d96-4f57-ad8e-58f424b2ed7e" /> ### **After** <img width="300" alt="perps_row_after" src="https://github.com/user-attachments/assets/e4aa46d7-a6be-45e6-80ac-6204ff247246" /> ## **Pre-merge author checklist** - [x] 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). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] 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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk UI refactor in the homepage Perps section that changes which component renders position rows and removes TP/SL-related loading behavior; main risk is minor display/regression or performance differences during live updates. > > **Overview** > **Homepage Perps position rows are reverted to the simpler `PerpsCard` UI.** The section no longer uses the memoized `PerpsPositionCard` wrapper (and its `positionDisplayKey` comparator), so position rows stop showing TP/SL/ROE-specific behavior and related loading states. > > Tests were updated to match the new rendering: the `PerpsCard` mock now supports both `position` and `order` rows with `onPress`, and TP/SL/key-stability assertions were removed/rewritten to validate the simpler position-row output and leverage display. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 811fabe. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 6fe5eee commit 348c47e

2 files changed

Lines changed: 37 additions & 245 deletions

File tree

‎app/components/Views/Homepage/Sections/Perpetuals/PerpsSection.test.tsx‎

Lines changed: 34 additions & 176 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import React from 'react';
2-
import { screen, fireEvent, act } from '@testing-library/react-native';
2+
import { screen, fireEvent } from '@testing-library/react-native';
33
import renderWithProvider from '../../../../../util/test/renderWithProvider';
4-
import PerpsSection, { positionDisplayKey } from './PerpsSection';
4+
import PerpsSection from './PerpsSection';
55
import Routes from '../../../../../constants/navigation/Routes';
66
import { MetaMetricsEvents } from '../../../../../core/Analytics/MetaMetrics.events';
77
import {
@@ -85,16 +85,28 @@ jest.mock('../../../../UI/Perps/components/PerpsCard', () => {
8585
return {
8686
__esModule: true,
8787
default: ({
88+
position,
8889
order,
90+
onPress,
8991
testID,
9092
}: {
91-
order: { symbol: string; side: string; orderId: string };
92-
testID: string;
93+
position?: { symbol: string; leverage?: { type: string; value: number } };
94+
order?: { symbol: string; side: string; orderId: string };
95+
onPress?: () => void;
96+
testID?: string;
9397
}) => (
94-
<TouchableOpacity testID={testID}>
95-
<Text>
96-
{order.symbol} {order.side === 'buy' ? 'long' : 'short'} order
97-
</Text>
98+
<TouchableOpacity testID={testID} onPress={onPress}>
99+
{position && (
100+
<Text>
101+
{position.symbol}{' '}
102+
{position.leverage ? `${position.leverage.value}X` : ''} position
103+
</Text>
104+
)}
105+
{order && (
106+
<Text>
107+
{order.symbol} {order.side === 'buy' ? 'long' : 'short'} order
108+
</Text>
109+
)}
98110
</TouchableOpacity>
99111
),
100112
};
@@ -217,73 +229,6 @@ jest.mock('../../hooks/useHomeViewedEvent', () => ({
217229
},
218230
}));
219231

220-
describe('positionDisplayKey', () => {
221-
it('returns stable key from position display fields', () => {
222-
const position = makePosition({
223-
symbol: 'BTC',
224-
entryPrice: '98500',
225-
size: '-0.0015',
226-
unrealizedPnl: '9.4',
227-
takeProfitPrice: undefined,
228-
stopLossPrice: undefined,
229-
}) as Parameters<typeof positionDisplayKey>[0];
230-
expect(positionDisplayKey(position)).toBe('BTC:98500:-0.0015:9.4::');
231-
});
232-
233-
it('uses empty string for undefined optional fields', () => {
234-
const position = makePosition({
235-
symbol: 'ETH',
236-
entryPrice: undefined,
237-
size: '1',
238-
unrealizedPnl: undefined,
239-
takeProfitPrice: undefined,
240-
stopLossPrice: undefined,
241-
}) as Parameters<typeof positionDisplayKey>[0];
242-
expect(positionDisplayKey(position)).toBe('ETH::1:::');
243-
});
244-
245-
it('includes takeProfitPrice and stopLossPrice when set', () => {
246-
const position = makePosition({
247-
symbol: 'SOL',
248-
entryPrice: '180',
249-
size: '10',
250-
unrealizedPnl: '-5',
251-
takeProfitPrice: '200',
252-
stopLossPrice: '160',
253-
}) as Parameters<typeof positionDisplayKey>[0];
254-
expect(positionDisplayKey(position)).toBe('SOL:180:10:-5:200:160');
255-
});
256-
257-
it('returns different keys when display-relevant fields differ', () => {
258-
const base = makePosition({ symbol: 'BTC' }) as Parameters<
259-
typeof positionDisplayKey
260-
>[0];
261-
const withPnl = makePosition({
262-
symbol: 'BTC',
263-
unrealizedPnl: '100',
264-
}) as Parameters<typeof positionDisplayKey>[0];
265-
expect(positionDisplayKey(base)).not.toBe(positionDisplayKey(withPnl));
266-
});
267-
268-
it('returns same key when only non-display fields differ', () => {
269-
const a = makePosition({
270-
symbol: 'BTC',
271-
entryPrice: '50000',
272-
size: '1',
273-
unrealizedPnl: '100',
274-
positionValue: '50000',
275-
}) as Parameters<typeof positionDisplayKey>[0];
276-
const b = makePosition({
277-
symbol: 'BTC',
278-
entryPrice: '50000',
279-
size: '1',
280-
unrealizedPnl: '100',
281-
positionValue: '99999',
282-
}) as Parameters<typeof positionDisplayKey>[0];
283-
expect(positionDisplayKey(a)).toBe(positionDisplayKey(b));
284-
});
285-
});
286-
287232
describe('PerpsSection', () => {
288233
beforeEach(() => {
289234
jest.clearAllMocks();
@@ -312,7 +257,7 @@ describe('PerpsSection', () => {
312257
expect(screen.getByText('Perpetuals')).toBeOnTheScreen();
313258
});
314259

315-
it('renders live positions', () => {
260+
it('renders live positions with leverage info', () => {
316261
usePerpsLivePositions.mockReturnValue({
317262
positions: [
318263
makePosition({ symbol: 'BTC', size: '-0.0015' }),
@@ -321,29 +266,6 @@ describe('PerpsSection', () => {
321266
size: '0.03',
322267
entryPrice: '3200',
323268
leverage: { type: 'isolated', value: 40 },
324-
takeProfitPrice: '3680',
325-
stopLossPrice: '2720',
326-
}),
327-
],
328-
isInitialLoading: false,
329-
});
330-
331-
renderWithProvider(
332-
<PerpsSection sectionIndex={0} totalSectionsLoaded={1} />,
333-
);
334-
335-
expect(screen.getByText('Short BTC')).toBeOnTheScreen();
336-
expect(screen.getByText('Long ETH')).toBeOnTheScreen();
337-
});
338-
339-
it('shows leverage badges', () => {
340-
usePerpsLivePositions.mockReturnValue({
341-
positions: [
342-
makePosition({ symbol: 'BTC', size: '-1' }),
343-
makePosition({
344-
symbol: 'ETH',
345-
size: '1',
346-
leverage: { type: 'isolated', value: 40 },
347269
}),
348270
],
349271
isInitialLoading: false,
@@ -353,75 +275,11 @@ describe('PerpsSection', () => {
353275
<PerpsSection sectionIndex={0} totalSectionsLoaded={1} />,
354276
);
355277

356-
expect(screen.getByText('10X short')).toBeOnTheScreen();
357-
expect(screen.getByText('40X long')).toBeOnTheScreen();
358-
});
359-
360-
it('shows TP/SL immediately when any position has TP/SL data', () => {
361-
usePerpsLivePositions.mockReturnValue({
362-
positions: [
363-
makePosition({ symbol: 'BTC' }),
364-
makePosition({
365-
symbol: 'ETH',
366-
size: '0.03',
367-
entryPrice: '3200',
368-
takeProfitPrice: '3680',
369-
stopLossPrice: '2720',
370-
}),
371-
],
372-
isInitialLoading: false,
373-
});
374-
375-
renderWithProvider(
376-
<PerpsSection sectionIndex={0} totalSectionsLoaded={1} />,
377-
);
378-
379-
expect(screen.getByText('TP 15%, SL 15%')).toBeOnTheScreen();
380-
expect(screen.getByText('No TP/SL')).toBeOnTheScreen();
381-
expect(screen.queryByTestId('tp-sl-skeleton')).toBeNull();
382-
});
383-
384-
it('shows TP/SL skeleton when no position has TP/SL data yet', () => {
385-
usePerpsLivePositions.mockReturnValue({
386-
positions: [makePosition({ symbol: 'BTC' })],
387-
isInitialLoading: false,
388-
});
389-
390-
renderWithProvider(
391-
<PerpsSection sectionIndex={0} totalSectionsLoaded={1} />,
392-
);
393-
394-
expect(screen.getByTestId('tp-sl-skeleton')).toBeOnTheScreen();
395-
expect(screen.queryByText('No TP/SL')).toBeNull();
396-
});
397-
398-
it('shows "No TP/SL" after fallback timeout settles', () => {
399-
jest.useFakeTimers();
400-
401-
try {
402-
usePerpsLivePositions.mockReturnValue({
403-
positions: [makePosition({ symbol: 'BTC' })],
404-
isInitialLoading: false,
405-
});
406-
407-
renderWithProvider(
408-
<PerpsSection sectionIndex={0} totalSectionsLoaded={1} />,
409-
);
410-
411-
expect(screen.getByTestId('tp-sl-skeleton')).toBeOnTheScreen();
412-
413-
act(() => {
414-
jest.advanceTimersByTime(5500);
415-
});
416-
417-
expect(screen.getByText('No TP/SL')).toBeOnTheScreen();
418-
expect(screen.queryByTestId('tp-sl-skeleton')).toBeNull();
419-
} finally {
420-
jest.useRealTimers();
421-
}
278+
expect(screen.getByText('BTC 10X position')).toBeOnTheScreen();
279+
expect(screen.getByText('ETH 40X position')).toBeOnTheScreen();
422280
});
423281

424-
it('shows position value and ROE', () => {
282+
it('renders multiple position rows', () => {
425283
usePerpsLivePositions.mockReturnValue({
426284
positions: [
427285
makePosition(),
@@ -434,8 +292,8 @@ describe('PerpsSection', () => {
434292
<PerpsSection sectionIndex={0} totalSectionsLoaded={1} />,
435293
);
436294

437-
const roeElements = screen.getAllByText('+9.40%');
438-
expect(roeElements.length).toBeGreaterThanOrEqual(2);
295+
expect(screen.getByText('BTC 10X position')).toBeOnTheScreen();
296+
expect(screen.getByText('ETH 10X position')).toBeOnTheScreen();
439297
});
440298

441299
it('navigates to perps home on title press with home_section source', () => {
@@ -555,12 +413,12 @@ describe('PerpsSection', () => {
555413
<PerpsSection sectionIndex={0} totalSectionsLoaded={1} />,
556414
);
557415

558-
expect(screen.getByText('Short BTC')).toBeOnTheScreen();
559-
expect(screen.getByText('Long ETH')).toBeOnTheScreen();
560-
expect(screen.getByText('Long SOL')).toBeOnTheScreen();
561-
expect(screen.getByText('Short DOGE')).toBeOnTheScreen();
562-
expect(screen.getByText('Long AVAX')).toBeOnTheScreen();
563-
expect(screen.queryByText('Long LINK')).toBeNull();
416+
expect(screen.getByText('BTC 10X position')).toBeOnTheScreen();
417+
expect(screen.getByText('ETH 10X position')).toBeOnTheScreen();
418+
expect(screen.getByText('SOL 10X position')).toBeOnTheScreen();
419+
expect(screen.getByText('DOGE 10X position')).toBeOnTheScreen();
420+
expect(screen.getByText('AVAX 10X position')).toBeOnTheScreen();
421+
expect(screen.queryByText('LINK 10X position')).toBeNull();
564422
expect(screen.queryByTestId('perps-order-row-order-1')).toBeNull();
565423
});
566424

@@ -584,8 +442,8 @@ describe('PerpsSection', () => {
584442
<PerpsSection sectionIndex={0} totalSectionsLoaded={1} />,
585443
);
586444

587-
expect(screen.getByText('Short BTC')).toBeOnTheScreen();
588-
expect(screen.getByText('Long ETH')).toBeOnTheScreen();
445+
expect(screen.getByText('BTC 10X position')).toBeOnTheScreen();
446+
expect(screen.getByText('ETH 10X position')).toBeOnTheScreen();
589447
expect(screen.getByTestId('perps-order-row-o1')).toBeOnTheScreen();
590448
expect(screen.getByTestId('perps-order-row-o2')).toBeOnTheScreen();
591449
});

‎app/components/Views/Homepage/Sections/Perpetuals/PerpsSection.tsx‎

Lines changed: 3 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import { usePerpsConnection } from '../../../../UI/Perps/hooks/usePerpsConnectio
3232
import { filterAndSortMarkets } from '../../../../UI/Perps/utils/filterAndSortMarkets';
3333
import { selectPerpsWatchlistMarkets } from '../../../../UI/Perps/selectors/perpsController';
3434
import type { PerpsNavigationParamList } from '../../../../UI/Perps/types/navigation';
35-
import PerpsPositionCard from '../../../../UI/Perps/components/PerpsPositionCard/PerpsPositionCard';
3635
import PerpsCard from '../../../../UI/Perps/components/PerpsCard';
3736
import PerpsPositionSkeleton from './components/PerpsPositionSkeleton';
3837
import PerpsMarketTileCard from './components/PerpsMarketTileCard';
@@ -51,41 +50,6 @@ const MAX_ITEMS = 5;
5150
const MAX_TRENDING_MARKETS = 5;
5251
const HOMEPAGE_THROTTLE_MS = 5000;
5352

54-
/** Key fields that affect position card display; skip re-render if unchanged. Exported for testing. */
55-
export function positionDisplayKey(p: Position): string {
56-
return `${p.symbol}:${p.entryPrice ?? ''}:${p.size ?? ''}:${p.unrealizedPnl ?? ''}:${p.takeProfitPrice ?? ''}:${p.stopLossPrice ?? ''}`;
57-
}
58-
59-
/**
60-
* Memoized row so only the position card whose data changed re-renders on stream updates.
61-
*/
62-
const PositionCardItem = React.memo<{
63-
position: Position;
64-
tpSlLoading: boolean;
65-
onPositionPress: (position: Position) => void;
66-
}>(
67-
({ position, tpSlLoading, onPositionPress }) => {
68-
const handlePress = useCallback(
69-
() => onPositionPress(position),
70-
[onPositionPress, position],
71-
);
72-
return (
73-
<PerpsPositionCard
74-
position={position}
75-
compact
76-
compactVariant="position"
77-
tpSlLoading={tpSlLoading}
78-
onPress={handlePress}
79-
testID={`perps-position-row-${position.symbol}`}
80-
/>
81-
);
82-
},
83-
(prev, next) =>
84-
prev.tpSlLoading === next.tpSlLoading &&
85-
prev.onPositionPress === next.onPositionPress &&
86-
positionDisplayKey(prev.position) === positionDisplayKey(next.position),
87-
);
88-
8953
/**
9054
* PerpsSection — single "Perpetuals" section on the homepage.
9155
*
@@ -129,36 +93,6 @@ const PerpsSection = forwardRef<SectionRefreshHandle, PerpsSectionProps>(
12993

13094
const showSkeleton = hookLoading || deferredLoading;
13195

132-
// TP/SL is extracted from orders and merged into positions by the
133-
// subscription service. Due to the 5s throttle the merged update can be
134-
// delayed — positions appear first without TP/SL. If any position already
135-
// has TP/SL the merge is done for all of them; otherwise wait for a
136-
// fallback timeout (throttle interval + margin).
137-
const anyPositionHasTpSl = useMemo(
138-
() =>
139-
positions.some(
140-
(p) => p.takeProfitPrice != null || p.stopLossPrice != null,
141-
),
142-
[positions],
143-
);
144-
145-
const [tpSlSettled, setTpSlSettled] = useState(false);
146-
147-
useEffect(() => {
148-
if (showSkeleton) {
149-
setTpSlSettled(false);
150-
return undefined;
151-
}
152-
if (anyPositionHasTpSl) return undefined;
153-
const timer = setTimeout(
154-
() => setTpSlSettled(true),
155-
HOMEPAGE_THROTTLE_MS + 500,
156-
);
157-
return () => clearTimeout(timer);
158-
}, [showSkeleton, anyPositionHasTpSl]);
159-
160-
const tpSlReady = anyPositionHasTpSl || tpSlSettled;
161-
16296
const { markets, isLoading: marketsLoading } = usePerpsMarkets();
16397
const watchlistSymbols = useSelector(selectPerpsWatchlistMarkets);
16498

@@ -333,11 +267,11 @@ const PerpsSection = forwardRef<SectionRefreshHandle, PerpsSectionProps>(
333267
<SectionRow>
334268
<Box testID="homepage-perps-positions">
335269
{displayPositions.map((position) => (
336-
<PositionCardItem
270+
<PerpsCard
337271
key={position.symbol}
338272
position={position}
339-
tpSlLoading={!tpSlReady}
340-
onPositionPress={handlePositionPress}
273+
onPress={() => handlePositionPress(position)}
274+
testID={`perps-position-row-${position.symbol}`}
341275
/>
342276
))}
343277
{displayOrders.map((order) => (

0 commit comments

Comments
 (0)