# Domain Pitfalls: UI Polish for SimpleFinanceDash

**Domain:** React + shadcn/ui frontend overhaul — pastel design system on an existing functional app
**Researched:** 2026-03-11
**Confidence:** HIGH (grounded in direct codebase inspection)

---

## Critical Pitfalls

Mistakes that cause rewrites, visual regressions across the whole app, or consistency breakdown.

---

### Pitfall 1: Hardcoded Color Values Bypassing the Token System

**What goes wrong:** The codebase already mixes two color approaches. Some components use shadcn CSS variable tokens (`bg-muted`, `text-destructive`, `hover:bg-muted`) while others hardcode Tailwind palette values (`bg-emerald-50`, `bg-violet-100 text-violet-800`, `fill="#fcd34d"`, `#93c5fd`, `#f9a8d4`). If the pastel palette is defined purely as CSS variable tokens in `index.css` but chart fills and badge color classes remain as inline hex strings and raw Tailwind utilities, changing the theme or adjusting a single color requires hunting down every call site.

**Why it happens:** shadcn components use CSS variables natively. But Recharts does not — `fill` props require string hex or named colors. Developers add those inline and then replicate the same approach in badge class strings, gradient headers, and row highlights.

**Consequences:**
- A "pastel blue" may be `#93c5fd` in one chart, `blue-300` in a gradient, and `--chart-1` in another chart. They look similar but are not the same.
- Adjusting saturation for accessibility requires touching 10+ places instead of 1.
- Dark mode (even if not this milestone) is impossible to add cleanly later.

**Detection warning signs:**
- `grep` finds hex color strings (`#`) in component `.tsx` files
- Tailwind color classes like `bg-pink-50`, `text-emerald-800` in component files alongside `bg-card`, `text-muted-foreground` in the same codebase
- `PASTEL_COLORS` arrays defined per-component (`AvailableBalance.tsx` and `ExpenseBreakdown.tsx` each define their own separate array with partially overlapping values)

**Prevention:**
1. Define the full pastel palette as CSS variables in `index.css` — one variable per semantic purpose: `--color-income`, `--color-bills`, `--color-expenses`, etc.
2. Map those variables into Tailwind's `@theme inline` block so `bg-income`, `text-bills` etc. work as utility classes.
3. For Recharts fills, create a single exported constant array that references the CSS variable resolved values (use `getComputedStyle` or a constant palette object that is the single source of truth).
4. Replace per-component `PASTEL_COLORS` arrays with imports from a central `lib/palette.ts`.

**Phase:** Must be addressed in Phase 1 (design token foundation) before touching any components. Fixing it mid-polish causes re-work.

---

### Pitfall 2: Overriding shadcn Components by Editing Their Source Files

**What goes wrong:** shadcn/ui is not a dependency — the components live in `src/components/ui/`. When developers want to change the default appearance of `Button`, `Card`, `Input`, etc., the temptation is to edit those source files directly (add default className values, change variant definitions, etc.).

**Why it happens:** It feels like the "right" place — the code is right there, you own it, and `className` overrides in every call site feel verbose.

**Consequences:**
- If `shadcn add` is run later to add a new component or update an existing one, it overwrites your customizations or creates merge conflicts.
- The diff between the upstream shadcn component and your version becomes invisible — future developers don't know what was intentionally changed vs. what is default shadcn behavior.
- You lose the ability to reference shadcn docs accurately.

**Detection warning signs:**
- Any `className` default changed in `src/components/ui/*.tsx` files
- Variant maps in `button.tsx`, `badge.tsx`, `card.tsx` expanded beyond the shadcn defaults

**Prevention:**
- Customize appearance exclusively through CSS variables in `index.css`. shadcn components read `--primary`, `--card`, `--border`, `--radius` etc. — those are the intended extension points.
- For structural changes (adding a new button variant, a custom card subcomponent), create wrapper components like `src/components/ui/pastel-card.tsx` that compose shadcn primitives rather than modifying them.
- Treat `src/components/ui/` as vendor code. Only modify it if the change would be appropriate to upstream.

**Phase:** Establish this rule before any component work begins. A short note in `CLAUDE.md` prevents accidental violations.

---

### Pitfall 3: Duplicated InlineEditRow Logic Diverging During Polish

**What goes wrong:** `BillsTracker.tsx`, `VariableExpenses.tsx`, and `DebtTracker.tsx` all contain a local `InlineEditRow` function with identical logic and structure. The visual polish work will need to change the edit affordance (hover state, active state, input styling, focus ring) in all three. If they remain separate, each will be polished independently and end up slightly different.

**Why it happens:** The component was created inline as a local helper and never extracted. Since each file is self-contained it doesn't feel wrong — until the polish pass.

**Consequences:**
- Three components get slightly different hover colors, different input widths, different transition timings.
- A bug fix or interaction change (e.g., adding an ESC key handler or optimistic update) must be applied three times.
- Reviewers approve them one at a time and miss inconsistencies.

**Detection warning signs:**
- `grep -r "InlineEditRow"` returns multiple files
- Visual comparison of Bills, Variable Expenses, and Debt tables reveals small inconsistencies in edit behavior

**Prevention:**
- Extract `InlineEditRow` to `src/components/InlineEditCell.tsx` before the polish phase begins.
- Unify props interface; handle all three use cases (bills, variable expenses, debts are all "click to edit actual amount").
- Polish the one component once.

**Phase:** Phase 1 or early Phase 2, before any visual work on those table components.

---

### Pitfall 4: Chart Colors Not Connected to the Semantic Color System

**What goes wrong:** Recharts charts (`PieChart` in `AvailableBalance`, `PieChart` in `ExpenseBreakdown`, `BarChart` in `VariableExpenses`) all use hardcoded hex palettes. The donut chart in `AvailableBalance` slices map to budget categories (bills, expenses, debts, savings, investments). If the color for "debts" in the donut is different from the color for "Debts" row in the `FinancialOverview` table, the user sees two different colors for the same concept on the same screen.

**Why it happens:** The table rows use Tailwind classes (`bg-red-50`) while the chart uses an index-based array (`PASTEL_COLORS[index]`). There is no mapping from category type to a single canonical color.

**Consequences:**
- Cognitive load: user cannot use color as a navigation cue across widgets.
- "Bills" is blue in the overview table header gradient, blue-300 in one chart, and whatever position it lands at in the pie.
- The donut in `AvailableBalance` has no legend — users must already be confused about which slice is which.

**Detection warning signs:**
- Side-by-side view of `AvailableBalance` donut and `FinancialOverview` table with different colors for the same categories
- `PASTEL_COLORS` arrays that do not reference a category-to-color map

**Prevention:**
- Define a `CATEGORY_COLORS` map in `lib/palette.ts`: `{ income: '#...', bill: '#...', variable_expense: '#...', debt: '#...', saving: '#...', investment: '#...' }`
- All chart `fill` props, all badge class strings, all row background classes derive from this map.
- Do not use index-based color arrays for category data — always key by category type.

**Phase:** Phase 1 (design token foundation). Required before polishing any chart component.

---

### Pitfall 5: Polish Feels "Off" Because Layout Spacing Is Inconsistent, Not Just Colors

**What goes wrong:** A common mistake in UI polish projects is focusing on colors and typography while leaving spacing inconsistent. Looking at the current layout: `DashboardPage` uses `p-6` and `gap-6`. `CardContent` sometimes uses `p-0` (for tables), sometimes `pt-4`, sometimes `pt-6`. `CardHeader` has gradient backgrounds but no consistent padding treatment relative to card content. Auth forms (`LoginPage`) have no background treatment — just `bg-background` which is white.

**Why it happens:** Spacing decisions were made locally per component during initial development. The Tailwind utility model makes it easy to add `p-4` or `p-6` without thinking about global rhythm.

**Consequences:**
- Even if colors are right, the layout feels amateur because card headers have different internal padding, section gaps vary, and the auth page has no visual brand presence.
- Dashboard looks like a collection of separate components rather than a unified screen.

**Detection warning signs:**
- `CardHeader` padding varies across components
- `CardContent` uses `p-0` in some cards and `pt-4`/`pt-6` in others without a clear rule
- Auth pages have no background color or brand element beyond the card itself
- Sidebar lacks any visual weight or brand mark beyond plain text

**Prevention:**
- Define a spacing scale decision early: what is the standard gap between dashboard sections? Between card header and content? Between form fields?
- Encode those decisions in either Tailwind config (spacing tokens) or a layout component.
- Treat the auth pages as a first-class design surface — they are the first screen users see.

**Phase:** Phase 2 (layout and structure), before polishing individual components.

---

## Moderate Pitfalls

---

### Pitfall 6: i18n Keys Missing for New UI Text Introduced During Polish

**What goes wrong:** The polish phase will add new UI elements — empty states, tooltips, chart legends, aria labels, section descriptions, helper text. Each piece of text needs both `en.json` and `de.json` entries. During fast iteration it is easy to hardcode English strings directly in JSX.

**Why it happens:** Adding `t('some.new.key')` requires updating two JSON files. Under time pressure developers skip it and plan to "add translations later."

**Consequences:**
- German users see raw English strings or key names like `dashboard.expenseTooltip`
- Text added without translation keys becomes invisible debt — only discovered when someone switches the language

**Detection warning signs:**
- String literals in JSX that are not wrapped in `t()`: `<p>No transactions yet</p>`
- `t()` calls referencing keys that exist in `en.json` but not `de.json`

**Prevention:**
- As a discipline: never commit a new UI string without both translation files updated.
- At the start of the milestone, run a diff between `en.json` and `de.json` key sets to verify they're in sync.
- For every new UI element added during polish, add the translation key immediately, even if the German translation is a placeholder.

**Phase:** Ongoing across all phases. Establish the discipline at the start.

---

### Pitfall 7: Recharts Tooltip and Legend Styling Not Customized — Default Gray Box Breaks Pastel Aesthetic

**What goes wrong:** Recharts renders its `<Tooltip>` as a white box with gray border by default. Its `<Legend>` uses its own internal color swatches. Neither reads from CSS variables. The `VariableExpenses` chart has `<Tooltip />` and `<Legend />` without any custom styling. The `ExpenseBreakdown` pie chart uses inline label rendering with `{name} {percent}%` which overflows on small slices and has no connection to the design system.

**Why it happens:** Recharts customization requires passing `content` prop with a custom component, which feels like extra work when the default "works."

**Consequences:**
- Charts look designed but tooltips and legends look like placeholder UI — this is one of the most noticeable quality signals in data dashboards.
- Pie chart labels on small slices (`ExpenseBreakdown`) collide and become unreadable.

**Prevention:**
- Create a shared `ChartTooltip` component that uses the design system's card/border/text tokens.
- For pie charts, remove inline labels and use a separate legend or tooltip instead.
- The `shadcn/ui` chart component (`src/components/ui/chart.tsx`) wraps Recharts and provides `ChartTooltipContent` — check if it covers the use case before building custom.

**Phase:** Phase 3 (chart polish), but the decision to use `chart.tsx` vs. raw Recharts should be made in Phase 1.

---

### Pitfall 8: The "No Data" State Is Not Designed

**What goes wrong:** Several components conditionally return `null` when data is absent (`ExpenseBreakdown` returns `null` if no expenses, `DebtTracker` returns `null` if no debts). This means sections of the dashboard silently disappear rather than showing a welcoming empty state. On first use (no budget created yet), the whole dashboard shows only a budget selector and a card saying "No budgets yet."

**Why it happens:** Empty states are not considered until user testing — during initial development, returning `null` is the simplest valid behavior.

**Consequences:**
- First-run experience feels broken — user sees a mostly-empty screen and doesn't know what to do.
- A polished product feels abandoned when sections disappear instead of explaining why.

**Prevention:**
- Design empty states for each major section during the polish phase.
- At minimum: an icon, a short explanation, and a call-to-action where appropriate.
- Consider whether conditional `null` returns are the right pattern or whether a section should always be present with an empty state.

**Phase:** Phase 2 or Phase 3. Must be addressed before considering any screen "polished."

---

### Pitfall 9: Delete Operations Have No Confirmation — Silent Destructive Actions

**What goes wrong:** In `CategoriesPage`, the Delete button calls `handleDelete(cat.id)` directly with no confirmation dialog. Categories are reused across budget periods — deleting one could orphan or break historical budget data.

**Why it happens:** Confirmation dialogs feel like UI polish work but are actually a correctness concern. Easy to defer.

**Consequences:**
- User accidentally deletes a category they use in 12 months of budgets.
- The backend may or may not cascade delete budget items — either way, the user has no warning.

**Prevention:**
- Add a confirmation step to all delete actions during the polish phase.
- The `Dialog` component is already in use in `CategoriesPage` — the pattern is available.
- Consider showing the impact ("This category is used in 3 budgets") as part of the confirmation.

**Phase:** Phase 3 (page polish), but should be flagged as a correctness concern, not just cosmetic.

---

## Minor Pitfalls

---

### Pitfall 10: `formatCurrency` Hardcodes `de-DE` Locale Regardless of User Language Preference

**What goes wrong:** `lib/format.ts` uses `new Intl.NumberFormat('de-DE', ...)`. Users with the English locale preference will still see `1.234,56 €` (German decimal notation) instead of `1,234.56 €`.

**Detection:** Switch to English in settings, observe currency formatting in the dashboard.

**Prevention:** Pass the user's locale to `formatCurrency` or read it from the i18n context. The user's `preferred_locale` is available from the settings API.

**Phase:** Can be addressed during any phase where currency display is touched, but should be caught before final review.

---

### Pitfall 11: The Sidebar Has No Visual Hierarchy or Brand Identity

**What goes wrong:** `AppLayout.tsx` renders the sidebar with a plain text `<h2>Budget Dashboard</h2>` and no logo, icon, or brand color. The active nav item uses shadcn's default `isActive` styling which is just a background highlight. In the pastel design vision, the sidebar is a primary brand surface.

**Prevention:** Treat the sidebar header as a branding opportunity — a simple icon or wordmark with the pastel primary color makes the application feel intentional. Active nav items should use a pastel accent, not the default dark highlight.

**Phase:** Phase 2 (layout).

---

### Pitfall 12: Budget Selector in the Dashboard Header Has No Visual Context

**What goes wrong:** The budget selector (`Select` component in `DashboardPage`) floats in the top-left with just a "Create Budget" button beside it. There is no visual framing — no page header, no date context, no quick-stat summary near the selector. Users can't tell at a glance which month they are viewing.

**Prevention:** The budget has a name (which presumably includes the month). Displaying it prominently with supporting context (date range, currency) as part of a proper page header would substantially improve orientation. This is a layout decision, not a data model change.

**Phase:** Phase 2 (layout).

---

## Phase-Specific Warnings

| Phase Topic | Likely Pitfall | Mitigation |
|-------------|----------------|------------|
| Design token foundation | Defining CSS variables but not connecting them to Recharts fills (Pitfall 1, 4) | Create `lib/palette.ts` as the single source of truth for all colors including chart fills |
| Component extraction | Leaving InlineEditRow duplicated (Pitfall 3) | Extract before any polish touches the table components |
| Auth page polish | Treating it as "just style the card" vs. a brand surface (Pitfall 5) | Design the full viewport, not just the card |
| Chart polish | Default Recharts tooltip/legend styling (Pitfall 7) | Decide early whether to use `chart.tsx` wrapper or custom tooltip component |
| Dashboard layout | Relying on component visibility returning `null` (Pitfall 8) | Design empty states for every conditional section |
| Categories page polish | No delete confirmation (Pitfall 9) | Treat confirmation as a requirement, not a stretch |
| Any new UI text | Missing i18n translations (Pitfall 6) | Two-JSON rule: never commit text without both `en.json` and `de.json` |
| Currency display | Hardcoded `de-DE` locale in formatCurrency (Pitfall 10) | Pass user locale when touching any numeric display |

---

## Sources

- Direct inspection of `frontend/src/` codebase (HIGH confidence — grounded in actual code)
- Observed patterns: `index.css` CSS variable definitions vs. inline hex in component files
- Observed duplication: `InlineEditRow` in `BillsTracker.tsx`, `VariableExpenses.tsx`, `DebtTracker.tsx`
- Observed color divergence: `PASTEL_COLORS` arrays in `AvailableBalance.tsx` and `ExpenseBreakdown.tsx` are different arrays
- shadcn/ui design: components in `src/components/ui/` read from CSS variables — `--primary`, `--card`, `--border`, `--muted`, etc.
- Recharts constraint: `fill`, `stroke` props accept strings only, no CSS variable resolution natively