diff --git a/.impeccable/critique/2026-07-13T21-29-24Z__rc-app-portfolio-slug-components-propertytable-tsx.md b/.impeccable/critique/2026-07-13T21-29-24Z__rc-app-portfolio-slug-components-propertytable-tsx.md new file mode 100644 index 00000000..73736a11 --- /dev/null +++ b/.impeccable/critique/2026-07-13T21-29-24Z__rc-app-portfolio-slug-components-propertytable-tsx.md @@ -0,0 +1,78 @@ +--- +target: portfolio page (property table + toolbar + no-plans state) +total_score: 27 +p0_count: 1 +p1_count: 1 +timestamp: 2026-07-13T21-29-24Z +slug: rc-app-portfolio-slug-components-propertytable-tsx +--- +Method: dual-agent (A: ad10a436 design-review · B: a517f2c1 detector+evidence) + +## Design Health Score + +| # | Heuristic | Score | Key Issue | +|---|-----------|-------|-----------| +| 1 | Visibility of System Status | 3 | Good results count + spinner overlay, but first-load is bare "Loading properties…" (no skeleton) and a no-plans portfolio gives zero signal that modelling is the missing step. | +| 2 | Match System / Real World | 3 | "Plan Cost / No cost", "Expected EPC" read well; but blank EPC bubbles + "—" Ref communicate nothing about *why* they're empty. | +| 3 | User Control and Freedom | 3 | Clear/Clear-all on filters + selection; no undo on optimistic tag assign, no cancel-export. | +| 4 | Consistency and Standards | 2 | Two colour systems: toolbar uses brandblue/primary; filter sidebar + Edit-Columns checkboxes use raw black/bg-black/accent-black. | +| 5 | Error Prevention | 3 | Delete has a preview modal, export gated >6000 — but the delete modal has Cancel and no wired Confirm (dead-end). | +| 6 | Recognition Rather Than Recall | 3 | Columns labelled, Current EPC has a "?" tooltip; but Current/Lodged/Expected are three identical blank bubbles with no legend when empty. | +| 7 | Flexibility and Efficiency | 3 | Column visibility, quick filters, filter builder, export — strong; undercut by a 7-row page size on a desk tool. | +| 8 | Aesthetic and Minimalist Design | 2 | The no-plans grid is a wall of "—", blank bubbles, italic "No cost"; 6 top-level toolbar actions + two dark CTAs + shouty micro-labels. | +| 9 | Error Recovery | 3 | load-more + export fail silently; tag-assign error only shows inline in the popover footer. | +| 10 | Help and Documentation | 2 | One tooltip (Current EPC); no inline guidance connecting an unmodelled portfolio to "Run modelling". | +| **Total** | | **27/40** | **Acceptable — solid plumbing, dragged down by the empty state + consistency** | + +## Anti-Patterns Verdict + +**LLM assessment:** Not egregious slop — the table is competently built and restrained — but three tells betray a generated-template lineage: tiny uppercase black micro-labels everywhere (`RESULTS:` font-black uppercase, `CURATE SELECTION` at 9px), `rounded-xl` + `shadow-sm` cards the brief names as an anti-reference, and em-dash-as-content filling 5–6 of 9 columns. Bones are institutional; finish is generic-admin. + +**Deterministic scan:** detect.mjs exit 0 — **zero findings**. No gradient text, decorative gradients, side-stripe borders, glassmorphism, over-rounded radii (only rounded-lg/xl/full), heavy shadows (only shadow-sm), or arbitrary z-index. The mechanical slop tells are genuinely absent — the problems here are IA/hierarchy/contrast that a static scanner can't see, which is exactly where the design review earns its keep. + +**Visual overlays:** none — browser automation unavailable (arm64 container can't run the bundled x86 Chromium under qemu). Fallback: static grep evidence corroborated the review (8 placeholder "—" cells in propertyTableColumns.tsx: 256/336/442/451/459/470/483/494; text-slate-300 at 384/442/451/494/524; 6 toolbar controls at 602/617/658/686/693/742; no dedicated no-plans empty-state component). + +## Overall Impression + +The interaction plumbing is genuinely good (optimistic tag writes, filter-scoped selection, provenance signalling). What lets it down is a single unaddressed truth: **the default state of every brand-new portfolio — properties added, nothing modelled — is the one state with zero design investment.** A first-time housing-association user lands on 185 rows where Ref is "—", three EPC columns are blank, Expiry is "—", and every Plan Cost is a faint italic "No cost". The read isn't "fresh start", it's "this tool is broken / my import failed" — the worst possible first impression for a product whose promise is *trustworthy numbers*. The single biggest opportunity is to design the unmodelled state as a first-class surface, not a fallthrough. + +## What's Working + +1. **The Current-EPC "?" tooltip** — exactly right for the institutional register: explains a load-bearing figure inline without cluttering the header. A pattern to extend, not remove. +2. **The Predicted pill + provenance model** (amber "Predicted", ADR-0002) — genuine trust-signalling that tells analysts which EPCs to trust less; precisely the "trustworthy numbers" value the brand claims. +3. **Optimistic tag writes across every cached page + filter-scoped selection surviving pagination** — Linear/Stripe-class plumbing, invisible when it works. + +## Priority Issues + +**[P0] The no-plans / unmodelled state has no design at all.** `EmptyPropertyState` only fires when there are literally zero rows; a portfolio with 185 unmodelled properties falls straight through to the full table with 5–6 empty columns. This is the *default* state of every new portfolio and the exact case flagged as "terrible". **Fix:** detect "has properties, zero plans/EPC" and render a purpose-built view — collapse/hide the three EPC + Expiry + Plan Cost columns, show Address/Ref/Tags at full density, and surface one reassuring banner ("185 properties added — run modelling to generate EPC ratings and retrofit costs →") wired to the existing Run-modelling route. *Command: `onboard`* + +**[P1] Empty cells fail contrast and read as breakage.** `text-slate-300 italic` "No cost" and slate-300 em-dashes / "Unknown" are below WCAG AA on white and make populated data indistinguishable from missing data at a glance. Four different "empty" renderings coexist (blank `w-5 h-5` div, "—", italic "No cost", "Unknown"). **Fix:** raise empty text to slate-400/500 and pick ONE absence vocabulary. *Command: `harden`* + +**[P2] Six top-level toolbar actions with two competing dark CTAs.** Filters, Edit Columns, Export, Tags▾, Add properties▾, Run modelling in one undifferentiated row; Add properties and Run modelling are both navy `bg-primary` — no primary focus, and the brief says navy belongs "in small doses". **Fix:** one dark CTA at a time (Add properties when empty → Run modelling once populated), fold Export/Edit Columns/Tags into a quieter secondary group. *Command: `distill`* + +**[P3] Colour-system drift between toolbar and filter sidebar.** Toolbar uses brandblue/primary; `PropertyFilters.tsx` and the Edit-Columns checkboxes use raw black/bg-black/accent-black — two systems on one page. **Fix:** replace every black token with the brand navy/brandblue scale. *Command: `colorize`* + +**[P3] No first-load skeleton; silent failures.** First load is a lone "Loading properties…" string; load-more and export swallow errors (`// silently ignore`). For a data-trust product a silent export failure is a credibility risk. **Fix:** skeleton rows + a toast/inline error on export and load-more failure. *Command: `polish`* + +## Persona Red Flags + +**Alex (power user):** `pageSize: 7` on a large desk screen means paging 27× through 185 homes; lazy load-more only triggers on reaching the last page — no jump-to-end or density control. Wants 50–100 rows + persistent EPC/cost sort. + +**Sam (accessibility):** slate-300 "No cost"/"Unknown" fail AA outright; the quick-filter clear control is a `` nested *inside* a ` @@ -939,7 +939,7 @@ export default function PropertyFilters({ diff --git a/src/app/portfolio/[slug]/components/PropertyTable.tsx b/src/app/portfolio/[slug]/components/PropertyTable.tsx index 8c8b86c9..a64be71d 100644 --- a/src/app/portfolio/[slug]/components/PropertyTable.tsx +++ b/src/app/portfolio/[slug]/components/PropertyTable.tsx @@ -47,21 +47,20 @@ import { DropdownMenuTrigger, } from "@/app/shadcn_components/ui/dropdown-menu"; -import { - Dialog, - DialogContent, - DialogHeader, - DialogTitle, - DialogDescription, - DialogFooter, -} from "@/app/shadcn_components/ui/dialog"; -import { Button } from "@/app/shadcn_components/ui/button"; /* ---------------------------------------- Export helpers ----------------------------------------- */ const EXPORT_LIMIT = 6000; +// Exactly one navy CTA is primary at a time (impeccable distill): Add properties +// leads an empty portfolio, Run modelling leads a populated one; the other demotes +// to the neutral toolbar-button style so the toolbar has a single focal action. +const PRIMARY_CTA = + "flex items-center gap-1.5 h-8 px-3 rounded-lg bg-primary text-white text-xs font-semibold hover:opacity-90 transition"; +const SECONDARY_CTA = + "flex items-center gap-1.5 h-8 px-3 rounded-lg border border-slate-200 bg-white text-primary text-xs font-semibold hover:bg-slate-50 transition"; + function resolveEnumLabel( options: EnumOption[], dbValue: string | null | undefined, @@ -237,43 +236,44 @@ function QuickFilterDropdown({ return (
- + {isActive && ( + )} - +
{isOpen && (
@@ -422,6 +422,7 @@ export default function PropertyTable({ filterGroups: [], }); const totalCount = allResponse?.total ?? 0; + const [modelNudgeDismissed, setModelNudgeDismissed] = useState(false); const DISPLAY_LIMIT = 250; const LOAD_MORE_SIZE = 100; @@ -448,6 +449,11 @@ export default function PropertyTable({ [queryData, extraRows], ); + // Properties with no default plan yet — the "not modelled" set. Counted from the + // loaded rows (cheap; avoids a hook per project convention): exact for a + // fully-loaded portfolio, a floor when more rows lazy-load beyond view. + const unmodelledInView = tableData.filter((p) => p.planId == null).length; + // Controlled pagination (lifted from DataTable so we can detect the last page) const [pagination, setPagination] = useState({ pageIndex: 0, @@ -519,9 +525,11 @@ export default function PropertyTable({ // Export fetches its own batch (up to EXPORT_LIMIT) for the current filters, // independent of the on-screen page — the display fetch is deliberately small. const [isExporting, setIsExporting] = useState(false); + const [exportError, setExportError] = useState(null); const handleExport = useCallback(async () => { if (isExporting) return; setIsExporting(true); + setExportError(null); try { const res = await fetch("/api/properties", { method: "POST", @@ -533,24 +541,17 @@ export default function PropertyTable({ offset: 0, }), }); - if (!res.ok) return; + if (!res.ok) throw new Error(); const json: { data: PropertyWithRelations[] } = await res.json(); exportToCsv(json.data); + } catch { + // Don't fail silently — a data-trust product must say when an export dies. + setExportError("Export failed — please try again."); } finally { setIsExporting(false); } }, [isExporting, portfolioId, allFilterGroups]); - /* ---------------------------------------- - Delete preview state - ----------------------------------------- */ - const [deletePropertyId, setDeletePropertyId] = useState(null); - const [deletePreview, setDeletePreview] = useState< - { table: string; count: number }[] | null - >(null); - const [previewLoading] = useState(false); - const [previewError] = useState(null); - return (
{/* Action bar */} @@ -586,6 +587,18 @@ export default function PropertyTable({ )} + {exportError && ( + + {exportError} + + + )} {hasActiveFilters && ( + +
+
+ )} + {/* Display-limit notice */} {isAtDisplayLimit && hasMore && (
@@ -890,7 +946,6 @@ export default function PropertyTable({ setDeletePropertyId(id)} columnVisibility={columnVisibility} onColumnVisibilityChange={ setColumnVisibility as ( @@ -911,57 +966,6 @@ export default function PropertyTable({ )}
- - {/* Delete preview modal */} - { - if (!open) { - setDeletePropertyId(null); - setDeletePreview(null); - } - }} - > - - - Delete property? - - {previewLoading - ? "Calculating what will be deleted…" - : "This action is permanent. Review the impact below."} - - - - {previewError && ( -

{previewError}

- )} - - {deletePreview && !previewLoading && ( -
    - {deletePreview.map((row) => ( -
  • - - {row.table.replace(/_/g, " ")} - - {row.count} -
    • - ))} -
- )} - - - - - - ); } diff --git a/src/app/portfolio/[slug]/components/propertyTableColumns.tsx b/src/app/portfolio/[slug]/components/propertyTableColumns.tsx index 3142be4a..3307ca92 100644 --- a/src/app/portfolio/[slug]/components/propertyTableColumns.tsx +++ b/src/app/portfolio/[slug]/components/propertyTableColumns.tsx @@ -53,8 +53,29 @@ function Pill({ /* ----------------------------------------------------------------------- EPC letter bubble ------------------------------------------------------------------------ */ +/** + * The one treatment for an absent value across the table (ADR-0013 impeccable + * pass): a single quiet, AA-legible em-dash with an accessible label — replacing + * the four different empty renderings (blank div, slate-300 "—", italic + * "No cost", "Unknown") the critique flagged. + */ +const EmptyCell = ({ label = "No data" }: { label?: string }) => ( + + — + +); + const EpcLetterBubble = ({ letter }: { letter: string }) => { - if (!letter) return
; + if (!letter) + return ( +
+ – +
+ ); return (
[] = [ header: () =>
Property Ref
, cell: ({ row }) => (
- {row.original.landlordPropertyId ?? "—"} + {row.original.landlordPropertyId ?? }
), }, @@ -333,7 +354,11 @@ const coreColumns: ColumnDef[] = [ const expired = row.original.epcIsExpired; if (!dateStr) { - return
—
; + return ( +
+ +
+ ); } const lodgementDate = new Date(dateStr); @@ -375,13 +400,13 @@ const coreColumns: ColumnDef[] = [ } return ( -
+
{cost ? ( £{formatNumber(cost)} ) : ( - No cost + )}
); @@ -439,7 +464,7 @@ const optionalColumns: ColumnDef[] = [ header: () =>
Property Type
, cell: ({ row }) => { const val = row.original.propertyType; - return val ? {val} : —; + return val ? {val} : ; }, }, { @@ -448,7 +473,7 @@ const optionalColumns: ColumnDef[] = [ header: () =>
Built Form
, cell: ({ row }) => { const val = row.original.builtForm; - return val ? {val} : —; + return val ? {val} : ; }, }, { @@ -456,7 +481,9 @@ const optionalColumns: ColumnDef[] = [ accessorKey: "yearBuilt", header: () =>
Construction Age
, cell: ({ row }) => ( -
{row.original.yearBuilt ?? "—"}
+
+ {row.original.yearBuilt ?? } +
), }, { @@ -467,7 +494,7 @@ const optionalColumns: ColumnDef[] = [ const val = row.original.totalFloorArea; return (
- {val != null ? `${val.toFixed(1)} m²` : "—"} + {val != null ? `${val.toFixed(1)} m²` : }
); }, @@ -480,7 +507,7 @@ const optionalColumns: ColumnDef[] = [ const val = row.original.co2Emissions; return (
- {val != null ? `${val.toFixed(1)} kg/m²/yr` : "—"} + {val != null ? `${val.toFixed(1)} kg/m²/yr` : }
); }, @@ -491,7 +518,7 @@ const optionalColumns: ColumnDef[] = [ header: () =>
Main Fuel
, cell: ({ row }) => { const label = resolveEnumLabel(MAINFUEL_OPTIONS, row.original.mainfuel); - return label ? {label} : —; + return label ? {label} : ; }, }, { @@ -521,7 +548,7 @@ function DescriptorCell({ value }: { value: string | null }) {