feat: add week/month/year range selector to token price chart#2121
Merged
feat: add week/month/year range selector to token price chart#2121
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a 7/30/365-day range toggle to LineChart, replaces prior fetch with CoinGecko market_chart endpoint driven by Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as ToggleButtonGroup
participant ChartComp as LineChart Component
participant Utils as fetchWithTimeout
participant API as CoinGecko API
participant Renderer as Chart Renderer
User->>UI: select range (7 / 30 / 365)
UI->>ChartComp: set selectedRange
ChartComp->>ChartComp: compute endpoint & interval based on range
ChartComp->>Utils: fetchWithTimeout(GET /coins/{id}/market_chart?days={selectedRange})
Utils->>API: HTTP request
API-->>Utils: MarketChartResponse (prices: [[time, value], ...])
Utils-->>ChartComp: Response data
ChartComp->>ChartComp: parse prices -> priceData[{time, value}, ...]
ChartComp->>Renderer: update dataset, tooltips, axes, title
Renderer-->>User: render updated chart
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/extension-polkagate/src/fullscreen/components/LineChart.tsx (1)
129-218:⚠️ Potential issue | 🟠 MajorPrevent stale responses from overwriting the latest selected range.
Line 207 updates chart state for any completed request. If the user switches ranges quickly, an older request can finish last and replace newer data, causing a range/title mismatch.
Suggested fix
@@ const chartRef = useRef<ChartJS<'line'>>(null); + const latestRequestIdRef = useRef(0); @@ const fetchPriceData = useCallback(async () => { + const requestId = ++latestRequestIdRef.current; try { @@ - setPriceData(prices); + if (requestId !== latestRequestIdRef.current) { + return; // stale response + } + setPriceData(prices);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension-polkagate/src/fullscreen/components/LineChart.tsx` around lines 129 - 218, fetchPriceData can allow an older fetch to overwrite newer state when the user switches ranges quickly; capture the current selectedRange at the start of fetchPriceData (e.g., const requestRange = selectedRange) and before calling setPriceData compare selectedRange to requestRange and abort updating if they differ, or alternatively wire an AbortController to the fetch (use fetchWithTimeout's signal) and abort prior requests when selectedRange changes; update the function identifiers fetchPriceData and setPriceData and the dependency array accordingly so only the intended response updates the chart.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/extension-polkagate/src/fullscreen/components/LineChart.tsx`:
- Around line 193-205: The component currently conflates "still loading" with
"parsed-but-empty" by using priceData.length === 0 to decide to show a loader;
change the loading flag so it reflects an undefined/null input or an explicit
isLoading prop instead of an empty parsed array. In LineChart.tsx adjust the
parsing block around maybePrices/needsSparkline (the code that sets prices from
maybePrices) to always set prices = [] when parsing yields no points, but do not
set isLoading there; ensure the component uses a separate boolean (e.g.,
isLoading or props.isLoading) to show the spinner and use priceData.length === 0
to render a no-data state. Update any places that currently derive loading from
priceData (the render logic around priceData.length checks) to read the new
loading flag instead.
- Around line 338-340: The three ToggleButton labels ("Week", "Month", "Year")
are hardcoded; replace them with localized strings using the component's
translation function (e.g., change ToggleButton value={7}>Week</ToggleButton to
ToggleButton value={7}>{t('...')}</ToggleButton), using appropriate translation
keys (e.g., 'range.week', 'range.month', 'range.year') to match existing i18n
patterns used in this component (refer to the ToggleButton elements near
value={7}, value={30}, value={365} and the t(...) usage elsewhere).
---
Outside diff comments:
In `@packages/extension-polkagate/src/fullscreen/components/LineChart.tsx`:
- Around line 129-218: fetchPriceData can allow an older fetch to overwrite
newer state when the user switches ranges quickly; capture the current
selectedRange at the start of fetchPriceData (e.g., const requestRange =
selectedRange) and before calling setPriceData compare selectedRange to
requestRange and abort updating if they differ, or alternatively wire an
AbortController to the fetch (use fetchWithTimeout's signal) and abort prior
requests when selectedRange changes; update the function identifiers
fetchPriceData and setPriceData and the dependency array accordingly so only the
intended response updates the chart.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 13555b84-8394-4325-80a2-519e3555cddb
📒 Files selected for processing (1)
packages/extension-polkagate/src/fullscreen/components/LineChart.tsx
packages/extension-polkagate/src/fullscreen/components/LineChart.tsx
Outdated
Show resolved
Hide resolved
packages/extension-polkagate/src/fullscreen/components/LineChart.tsx
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/extension-polkagate/src/fullscreen/components/LineChart.tsx (2)
142-143:⚠️ Potential issue | 🟠 MajorThrottle 429 warnings to avoid alert spam during polling.
With interval polling enabled (Lines 219-221), every 429 triggers
notify(...)again at Line 143.useAlertscurrently appends every alert (no dedupe), so this can spam users repeatedly.Suggested fix (cooldown for rate-limit alerts)
@@ const { notify } = useAlerts(); + const lastRateLimitNotifyAtRef = useRef(0); @@ if (!res.ok) { if (res.status === 429) { - notify(t('Rate limited by CoinGecko. Please try again shortly.'), 'warning'); + const now = Date.now(); + const cooldownMs = 5 * 60 * 1000; + if (now - lastRateLimitNotifyAtRef.current > cooldownMs) { + lastRateLimitNotifyAtRef.current = now; + notify(t('Rate limited by CoinGecko. Please try again shortly.'), 'warning'); + } } else { notify(t(`Failed to fetch token data (status ${res.status}).`), 'error'); }Also applies to: 219-221
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension-polkagate/src/fullscreen/components/LineChart.tsx` around lines 142 - 143, The rate-limit notify call (notify(...)) in LineChart.tsx is spamming users during interval polling; add a cooldown guard (e.g., a module-level or component ref/state like lastRateLimitAt or isRateLimitNotified) and only call notify for a 429 if the last notification timestamp is older than a chosen cooldown (e.g., 60s); update both the 429 branch where res.status === 429 and the polling callback (the code around the interval polling at lines ~219-221) to consult and update that guard so alerts are throttled rather than appended on every poll.
126-205:⚠️ Potential issue | 🟠 MajorPrevent stale responses from overwriting the currently selected range.
If users switch ranges quickly, an older request can resolve later and overwrite newer data via
setPriceData(prices), causing range/title mismatch.Suggested fix (request sequencing guard)
@@ const [priceData, setPriceData] = useState<PricePoint[]>([]); const [selectedRange, setSelectedRange] = useState<number>(7); + const latestRequestIdRef = useRef(0); const fetchPriceData = useCallback(async () => { + const requestId = ++latestRequestIdRef.current; try { @@ - setPriceData(prices); + if (requestId !== latestRequestIdRef.current) { + return; + } + setPriceData(prices); } catch (err: unknown) { @@ - notify(message, isTimeOut ? 'warning' : 'error'); + if (requestId === latestRequestIdRef.current) { + notify(message, isTimeOut ? 'warning' : 'error'); + } } }, [coinId, notify, selectedRange, t, vsCurrency]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension-polkagate/src/fullscreen/components/LineChart.tsx` around lines 126 - 205, The fetchPriceData routine can be overwritten by stale async responses when selectedRange changes; add a request-sequencing guard: create a mutable ref (e.g., latestFetchIdRef) and increment it each time fetchPriceData runs, capture the current id inside fetchPriceData, and before calling setPriceData(prices) verify the captured id matches latestFetchIdRef.current (if not, return early). Update the same guard on cleanup/unmount or when selectedRange changes (or alternatively wire an AbortController into fetchWithTimeout and abort previous requests) so only the most recent fetch updates state.
♻️ Duplicate comments (1)
packages/extension-polkagate/src/fullscreen/components/LineChart.tsx (1)
319-325:⚠️ Potential issue | 🟠 MajorSeparate loading state from empty-data state.
Line 319 still treats
priceData.length === 0as loading. If parsing yields an empty array, the loader can persist indefinitely instead of showing a no-data state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension-polkagate/src/fullscreen/components/LineChart.tsx` around lines 319 - 325, The current conditional uses priceData.length === 0 to show SineWaveLoader which confuses "loading" with "empty data"; change the render logic to check a distinct loading boolean (e.g., isLoading or loadingPriceData) and only render SineWaveLoader when that flag is true, otherwise if loading is false and priceData.length === 0 render a no-data placeholder component/message, and only render <Line data={chartData} options={options} plugins={[gradientFillPlugin]} ref={chartRef} /> when loading is false and priceData.length > 0; update the component state/props to provide the new loading flag and adjust references to priceData, SineWaveLoader, chartData, gradientFillPlugin, options, and chartRef accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/extension-polkagate/src/fullscreen/components/LineChart.tsx`:
- Around line 142-143: The rate-limit notify call (notify(...)) in LineChart.tsx
is spamming users during interval polling; add a cooldown guard (e.g., a
module-level or component ref/state like lastRateLimitAt or isRateLimitNotified)
and only call notify for a 429 if the last notification timestamp is older than
a chosen cooldown (e.g., 60s); update both the 429 branch where res.status ===
429 and the polling callback (the code around the interval polling at lines
~219-221) to consult and update that guard so alerts are throttled rather than
appended on every poll.
- Around line 126-205: The fetchPriceData routine can be overwritten by stale
async responses when selectedRange changes; add a request-sequencing guard:
create a mutable ref (e.g., latestFetchIdRef) and increment it each time
fetchPriceData runs, capture the current id inside fetchPriceData, and before
calling setPriceData(prices) verify the captured id matches
latestFetchIdRef.current (if not, return early). Update the same guard on
cleanup/unmount or when selectedRange changes (or alternatively wire an
AbortController into fetchWithTimeout and abort previous requests) so only the
most recent fetch updates state.
---
Duplicate comments:
In `@packages/extension-polkagate/src/fullscreen/components/LineChart.tsx`:
- Around line 319-325: The current conditional uses priceData.length === 0 to
show SineWaveLoader which confuses "loading" with "empty data"; change the
render logic to check a distinct loading boolean (e.g., isLoading or
loadingPriceData) and only render SineWaveLoader when that flag is true,
otherwise if loading is false and priceData.length === 0 render a no-data
placeholder component/message, and only render <Line data={chartData}
options={options} plugins={[gradientFillPlugin]} ref={chartRef} /> when loading
is false and priceData.length > 0; update the component state/props to provide
the new loading flag and adjust references to priceData, SineWaveLoader,
chartData, gradientFillPlugin, options, and chartRef accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 897bb79c-2504-4922-9632-fcbd1fc98214
📒 Files selected for processing (1)
packages/extension-polkagate/src/fullscreen/components/LineChart.tsx
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/extension-polkagate/src/fullscreen/components/LineChart.tsx (1)
342-345:⚠️ Potential issue | 🟠 MajorLoading state is still coupled to empty data.
At Line 342,
priceData.length === 0is treated as “loading”, so valid empty responses can leave the loader spinning indefinitely instead of showing a no-data state.Proposed fix
const [priceData, setPriceData] = useState<PricePoint[]>([]); +const [isLoading, setIsLoading] = useState<boolean>(true); const fetchPriceData = useCallback(async () => { + setIsLoading(true); try { @@ setPriceData(prices); } catch (err: unknown) { @@ notify(message, isTimeOut ? 'warning' : 'error'); + } finally { + setIsLoading(false); } }, [coinId, notify, selectedRange, t, vsCurrency]); @@ - {priceData.length === 0 + {isLoading ? ( <SineWaveLoader height={300} width={637} /> ) + : priceData.length === 0 + ? ( + <Typography variant='S-2'>{t('Nothing Found')}</Typography> + ) : ( <Line data={chartData} options={options} plugins={[gradientFillPlugin]} ref={chartRef} /> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension-polkagate/src/fullscreen/components/LineChart.tsx` around lines 342 - 345, The component treats priceData.length === 0 as the loading condition causing spinners for legitimate empty responses; change the logic in LineChart to use an explicit loading flag (e.g., isLoading prop/state) instead of priceData.length, render <SineWaveLoader> only when isLoading is true, and render a distinct no-data UI when isLoading is false and priceData is empty; locate the conditional that currently checks priceData.length === 0 and replace it to check the new isLoading flag (or hook-provided loading state), and add a NoData/empty-state element to display when priceData is empty but not loading.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/extension-polkagate/src/fullscreen/components/LineChart.tsx`:
- Around line 342-345: The component treats priceData.length === 0 as the
loading condition causing spinners for legitimate empty responses; change the
logic in LineChart to use an explicit loading flag (e.g., isLoading prop/state)
instead of priceData.length, render <SineWaveLoader> only when isLoading is
true, and render a distinct no-data UI when isLoading is false and priceData is
empty; locate the conditional that currently checks priceData.length === 0 and
replace it to check the new isLoading flag (or hook-provided loading state), and
add a NoData/empty-state element to display when priceData is empty but not
loading.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d1f62157-af39-400a-a6d5-4fa898720b8f
📒 Files selected for processing (2)
packages/extension-polkagate/src/fullscreen/components/LineChart.tsxpackages/extension/public/locales/en/translation.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/extension-polkagate/src/fullscreen/components/LineChart.tsx (1)
284-290:⚠️ Potential issue | 🟠 MajorLoading state is still coupled to empty-data state.
At Lines 284-287, using
priceData.length === 0as loading means an empty API result can keep the spinner visible indefinitely instead of showing a no-data state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension-polkagate/src/fullscreen/components/LineChart.tsx` around lines 284 - 290, The current render uses priceData.length === 0 to decide loading, which conflates an empty API result with loading; introduce and use an explicit loading flag (e.g., isLoading from the fetch state/props) instead of priceData length when rendering SineWaveLoader, and add a separate no-data UI when not loading and priceData is empty; update the JSX around priceData / SineWaveLoader / Line (and the surrounding logic that supplies priceData, e.g., the fetch hook or parent prop) so the conditional becomes: if isLoading show SineWaveLoader, else if priceData.length === 0 show a NoData placeholder, else render <Line data={chartData} options={options} plugins={[gradientFillPlugin]} ref={chartRef} />.
🧹 Nitpick comments (1)
packages/extension-polkagate/src/fullscreen/components/LineChart.tsx (1)
127-133: Remove unreachable check and stale “Sparkline” message.After Line 121-125,
maybePricesis already guaranteed to be an array, so Line 129-133 is dead code and the message is outdated.Proposed cleanup
- const maybePrices = data.prices; - - if (!maybePrices) { - notify(t('Sparkline data not available for this token.'), 'info'); - - return; - } + const maybePrices = data.prices;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension-polkagate/src/fullscreen/components/LineChart.tsx` around lines 127 - 133, The check for maybePrices being falsy and the notify call are unreachable because data.prices is already guaranteed to be an array earlier; remove the entire conditional block that references maybePrices and the stale t('Sparkline data not available for this token.') message (i.e., delete the if (!maybePrices) { notify(...); return; } block) so the component no longer contains dead code or outdated user-facing text; ensure no other logic depends on that early return and keep using maybePrices directly wherever used in this component (e.g., in LineChart rendering logic).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/extension-polkagate/src/fullscreen/components/LineChart.tsx`:
- Line 281: The chart title is hardcoded in LineChart.tsx
(title={`${coinId.toUpperCase()} Price — Last ${selectedRange} Days`}); update
it to use the i18n translator (t) instead of a literal string. Replace the
current title expression with a call to t(...) that includes coinId and
selectedRange as interpolation variables (e.g., pass coinId.toUpperCase() and
selectedRange), and add or use an appropriate translation key (e.g.,
"chart.priceLastDays") so the title becomes localized while preserving the coin
and day values.
---
Duplicate comments:
In `@packages/extension-polkagate/src/fullscreen/components/LineChart.tsx`:
- Around line 284-290: The current render uses priceData.length === 0 to decide
loading, which conflates an empty API result with loading; introduce and use an
explicit loading flag (e.g., isLoading from the fetch state/props) instead of
priceData length when rendering SineWaveLoader, and add a separate no-data UI
when not loading and priceData is empty; update the JSX around priceData /
SineWaveLoader / Line (and the surrounding logic that supplies priceData, e.g.,
the fetch hook or parent prop) so the conditional becomes: if isLoading show
SineWaveLoader, else if priceData.length === 0 show a NoData placeholder, else
render <Line data={chartData} options={options} plugins={[gradientFillPlugin]}
ref={chartRef} />.
---
Nitpick comments:
In `@packages/extension-polkagate/src/fullscreen/components/LineChart.tsx`:
- Around line 127-133: The check for maybePrices being falsy and the notify call
are unreachable because data.prices is already guaranteed to be an array
earlier; remove the entire conditional block that references maybePrices and the
stale t('Sparkline data not available for this token.') message (i.e., delete
the if (!maybePrices) { notify(...); return; } block) so the component no longer
contains dead code or outdated user-facing text; ensure no other logic depends
on that early return and keep using maybePrices directly wherever used in this
component (e.g., in LineChart rendering logic).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e18f0fef-3f44-4b94-94f0-52bfc37e5b78
📒 Files selected for processing (2)
packages/extension-polkagate/src/fullscreen/components/LineChart.tsxpackages/extension-polkagate/src/fullscreen/components/utils.ts
github-actions bot
pushed a commit
that referenced
this pull request
Mar 4, 2026
# [1.27.0](v1.26.2...v1.27.0) (2026-03-04) ### Features * add week/month/year range selector to token price chart ([#2121](#2121)) ([5820a3e](5820a3e))
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation