makiolaj/SimpleFinanceDash
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs(phase-04): research chart polish and bug fixes

Browse Source
This commit is contained in:
Jean-Luc Makiola
2026-03-12 09:16:09 +01:00
parent be22fcc808
commit dd098d723e
1 changed files with 340 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

340
.planning/phases/04-chart-polish-and-bug-fixes/04-RESEARCH.md Normal file
View File

@@ -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 `<PieChart>` donut with no `<Tooltip>` at all. `ExpenseBreakdown` has a bare `<Tooltip />` 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 `<Tooltip>`.
**Primary recommendation:** Add an optional `locale` param to `formatCurrency`, then pass a custom `content` renderer to Recharts `<Tooltip>` in both chart components using the budget's currency and the user's preferred locale.
<phase_requirements>
## Phase Requirements
| ID | Description | Research Support |
|----|-------------|-----------------|
| IXTN-04 | Chart tooltips display values formatted with the budget's currency | Recharts `<Tooltip content={...}>` 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 |
</phase_requirements>
## 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 `<Tooltip>` 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
<Tooltip
content={({ active, payload }) => {
if (!active || !payload?.length) return null
const item = payload[0]
return (
<div className="rounded-lg border border-border/50 bg-background px-2.5 py-1.5 text-xs shadow-xl">
<p className="font-medium text-foreground">{item.name}</p>
<p className="text-muted-foreground">
{formatCurrency(item.value as number, budget.currency, userLocale)}
</p>
</div>
)
}}
/>
```
### 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 `<Tooltip>` to `AvailableBalance`** donut without a `content` prop — bare `<Tooltip />` 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 `<Tooltip />` 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
<Tooltip
content={({ active, payload }) => {
if (!active || !payload?.length) return null
const item = payload[0]
return (
<div className="rounded-lg border border-border/50 bg-background px-2.5 py-1.5 text-xs shadow-xl">
<p className="font-medium text-foreground">{item.name}</p>
<p className="font-mono tabular-nums text-muted-foreground">
{formatCurrency(Number(item.value), budget.currency, locale)}
</p>
</div>
)
}}
/>
```
### Custom Tooltip for AvailableBalance (Donut Chart)
```tsx
// Same pattern — AvailableBalance also needs locale and budget.currency props added
<Tooltip
content={({ active, payload }) => {
if (!active || !payload?.length) return null
const item = payload[0]
return (
<div className="rounded-lg border border-border/50 bg-background px-2.5 py-1.5 text-xs shadow-xl">
<p className="font-medium text-foreground">{item.name}</p>
<p className="font-mono tabular-nums text-muted-foreground">
{formatCurrency(Number(item.value), budget.currency, locale)}
</p>
</div>
)
}}
/>
```
### 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
<ExpenseBreakdown budget={current} locale={userLocale} />
<AvailableBalance budget={current} locale={userLocale} />
```
## 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 `<Tooltip />` 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 — `<Tooltip content={renderer}>` 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)