Frontend modernization: polish pass (PR 8) + unified type system (PR A) - #29
Merged
Conversation
Phase 2 PRs 1–7 all shipped under THI-45 through THI-54 before this tracker was committed. Audit on 2026-04-19 confirmed `frontend/src/index.css` tokens match `README.md` spec exactly (no drift, light + dark). Phase 2 PR 8 (polish pass) is in-progress with #26/#27/#28 landed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Record that PR 2 (THI-46) shipped the README's primitive inventory — Field, FormSection, MetricCell, OccupancyGauge, PropertyThumb, RentalBadge, Segmented, SliderField, Toggle — and that Card/Button/Tabs were deliberately not extracted. PageHeader is deferred to Phase 3 PR A (unified type system). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… classes Add a global :focus-visible rule in index.css (2px var(--accent) outline, 2px offset), matching the README spec. Exclude .field so its bottom-border focus treatment wins on form fields. Strip the lingering focus:ring-indigo-500 classes on the snapshot-name input so it inherits the new rule. Part of PR 8 (polish pass), tier 1. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add shared/Skeleton.tsx (<Skeleton>, <SkeletonLine>) and shared/EmptyState.tsx (extracted from the local copy in Dashboard). Add a token-based .skeleton utility in index.css with a pulse animation and prefers-reduced-motion guard. Replace plain-text loading placeholders with structured skeletons matching each view's grid, so layout doesn't jump when data lands: - Dashboard: hero, toolbar, 6 table rows - PropertyPage: breadcrumb, title block, metric strip, tab bar - PropertyDetail: Financing (280px rail + editor), Revenue & Expenses (3 form sections) - ComparisonView: 8-row comparison table Each skeleton container carries role="status" + aria-label; inner shapes are aria-hidden. Part of PR 8 (polish pass), tier 2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add useFocusTrap and useEscapeKey hooks. Wire them into every dialog and drawer in the app. Tablists now use roving tabindex + arrow/Home/End navigation per ARIA spec — Tab enters at the active option, arrows move between siblings. Applied to Segmented (all consumers: Dashboard filters, Settings preferences, Glossary category, etc.) and the PropertyDetail tab bar. Dialogs gain role="dialog"/"alertdialog", aria-modal, aria-labelledby, focus trap, and Escape to close: - ConfirmDialog - HistoryDrawer - SnapshotButton's save dialog Part of PR 8 (polish pass), tier 3. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Install axe-core and wire it into main.tsx behind import.meta.env.DEV. Patches history.pushState / replaceState and listens for popstate so client-side route changes trigger a re-run. Violations are grouped in the devtools console with severity, help URL, and the offending selectors. Prod bundle unaffected — the DEV branch is tree-shaken (bundle grew 20B). Part of PR 8 (polish pass), tier 4a. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Lets you run axe.run(document.body).then(...) directly from DevTools. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wire htmlFor + id via useId() in Field, SliderField, CurrencyInput, and PercentInput — associates every rendered label with its input so screen readers announce the control. Also: - Raw <select> on Property Info "Property Type" gets htmlFor/id. - File input on Settings logo upload gets aria-label="Upload company logo". - FormSection <h3> → <h2> (top-level sections under page h1). - PropertyCard <h3> → <h2> (card titles on Dashboard). - "New property" dialog <h3> → <h2>. - Import status / Shortcuts / Live preview <h4> → <h3> so the hierarchy is h1 → h2 → h3 with no skips. Addresses axe rules: label, select-name, heading-order. Color-contrast violations persist across all views and trace back to the --ink-3 token at oklch(0.62) vs --paper failing 4.5:1 — that's a design-spec call, not a code fix. Part of PR 8 (polish pass), tier 4b. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Update the PR 8 row with every tier's status (1 focus, 2 skeletons, 3 keyboard nav, 4a axe reporter, 4b a11y fixes). Color-contrast is systemic across all views and traces to the --ink-3 token vs spec; filed as THI-58 for the design conversation. Tier 5 (test infra) not started. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Install vitest, @testing-library/react, @testing-library/jest-dom, @testing-library/user-event, jsdom, jest-axe, @types/jest-axe. Add vitest.config.ts (jsdom env, setup file, excludes macOS "* 2.tsx" duplicates) and src/test/setup.ts (jest-dom matchers + cleanup + toHaveNoViolations). Tests cover the primitives introduced in PR 8: - Skeleton / SkeletonLine — render, aria-hidden, classes - EmptyState — title, body, actions slot, axe clean - Segmented — tablist roles, roving tabindex, Arrow/Home/End/click - useEscapeKey — fires on Escape, respects active flag, cleans up - useFocusTrap — auto-focus, Tab/Shift+Tab wrapping - ConfirmDialog — open/closed, role="alertdialog", labelled by, Escape → onCancel, Confirm → onConfirm Drop the offsetParent !== null filter in useFocusTrap in favor of a [hidden] attribute check — jsdom never computes offsetParent, which was excluding every button in tests without changing real-browser behavior meaningfully. CI integration intentionally deferred (.github is out of scope per CLAUDE.md). Run locally with `npm test`. Part of PR 8 (polish pass), tier 5. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tier 5 (vitest + RTL + jest-axe + tests for PR 8 primitives, 23 tests) committed as aec4fbf. All tiers of PR 8 complete. Deferred items: color-contrast token decision (THI-58), CI integration of npm test, and test backfill for pre-PR-8 primitives. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Lock the type scale from "Font & Chart Proposals" section 01 into CSS
variables — --h1: 48px, --h2: 22px, --hero: 56px, --caps-eyebrow: 10px.
Add .h1 / .h2 / .hero utility classes (Instrument Serif, weight 400,
no font-semibold) and .caps-eyebrow (Inter 10px, 0.14em tracking, ink-3).
New shared/PageHeader.tsx — { eyebrow?, title, subtitle?, hero?, actions? }.
Renders the unified "caps eyebrow + serif H1 + optional subtitle" pattern
the spec calls for on every page.
Migrate the four top-level page titles to PageHeader:
- Dashboard — "Portfolio" eyebrow + dynamic property-count title
- Settings — "Workspace" / "Settings" / workspace blurb
- Glossary — "Glossary" / "Every term, defined." + search in actions slot
- Comparison — "Compare · N" / "Side by side"
Titles bump from text-[44px] to var(--h1) = 48px per spec. PropertyDetail
is deliberately untouched; per-tab header rework is Tier A2 pending an IA
decision.
Tier 5 tests cover the new primitive (5 tests; axe clean).
Part of Phase 3 PR A (unified type system), tier A1.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the persistent property header (h1 + rental badge + address + Export PDF) with two pieces: a compact meta row (rental badge left, Export PDF right) and a PageHeader that switches by activeTab. Each tab now owns its own h1 per the spec's "one H1 per page/tab" rule. Eyebrow / title / subtitle mapping: - Property Info — eyebrow "Property · Information", title = property.name, subtitle = full address. The property name is the h1 on the tab that's semantically about the property itself. - Financing — eyebrow = property.name, title "Financing" - Revenue & Expenses — eyebrow = property.name, title "Revenue & expenses" - Results — eyebrow = property.name, title "Results" - Sensitivity — eyebrow = property.name, title "Sensitivity" Property name is visible on every tab (as h1 on Info, as eyebrow elsewhere) so the user never loses context. Metric strip, tab bar, breadcrumb, and tab content are untouched. Part of Phase 3 PR A (unified type system), tier A2a. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace every remaining inline text-[NNpx] + font-serif on heading elements with the .h2 utility class landed in A1. Strips redundant leading-tight / leading-none / text-ink where the token class already sets them. Cleans up: - ResultsTab — 7 section sub-headings - SensitivityTab — 2 section sub-headings (one promoted 20px → 22px for consistency with siblings) - RevenueExpensesTab — Occupancy section title - FormSection — 22px serif title - PropertyCard — card title (promoted 20px → 22px) - Dashboard — "New property" dialog heading - GlossaryPage — entry term (stays <h3> semantically under letter <h2>, uses .h2 for visual 22px) Demote three <h3 className="caps"> sites to <p className="caps"> — they're small labels, not real sub-section headings, and the spec's "no sans-serif headings anywhere" rule makes caps-styled <h[1-6]> elements a conflict: - PropertyInfoTab — "Import status" and "Shortcuts" aside labels - FinancingTab — scenario rail "Scenarios" label Explicit exceptions kept as-is: - GlossaryPage's 48px decorative letter-section <h2> (A, B, C) — design call; the 4-token scale has no big-letter tier. - Dialog <h3>s (ConfirmDialog, HistoryDrawer, SnapshotButton) — the "no sans headings" rule is ambiguous for modal chrome. - Legacy QuickTest.tsx — pre-modernization; separate rewrite. Part of Phase 3 PR A (unified type system), tier A3. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
All three tiers of PR A (A1 infrastructure + top-page migration, A2a PropertyDetail per-tab PageHeader, A3 inline heading cleanup) committed. Exceptions documented. PR A is now in review state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes out two PRs from
design_handoff_modernization/PROGRESS.md:PageHeaderprimitive + four token-based type utilities (.h1/.h2/.hero/.caps-eyebrow). Every top-level page plus PropertyDetail now emits the same eyebrow / H1 / subtitle pattern. Every inlinetext-[NNpx]on a heading is gone.15 feature/fix commits + 4 bookkeeping commits on
design/modernization, stacked on top ofmain(last SHA:8886ff9).What shipped
PR 8 — Polish pass (6 tiers)
b925ee3:focus-visibleoutline inindex.cssusingvar(--accent);.fieldopts out via its own bottom-rule focus. Dropped legacyfocus:ring-indigo-500fromSnapshotButton.5fabe8dSkeleton/SkeletonLine+EmptyStateprimitives. Loading text placeholders replaced with structured skeletons on Dashboard, PropertyPage, PropertyDetail (Financing / Revenue tab loaders), ComparisonView. All skeleton containers carryrole="status"+aria-label; shapes arearia-hidden. Supportsprefers-reduced-motion.2e9a622useFocusTrapanduseEscapeKeyhooks.Segmentedand the PropertyDetail tab bar use rovingtabIndex+ Arrow / Home / End nav.ConfirmDialog,HistoryDrawer, andSnapshotButton's dialog gainrole="dialog"/aria-modal/aria-labelledby+ focus trap + Escape to close.606e974+e09fd6faxe-coreinstalled as a dev dep; DEV-only runtime reporter inmain.tsxpatcheshistory.pushState/replaceStateand fires on popstate. Prod bundle unaffected.window.axeexposed for ad-hoc console audits.f4f286cField/SliderField/CurrencyInput/PercentInputwirehtmlForviauseId. Raw<select>in PropertyInfoTab getshtmlFor. Settings file input getsaria-label.FormSection<h3>→<h2>,PropertyCard<h3>→<h2>, "New property" dialog<h3>→<h2>, aside<h4>s (Import status / Shortcuts / Live preview) →<h3>so hierarchy is h1 → h2 → h3 with no skips. Clears every axe violation except color-contrast.aec4fbfvitest.config.ts(jsdom, excludes macOS* 2.tsxdupes),src/test/setup.ts(jest-dom matchers +toHaveNoViolations+ cleanup). Tests for Skeleton (2), EmptyState (3, axe clean), Segmented (6 incl. keyboard nav), useEscapeKey (4), useFocusTrap (4), ConfirmDialog (4).Deferred from PR 8 → THI-58: systemic
color-contrastfailures trace to--ink-3atoklch(0.62)vs--paperatoklch(0.995)computing to ~3.8:1. Requires a design decision on token values — not a mechanical fix — so filed with root cause, 4 remediation options, and acceptance criteria.PR A — Unified type system (3 tiers)
b0e37d3--h1: 48,--h2: 22,--hero: 56,--caps-eyebrow: 10). New utility classes (.h1,.h2,.hero,.caps-eyebrow), all Instrument Serif at weight 400 — nofont-semibold. NewPageHeader { eyebrow, title, subtitle, hero, actions }primitive inshared/. Migrated Dashboard, Settings, Glossary, Comparison top-level headers. Includes 5 render tests + axe assertion.43c7856PageHeaderthat switches byactiveTab. Info tab's h1 is the property name; other tabs use the property name as the eyebrow so context never disappears. Metric strip and tab bar untouched.ff81759text-[NNpx]+font-seriffrom heading elements. Swap to.h2utility across Results (7 sites), Sensitivity (2), Revenue (1), FormSection, PropertyCard, Dashboard, Glossary entry terms. Demoted three<h3 className="caps">sites (Import status, Shortcuts, Scenarios rail) to<p>— they're labels, not sub-sections. Exceptions documented: Glossary 48px decorative letter h2s; dialog h3s in ConfirmDialog/HistoryDrawer/SnapshotButton (spec ambiguous for modal chrome).Bookkeeping
27dbe7d— PROGRESS.md corrected to reflect that Phase 2 PRs 1–7 had pre-dated the tracker. No code change.996a850— PROGRESS.md clarified PR 2's actually-shipped primitive set (README inventory, not the original one-liner list) soPageHeaderwas explicitly deferred into PR A.1759792— PROGRESS.md recorded PR 8 tier status + color-contrast deferral to THI-58.179565c+aae6f06— closing bookkeeping for PR 8 and PR A respectively.Out of scope for this PR
npm test—.github/is off-limits per CLAUDE.md. Test suite runs locally; wire it into CI in a separate PR when ready.QuickTest.tsx— pre-modernization; rewrite is separate.Test plan
Local verification performed on
design/modernizationbefore push:npm run lint— cleannpm run build— clean (463.93 kB JS, 48.36 kB CSS, 123 modules)npm test— 28/28 tests pass across 7 filesPlease also check in your review:
🤖 Generated with Claude Code