Skip to content

Commit 3cb5e1b

Browse files
authored
chore: add buy/sell trade markers to trader position chart (#29478)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until this PR meets the canonical Definition of Ready For Review in `docs/readme/ready-for-review.md`. In short: the template must be materially complete (not just section titles present), all status checks must be currently passing, and the only expected follow-up commits must be reviewer-driven. --> ## **Description** Adds buy/sell dot markers to the price chart on the Trader Position view, mapping each trade's timestamp to the nearest price-data index on the chart line. <img height="800" alt="Simulator Screenshot - iPhone 17 Pro - 2026-04-29 at 11 44 47" src="https://github.com/user-attachments/assets/beeda603-8f8f-42a6-b778-64c9a5159776" /> <img height="800" alt="Simulator Screenshot - iPhone 17 Pro - 2026-04-29 at 11 46 14" src="https://github.com/user-attachments/assets/e5d58dc6-3871-411f-bbbe-705e4163b662" /> Why some markers may not appear: Trade predates the chart's price data, because the price API window may start later (recently-listed token). Trades before the first price point are correctly excluded since plotting them at a position the chart isn't drawing would be misleading. Multiple trades collapse to the same index, on wider timeframes (1M: ~6h candle spacing), trades that are hours apart on the same day snap to the same nearest index and stack. ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: null ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** <!-- Every checklist item must be consciously assessed before marking this PR as "Ready for review". A checked box means you deliberately considered that responsibility, not that you literally performed every action listed. Unchecked boxes are ambiguous: they are not an implicit "N/A" and they are not a silent "skip". See `docs/readme/ready-for-review.md` for the full checklist semantics. --> - [ ] 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. #### Performance checks (if applicable) - [ ] I've tested on Android - Ideally on a mid-range device; emulator is acceptable - [ ] I've tested with a power user scenario - Use these [power-user SRPs](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93) to import wallets with many accounts and tokens - [ ] I've instrumented key operations with Sentry traces for production performance metrics - See [`trace()`](/app/util/trace.ts) for usage and [`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274) for an example For performance guidelines and tooling, see the [Performance Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers). ## **Pre-merge reviewer checklist** <!-- Reviewer checklist items follow the same semantics as the author checklist: an unchecked box is ambiguous, a checked box means the reviewer consciously assessed that responsibility. See `docs/readme/ready-for-review.md`. --> - [ ] 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] > **Medium Risk** > Introduces a new chart implementation with custom touch/overlay rendering and timestamp-to-index mapping, which could affect chart correctness and performance on edge-case data windows; changes are UI-only but non-trivial. > > **Overview** > Adds a dedicated `TraderPriceChart` to the Trader Position view that overlays buy/sell trade markers on the price line by mapping each trade timestamp to the nearest historical price index (dropping out-of-window trades and normalizing seconds vs ms timestamps). > > Wires the filtered `trades` list through `TraderPositionView` → `TraderPositionChartSection`, suppresses the chart end-dot when recent trade markers would overlap it, and adds focused unit tests for both the marker-mapping logic and chart rendering behavior. Separately, the homepage Top Traders carousel cards switch from fixed heights to `h-auto` (including skeletons and “View more”) to avoid hard-coded tile height constraints. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit f086b5b. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent e1c851c commit 3cb5e1b

10 files changed

Lines changed: 937 additions & 14 deletions

File tree

app/components/Views/Homepage/Sections/TopTraders/TopTradersSection.tsx

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,7 @@ import useHomeViewedEvent, {
2323
import { useSectionPerformance } from '../../hooks/useSectionPerformance';
2424
import { SectionRefreshHandle } from '../../types';
2525
import { TopTraderCard, TopTraderCardSkeleton } from './components';
26-
import {
27-
TOP_TRADER_CARD_HEIGHT,
28-
TOP_TRADER_CARD_WIDTH,
29-
} from './components/TopTraderCard';
26+
import { TOP_TRADER_CARD_WIDTH } from './components/TopTraderCard';
3027
import { useTopTraders } from './hooks';
3128

3229
const HOME_TRADER_LIMIT = 10;
@@ -135,7 +132,7 @@ const TopTradersSection = forwardRef<
135132
{!isLoading && traders.length > 0 && (
136133
<ViewMoreCard
137134
onPress={handleViewAll}
138-
twClassName={`w-[${TOP_TRADER_CARD_WIDTH}px] h-[${TOP_TRADER_CARD_HEIGHT}px]`}
135+
twClassName={`w-[${TOP_TRADER_CARD_WIDTH}px] h-auto`}
139136
testID="top-traders-view-more-card"
140137
/>
141138
)}

app/components/Views/Homepage/Sections/TopTraders/components/TopTraderCard.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ export interface TopTraderCardProps {
3838
// Fixed dimensions for every tile in the homepage Top Traders carousel
3939
const AVATAR_SIZE = 60;
4040
export const TOP_TRADER_CARD_WIDTH = 200;
41-
export const TOP_TRADER_CARD_HEIGHT = 188;
4241

4342
/**
4443
* TopTraderCard -- compact card for the homepage horizontal scroll.
@@ -59,7 +58,7 @@ const TopTraderCard: React.FC<TopTraderCardProps> = ({
5958

6059
return (
6160
<Box
62-
twClassName={`w-[${TOP_TRADER_CARD_WIDTH}px] h-[${TOP_TRADER_CARD_HEIGHT}px] rounded-2xl bg-muted p-4 overflow-hidden gap-1`}
61+
twClassName={`w-[${TOP_TRADER_CARD_WIDTH}px] h-auto rounded-2xl bg-muted p-4 overflow-hidden gap-1`}
6362
testID={testID ?? `top-trader-card-${trader.id}`}
6463
>
6564
<TouchableOpacity
@@ -134,6 +133,7 @@ const TopTraderCard: React.FC<TopTraderCardProps> = ({
134133
trader.isFollowing ? ButtonVariant.Secondary : ButtonVariant.Primary
135134
}
136135
size={ButtonSize.Md}
136+
twClassName="mt-2"
137137
isFullWidth
138138
onPress={() => onFollowPress(trader.id)}
139139
>

app/components/Views/Homepage/Sections/TopTraders/components/TopTraderCardSkeleton.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import React from 'react';
33
import { View } from 'react-native';
44
import SkeletonPlaceholder from 'react-native-skeleton-placeholder';
55
import { useTheme } from '../../../../../../util/theme';
6-
import { TOP_TRADER_CARD_HEIGHT, TOP_TRADER_CARD_WIDTH } from './TopTraderCard';
6+
import { TOP_TRADER_CARD_WIDTH } from './TopTraderCard';
77

88
/**
99
* TopTraderCardSkeleton -- loading placeholder that mirrors the TopTraderCard layout.
@@ -18,7 +18,7 @@ const TopTraderCardSkeleton: React.FC = () => {
1818
return (
1919
<View
2020
style={tw.style(
21-
`w-[${TOP_TRADER_CARD_WIDTH}px] h-[${TOP_TRADER_CARD_HEIGHT}px] rounded-2xl bg-muted p-4`,
21+
`w-[${TOP_TRADER_CARD_WIDTH}px] h-auto rounded-2xl bg-muted p-4`,
2222
)}
2323
>
2424
<SkeletonPlaceholder

app/components/Views/SocialLeaderboard/TraderPositionView/TraderPositionView.test.tsx

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const mockGoBack = jest.fn();
1111
const mockGetAssetImageUrl = jest.fn();
1212
const mockHandleFetch = handleFetch as jest.MockedFunction<typeof handleFetch>;
1313
const mockPriceChart = jest.fn();
14+
const mockTraderPriceChart = jest.fn();
1415

1516
interface MockRouteParams {
1617
positionId?: string;
@@ -132,6 +133,16 @@ jest.mock('../../../UI/AssetOverview/PriceChart', () => {
132133
},
133134
};
134135
});
136+
jest.mock('./components/TraderPriceChart', () => {
137+
const { View } = jest.requireActual('react-native');
138+
return {
139+
__esModule: true,
140+
default: (props: unknown) => {
141+
mockTraderPriceChart(props);
142+
return <View testID="trader-price-chart-mock" />;
143+
},
144+
};
145+
});
135146
jest.mock('../../../UI/AssetOverview/PriceChart/PriceChart.context', () => {
136147
const ReactActual = jest.requireActual('react');
137148
return {
@@ -229,7 +240,7 @@ describe('TraderPositionView', () => {
229240
screen.getByTestId(TraderPositionViewSelectorsIDs.FALLBACK),
230241
).toBeOnTheScreen();
231242
// Price chart should not be rendered in the fallback state
232-
expect(screen.queryByTestId('price-chart-mock')).toBeNull();
243+
expect(screen.queryByTestId('trader-price-chart-mock')).toBeNull();
233244
});
234245

235246
it('renders the skeleton while a positionId fetch is in flight', () => {
@@ -296,7 +307,9 @@ describe('TraderPositionView', () => {
296307

297308
await waitFor(() => {
298309
const lastCall =
299-
mockPriceChart.mock.calls[mockPriceChart.mock.calls.length - 1]?.[0];
310+
mockTraderPriceChart.mock.calls[
311+
mockTraderPriceChart.mock.calls.length - 1
312+
]?.[0];
300313
expect(lastCall).toMatchObject({ isLoading: false });
301314
});
302315
});
@@ -309,6 +322,19 @@ describe('TraderPositionView', () => {
309322
).toBeOnTheScreen();
310323
});
311324

325+
it('forwards the filtered trades to the chart component', async () => {
326+
renderWithProvider(<TraderPositionView />, { state: mockState });
327+
328+
await waitFor(() => {
329+
const lastCall =
330+
mockTraderPriceChart.mock.calls[
331+
mockTraderPriceChart.mock.calls.length - 1
332+
]?.[0];
333+
expect(lastCall).toHaveProperty('trades');
334+
expect(Array.isArray(lastCall.trades)).toBe(true);
335+
});
336+
});
337+
312338
it('fires a medium impact haptic when the buy button is pressed', () => {
313339
renderWithProvider(<TraderPositionView />, { state: mockState });
314340

@@ -487,7 +513,9 @@ describe('TraderPositionView', () => {
487513

488514
await waitFor(() => {
489515
const lastCall =
490-
mockPriceChart.mock.calls[mockPriceChart.mock.calls.length - 1]?.[0];
516+
mockTraderPriceChart.mock.calls[
517+
mockTraderPriceChart.mock.calls.length - 1
518+
]?.[0];
491519

492520
expect(lastCall).toMatchObject({
493521
prices: weeklyPrices,

app/components/Views/SocialLeaderboard/TraderPositionView/TraderPositionView.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ const TraderPositionView = () => {
140140
priceDiff={priceDiff}
141141
isPricesLoading={isPricesLoading}
142142
onChartIndexChange={handleChartIndexChange}
143+
trades={trades}
143144
/>
144145

145146
<TraderTimePeriodSelector

app/components/Views/SocialLeaderboard/TraderPositionView/components/TraderPositionChartSection.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,33 @@
11
import React from 'react';
22
import { Box } from '@metamask/design-system-react-native';
3+
import type { Trade } from '@metamask/social-controllers';
34
import type { TokenPrice } from '../../../../hooks/useTokenHistoricalPrices';
4-
import PriceChart from '../../../../UI/AssetOverview/PriceChart';
55
import { PriceChartProvider } from '../../../../UI/AssetOverview/PriceChart/PriceChart.context';
6+
import TraderPriceChart from './TraderPriceChart';
67

78
export interface TraderPositionChartSectionProps {
89
historicalPrices: TokenPrice[];
910
priceDiff: number;
1011
isPricesLoading: boolean;
1112
onChartIndexChange: (index: number) => void;
13+
trades: readonly Trade[];
1214
}
1315

1416
const TraderPositionChartSection: React.FC<TraderPositionChartSectionProps> = ({
1517
historicalPrices,
1618
priceDiff,
1719
isPricesLoading,
1820
onChartIndexChange,
21+
trades,
1922
}) => (
2023
<PriceChartProvider>
2124
<Box twClassName="mx-4 my-3">
22-
<PriceChart
25+
<TraderPriceChart
2326
prices={historicalPrices}
2427
priceDiff={priceDiff}
2528
isLoading={isPricesLoading}
2629
onChartIndexChange={onChartIndexChange}
30+
trades={trades}
2731
/>
2832
</Box>
2933
</PriceChartProvider>

0 commit comments

Comments
 (0)