diff --git a/.planning/phases/04-chart-polish-and-bug-fixes/04-RESEARCH.md b/.planning/phases/04-chart-polish-and-bug-fixes/04-RESEARCH.md new file mode 100644 index 0000000..0e6db88 --- /dev/null +++ b/.planning/phases/04-chart-polish-and-bug-fixes/04-RESEARCH.md @@ -0,0 +1,340 @@ +# Phase 4: Chart Polish and Bug Fixes - Research + +**Researched:** 2026-03-12 +**Domain:** Recharts tooltip customization, JavaScript Intl.NumberFormat locale, React context propagation +**Confidence:** HIGH + +## Summary + +Phase 4 covers two tightly scoped changes. IXTN-04 requires chart tooltips in `ExpenseBreakdown` and `AvailableBalance` to display values formatted using the budget's currency and the user's locale. FIX-01 requires removing the hardcoded `'de-DE'` locale from `formatCurrency` in `lib/format.ts` and replacing it with the user's preferred locale from their settings. + +The codebase already has all the infrastructure in place. `palette.ts` already drives `Cell` fills correctly via `palette[entry.categoryType].base` — the color token problem is already solved. The gap is purely in tooltip content formatting. `Intl.NumberFormat` accepts any valid BCP 47 locale tag; the user's `preferred_locale` is already available on the `User` object returned by `auth.me()` and stored in `useAuth`. The main design question for FIX-01 is how to thread the locale down to `formatCurrency` — the cleanest approach is adding an optional `locale` parameter to the function signature with a sensible fallback. + +The `AvailableBalance` component currently renders a `` donut with no `` at all. `ExpenseBreakdown` has a bare `` with no formatting. Neither uses the shadcn `ChartContainer`/`ChartTooltipContent` wrapper — they use raw Recharts primitives. This is fine and consistent; the fix only needs a custom `content` prop on the Recharts ``. + +**Primary recommendation:** Add an optional `locale` param to `formatCurrency`, then pass a custom `content` renderer to Recharts `` in both chart components using the budget's currency and the user's preferred locale. + + +## Phase Requirements + +| ID | Description | Research Support | +|----|-------------|-----------------| +| IXTN-04 | Chart tooltips display values formatted with the budget's currency | Recharts `` custom renderer accepts the active payload with `value` and `name`; `formatCurrency` can be called inside the renderer with the budget's currency | +| FIX-01 | `formatCurrency` uses the user's locale preference instead of hardcoded `de-DE` | `User.preferred_locale` is already on the API type; adding an optional second positional param `locale` to `formatCurrency` and defaulting to `navigator.language` or `'en'` covers all call sites without breakage | + + +## Standard Stack + +### Core +| Library | Version | Purpose | Why Standard | +|---------|---------|---------|--------------| +| recharts | installed (peer of shadcn chart) | Charting — `PieChart`, `Pie`, `Cell`, `Tooltip` | Already in project; `ExpenseBreakdown` and `AvailableBalance` use it directly | +| Intl.NumberFormat | Web platform built-in | Locale-aware number and currency formatting | No third-party dep needed; `formatCurrency` already uses it | + +### Supporting +| Library | Version | Purpose | When to Use | +|---------|---------|---------|-------------| +| shadcn `chart.tsx` (`ChartTooltipContent`) | installed | Pre-styled tooltip shell with color indicator dot | Optional — can be used as the custom `content` renderer if its `formatter` prop is threaded in | + +### Alternatives Considered +| Instead of | Could Use | Tradeoff | +|------------|-----------|----------| +| Custom inline tooltip renderer | `ChartTooltipContent` with `formatter` prop | `ChartTooltipContent` is already styled correctly and accepts a `formatter` callback — either approach works; direct `content` prop is simpler since charts don't use `ChartContainer` | + +**Installation:** +No new packages required. + +## Architecture Patterns + +### Current Chart Structure + +Both chart components follow the same pattern — they use raw Recharts primitives without the `ChartContainer` wrapper: + +``` +ExpenseBreakdown.tsx + ResponsiveContainer + PieChart + Pie > Cell[] ← fill via palette[entry.categoryType].base (ALREADY CORRECT) + Tooltip ← bare, no formatter (NEEDS FIX) + +AvailableBalance.tsx + ResponsiveContainer + PieChart + Pie > Cell[] ← fill via palette[entry.categoryType].base (ALREADY CORRECT) + (no Tooltip) ← NEEDS to be added +``` + +### Pattern 1: Recharts Custom Tooltip Content Renderer + +**What:** Pass a `content` render prop to `` that receives the active payload and renders formatted values. + +**When to use:** When the chart does not use `ChartContainer` and needs full control over tooltip display. + +```tsx +// Source: Recharts official docs — https://recharts.org/en-US/api/Tooltip + { + if (!active || !payload?.length) return null + const item = payload[0] + return ( +
+

{item.name}

+

+ {formatCurrency(item.value as number, budget.currency, userLocale)} +

+
+ ) + }} +/> +``` + +### Pattern 2: Locale-Aware formatCurrency + +**What:** Add an optional `locale` parameter to `formatCurrency` with a fallback. + +**When to use:** Everywhere currency values are displayed — the function already handles currency code; adding locale makes it complete. + +```ts +// lib/format.ts — proposed signature +export function formatCurrency( + amount: number, + currency: string = 'EUR', + locale: string = 'en' +): string { + return new Intl.NumberFormat(locale, { + style: 'currency', + currency, + }).format(amount) +} +``` + +All existing call sites already pass `budget.currency` as the second argument — they will continue to work unchanged. Sites that want locale formatting pass `userLocale` as the third argument. + +### Pattern 3: Accessing User Locale from useAuth + +**What:** The `useAuth` hook already exposes `user.preferred_locale`. Charts are rendered inside `DashboardPage`, which does not receive `user` as a prop. The locale must be threaded in or read from a context. + +**Current auth flow:** +- `App.tsx` calls `useAuth()` and passes `auth.user` as a prop to `SettingsPage` but NOT to `DashboardPage` +- `DashboardPage` does not currently call `useAuth` — it calls `useBudgets` only + +**Options:** +1. **Call `useAuth()` inside `DashboardPage`** — simplest; hook is idempotent and reads from state already initialized by `App.tsx` +2. **Pass locale as prop** — adds prop drilling through `DashboardPage` → `ExpenseBreakdown`/`AvailableBalance` + +Option 1 is correct: call `useAuth()` in `DashboardPage` to get `user.preferred_locale`, then pass it down to the two chart components as a `locale` prop. The hook shares the same React state so there is no double-fetch. + +### Recommended File Change List + +``` +frontend/src/lib/format.ts ← add locale param with fallback +frontend/src/components/ExpenseBreakdown.tsx ← add locale prop, wire custom Tooltip +frontend/src/components/AvailableBalance.tsx ← add locale prop, add Tooltip with formatter +frontend/src/pages/DashboardPage.tsx ← call useAuth(), pass locale to chart components +``` + +### Anti-Patterns to Avoid + +- **Hardcoding `'de-DE'`** in any new tooltip code — defeats FIX-01 +- **Using `toLocaleString()` raw** in the tooltip without passing the locale explicitly — inherits the browser/OS locale instead of the user's stored preference +- **Adding `` to `AvailableBalance`** donut without a `content` prop — bare `` will show raw numeric values, not currency-formatted ones +- **Making `locale` a required prop on chart components** — would break existing callers; should default to `'en'` +- **Using `ChartContainer`/`ChartTooltipContent`** just for this change — these components exist in `chart.tsx` (a shadcn/ui source file) and should not be modified per project rule: "Customize shadcn via CSS variables only — never edit `src/components/ui/` source files directly" + +## Don't Hand-Roll + +| Problem | Don't Build | Use Instead | Why | +|---------|-------------|-------------|-----| +| Currency formatting with locale | Custom number formatter | `Intl.NumberFormat` (already in use) | Handles grouping separators, decimal symbols, currency symbol placement per locale — dozens of edge cases | +| Tooltip styled container | Custom `div` with bespoke shadows/borders | Replicate shadcn tooltip class list (`rounded-lg border border-border/50 bg-background px-2.5 py-1.5 text-xs shadow-xl`) | Consistent with app design system without modifying `chart.tsx` | + +**Key insight:** The work is threading data correctly — color tokens and the Recharts API are already in place. + +## Common Pitfalls + +### Pitfall 1: `Intl.NumberFormat` with Invalid Locale Tag +**What goes wrong:** Passing `preferred_locale` values like `'en'` or `'de'` without BCP 47 region subtag (e.g. `'en-US'`, `'de-DE'`) — both work fine in all modern browsers. Passing an empty string `''` or `undefined` throws `RangeError: invalid language tag`. + +**Why it happens:** A new user might have a blank `preferred_locale` in the DB if the field has no default. + +**How to avoid:** Defensive fallback: `locale || 'en'` before calling `Intl.NumberFormat`. The DB has `preferred_locale` on users; the Settings page initializes it to `'en'` if falsy. + +**Warning signs:** `RangeError: Incorrect locale information provided` in the console. + +### Pitfall 2: Tooltip Not Appearing on AvailableBalance Donut +**What goes wrong:** Adding `` to the donut inside `AvailableBalance` without a `content` prop — the default Recharts tooltip on a donut shows numeric values, not names, because the `name` field of the data items must match what Recharts expects. + +**How to avoid:** Always use a custom `content` renderer for this chart so you control what fields are displayed. + +### Pitfall 3: AvailableBalance Center Text Already Calls formatCurrency +**What goes wrong:** FIX-01 fix to `formatCurrency` will change the center text display in `AvailableBalance` (`{formatCurrency(available, budget.currency)}`). This is correct behavior but must be tested — English-locale users will see `$1,234.56` instead of `1.234,56 $`. + +**How to avoid:** Verify `formatCurrency` call in `AvailableBalance` also receives the locale parameter after the fix. + +### Pitfall 4: FinancialOverview and Other Components Using formatCurrency +**What goes wrong:** `FinancialOverview`, `AvailableBalance`, `BillsTracker`, `VariableExpenses`, `DebtTracker` all call `formatCurrency(amount, budget.currency)` — they will continue using the `locale` default. If the default is wrong, all tables are broken. + +**How to avoid:** The default in `formatCurrency` should be `'en'` not `'de-DE'`. Alternatively, accept that all these components also need the locale threaded through. Scoping to chart tooltips + the `formatCurrency` signature fix satisfies the requirements without refactoring every table. + +**Note:** FIX-01 requirement is specifically about `formatCurrency` using the user's locale. For the fullest fix, the locale default should change and call sites should pass the user locale — but minimum viable FIX-01 is changing the hardcoded default from `'de-DE'` to a sensible fallback like `'en'` or `navigator.language`. The planner should decide the scope: change only the default, or thread locale to all callers. + +### Pitfall 5: STATE.md Blocker About preferred_locale +**What goes wrong:** STATE.md notes: "Confirm `preferred_locale` field is available on the settings API response before implementing FIX-01." + +**Resolved:** `api.ts` already shows `settings.get()` returns `User` which includes `preferred_locale: string`. The `User` interface at line 38 of `api.ts` contains `preferred_locale`. The field is confirmed available on the API type. The backend endpoint is `/api/settings` which returns `User`. This blocker is resolved at the API type level; the plan should note to verify the backend migration ensures the column exists with a default value. + +## Code Examples + +### Custom Tooltip for ExpenseBreakdown (Pie Chart) + +```tsx +// Source: Recharts Tooltip API — https://recharts.org/en-US/api/Tooltip +// Applied to ExpenseBreakdown.tsx + { + if (!active || !payload?.length) return null + const item = payload[0] + return ( +
+

{item.name}

+

+ {formatCurrency(Number(item.value), budget.currency, locale)} +

+
+ ) + }} +/> +``` + +### Custom Tooltip for AvailableBalance (Donut Chart) + +```tsx +// Same pattern — AvailableBalance also needs locale and budget.currency props added + { + if (!active || !payload?.length) return null + const item = payload[0] + return ( +
+

{item.name}

+

+ {formatCurrency(Number(item.value), budget.currency, locale)} +

+
+ ) + }} +/> +``` + +### Updated formatCurrency Signature + +```ts +// frontend/src/lib/format.ts +export function formatCurrency( + amount: number, + currency: string = 'EUR', + locale: string = 'en' +): string { + return new Intl.NumberFormat(locale, { + style: 'currency', + currency, + }).format(amount) +} +``` + +### DashboardPage — Wiring Locale + +```tsx +// frontend/src/pages/DashboardPage.tsx +// Add useAuth call (already imported in hooks) +const { user } = useAuth() +const userLocale = user?.preferred_locale || 'en' + +// Pass to chart components + + +``` + +## State of the Art + +| Old Approach | Current Approach | When Changed | Impact | +|--------------|------------------|--------------|--------| +| Hardcoded `'de-DE'` locale | User's `preferred_locale` from settings | Phase 4 | English-locale users see correct number formatting | +| No tooltip on charts | Custom currency-formatted tooltip | Phase 4 | Charts become informative on hover | + +**Current status (before Phase 4):** +- `ExpenseBreakdown` has `` with default Recharts formatting (raw numbers, no currency symbol) +- `AvailableBalance` has no tooltip at all +- `formatCurrency` hardcodes `'de-DE'` regardless of user preference + +## Open Questions + +1. **Should all `formatCurrency` call sites receive the user locale, or just the tooltip fix?** + - What we know: FinancialOverview, BillsTracker, VariableExpenses, DebtTracker, AvailableBalance center text all call `formatCurrency(amount, budget.currency)` with no locale + - What's unclear: FIX-01 says "fix `formatCurrency`" — this could mean fix the default, or thread locale everywhere + - Recommendation: Plan as two sub-tasks — (a) fix the default in `format.ts` from `'de-DE'` to `'en'`, and (b) thread locale to chart tooltips for IXTN-04. A full locale-threading pass across all components can be a follow-up or included in Phase 4 depending on scope the planner defines. + +2. **Does the backend `preferred_locale` column have a NOT NULL default?** + - What we know: `User.preferred_locale` is typed as `string` (not `string | null`) in `api.ts`; Settings page defaults to `'en'` + - What's unclear: Whether old user rows have a value or empty string + - Recommendation: Add defensive `|| 'en'` fallback at the `formatCurrency` call in `DashboardPage`; this is low risk. + +## Validation Architecture + +### Test Framework + +| Property | Value | +|----------|-------| +| Framework | Vitest (configured in `vite.config.ts`) | +| Config file | `frontend/vite.config.ts` — `test` block with `environment: 'jsdom'`, `setupFiles: ['./src/test-setup.ts']` | +| Quick run command | `cd frontend && bun vitest run src/lib/format.test.ts` | +| Full suite command | `cd frontend && bun vitest run` | + +### Phase Requirements → Test Map + +| Req ID | Behavior | Test Type | Automated Command | File Exists? | +|--------|----------|-----------|-------------------|-------------| +| FIX-01 | `formatCurrency('en')` formats as `$1,234.56`; `formatCurrency('de')` formats as `1.234,56 €` | unit | `cd frontend && bun vitest run src/lib/format.test.ts` | ❌ Wave 0 | +| FIX-01 | Default locale is not `'de-DE'` — calling `formatCurrency(1234.56, 'EUR')` without locale arg does not produce German format | unit | `cd frontend && bun vitest run src/lib/format.test.ts` | ❌ Wave 0 | +| IXTN-04 | `ExpenseBreakdown` tooltip renders formatted currency string | unit | `cd frontend && bun vitest run src/components/ExpenseBreakdown.test.tsx` | ❌ Wave 0 | +| IXTN-04 | `AvailableBalance` tooltip renders formatted currency string | unit | `cd frontend && bun vitest run src/components/AvailableBalance.test.tsx` | ❌ Wave 0 | + +**Note:** Recharts renders its tooltip conditionally on mouseover — tooltip tests should mock the Recharts `Tooltip` render prop or test the formatter function directly rather than simulating hover events (jsdom does not simulate SVG pointer events reliably). + +### Sampling Rate + +- **Per task commit:** `cd frontend && bun vitest run src/lib/format.test.ts` +- **Per wave merge:** `cd frontend && bun vitest run` +- **Phase gate:** Full suite green before `/gsd:verify-work` + +### Wave 0 Gaps + +- [ ] `frontend/src/lib/format.test.ts` — covers FIX-01 locale tests +- [ ] `frontend/src/components/ExpenseBreakdown.test.tsx` — covers IXTN-04 tooltip formatter +- [ ] `frontend/src/components/AvailableBalance.test.tsx` — covers IXTN-04 tooltip formatter on donut + +## Sources + +### Primary (HIGH confidence) + +- Direct codebase inspection — `frontend/src/lib/format.ts`, `frontend/src/components/ExpenseBreakdown.tsx`, `frontend/src/components/AvailableBalance.tsx`, `frontend/src/lib/api.ts`, `frontend/src/lib/palette.ts`, `frontend/src/pages/DashboardPage.tsx`, `frontend/src/hooks/useAuth.ts` +- MDN Web Docs (Intl.NumberFormat) — locale parameter is a standard BCP 47 tag; empty string throws RangeError; `'en'`, `'de'` short tags are valid + +### Secondary (MEDIUM confidence) + +- Recharts official documentation — `` accepts a React render prop receiving `{ active, payload, label }`; `payload[0].value` and `payload[0].name` are always available for Pie charts + +### Tertiary (LOW confidence) + +- None — all findings are verifiable from codebase or standard web platform documentation + +## Metadata + +**Confidence breakdown:** +- Standard stack: HIGH — no new libraries; all existing +- Architecture: HIGH — pattern directly derived from reading actual component source +- Pitfalls: HIGH — identified from reading current code and known Intl.NumberFormat behavior +- FIX-01 scope question: MEDIUM — requirement wording is ambiguous about full vs partial locale threading + +**Research date:** 2026-03-12 +**Valid until:** 2026-04-12 (stable domain — Recharts API and Intl.NumberFormat are stable)