fix(predict): Limit chart data points to ensure low-end device support#25016
fix(predict): Limit chart data points to ensure low-end device support#25016kevinbluer wants to merge 0 commit into
Conversation
kevinbluer
force-pushed
the
fix/predict-market-details-chart-max
branch
from
ac56a7a to
a7fc655
Compare
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsThe PR makes changes exclusively to the Predict (Predictions) feature:
All changes are isolated to the Predict feature with no impact on core wallet functionality, navigation, or shared components. The E2E tests in Risk is low because:
|
|
| seriesToRender.map((series) => { | ||
| // Limit data points to prevent performance issues on lower-end devices | ||
| const limitedData = | ||
| series.data.length > MAX_CHART_POINTS | ||
| ? series.data.filter( | ||
| (_, i, arr) => | ||
| i % Math.ceil(arr.length / MAX_CHART_POINTS) === 0 || | ||
| i === arr.length - 1, | ||
| ) | ||
| : series.data; | ||
| return { | ||
| ...series, | ||
| data: limitedData.map((point) => ({ | ||
| ...point, | ||
| label: formatPriceHistoryLabel(point.timestamp, selectedTimeframe, { | ||
| timeRangeMs: chartTimeRangeMs, | ||
| }), | ||
| })), | ||
| }; | ||
| }), |
There was a problem hiding this comment.
Is this fetching everything and filtering data out on the React component?
My suggestion is we move this logic to the hook and also do not over-fetch, since the API allows more granular control over the data we get.
| ? PredictMarketStatus.CLOSED | ||
| : PredictMarketStatus.OPEN, | ||
| recurrence: getRecurrence(event.series), | ||
| startTime: event.startTime, |
There was a problem hiding this comment.
Did you confirm this event.startTime? When I fetch events from the API I see startDate, rather than startTime
| const startTs = market?.startTime | ||
| ? new Date(market.startTime).getTime() | ||
| : null; | ||
|
|
||
| if (!startTs) { | ||
| // Fallback to default if no start time available | ||
| return DEFAULT_FIDELITY_BY_INTERVAL[PredictPriceHistoryInterval.MAX]; | ||
| } | ||
|
|
||
| const rangeMinutes = (Date.now() - startTs) / (60 * 1000); | ||
| const calculatedFidelity = Math.ceil(rangeMinutes / MAX_HISTORY_SAMPLES); | ||
|
|
||
| // Ensure fidelity is at least 1 minute | ||
| return Math.max(1, calculatedFidelity); |
There was a problem hiding this comment.
same here... I would take this opportunity to remove ALL chart / history data logic into the hook, so we're not adding so much logic directly into components
kevinbluer
force-pushed
the
fix/predict-market-details-chart-max
branch
from
a7fc655 to
51f8b9a
Compare
3 participants
Description
Fixes chart rendering performance issues on low-end devices (e.g., Galaxy A32) when the
MAXtimeframe is selected with large datasets (e.g. longer running markets). Implements data limiting via dynamic fidelity calculation, with "client-side" safety net.Changes
startTimefield to PredictMarket typeparsePolymarketEventsMAXtimeframe based on market ageMAX_CHART_POINTSconstant (150) inPredictDetailsChartChangelog
CHANGELOG entry: null
Related issues
Fixes: 24523
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
No changes detected in the diff; no files were added, modified, or removed.
Written by Cursor Bugbot for commit 51f8b9a. This will update automatically on new commits. Configure here.