Finish developing the stage 2 function, it is stable but not yet optimized. #23
Conversation
finish API design for historical data #20
…ntend Finish the frontend design but not yet optimized
Boxuan-Matty-Lin
added
documentation
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull Request Overview
This PR adds historical exchange rate tracking and visualization to the currency converter application. The main enhancement enables users to view time-series charts of exchange rates over the past 13 days, with interactive currency selection and responsive design that adapts between desktop and mobile views.
Key changes:
- Adds historical rate fetching from Open Exchange Rates API with retry logic and concurrency control
- Implements interactive chart components using Recharts with collapsible info sections
- Refactors
CurrencySidebarto be a controlled component receiving data and event handlers from parent - Adds currency selection feature with visual highlighting and chart synchronization
Reviewed Changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds @radix-ui/react-collapsible dependency and its transitive dependencies |
| package.json | Registers @radix-ui/react-collapsible as a project dependency |
| lib/time_utils.ts | Provides utilities for generating UTC date ranges for historical data fetching |
| lib/server/oxr_history.ts | Implements historical rate fetching with retry logic, concurrent workers, and data transformation |
| lib/server/oxr_convert.ts | Updates DEFAULTS_CURRENCY from const array to mutable array for flexible usage |
| lib/server/oxr.ts | Adds getHistorical function and HistoricalResponse type for historical API calls |
| lib/server/tests/oxr_history.test.ts | Comprehensive unit tests for history fetching and transformation logic |
| lib/server/tests/oxr.integration.test.ts | Adds integration test for getHistorical endpoint with real API calls |
| components/ui/collapsible.tsx | Wraps Radix UI collapsible primitives for consistent UI patterns |
| components/sidebar/tests/CurrencySidebar.test.tsx | Updates tests to reflect controlled component refactor with new props |
| components/sidebar/tests/CurrencyCard.test.tsx | Adds tests for selection interactions and keyboard navigation |
| components/sidebar/CurrencySidebar.tsx | Refactors to controlled component accepting rates, loading state, and event handlers |
| components/sidebar/CurrencyCard.tsx | Adds selection state, keyboard navigation, and interactive styling |
| components/charts/InfoCollapsible.tsx | Creates reusable collapsible component for displaying data provenance information |
| components/charts/HistoricalChart.tsx | Implements area chart with dynamic domain calculation and theme-aware tooltips |
| components/charts/ChartWithInfo.tsx | Combines chart and collapsible info section into a single component |
| app/page.tsx | Refactors to client component managing rates, history, selection state, and modal chart display |
| app/api/rates/history/route.ts | Exposes historical rates API endpoint with configurable date ranges |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
|
|
||
|
|
||
| /** * Shape of Open Exchange Rates historical payload. |
There was a problem hiding this comment.
The JSDoc comment has an extra space between /** and *. Should be /** without the extra asterisk.
| /** * Shape of Open Exchange Rates historical payload. | |
| /** | |
| * Shape of Open Exchange Rates historical payload. |
|
|
||
|
|
||
|
|
||
| /** * Fetches historical exchange rates for a specific date from Open Exchange Rates. |
There was a problem hiding this comment.
The JSDoc comment has an extra space between /** and *. Should be /** without the extra asterisk.
| /** * Fetches historical exchange rates for a specific date from Open Exchange Rates. | |
| /** | |
| * Fetches historical exchange rates for a specific date from Open Exchange Rates. |
| for (const t of targets) series[t] = []; | ||
|
|
||
| for (const row of points) { | ||
| const d = row.date; // 已是 string |
There was a problem hiding this comment.
Comment contains Chinese text '已是 string' (meaning 'already is string'). Should be in English: 'already a string' or removed if redundant.
| const d = row.date; // 已是 string | |
| const d = row.date; |
| * @returns A promise resolving to the `HistoricalResponse` payload. | ||
| */ | ||
| export async function getHistorical(dateISO: string): Promise<HistoricalResponse> { | ||
| // dateISO = YYYY-MM-DD（UTC） |
There was a problem hiding this comment.
Comment uses Chinese full-width parentheses '（UTC）'. Should use standard ASCII parentheses: '(UTC)'.
| // dateISO = YYYY-MM-DD（UTC） | |
| // dateISO = YYYY-MM-DD (UTC) |
| let index = 0; | ||
|
|
||
| // Small worker pool that processes pending dates until exhausted. | ||
| const worker = async () => { | ||
| while (true) { | ||
| const current = index++; | ||
| if (current >= dates.length) break; | ||
| const date = dates[current]; | ||
| const result = await fetchHistoricalWithRetry(date); | ||
| if (result && "rates" in result) { | ||
| points[current] = toAudRow(result.rates, upperTargets, date); | ||
| } else { | ||
| points[current] = { date, error: "fetch_failed" }; |
There was a problem hiding this comment.
The shared index variable is mutated by multiple concurrent workers without synchronization. While JavaScript's single-threaded event loop prevents true race conditions, the post-increment index++ is not atomic in async contexts. This could lead to duplicate work or skipped indices if workers interleave. Consider using a proper queue or generator pattern for work distribution.
| let index = 0; | |
| // Small worker pool that processes pending dates until exhausted. | |
| const worker = async () => { | |
| while (true) { | |
| const current = index++; | |
| if (current >= dates.length) break; | |
| const date = dates[current]; | |
| const result = await fetchHistoricalWithRetry(date); | |
| if (result && "rates" in result) { | |
| points[current] = toAudRow(result.rates, upperTargets, date); | |
| } else { | |
| points[current] = { date, error: "fetch_failed" }; | |
| // Use a queue to safely distribute work among workers. | |
| const dateQueue = dates.map((date, idx) => ({ date, idx })); | |
| // Small worker pool that processes pending dates until exhausted. | |
| const worker = async () => { | |
| while (true) { | |
| const item = dateQueue.shift(); | |
| if (!item) break; | |
| const { date, idx } = item; | |
| const result = await fetchHistoricalWithRetry(date); | |
| if (result && "rates" in result) { | |
| points[idx] = toAudRow(result.rates, upperTargets, date); | |
| } else { | |
| points[idx] = { date, error: "fetch_failed" }; |
| decimals = 4, | ||
| className, | ||
| }: Props) { | ||
| // 最后一个点，用于高亮当前值 |
There was a problem hiding this comment.
Comment contains Chinese text '最后一个点，用于高亮当前值' (meaning 'last point, used to highlight current value'). Should be in English: 'Last point, used to highlight the current value'.
| // 最后一个点，用于高亮当前值 | |
| // Last point, used to highlight the current value |
| <div className="h-72 w-full"> | ||
| <ResponsiveContainer> | ||
| <AreaChart data={data}> | ||
| {/* 时间轴保持简洁，不改默认配色 */} |
There was a problem hiding this comment.
Comment contains Chinese text '时间轴保持简洁，不改默认配色' (meaning 'Keep time axis simple, don't change default colors'). Should be in English: 'Keep time axis simple, use default colors'.
| {/* 时间轴保持简洁，不改默认配色 */} | |
| {/* Keep time axis simple, use default colors */} |
| {/* 只调整 Tooltip 尺寸与圆角 | ||
| 暗黑模式下灰底和灰白字，其余保持原生 */} |
There was a problem hiding this comment.
Comment contains Chinese text '只调整 Tooltip 尺寸与圆角 暗黑模式下灰底和灰白字，其余保持原生' (meaning 'Only adjust Tooltip size and border-radius; in dark mode use gray background and light text, keep rest native'). Should be in English: 'Only adjust Tooltip size and border-radius; in dark mode use gray background and light text, keep rest native'.
| {/* 只调整 Tooltip 尺寸与圆角 | |
| 暗黑模式下灰底和灰白字，其余保持原生 */} | |
| {/* Only adjust Tooltip size and border-radius | |
| in dark mode use gray background and light text, keep rest native */} |
| fill="url(#fxFill)" | ||
| /> | ||
|
|
||
| {/* 渐变不改颜色，仅保留透明度过渡 */} |
There was a problem hiding this comment.
Comment contains Chinese text '渐变不改颜色，仅保留透明度过渡' (meaning 'Gradient keeps color, only varies opacity'). Should be in English: 'Gradient keeps color, only varies opacity'.
| {/* 渐变不改颜色，仅保留透明度过渡 */} | |
| {/* Gradient keeps color, only varies opacity */} |
| </ResponsiveContainer> | ||
| </div> | ||
|
|
||
| {/* 末端小圆点的文本提示 */} |
There was a problem hiding this comment.
Comment contains Chinese text '末端小圆点的文本提示' (meaning 'Text label for the last point'). Should be in English: 'Text label for the last point'.
| {/* 末端小圆点的文本提示 */} | |
| {/* Text label for the last point */} |
This pull request introduces a major refactor and feature upgrade to the currency converter app. The main focus is on adding a responsive, interactive historical exchange rate chart for each currency, improving the sidebar's architecture, and enhancing accessibility and user experience across devices. The sidebar is now a presentational component, with all data fetching and state management moved to the page level, and several new reusable chart components are introduced.
Key changes include:
1. Historical Chart Feature & Components
app/api/rates/history/route.tsto serve historical exchange rate data, supporting up to 60 days and a "byCurrency" orientation.components/charts/HistoricalChart.tsx: Renders a responsive area chart for historical rates, with custom tooltip and styling.components/charts/ChartWithInfo.tsx: Wraps the chart with a collapsible info section explaining data provenance and limitations.components/charts/InfoCollapsible.tsx: Generic collapsible info panel with accessible controls.2. Page Architecture & State Management
app/page.tsxto manage all data fetching (latest rates, history), error/loading states, and UI state (selected currency, modal visibility, desktop/mobile detection) at the page level. The sidebar now receives all data and callbacks as props, making it a pure presentational component.3. Sidebar & Currency Card Improvements
CurrencySidebarto accept props for all data and event handlers, removing its own state and side effects. Exported types and constants for better type safety and reusability. [1] [2]CurrencyCardto support selection, keyboard accessibility, and click/keyboard event handlers for improved usability and a11y. [1] [2] [3]4. Type Safety & Code Cleanliness
Rates,TargetCode,CURRENCY_META) to be exported for shared usage across components.References: