Skip to content

Commit f088cd1

Browse files
chore: relax-price-api-condition-legacy-chart and update to time based index (#33204)
## **Description** ### Purpose This PR improves the legacy price chart on the Token Details Page (TDP) with two changes: 1. **Time-based x-axis** — switched PriceChart from index-based to time-based x-axis so partial data (e.g. 14h of data in a 24h window) renders at its correct temporal position within the full time range, with a visible gap on the left. 2. **Relaxed coverage threshold** — lowered `MIN_COVERAGE_RATIO` from 0.95 to 0.20. With the time-based x-axis, partial data is no longer misleading, so the no-data overlay only triggers for extremely sparse responses (<20% of the requested period). ### Changes **`app/components/UI/AssetOverview/PriceChart/PriceChart.tsx`** - Added `timePeriodMs?: number` prop - Uses `xAccessor`/`yAccessor`/`xScale` with d3 `scaleTime` for time-based positioning - Sets `xMin = Date.now() - timePeriodMs` and `xMax = Date.now()` - Rewrote touch logic: pixel → inverse scale → timestamp → binary search nearest point - Fixed stale PanResponder closure by routing through refs - Fixed Tooltip and EndDot to use `x(timestamp)` instead of `x(index)` **`app/components/UI/AssetOverview/Price/Price.legacy.tsx`** - Passes `TIME_PERIOD_MS[timePeriod]` as `timePeriodMs` to PriceChart **`app/components/UI/AssetOverview/Price/tokenOverviewChart.constants.ts`** - Added `TIME_PERIOD_MS` mapping (`TimePeriod → number | null`) **`app/components/hooks/useTokenHistoricalPrices.ts`** - Lowered `MIN_COVERAGE_RATIO` from 0.95 to 0.20 ### Scope Only the Token Details Page (TDP) is affected. `TraderPriceChart.tsx` and other consumers are **not** modified. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/ASSETS-3618 ## **Manual testing steps** ```gherkin Feature: Legacy price chart renders partial data with correct time positioning Scenario: Chart shows gap for partial 1D data Given the user navigates to Token Details for a newly listed token And the price API returns less than 24h of data for the 1D period When the chart renders Then there is a visible gap on the left side of the chart And the price line starts at the actual first data timestamp And the right edge of the chart represents "now" Scenario: Touch/scrub selects correct data point Given the user is viewing a chart with partial data and a gap on the left When the user long-presses and drags on the chart Then the tooltip snaps to the nearest actual data point And does not select phantom points in the gap area Scenario: ALL time range falls back to index-based Given the user selects the "ALL" time period When the chart renders Then data fills the full chart width (no gap, index-based) Scenario: No-data overlay shows for extremely sparse data Given the price API returns data covering less than 20% of the requested period When the chart renders Then the no-data overlay is displayed ``` ## **Screenshots/Recordings** ### **Before** Chart stretches partial data (14h) across full width, making it look like 24h of data. ### **After** Chart correctly shows a gap on the left, with data starting at its actual first timestamp. https://github.com/user-attachments/assets/97ffcc5e-4f97-4b94-8330-763013f67037 ## **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 - [ ] 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) - [x] I've tested on iOS - [ ] I've tested on Android - [ ] I've tested with a power user scenario - [ ] I've instrumented key operations with Sentry traces for production performance metrics ## **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] > **Medium Risk** > Chart rendering and touch-selection behavior change on token details; scope is UI-only but users may misread partial history if the new gap layout is wrong. > > **Overview** > The legacy token overview **PriceChart** can use a **time-based x-axis** when `timePeriodMs` is passed from `Price.legacy` via new `TIME_PERIOD_MS` constants. Fixed windows anchor from `now - period` to `now`, so sparse history (e.g. 14h in 1D) draws at the correct time with a **left gap** instead of stretching across the full width. **ALL** still omits `timePeriodMs` and keeps **index-based** layout. > > Touch/scrub maps pixels to timestamps and uses exported **`findNearestIndex`**; tooltip, end dot, and **`AreaChart`** accessors switch between timestamp and index. **PanResponder** handlers go through refs to avoid stale closures. > > **`hasInsufficientTimeCoverage`** threshold drops from **95% to 20%** (`MIN_COVERAGE_RATIO`), so the no-data overlay only appears for very sparse API responses; docs/comments align with the 50% boundary behavior. Tests cover pass-through, time-based touch, `findNearestIndex`, and updated coverage cases. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit dd06940. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
1 parent 53996cd commit f088cd1

7 files changed

Lines changed: 302 additions & 52 deletions

File tree

app/components/UI/AssetOverview/Price/Price.legacy.test.tsx

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ import { Button, ButtonVariant } from '@metamask/design-system-react-native';
1010
import { TokenOverviewSelectorsIDs } from '../TokenOverview.testIds';
1111
import type { TokenPrice } from '../../../../components/hooks/useTokenHistoricalPrices';
1212
import { selectTokenDetailsTechnicalIndicatorsEnabled } from '../../../../selectors/featureFlagController/tokenDetailsTechnicalIndicators';
13-
import { CHART_DATA_THRESHOLD } from './tokenOverviewChart.constants';
13+
import {
14+
CHART_DATA_THRESHOLD,
15+
TIME_PERIOD_MS,
16+
} from './tokenOverviewChart.constants';
1417

1518
const mockSelectTechnicalIndicatorsEnabled = jest.fn(() => false);
1619

@@ -195,6 +198,28 @@ describe('PriceLegacy', () => {
195198
expect(getByTestId('price-chart-insufficient-data')).toBeOnTheScreen();
196199
});
197200

201+
describe('timePeriodMs pass-through', () => {
202+
it('passes the correct timePeriodMs for a known time period', () => {
203+
renderWithProvider(<PriceLegacy {...baseProps} timePeriod="1d" />);
204+
expect(PriceChart).toHaveBeenCalledWith(
205+
expect.objectContaining({
206+
timePeriodMs: TIME_PERIOD_MS['1d'],
207+
}),
208+
undefined,
209+
);
210+
});
211+
212+
it('passes undefined timePeriodMs for the "all" time period', () => {
213+
renderWithProvider(<PriceLegacy {...baseProps} timePeriod="all" />);
214+
expect(PriceChart).toHaveBeenCalledWith(
215+
expect.objectContaining({
216+
timePeriodMs: undefined,
217+
}),
218+
undefined,
219+
);
220+
});
221+
});
222+
198223
describe('chart navigation buttons', () => {
199224
it('calls onTimePeriodChange when a button is pressed with flag OFF (below chart)', () => {
200225
mockSelectTechnicalIndicatorsEnabled.mockReturnValue(false);

app/components/UI/AssetOverview/Price/Price.legacy.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,10 @@ import styleSheet from './Price.styles';
2020
import ChartNavigationButton from '../ChartNavigationButton';
2121
import { useSelector } from 'react-redux';
2222
import { selectTokenDetailsTechnicalIndicatorsEnabled } from '../../../../selectors/featureFlagController/tokenDetailsTechnicalIndicators';
23-
import { TOKEN_OVERVIEW_TIME_RANGE_ROW_HEIGHT } from './tokenOverviewChart.constants';
23+
import {
24+
TIME_PERIOD_MS,
25+
TOKEN_OVERVIEW_TIME_RANGE_ROW_HEIGHT,
26+
} from './tokenOverviewChart.constants';
2427

2528
export interface PriceLegacyProps {
2629
prices: TokenPrice[];
@@ -201,6 +204,7 @@ const PriceLegacy = ({
201204
onChartIndexChange={handleChartInteraction}
202205
chartColorOverride={initialAmbientColor}
203206
hasInsufficientCoverage={hasInsufficientCoverage}
207+
timePeriodMs={TIME_PERIOD_MS[timePeriod] ?? undefined}
204208
/>
205209
</Box>
206210
{/* Technical indicators flag OFF: time range below chart (legacy position) */}

app/components/UI/AssetOverview/Price/tokenOverviewChart.constants.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Dimensions } from 'react-native';
22
import type { OHLCVTimePeriod } from '../../Charts/AdvancedChart/TimeRangeSelector';
3+
import { TimePeriod } from '../../../hooks/useTokenHistoricalPrices';
34

45
/**
56
* Token overview chart column height (AdvancedChart / TradingView + legacy line when shown).
@@ -18,6 +19,23 @@ export const CHART_DATA_THRESHOLD = 5;
1819
*/
1920
export const TOKEN_OVERVIEW_TIME_RANGE_ROW_HEIGHT = 34;
2021

22+
/**
23+
* Duration in milliseconds for each time period.
24+
* `null` for "all" (no fixed duration — falls back to index-based x-axis).
25+
*/
26+
const HOURS = 3_600_000;
27+
const DAYS = 24 * HOURS;
28+
export const TIME_PERIOD_MS: Record<TimePeriod, number | null> = {
29+
'1d': 1 * DAYS,
30+
'1w': 7 * DAYS,
31+
'7d': 7 * DAYS,
32+
'1m': 30 * DAYS,
33+
'3m': 90 * DAYS,
34+
'1y': 365 * DAYS,
35+
'3y': 3 * 365 * DAYS,
36+
all: null,
37+
};
38+
2139
/**
2240
* Single source of truth for candle intervals in the IntervalBar.
2341
* Maps each interval to the API timePeriod that returns enough history.

app/components/UI/AssetOverview/PriceChart/PriceChart.test.tsx

Lines changed: 126 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import React from 'react';
22
import { render, fireEvent } from '@testing-library/react-native';
3-
import PriceChart from './PriceChart';
3+
import PriceChart, { findNearestIndex } from './PriceChart';
44
import { TokenPrice } from '../../../hooks/useTokenHistoricalPrices';
55
import { PriceChartProvider } from './PriceChart.context';
66
import { useAnalytics } from '../../../hooks/useAnalytics/useAnalytics';
@@ -258,8 +258,10 @@ describe('PriceChart', () => {
258258
* Synthetic responder event with the touchHistory shape
259259
* that PanResponder's internal state machine expects.
260260
*/
261+
const FIXED_TIMESTAMP = 1_736_761_237_983;
262+
261263
const createResponderEvent = (locationX: number, locationY: number) => {
262-
const timestamp = Date.now();
264+
const timestamp = FIXED_TIMESTAMP;
263265
return {
264266
nativeEvent: {
265267
locationX,
@@ -315,5 +317,127 @@ describe('PriceChart', () => {
315317
fireEvent(chartArea, 'responderRelease', createResponderEvent(200, 55));
316318
expect(mockOnChartIndexChange).toHaveBeenCalledTimes(3);
317319
});
320+
321+
it('uses time-based updatePosition when timePeriodMs is provided', () => {
322+
const now = Date.now();
323+
const prices: TokenPrice[] = Array.from({ length: 5 }, (_, i) => [
324+
String(now - 86_400_000 + i * 20_000_000),
325+
100 + i,
326+
]) as TokenPrice[];
327+
328+
const { getByTestId } = render(
329+
<PriceChartProvider>
330+
<PriceChart
331+
{...defaultProps}
332+
prices={prices}
333+
priceDiff={5}
334+
timePeriodMs={86_400_000}
335+
/>
336+
</PriceChartProvider>,
337+
);
338+
339+
const chartArea = getByTestId('price-chart-area');
340+
fireEvent(chartArea, 'responderGrant', createResponderEvent(50, 50));
341+
expect(mockOnChartIndexChange).toHaveBeenCalled();
342+
const idx = mockOnChartIndexChange.mock.calls[0][0];
343+
expect(idx).toBeGreaterThanOrEqual(0);
344+
expect(idx).toBeLessThan(prices.length);
345+
});
346+
});
347+
348+
describe('findNearestIndex', () => {
349+
it('returns -1 for an empty array', () => {
350+
expect(findNearestIndex([], 100)).toBe(-1);
351+
});
352+
353+
it('returns 0 for a single-element array', () => {
354+
const prices: TokenPrice[] = [['1000', 42]];
355+
expect(findNearestIndex(prices, 999)).toBe(0);
356+
expect(findNearestIndex(prices, 1000)).toBe(0);
357+
expect(findNearestIndex(prices, 1001)).toBe(0);
358+
});
359+
360+
it('returns exact match index', () => {
361+
const prices: TokenPrice[] = [
362+
['100', 1],
363+
['200', 2],
364+
['300', 3],
365+
];
366+
expect(findNearestIndex(prices, 200)).toBe(1);
367+
});
368+
369+
it('returns the closer element when target is between two points', () => {
370+
const prices: TokenPrice[] = [
371+
['100', 1],
372+
['200', 2],
373+
['300', 3],
374+
];
375+
expect(findNearestIndex(prices, 190)).toBe(1);
376+
expect(findNearestIndex(prices, 110)).toBe(0);
377+
});
378+
379+
it('returns 0 when target is before all data', () => {
380+
const prices: TokenPrice[] = [
381+
['100', 1],
382+
['200', 2],
383+
];
384+
expect(findNearestIndex(prices, 50)).toBe(0);
385+
});
386+
387+
it('returns last index when target is after all data', () => {
388+
const prices: TokenPrice[] = [
389+
['100', 1],
390+
['200', 2],
391+
['300', 3],
392+
];
393+
expect(findNearestIndex(prices, 999)).toBe(2);
394+
});
395+
});
396+
397+
describe('Time-based rendering', () => {
398+
it('renders the chart with time-based xAccessor when timePeriodMs is provided', () => {
399+
const prices = fiveTokenPrices();
400+
const { getByTestId } = render(
401+
<PriceChart
402+
{...defaultProps}
403+
prices={prices}
404+
priceDiff={5}
405+
timePeriodMs={86_400_000}
406+
/>,
407+
);
408+
expect(getByTestId('price-chart-area')).toBeOnTheScreen();
409+
});
410+
411+
it('renders the chart with index-based xAccessor when timePeriodMs is undefined', () => {
412+
const prices = fiveTokenPrices();
413+
const { getByTestId } = render(
414+
<PriceChart {...defaultProps} prices={prices} priceDiff={5} />,
415+
);
416+
expect(getByTestId('price-chart-area')).toBeOnTheScreen();
417+
});
418+
419+
it('renders EndDot using time-based x position when timePeriodMs is set', () => {
420+
const prices = fiveTokenPrices();
421+
const result = render(
422+
<PriceChart
423+
{...defaultProps}
424+
prices={prices}
425+
priceDiff={5}
426+
timePeriodMs={86_400_000}
427+
/>,
428+
);
429+
430+
const chartArea = result.getByTestId('price-chart-area');
431+
const layoutViews = chartArea.findAll(
432+
(node) => typeof node.props?.onLayout === 'function',
433+
);
434+
layoutViews.forEach((v) =>
435+
fireEvent(v, 'layout', {
436+
nativeEvent: { layout: { x: 0, y: 0, width: 300, height: 200 } },
437+
}),
438+
);
439+
440+
expect(result.getByTestId('price-chart-end-dot')).toBeOnTheScreen();
441+
});
318442
});
319443
});

0 commit comments

Comments
 (0)