fire-planner: UX review pass 1 — fix sidebar/route/PATCH/badges issues
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
Round-1 fixes from the headless UI review:
Backend
- scenarios PATCH now allows config_json/name/description on cartesian
scenarios (so users can pin flex_rules + notes that recompute will
preserve). Core fields (jurisdiction/strategy/etc.) still blocked
because they're rebuilt on recompute. Existing test updated.
Frontend
- Sidebar Plans switcher: drop the kind=user filter so the switcher
surfaces all 120 cartesian scenarios that ship out of the box.
- Settings → Milestones now reachable at both /settings (index) and
/settings/milestones (explicit) — the agent navigated to the latter
and got a blank page.
- EventGantt background click capture: explicit pointerEvents="all" +
fillOpacity=0 so click-to-add reliably fires on empty regions
between bars.
- Plan tab stat badges moved out of the chart card into a dedicated
row above the fan — previously they overlapped the chart's title,
legend caption ("p10/p50/p..."), and right-side withdrawal axis.
- Stub tabs (Tax Analytics / Compare / Reports / Estate) and stub
Settings sub-pages (Dividends / Bonds / Tax / Metrics / Other) get
a "soon" badge + slate-300 styling so they're clearly placeholders.
- New "Portfolio depleted at this year" pill renders in the badge
row when the scrubbed year's NW is 0 — previously the badges
silently went to £0 with no UI cue.
- Test life-event from the smoke run cleaned up from prod DB.
246 pytest pass; mypy/ruff clean; frontend typecheck/test/build green.
This commit is contained in:
parent
2f95c891fa
commit
cd1fc37f25
10 changed files with 133 additions and 56 deletions
|
|
@ -196,10 +196,22 @@ async def patch_scenario(
|
||||||
scen = await session.get(Scenario, scenario_id)
|
scen = await session.get(Scenario, scenario_id)
|
||||||
if scen is None:
|
if scen is None:
|
||||||
raise HTTPException(status_code=404, detail="Scenario not found")
|
raise HTTPException(status_code=404, detail="Scenario not found")
|
||||||
if scen.kind != "user":
|
|
||||||
raise HTTPException(status_code=400,
|
|
||||||
detail="Cannot patch cartesian scenarios — they're auto-generated")
|
|
||||||
updates = payload.model_dump(exclude_unset=True)
|
updates = payload.model_dump(exclude_unset=True)
|
||||||
|
if scen.kind != "user":
|
||||||
|
# Cartesian scenarios are rebuilt on every recompute — most core
|
||||||
|
# fields would be wiped by the next run, so we only allow updates
|
||||||
|
# to free-form metadata that we want to preserve across recomputes
|
||||||
|
# (notes, flex_rules, rate overrides). Hard-block edits to the
|
||||||
|
# parameters that define the scenario shape.
|
||||||
|
allowed_for_cartesian = {"config_json", "name", "description"}
|
||||||
|
bad = set(updates) - allowed_for_cartesian
|
||||||
|
if bad:
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=400,
|
||||||
|
detail=("Cannot patch cartesian scenario fields {sorted(bad)} — "
|
||||||
|
"they're auto-generated. Only config_json/name/description "
|
||||||
|
"may be updated."),
|
||||||
|
)
|
||||||
for k, v in updates.items():
|
for k, v in updates.items():
|
||||||
setattr(scen, k, v)
|
setattr(scen, k, v)
|
||||||
await session.commit()
|
await session.commit()
|
||||||
|
|
|
||||||
|
|
@ -75,6 +75,7 @@ export function App() {
|
||||||
/>
|
/>
|
||||||
<Route path="settings" element={<SettingsTab />}>
|
<Route path="settings" element={<SettingsTab />}>
|
||||||
<Route index element={<MilestonesSettings />} />
|
<Route index element={<MilestonesSettings />} />
|
||||||
|
<Route path="milestones" element={<MilestonesSettings />} />
|
||||||
<Route path="rates" element={<RatesSettings />} />
|
<Route path="rates" element={<RatesSettings />} />
|
||||||
<Route
|
<Route
|
||||||
path="dividends"
|
path="dividends"
|
||||||
|
|
|
||||||
|
|
@ -248,13 +248,17 @@ function Inner({
|
||||||
/>
|
/>
|
||||||
);
|
);
|
||||||
})}
|
})}
|
||||||
{/* Background click capture (must be drawn before bars) */}
|
{/* Background click capture — drawn before bars so they win
|
||||||
|
z-order, but `pointerEvents=all` ensures the empty regions
|
||||||
|
between bars still bubble clicks here. */}
|
||||||
<rect
|
<rect
|
||||||
x={0}
|
x={0}
|
||||||
y={0}
|
y={0}
|
||||||
width={innerW}
|
width={innerW}
|
||||||
height={innerH}
|
height={innerH}
|
||||||
fill="transparent"
|
fill="white"
|
||||||
|
fillOpacity={0}
|
||||||
|
pointerEvents="all"
|
||||||
onClick={onBackgroundClick}
|
onClick={onBackgroundClick}
|
||||||
style={{ cursor: 'crosshair' }}
|
style={{ cursor: 'crosshair' }}
|
||||||
/>
|
/>
|
||||||
|
|
|
||||||
|
|
@ -7,6 +7,7 @@ interface Item {
|
||||||
to: string;
|
to: string;
|
||||||
label: string;
|
label: string;
|
||||||
end?: boolean;
|
end?: boolean;
|
||||||
|
stub?: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
export function SettingsSubnav({ items }: { items: Item[] }) {
|
export function SettingsSubnav({ items }: { items: Item[] }) {
|
||||||
|
|
@ -23,11 +24,15 @@ export function SettingsSubnav({ items }: { items: Item[] }) {
|
||||||
'block rounded-md px-3 py-1.5 text-sm',
|
'block rounded-md px-3 py-1.5 text-sm',
|
||||||
isActive
|
isActive
|
||||||
? 'bg-slate-100 text-slate-900 font-medium'
|
? 'bg-slate-100 text-slate-900 font-medium'
|
||||||
: 'text-slate-600 hover:text-slate-900 hover:bg-slate-50',
|
: it.stub
|
||||||
|
? 'text-slate-300 hover:text-slate-500'
|
||||||
|
: 'text-slate-600 hover:text-slate-900 hover:bg-slate-50',
|
||||||
].join(' ')
|
].join(' ')
|
||||||
}
|
}
|
||||||
|
title={it.stub ? 'Coming soon — not yet wired' : undefined}
|
||||||
>
|
>
|
||||||
{it.label}
|
{it.label}
|
||||||
|
{it.stub && <span className="ml-1 text-[10px] uppercase">soon</span>}
|
||||||
</NavLink>
|
</NavLink>
|
||||||
</li>
|
</li>
|
||||||
))}
|
))}
|
||||||
|
|
|
||||||
|
|
@ -78,9 +78,12 @@ function SidebarLink({
|
||||||
|
|
||||||
function PlansSwitcher({ activeScenarioId }: { activeScenarioId?: number }) {
|
function PlansSwitcher({ activeScenarioId }: { activeScenarioId?: number }) {
|
||||||
const params = useParams();
|
const params = useParams();
|
||||||
|
// Show every scenario in the switcher — the prod DB is dominated by
|
||||||
|
// cartesian rows (120 by default) and a "user-only" filter would
|
||||||
|
// surface an empty list out of the box.
|
||||||
const scenarios = useQuery({
|
const scenarios = useQuery({
|
||||||
queryKey: ['scenarios', 'list', 'user'],
|
queryKey: ['scenarios', 'list', 'all'],
|
||||||
queryFn: () => api.scenarios.list('user'),
|
queryFn: () => api.scenarios.list(),
|
||||||
});
|
});
|
||||||
const active = activeScenarioId ?? Number(params.id);
|
const active = activeScenarioId ?? Number(params.id);
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -9,6 +9,9 @@ export interface TabSpec {
|
||||||
to: string;
|
to: string;
|
||||||
label: string;
|
label: string;
|
||||||
end?: boolean;
|
end?: boolean;
|
||||||
|
/** Mark stub/placeholder tabs so they're rendered de-emphasized — the
|
||||||
|
* user shouldn't mistake them for ready features. */
|
||||||
|
stub?: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
export function TabBar({ tabs }: { tabs: TabSpec[] }) {
|
export function TabBar({ tabs }: { tabs: TabSpec[] }) {
|
||||||
|
|
@ -24,11 +27,15 @@ export function TabBar({ tabs }: { tabs: TabSpec[] }) {
|
||||||
'py-3 px-1 -mb-px border-b-2 whitespace-nowrap',
|
'py-3 px-1 -mb-px border-b-2 whitespace-nowrap',
|
||||||
isActive
|
isActive
|
||||||
? 'border-slate-900 text-slate-900 font-medium'
|
? 'border-slate-900 text-slate-900 font-medium'
|
||||||
: 'border-transparent text-slate-500 hover:text-slate-800',
|
: t.stub
|
||||||
|
? 'border-transparent text-slate-300 hover:text-slate-500'
|
||||||
|
: 'border-transparent text-slate-500 hover:text-slate-800',
|
||||||
].join(' ')
|
].join(' ')
|
||||||
}
|
}
|
||||||
|
title={t.stub ? 'Coming soon — not yet wired' : undefined}
|
||||||
>
|
>
|
||||||
{t.label}
|
{t.label}
|
||||||
|
{t.stub && <span className="ml-1 text-[10px] uppercase">soon</span>}
|
||||||
</NavLink>
|
</NavLink>
|
||||||
))}
|
))}
|
||||||
</nav>
|
</nav>
|
||||||
|
|
|
||||||
|
|
@ -219,8 +219,22 @@ export function ScenarioDetail() {
|
||||||
|
|
||||||
{projection ? (
|
{projection ? (
|
||||||
<>
|
<>
|
||||||
{/* NW fan with floating stat badges */}
|
{/* Stats badges row — sits above the chart, not on top of it */}
|
||||||
<div className="relative rounded-lg border border-slate-200 bg-white p-5">
|
<StatsBadges
|
||||||
|
year={year}
|
||||||
|
netWorth={yearStats.data?.net_worth_p50}
|
||||||
|
changeNw={yearStats.data?.change_in_nw}
|
||||||
|
spending={yearStats.data?.spending}
|
||||||
|
taxes={yearStats.data?.taxes}
|
||||||
|
effectiveRate={yearStats.data?.effective_tax_rate}
|
||||||
|
age={yearStats.data?.age ?? null}
|
||||||
|
calendarYear={yearStats.data?.calendar_year}
|
||||||
|
successRate={projection.success_rate}
|
||||||
|
ruined={isRuined(yearStats.data?.net_worth_p50)}
|
||||||
|
/>
|
||||||
|
|
||||||
|
{/* NW fan + scrubber */}
|
||||||
|
<div className="rounded-lg border border-slate-200 bg-white p-5">
|
||||||
<div className="flex items-baseline justify-between mb-2">
|
<div className="flex items-baseline justify-between mb-2">
|
||||||
<h2 className="text-lg font-semibold">Portfolio fan</h2>
|
<h2 className="text-lg font-semibold">Portfolio fan</h2>
|
||||||
<span className="text-xs text-slate-500">
|
<span className="text-xs text-slate-500">
|
||||||
|
|
@ -229,7 +243,7 @@ export function ScenarioDetail() {
|
||||||
</div>
|
</div>
|
||||||
<FanChart
|
<FanChart
|
||||||
yearly={projection.yearly}
|
yearly={projection.yearly}
|
||||||
height={460}
|
height={420}
|
||||||
showWithdrawal
|
showWithdrawal
|
||||||
milestones={milestones}
|
milestones={milestones}
|
||||||
selectedYear={year}
|
selectedYear={year}
|
||||||
|
|
@ -238,19 +252,6 @@ export function ScenarioDetail() {
|
||||||
<div className="mt-3">
|
<div className="mt-3">
|
||||||
<YearScrubber value={year} min={0} max={maxYear} onChange={setYearAndUrl} />
|
<YearScrubber value={year} min={0} max={maxYear} onChange={setYearAndUrl} />
|
||||||
</div>
|
</div>
|
||||||
<FloatingStats
|
|
||||||
year={year}
|
|
||||||
maxYear={maxYear}
|
|
||||||
successRate={projection.success_rate}
|
|
||||||
p50End={projection.p50_ending_gbp}
|
|
||||||
netWorth={yearStats.data?.net_worth_p50}
|
|
||||||
changeNw={yearStats.data?.change_in_nw}
|
|
||||||
spending={yearStats.data?.spending}
|
|
||||||
taxes={yearStats.data?.taxes}
|
|
||||||
effectiveRate={yearStats.data?.effective_tax_rate}
|
|
||||||
age={yearStats.data?.age ?? null}
|
|
||||||
calendarYear={yearStats.data?.calendar_year}
|
|
||||||
/>
|
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
{/* Spending profile */}
|
{/* Spending profile */}
|
||||||
|
|
@ -336,11 +337,9 @@ export function ScenarioDetail() {
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
function FloatingStats(props: {
|
function StatsBadges(props: {
|
||||||
year: number;
|
year: number;
|
||||||
maxYear: number;
|
|
||||||
successRate: string;
|
successRate: string;
|
||||||
p50End: string;
|
|
||||||
netWorth?: string;
|
netWorth?: string;
|
||||||
changeNw?: string;
|
changeNw?: string;
|
||||||
spending?: string;
|
spending?: string;
|
||||||
|
|
@ -348,19 +347,36 @@ function FloatingStats(props: {
|
||||||
effectiveRate?: string;
|
effectiveRate?: string;
|
||||||
age: number | null;
|
age: number | null;
|
||||||
calendarYear?: number;
|
calendarYear?: number;
|
||||||
|
ruined: boolean;
|
||||||
}) {
|
}) {
|
||||||
return (
|
return (
|
||||||
<div className="absolute top-3 right-3 grid grid-cols-2 gap-2 max-w-xs pointer-events-none">
|
<div className="rounded-lg border border-slate-200 bg-white p-3">
|
||||||
<Badge label="Year" value={String(props.calendarYear ?? `y${props.year}`)} />
|
<div className="flex items-center gap-2 flex-wrap">
|
||||||
<Badge label="Age" value={props.age != null ? String(props.age) : '—'} />
|
<Badge label="Year" value={String(props.calendarYear ?? `y${props.year}`)} sub={`y${props.year}`} />
|
||||||
<Badge label="Net Worth" value={props.netWorth ? gbp(props.netWorth) : '—'} accent />
|
<Badge label="Age" value={props.age != null ? String(props.age) : '—'} />
|
||||||
<Badge
|
<Badge
|
||||||
label="Δ NW"
|
label="Net Worth"
|
||||||
value={props.changeNw ? gbp(props.changeNw) : '—'}
|
value={props.netWorth ? gbp(props.netWorth) : '—'}
|
||||||
signed={props.changeNw}
|
accent
|
||||||
/>
|
/>
|
||||||
<Badge label="Spending" value={props.spending ? gbp(props.spending) : '—'} />
|
<Badge
|
||||||
<Badge label="Eff. tax" value={props.effectiveRate ? pct(props.effectiveRate) : '—'} />
|
label="Δ NW"
|
||||||
|
value={props.changeNw ? gbp(props.changeNw) : '—'}
|
||||||
|
signed={props.changeNw}
|
||||||
|
/>
|
||||||
|
<Badge label="Spending" value={props.spending ? gbp(props.spending) : '—'} />
|
||||||
|
<Badge label="Taxes" value={props.taxes ? gbp(props.taxes) : '—'} />
|
||||||
|
<Badge
|
||||||
|
label="Eff. tax"
|
||||||
|
value={props.effectiveRate ? pct(props.effectiveRate) : '—'}
|
||||||
|
/>
|
||||||
|
<Badge label="Success" value={pct(props.successRate)} accent />
|
||||||
|
{props.ruined && (
|
||||||
|
<span className="ml-auto rounded-md bg-amber-100 text-amber-800 px-2 py-1 text-xs font-medium">
|
||||||
|
Portfolio depleted at this year
|
||||||
|
</span>
|
||||||
|
)}
|
||||||
|
</div>
|
||||||
</div>
|
</div>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
@ -368,15 +384,17 @@ function FloatingStats(props: {
|
||||||
function Badge({
|
function Badge({
|
||||||
label,
|
label,
|
||||||
value,
|
value,
|
||||||
|
sub,
|
||||||
accent,
|
accent,
|
||||||
signed,
|
signed,
|
||||||
}: {
|
}: {
|
||||||
label: string;
|
label: string;
|
||||||
value: string;
|
value: string;
|
||||||
|
sub?: string;
|
||||||
accent?: boolean;
|
accent?: boolean;
|
||||||
signed?: string;
|
signed?: string;
|
||||||
}) {
|
}) {
|
||||||
let cls = 'text-slate-700';
|
let cls = 'text-slate-800';
|
||||||
if (accent) cls = 'text-emerald-700';
|
if (accent) cls = 'text-emerald-700';
|
||||||
if (signed != null) {
|
if (signed != null) {
|
||||||
const n = Number(signed);
|
const n = Number(signed);
|
||||||
|
|
@ -384,13 +402,21 @@ function Badge({
|
||||||
else if (n < 0) cls = 'text-red-600';
|
else if (n < 0) cls = 'text-red-600';
|
||||||
}
|
}
|
||||||
return (
|
return (
|
||||||
<div className="rounded-md border border-slate-200 bg-white/90 backdrop-blur px-2 py-1 shadow-sm pointer-events-auto">
|
<div className="rounded-md border border-slate-200 px-2.5 py-1">
|
||||||
<div className="text-[9px] uppercase tracking-wide text-slate-500">{label}</div>
|
<div className="text-[9px] uppercase tracking-wide text-slate-500 leading-none">{label}</div>
|
||||||
<div className={`text-xs font-semibold tabular-nums ${cls}`}>{value}</div>
|
<div className={`text-sm font-semibold tabular-nums leading-tight mt-0.5 ${cls}`}>
|
||||||
|
{value}
|
||||||
|
{sub && <span className="ml-1 text-[10px] font-normal text-slate-400">{sub}</span>}
|
||||||
|
</div>
|
||||||
</div>
|
</div>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function isRuined(networth?: string): boolean {
|
||||||
|
if (!networth) return false;
|
||||||
|
return Number(networth) <= 0;
|
||||||
|
}
|
||||||
|
|
||||||
function Legend() {
|
function Legend() {
|
||||||
return (
|
return (
|
||||||
<div className="flex items-center gap-3 text-xs">
|
<div className="flex items-center gap-3 text-xs">
|
||||||
|
|
|
||||||
|
|
@ -15,10 +15,10 @@ export function ScenarioShell() {
|
||||||
const tabs: TabSpec[] = [
|
const tabs: TabSpec[] = [
|
||||||
{ to: `/scenarios/${id}`, label: 'Plan', end: true },
|
{ to: `/scenarios/${id}`, label: 'Plan', end: true },
|
||||||
{ to: `/scenarios/${id}/cash-flow`, label: 'Cash Flow' },
|
{ to: `/scenarios/${id}/cash-flow`, label: 'Cash Flow' },
|
||||||
{ to: `/scenarios/${id}/tax-analytics`, label: 'Tax Analytics' },
|
{ to: `/scenarios/${id}/tax-analytics`, label: 'Tax Analytics', stub: true },
|
||||||
{ to: `/scenarios/${id}/compare`, label: 'Compare' },
|
{ to: `/scenarios/${id}/compare`, label: 'Compare', stub: true },
|
||||||
{ to: `/scenarios/${id}/reports`, label: 'Reports' },
|
{ to: `/scenarios/${id}/reports`, label: 'Reports', stub: true },
|
||||||
{ to: `/scenarios/${id}/estate`, label: 'Estate' },
|
{ to: `/scenarios/${id}/estate`, label: 'Estate', stub: true },
|
||||||
{ to: `/scenarios/${id}/settings`, label: 'Settings' },
|
{ to: `/scenarios/${id}/settings`, label: 'Settings' },
|
||||||
];
|
];
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -11,12 +11,12 @@ export function SettingsTab() {
|
||||||
const items = [
|
const items = [
|
||||||
{ to: `/scenarios/${id}/settings`, label: 'Milestones', end: true },
|
{ to: `/scenarios/${id}/settings`, label: 'Milestones', end: true },
|
||||||
{ to: `/scenarios/${id}/settings/rates`, label: 'Rates' },
|
{ to: `/scenarios/${id}/settings/rates`, label: 'Rates' },
|
||||||
{ to: `/scenarios/${id}/settings/dividends`, label: 'Dividends' },
|
|
||||||
{ to: `/scenarios/${id}/settings/bonds`, label: 'Bonds' },
|
|
||||||
{ to: `/scenarios/${id}/settings/tax`, label: 'Tax' },
|
|
||||||
{ to: `/scenarios/${id}/settings/metrics`, label: 'Metrics' },
|
|
||||||
{ to: `/scenarios/${id}/settings/other`, label: 'Other Settings' },
|
|
||||||
{ to: `/scenarios/${id}/settings/notes`, label: 'Notes' },
|
{ to: `/scenarios/${id}/settings/notes`, label: 'Notes' },
|
||||||
|
{ to: `/scenarios/${id}/settings/dividends`, label: 'Dividends', stub: true },
|
||||||
|
{ to: `/scenarios/${id}/settings/bonds`, label: 'Bonds', stub: true },
|
||||||
|
{ to: `/scenarios/${id}/settings/tax`, label: 'Tax', stub: true },
|
||||||
|
{ to: `/scenarios/${id}/settings/metrics`, label: 'Metrics', stub: true },
|
||||||
|
{ to: `/scenarios/${id}/settings/other`, label: 'Other', stub: true },
|
||||||
];
|
];
|
||||||
|
|
||||||
return (
|
return (
|
||||||
|
|
|
||||||
|
|
@ -153,11 +153,30 @@ async def test_patch_user_scenario(client: AsyncClient) -> None:
|
||||||
assert body["leave_uk_year"] == 2
|
assert body["leave_uk_year"] == 2
|
||||||
|
|
||||||
|
|
||||||
async def test_patch_cartesian_blocked(client: AsyncClient, session: AsyncSession) -> None:
|
async def test_patch_cartesian_core_fields_blocked(
|
||||||
|
client: AsyncClient, session: AsyncSession,
|
||||||
|
) -> None:
|
||||||
|
"""Cartesian scenarios reject edits to fields that get rebuilt by
|
||||||
|
recompute (jurisdiction/strategy/etc), but allow free-form metadata
|
||||||
|
(config_json/name/description) so users can pin notes + flex_rules."""
|
||||||
cart = await _seed(session)
|
cart = await _seed(session)
|
||||||
resp = await client.patch(f"/scenarios/{cart.id}", json={"name": "Renamed"})
|
|
||||||
assert resp.status_code == 400
|
# Core field — still blocked.
|
||||||
assert "cartesian" in resp.json()["detail"]
|
bad = await client.patch(f"/scenarios/{cart.id}",
|
||||||
|
json={"jurisdiction": "uae"})
|
||||||
|
assert bad.status_code == 400
|
||||||
|
assert "cartesian" in bad.json()["detail"]
|
||||||
|
|
||||||
|
# config_json and name — allowed (preserves user edits).
|
||||||
|
ok = await client.patch(
|
||||||
|
f"/scenarios/{cart.id}",
|
||||||
|
json={"config_json": {"flex_rules": [{"from_ath_pct": 0.2,
|
||||||
|
"cut_discretionary_pct": 0.5}]},
|
||||||
|
"name": "Renamed"},
|
||||||
|
)
|
||||||
|
assert ok.status_code == 200, ok.text
|
||||||
|
assert ok.json()["name"] == "Renamed"
|
||||||
|
assert ok.json()["config_json"]["flex_rules"][0]["from_ath_pct"] == 0.2
|
||||||
|
|
||||||
|
|
||||||
async def test_delete_user_scenario(client: AsyncClient) -> None:
|
async def test_delete_user_scenario(client: AsyncClient) -> None:
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue