fix(sf-fe): poi-score TypeError + drawer non-overlay + CSS design tokens #353

Merged

lekss361 merged 1 commit from fix/sf-fe-ui-alignment-typeerror into main

2026-05-18 00:04:31 +00:00

lekss361 commented

2026-05-17 23:58:23 +00:00

Owner

Summary

Bug A — TypeError r is not iterable on /site-finder/analysis/[cad]: Backend /parcels/{cad}/poi-score returns {cad_num, radius_m, top_poi:[...]} but useParcelPoiScoreQuery expected {items, poi_weighted_score}. Added adapter in queryFn that maps top_poi to items and computes poi_weighted_score from weight field. P0 fix, no backend changes.
Bug B — Sidebar hidden on parcel select: ParcelDrawer was position:fixed over the whole page, causing the right sidebar (CadInput + RecentParcels + ParcelLegend) to hide. Fix: drawer is now position:absolute within the map container (parent has position:relative), overlapping only the map area. CadInput stays always visible.
Bug C — CSS design tokens missing: All var(--bg-card), var(--accent), var(--fg-primary) etc. were referenced but never defined — the Site Finder UI was invisible/unstyled. Created frontend/src/app/globals.css with all tokens from UI_Brief_GenDesign_May17. Imported in layout.tsx. Also sets font-variant-numeric:tabular-nums globally per design brief.

Test plan

Open /site-finder/analysis/66:41:0204016:10 — no TypeError, POI list renders
Open /site-finder — click parcel — drawer over map only, right sidebar remains visible
Both pages show correct design colors (CSS vars resolve)
tsc --noEmit passes on modified files
next lint passes

## Summary - **Bug A — TypeError `r is not iterable`** on `/site-finder/analysis/[cad]`: Backend `/parcels/{cad}/poi-score` returns `{cad_num, radius_m, top_poi:[...]}` but `useParcelPoiScoreQuery` expected `{items, poi_weighted_score}`. Added adapter in queryFn that maps `top_poi` to `items` and computes `poi_weighted_score` from `weight` field. P0 fix, no backend changes. - **Bug B — Sidebar hidden on parcel select**: ParcelDrawer was `position:fixed` over the whole page, causing the right sidebar (CadInput + RecentParcels + ParcelLegend) to hide. Fix: drawer is now `position:absolute` within the map container (parent has `position:relative`), overlapping only the map area. CadInput stays always visible. - **Bug C — CSS design tokens missing**: All `var(--bg-card)`, `var(--accent)`, `var(--fg-primary)` etc. were referenced but never defined — the Site Finder UI was invisible/unstyled. Created `frontend/src/app/globals.css` with all tokens from UI_Brief_GenDesign_May17. Imported in `layout.tsx`. Also sets `font-variant-numeric:tabular-nums` globally per design brief. ## Test plan - [ ] Open `/site-finder/analysis/66:41:0204016:10` — no TypeError, POI list renders - [ ] Open `/site-finder` — click parcel — drawer over map only, right sidebar remains visible - [ ] Both pages show correct design colors (CSS vars resolve) - [ ] `tsc --noEmit` passes on modified files - [ ] `next lint` passes

lekss361 added 1 commit 2026-05-17 23:58:24 +00:00

fix(sf-fe): poi-score adapter (TypeError) + drawer non-overlay + CSS tokens 807eb9ae0f

Bug A — TypeError 'r is not iterable' on /site-finder/analysis/[cad]:
  Backend /parcels/{cad}/poi-score returns {cad_num, radius_m, top_poi:[...]}
  but useParcelPoiScoreQuery expected {items, poi_weighted_score}. Added
  adapter in queryFn that maps top_poi → items and computes poi_weighted_score
  from weight field. No backend changes needed.

Bug B — Sidebar hides on parcel select (bad UX):
  ParcelDrawer was position:fixed over the whole page, causing sidebar
  (CadInput + RecentParcels + ParcelLegend) to hide. Fix: drawer is now
  position:absolute within the map container (parent given position:relative),
  overlapping only the map area. Sidebar stays permanently visible so user
  can type a cad directly alongside the drawer.

Bug C — CSS design tokens missing (all var(--*) undefined):
  Created frontend/src/app/globals.css with all design tokens from
  UI_Brief_GenDesign_May17 (surface, border, text, brand, semantic, viz).
  Imported in layout.tsx. Removes invisible UI caused by undefined CSS vars.
  Also sets font-variant-numeric: tabular-nums globally per brief.

lekss361 commented

2026-05-18 00:04:25 +00:00

Author

Owner

Deep Code Review — verdict: APPROVE

Summary

Status: APPROVE
Files reviewed: 6 (P2: 5 source, skip: package-lock.json)
Lines: +1094 / -357 (most churn is re-indentation in ParcelDrawer + lock file regen)
Head SHA: 807eb9ae (single commit, matches my scan)

Bug A — POI adapter (`site-finder-api.ts`)

Clean. PoiScoreRaw is precise (no any), nullish-coalescing on top_poi ?? [] handles missing-key case, reduce(..., 0) initial value prevents NaN on empty list. Section1ParcelInfo.tsx consumer keeps the stable PoiScoreResponse contract — no downstream churn.

Minor: score_contribution: Math.round(p.weight * 100) assumes weight ∈ [0,1] per item — if backend ever returns un-normalized weights, the computed poi_weighted_score could exceed expected bounds. Not blocking (matches B6 contract from #333).

Bug B — Drawer `position:absolute` + sidebar always visible

if (!parcel) return null; retained → drawer self-gates render. OK.
Map container parent gets position: relative (page.tsx:158) so absolute anchor works. OK.
Drawer scoped to map flex item; sidebar (CadInput / RecentParcels / ParcelLegend) is now unconditionally rendered. OK.
role="dialog" + aria-label retained. No focus trap existed before either — no regression.

Trade-off (acceptable): old backdrop <div onClick={onClose} inset:0> is gone, so click-outside-to-close no longer works. Close is via X button or map deselect. Per design brief this is intentional — drawer no longer occludes anything outside the map, so backdrop is unnecessary.

Cosmetic nit (not blocking): drawer style sets both borderLeft: 1px solid var(--border-card) and border: 1px solid var(--border-card) on lines 1087-1088 — the shorthand border overrides borderLeft, so the borderLeft line is dead. Squash if you touch the file again.

Bug C — `globals.css`

All tokens referenced by site-finder components defined: --bg-card, --bg-card-alt, --bg-headline, --bg-app, --border-soft/card/strong, --fg-primary/secondary/tertiary, --fg-on-dark, --fg-on-dark-muted, --accent, --accent-hover, --accent-soft, --accent-2, --success, --success-soft, --warn, --warn-soft, --danger, --danger-soft, viz-1..5, prediction line/band.
font-variant-numeric: tabular-nums + font-feature-settings: "tnum" on body — matches brief.
:focus-visible ring uses --accent — a11y-correct.
No Tailwind directives — but the project has tailwindcss@^4 in deps without a postcss.config / @tailwindcss/postcss wiring or @import "tailwindcss" anywhere. Codebase uses inline style={...} exclusively, so Tailwind utilities are not active either pre- or post-PR. No new conflict; just noting this dep is dead weight.
* { box-sizing: border-box } is a behavior shift but low risk: layout uses flex/grid with inline px, no measurable regression.
html, body { font-size: 14px } global — could shift any em/rem usage. Codebase uses absolute px → low impact.

Cross-file impact analysis

useParcelPoiScoreQuery consumers (Section1ParcelInfo.tsx, PoiList2Gis.tsx) unchanged contract. ✅
ParcelDrawer only consumer is site-finder/page.tsx, updated. ✅
layout.tsx body inline-style removed in favor of globals.css equivalents. ✅
tabular-nums per-component inline declarations now redundant (set globally) — keep them; they don't conflict.

Conventions

TS strict: clean, no any.
"use client" preserved.
Lucide icons unchanged.
No dangerouslySetInnerHTML.
No inline hex in new code (tokens used).

Pre-flight

Feature branch, no --no-verify / force / amend in history (single commit).
mergeable=true, no conflicts with main@9ffeee7e.

Blast-radius score

Risk: Low
Reversibility: easy revert (no DB / API changes).
Merge window: anytime.

Merging via deep-code-reviewer.

## Deep Code Review — verdict: APPROVE ### Summary - **Status**: APPROVE - **Files reviewed**: 6 (P2: 5 source, skip: package-lock.json) - **Lines**: +1094 / -357 (most churn is re-indentation in ParcelDrawer + lock file regen) - **Head SHA**: `807eb9ae` (single commit, matches my scan) ### Bug A — POI adapter (`site-finder-api.ts`) Clean. `PoiScoreRaw` is precise (no `any`), nullish-coalescing on `top_poi ?? []` handles missing-key case, `reduce(..., 0)` initial value prevents NaN on empty list. `Section1ParcelInfo.tsx` consumer keeps the stable `PoiScoreResponse` contract — no downstream churn. Minor: `score_contribution: Math.round(p.weight * 100)` assumes `weight ∈ [0,1]` per item — if backend ever returns un-normalized weights, the computed `poi_weighted_score` could exceed expected bounds. Not blocking (matches B6 contract from #333). ### Bug B — Drawer `position:absolute` + sidebar always visible - `if (!parcel) return null;` retained → drawer self-gates render. OK. - Map container parent gets `position: relative` (page.tsx:158) so absolute anchor works. OK. - Drawer scoped to map flex item; sidebar (CadInput / RecentParcels / ParcelLegend) is now unconditionally rendered. OK. - `role="dialog"` + aria-label retained. No focus trap existed before either — no regression. Trade-off (acceptable): old backdrop `<div onClick={onClose} inset:0>` is gone, so click-outside-to-close no longer works. Close is via X button or map deselect. Per design brief this is intentional — drawer no longer occludes anything outside the map, so backdrop is unnecessary. Cosmetic nit (not blocking): drawer style sets both `borderLeft: 1px solid var(--border-card)` and `border: 1px solid var(--border-card)` on lines 1087-1088 — the shorthand `border` overrides `borderLeft`, so the `borderLeft` line is dead. Squash if you touch the file again. ### Bug C — `globals.css` - All tokens referenced by site-finder components defined: `--bg-card`, `--bg-card-alt`, `--bg-headline`, `--bg-app`, `--border-soft/card/strong`, `--fg-primary/secondary/tertiary`, `--fg-on-dark`, `--fg-on-dark-muted`, `--accent`, `--accent-hover`, `--accent-soft`, `--accent-2`, `--success`, `--success-soft`, `--warn`, `--warn-soft`, `--danger`, `--danger-soft`, viz-1..5, prediction line/band. - `font-variant-numeric: tabular-nums` + `font-feature-settings: "tnum"` on body — matches brief. - `:focus-visible` ring uses `--accent` — a11y-correct. - No Tailwind directives — but the project has `tailwindcss@^4` in deps without a postcss.config / @tailwindcss/postcss wiring or `@import "tailwindcss"` anywhere. Codebase uses inline `style={...}` exclusively, so Tailwind utilities are not active either pre- or post-PR. No new conflict; just noting this dep is dead weight. - `* { box-sizing: border-box }` is a behavior shift but low risk: layout uses flex/grid with inline px, no measurable regression. - `html, body { font-size: 14px }` global — could shift any `em`/`rem` usage. Codebase uses absolute px → low impact. ### Cross-file impact analysis - `useParcelPoiScoreQuery` consumers (`Section1ParcelInfo.tsx`, `PoiList2Gis.tsx`) unchanged contract. ✅ - `ParcelDrawer` only consumer is `site-finder/page.tsx`, updated. ✅ - `layout.tsx` body inline-style removed in favor of globals.css equivalents. ✅ - `tabular-nums` per-component inline declarations now redundant (set globally) — keep them; they don't conflict. ### Conventions - TS strict: clean, no `any`. - "use client" preserved. - Lucide icons unchanged. - No `dangerouslySetInnerHTML`. - No inline hex in new code (tokens used). ### Pre-flight - Feature branch, no `--no-verify` / force / amend in history (single commit). - mergeable=true, no conflicts with `main@9ffeee7e`. ### Blast-radius score - Risk: **Low** - Reversibility: easy revert (no DB / API changes). - Merge window: anytime. Merging via deep-code-reviewer.