Skip to content

feat: Nothing design redesign (spec 028) — design system foundation#108

Merged
studert merged 19 commits into
mainfrom
worktree-nothing-design
Jun 2, 2026
Merged

feat: Nothing design redesign (spec 028) — design system foundation#108
studert merged 19 commits into
mainfrom
worktree-nothing-design

Conversation

@studert
Copy link
Copy Markdown
Member

@studert studert commented Jun 2, 2026

Implements spec 028 — Nothing Design Redesign: migrate the AI Developer Hub from the stock shadcn/green-oklch theme to one coherent Nothing design system — monochrome canvas, a single red interrupt (#d71921), Space Grotesk / Space Mono / Doto, flat bordered surfaces, segmented-bar data viz, inline status. Dark = instrument panel, light = printed manual.

Built phase-by-phase per specs/028-nothing-design-redesign/implementation-plan.html, verified in a browser on an isolated Neon DB branch (wt/nothing-design) in both modes, gated green (typecheck + lint zero-warnings + build all 40 routes), and a11y-audited.

What landed

  • P0 — Tokens & fonts. globals.css re-keys every shadcn semantic var onto Nothing values (light = :root, dark = .dark; next-themes class strategy unchanged). shadcn --accent stays a neutral hover bg; the one red + all error/destructive route through --destructive. Inter → Space Grotesk (UI) / Space Mono (data) / Doto (hero) via next/font.
  • P1 — Primitives & overlays. Every shadcn primitive re-skinned in place: pill mono-caps buttons (greyscale fill), border-only tags (+ success/warning), boxed mono inputs, square greyscale checkbox/switch, flat shadowless overlays with fade-only animation, neutral in-content tabs, mono-caps tables (no zebra), border-only alerts, select/command selected = left 2px red bar. New shared <StatusText>/useInlineStatus + <LoadingState>/segmented spinner.
  • P2 — App shell. CSS-grid shell replaces SidebarProvider: fixed Nothing text-nav rail (AI·HUB red-dot wordmark, 2px red active bar) + mobile drawer; segmented theme toggle (light/dark/system); AlertBanner → flat bordered status box; neutral auth brand panel.
  • P3 — Shared data & viz. <SegmentedBar> signature viz; SpendProgressBar rebuilt on it; DataTable chrome (icon search, mono pagination); ui/chart.tsx re-themed (mono axes, flat tooltip); all 14 route loaders → <LoadingState>; Claude charts → greyscale ramp + square bars; Copilot donut rebuilt as a SegmentedBar stat-list; KPI "up" delta no longer red.
  • P4 — Pages. Literal rainbow tints → value-text Nothing tokens; all 38 toast.* components converted to inline <StatusText> and the global <Toaster/>/sonner.tsx removed; FormLabel → Space-Mono ALL-CAPS instrument labels across every form; native confirm() → Nothing AlertDialog (request cancel, ingestion delete).
  • P5 — QA. Anti-pattern grep gate clean: 0 toasts, 0 confirm(), 0 literal rainbow classes, 0 animate-pulse, 0 <Skeleton>, 0 shadow-* on chrome. Dead shadcn sidebar.tsx/skeleton.tsx removed. /simplify applied.

Accessibility (verified in-browser, P5)

Lighthouse accessibility 98. Contrast (WCAG AA): light — foreground 15.96, destructive 4.76, success 4.70, warning 5.35 (all ≥4.5 ✓); dark — success 6.34, warning 9.48. The brand red #d71921 is 4.05 on #000 / 3.64 on #111 — clears AA for graphical/UI (≥3:1) and large text and matches the value the spec's own contrast table documents as accepted; kept verbatim per the spec's explicit brand decision.

Notable decisions & deviations

Full running log in specs/028-nothing-design-redesign/implementation-notes.html (Nothing-styled). Highlights: shadcn --accent kept neutral (red via --destructive); light-mode status greens/ambers darkened for WCAG (deviates from the mockups' identical-both-modes hues); simplified fixed-rail shell (no collapsible/Cmd+B/cookie).

Remaining (optional polish, non-blocking)

The two single-date pickers still use the Popover + react-day-picker calendar — re-skinned to monochrome tokens in P1 (a documented-acceptable exception, not an anti-pattern); converting to a Space-Mono date input is optional. Hero Doto display moments and the per-screen "one red" auto-demotion are remaining aesthetic refinements.

🤖 Generated with Claude Code

studert and others added 7 commits June 2, 2026 12:10
Re-key the shadcn semantic token layer in globals.css onto the Nothing
monochrome+red system (light=:root, dark=.dark, next-themes class strategy
unchanged). shadcn --accent stays a neutral hover bg; the single red interrupt
and all error/destructive states route through --destructive (#d71921, =--ring).
Adds ink/faint/success/warning/seg-empty/destructive-subtle tokens registered in
@theme inline. Light-mode status colors darkened for WCAG (see implementation
notes). Swaps Inter for Space Grotesk (UI) / Space Mono (data) / Doto (hero) via
next/font. Build green across all 40 routes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
P1 — re-skin all shadcn primitives/overlays in place to the Nothing look:
pill Space-Mono buttons (greyscale fill), border-only tags w/ success/warning,
boxed mono inputs, square greyscale checkbox/switch, flat shadowless overlays
with fade-only animation, neutral in-content tab underline, mono-caps table
headers (no zebra), border-only alert (no fill), select/command selected =
left 2px red bar. New shared helpers: <StatusText>/useInlineStatus (inline
[SAVED]/[ERROR] w/ aria-live) and <LoadingState>/segmented spinner ([LOADING…]).

P2 — replace SidebarProvider shell with a CSS-grid .app: fixed Nothing text-nav
rail (AI·HUB wordmark + red dot, ALL-CAPS items, 2px red active bar) + mobile
drawer; segmented theme toggle (light/dark/system) wired to next-themes;
AlertBanner -> flat bordered status box (no red fill); neutral auth brand panel;
Sonner flattened to a transitional Nothing toast (call-site removal in P4).

Typecheck + build + lint all green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…pper

- New <SegmentedBar> (signature ~20-segment viz: ink/success/warning/over-red).
- SpendProgressBar rebuilt on SegmentedBar (same API) — discrete segments,
  red overage past ceiling, ceiling tick.
- DataTable chrome: search w/ icon (pill), mono [No results], pagination with
  "PAGE x / y" mono indicator + secondary Prev/Next, re-skinned page-size select.
- ui/chart.tsx wrapper: mono axis ticks, flat shadowless tooltip on raised
  surface (border-input), inheriting the greyscale chart-1..5 ramp from P0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ec 028)

- All 14 route loading.tsx now render <LoadingState label="…"> (segmented
  spinner + [LOADING…]); animate-pulse placeholders in budget-report, claude
  page, sync page, users-month-picker replaced with LoadingState/InlineSpinner.
- Claude console charts (global-metrics, top-users, daily-by-user): 8-color
  spectral FALLBACK_PALETTE -> greyscale chart-token ramp, square bar ends,
  horizontal-only grid.
- Copilot activity-distribution donut rebuilt as a direct-labeled SegmentedBar
  stat-list (pies are banned in Nothing).
- KpiWithMom: "up" MoM delta no longer red (neutral tag); value -> mono ink.

typecheck + lint green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… notes

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… by Nothing shell + LoadingState)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 2, 2026 12:01
@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
ai-developer-hub Ready Ready Preview, Comment Jun 2, 2026 4:09pm

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Implements spec 028’s “Nothing” design-system foundation across the app: new monochrome/light+dark token set, updated typography (Space Grotesk / Space Mono / Doto), flattened primitives/overlays, a new fixed-rail shell, and shared Nothing-style loading + data-viz components.

Changes:

  • Re-key global theme tokens + fonts and propagate the look by re-skinning shadcn/Radix primitives.
  • Introduce Nothing-specific shared UI primitives (LoadingState, StatusText, SegmentedBar) and migrate loaders/viz to them.
  • Replace the prior sidebar framework with a custom fixed rail + mobile drawer layout.

Reviewed changes

Copilot reviewed 97 out of 98 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/components/ui/tooltip.tsx Restyles tooltip surface/arrow to Nothing popover styling.
src/components/ui/textarea.tsx Updates textarea typography, focus, and invalid-state styling.
src/components/ui/tabs.tsx Reworks tabs into underline style with mono-caps triggers.
src/components/ui/table.tsx Converts table chrome to mono headers, bordered rows, and Nothing spacing.
src/components/ui/switch.tsx Adjusts switch sizing/colors to match Nothing toggle treatment.
src/components/ui/status-text.tsx Adds inline status component + hook to replace toast patterns.
src/components/ui/sonner.tsx Flattens Sonner toast styling as transitional fallback.
src/components/ui/skeleton.tsx Removes skeleton primitive (loading now uses LoadingState).
src/components/ui/sheet.tsx Restyles sheet surfaces/borders/close button/title.
src/components/ui/select.tsx Restyles select and replaces checkmark with left red indicator bar.
src/components/ui/segmented-bar.tsx Adds segmented-bar visualization primitive.
src/components/ui/popover.tsx Restyles popover to flat bordered Nothing surface.
src/components/ui/loading-state.tsx Adds Nothing loading indicator + compact inline spinner.
src/components/ui/label.tsx Tweaks label text color to align with new tokens.
src/components/ui/json-viewer.tsx Removes multicolor JSON token styling in favor of monochrome tokens.
src/components/ui/input.tsx Updates input sizing/typography/focus/invalid styling for Nothing theme.
src/components/ui/form.tsx Strengthens form error message styling.
src/components/ui/dropdown-menu.tsx Flattens menu styling and updates item/label treatments.
src/components/ui/dialog.tsx Updates dialog overlay/surface/close/title styling to Nothing.
src/components/ui/command.tsx Restyles command palette to Nothing look and red-selection bar.
src/components/ui/checkbox.tsx Updates checkbox sizing/focus/disabled styling for Nothing.
src/components/ui/chart.tsx Re-themes chart container + tooltip to Nothing typography and surfaces.
src/components/ui/card.tsx Flattens card styling and updates title typography.
src/components/ui/calendar.tsx Applies mono label treatment and tokenized colors to calendar.
src/components/ui/button.tsx Rebuilds button variants/sizes to Nothing pill mono-caps system.
src/components/ui/badge.tsx Rebuilds badges as border-only tags with status variants.
src/components/ui/alert.tsx Converts alerts to border-only status boxes with warning variant.
src/components/ui/alert-dialog.tsx Restyles alert-dialog overlay/content/title to Nothing surfaces.
src/components/theme-toggle.tsx Replaces dropdown theme menu with segmented toggle control.
src/components/shared/spend-progress-bar.tsx Replaces continuous progress with Nothing SegmentedBar + ceiling tick.
src/components/reports/overview/what-changed.tsx Tokenizes severity styling (warning/destructive) for Nothing.
src/components/reports/overview/kpi-with-mom.tsx Updates KPI typography and delta badge variants.
src/components/reports/budget/budget-report.tsx Migrates dynamic chart skeletons to LoadingState.
src/components/data-table.tsx Updates table chrome (search icon, rounded container, mono pagination).
src/components/dashboard/viewer/my-usage-card.tsx Tokenizes savings color to text-success.
src/components/dashboard/viewer/identity-card.tsx Tokenizes stale warning callout to warning tokens.
src/components/dashboard/admin/admin-dashboard.tsx Tokenizes sync chip warning/success colors.
src/components/dashboard/admin/activity-timeline.tsx Tokenizes severity dot colors.
src/components/copilot/metrics-freshness-card.tsx Tokenizes stale warning styling to warning tokens.
src/components/copilot/activity-distribution.tsx Replaces pie chart with segmented-bar stat list.
src/components/claude/workspace-budget-list.tsx Tokenizes “big delta” warning color.
src/components/claude/users-month-picker.tsx Replaces “Loading…” text with InlineSpinner.
src/components/claude/user-kpi-strip.tsx Tokenizes success delta caption.
src/components/claude/top-users-bar-chart.tsx Switches palette to token ramp and removes bar rounding.
src/components/claude/sync-status-pill.tsx Converts pill to border-only, tokenized success/warning styles.
src/components/claude/org-credits-panel.tsx Tokenizes warning threshold color.
src/components/claude/kpi-strip.tsx Tokenizes success/warning tones.
src/components/claude/global-metrics-client.tsx Updates palette, loading indicator, grid, and bar rounding.
src/components/claude/daily-by-user-chart.tsx Updates palette, grid, and bar rounding.
src/components/claude/cumulative-pacing-chart.tsx Updates historical series colors to token ramp.
src/components/app-sidebar.tsx Replaces SidebarProvider-based shell with fixed rail + mobile drawer.
src/components/alert-banner.tsx Rebuilds alert banner as flat bordered Nothing status box.
src/app/users/loading.tsx Migrates route loader to LoadingState.
src/app/users/[id]/loading.tsx Migrates route loader to LoadingState.
src/app/tools/loading.tsx Migrates route loader to LoadingState.
src/app/tools/[id]/loading.tsx Migrates route loader to LoadingState.
src/app/settings/sync/page.tsx Updates Suspense fallback to LoadingState.
src/app/settings/license-templates/template-editor-dialog.tsx Tokenizes missing-variable warning box.
src/app/reports/loading.tsx Migrates route loader to LoadingState.
src/app/loading.tsx Migrates root loader to LoadingState.
src/app/layout.tsx Adds next/font Nothing font stack and replaces shell layout grid.
src/app/invoices/sync-results-dialog.tsx Tokenizes outcome badge colors for Nothing.
src/app/invoices/loading.tsx Migrates route loader to LoadingState.
src/app/copilot/seats/loading.tsx Migrates route loader to LoadingState.
src/app/copilot/seats/[userId]/page.tsx Tokenizes “Unmatched” warning text.
src/app/copilot/loading.tsx Migrates route loader to LoadingState.
src/app/copilot/billing/loading.tsx Migrates route loader to LoadingState.
src/app/copilot/analytics/loading.tsx Migrates route loader to LoadingState.
src/app/claude/page.tsx Updates Suspense fallback to LoadingState.
src/app/budget/loading.tsx Migrates route loader to LoadingState.
src/app/budget/[id]/loading.tsx Migrates route loader to LoadingState.
src/app/assignments/loading.tsx Migrates route loader to LoadingState.
src/app/(auth)/layout.tsx Restyles auth layout panel (dot-grid, brand mark, mono footer).
src/app/globals.css Introduces Nothing token system + font vars + nd-* helpers + spinner.
specs/028-nothing-design-redesign/README.md Adds spec README documenting design system goals/tokens.
specs/028-nothing-design-redesign/mockups/styles/theme.js Adds theme toggle script for static mockups.
specs/028-nothing-design-redesign/mockups/requests.html Adds Requests screen mockup.
specs/028-nothing-design-redesign/mockups/login.html Adds Login screen mockup.
specs/028-nothing-design-redesign/mockups/invoices.html Adds Invoices screen mockup.
specs/028-nothing-design-redesign/mockups/index.html Adds mockup gallery index.
specs/028-nothing-design-redesign/mockups/_CONTRACT.md Adds mockup authoring contract and component rules.
specs/028-nothing-design-redesign/implementation-notes.html Adds implementation log/notes page.
.claude/skills/nothing-design/SKILL.md Adds “Nothing design” skill documentation for Claude workflows.
.claude/skills/nothing-design/references/tokens.md Adds Nothing token reference doc.
.claude/skills/nothing-design/references/platform-mapping.md Adds platform mapping reference doc.
.claude/skills/nothing-design/references/components.md Adds component pattern reference doc.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +64 to +69
return (
<div
className={cn("flex w-full gap-0.5", SIZE_CLASS[size], className)}
role="img"
aria-label={ariaLabel}
>
Comment on lines +184 to +191
{mobileOpen ? (
<div className="fixed inset-0 z-50 md:hidden" role="dialog" aria-modal="true">
<button
type="button"
aria-label="Close navigation"
className="absolute inset-0 bg-black/80"
onClick={() => setMobileOpen(false)}
/>
import { requireAdmin } from "@/lib/auth-helpers";
import { redirect } from "next/navigation";
import { SyncDashboard } from "./sync-dashboard";
import { LoadingState, InlineSpinner } from "@/components/ui/loading-state";
CardTitle,
} from "@/components/ui/card";
import { Skeleton } from "@/components/ui/skeleton";
import { LoadingState, InlineSpinner } from "@/components/ui/loading-state";
Convert remaining page-level literal Tailwind tints to value-text-only tokens
across ~13 files: text-emerald/amber/green-* -> text-success/text-warning,
bg-amber-500 progress fills -> bg-warning, green success callouts -> token
borders on transparent bg, chart hex series -> greyscale chart tokens, and
routine month-over-month deltas neutralized (no red for ordinary up/down).
Remaining animate-pulse placeholders in the Claude detail clients -> LoadingState.

Anti-pattern gate now: 0 literal rainbow classes, 0 animate-pulse, 0 <Skeleton>,
0 shadow on ui primitives. typecheck + lint + build green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
studert and others added 2 commits June 2, 2026 14:26
- Toast popups eliminated: all 38 toast.* components converted to inline
  <StatusText>/useInlineStatus (success-on-navigate dropped, errors kept inline,
  aria-live preserved); global <Toaster/> removed from layout; sonner.tsx and
  the toast call in use-theme-preference removed/deleted. 0 sonner imports left.
- FormLabel now renders the Nothing instrument label (Space Mono ALL CAPS) so
  every RHF field caption matches the mockups (login EMAIL/PASSWORD, etc.);
  plain <Label> stays sentence-case for inline checkbox/switch labels.
- Native confirm() replaced with Nothing AlertDialogs in request cancel and
  ingestion-filter delete. 0 confirm() calls remain.

Anti-pattern grep gate: 0 toasts, 0 confirm(), 0 literal rainbow, 0 animate-pulse,
0 <Skeleton>, 0 shadow on chrome. typecheck + lint + build all green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s; record a11y audit

Verified in-browser contrast: light foreground 15.96 / destructive 4.76 /
success 4.70 / warning 5.35 (all AA normal-text). Dark success 6.34 / warning
9.48; brand red #d71921 at 4.05 on #000 (AA graphical/large-text, spec-locked).
Lighthouse accessibility 98. Implementation notes updated: toast removal,
mono FormLabels, and confirm()->AlertDialog all complete.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ic ramp, separators

The greyscale ramp made the Claude console's stacked daily-spend charts (by
workspace and by user) unreadable — 8-9 grey segments stacked blur together.
Now cap the stack at the top 4 ranks + an aggregated "Other" (≤5 segments),
colour by rank along a single monotonic --chart-1..5 ramp (no repeats, so the
legend reads as a gradient), and add a 1px --card separator stroke per segment
for crisp edges. Full per-row detail remains in the tooltip + the list below;
the "Use workspace colors" toggle still offers per-workspace hues.

typecheck + lint + build green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Top-10-users-by-cost bars defaulted to workspace display_color
unconditionally (off-system). Now default to monochrome ink (rank is shown by
bar length/order) with a "Use workspace colors" toggle in the card header —
mirroring the Workspaces tab. New TopUsersCard client wrapper owns the state and
persists it under the SAME localStorage key as the workspace daily chart, so the
preference stays in sync across both Claude tabs.

typecheck + lint green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ble headers)

P1's TableHead re-skin used `h-auto … pb-3` (bottom-only padding), so every
header label jammed against the top of the band instead of centering. Restore a
fixed `h-11` header height with align-middle. Also rewrite DataTableColumnHeader:
the sortable header was a full Button (leftover px-6 padding + h-8) that indented
and resized sortable columns vs plain ones — now a minimal inline button that
sets the mono/uppercase/tracking label style explicitly (a <button> doesn't
inherit text-transform/font from the <th>) and carries no padding, so sortable
and plain headers match and align flush with their column cells.

typecheck + lint green; verified on /tools and /users.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
WorkspaceModelBreakdown used a 6-colour spectral palette (purple/cyan/green/…)
for its proportion bar + legend dots — now a greyscale --chart-1..5 rank ramp
with 1px surface separators. WorkspaceDailyChart painted bars in the workspace
display_color (off-scheme tan); drop the `color` prop entirely, use monochrome
ink, square bar ends, and a horizontal-only grid. Updated both callers
(user-detail, workspace-detail). Fixes /claude/users/[id] and
/claude/workspaces/[id] to follow the Nothing scheme.

typecheck + lint green; verified on /claude/users/6.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Inactive nav items used text-faint (#666) at 11px — ~3.7:1 on the dark sidebar,
below WCAG AA and hard to read. Bump to text-muted-foreground (#999, ~7.4:1 dark
/ ~5.2:1 light) and 11px -> 12px. The active item still reads clearly via its
white text + 2px red bar. Same treatment for the Sign Out item. Covers desktop
rail + mobile drawer (shared NavLinks/Footer).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
From a 12-section read-only audit (77 findings, curated):
- Page titles: font-bold/semibold -> font-medium + tracking-tight + text-ink,
  consistently across all ~30 routes; settings page titles unified to text-3xl.
- Numeric values -> font-mono (Nothing numbers = Space Mono): copilot billing
  KPIs, overview-cards, kpi-strip, github-member-sync summary, reports
  at-a-glance/past-month, profile cost total, tool tier cost.
- Progress bars -> <SegmentedBar>: org-credits, workspace-budget rows,
  viewer my-usage model breakdown (continuous fills replaced w/ discrete segs).
- Anti-patterns: removed shadow-lg from auth cards; removed bg-destructive/10
  row tint on bulk-import (status stays on text/badge); bulk-import highlight
  text-primary -> text-ink.
- stat-tile: success tone text-primary -> text-success; rounded-lg -> 14px.
- sync-dashboard space-y-8 -> space-y-6.

Left intentionally (documented): budget-health multi-marker bar + past-month
plan-vs-actual comparison (bespoke monochrome comparison visuals, not simple
progress bars); status badges keep their success/warning text+border (the
Nothing tag treatment); circular avatars (match the sidebar).

typecheck + lint + build all green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
setLoadingTiers(false);
}
}, [assignment.tool.id]);
}, [assignment.tool.id, detailStatus]);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[high · correctness] Infinite re-render loop on the assignment detail page.

useInlineStatus() returns a brand-new object literal every render, so detailStatus in loadTiers's dependency array (line 135) makes loadTiers a new reference each render. The effect on line 137 depends on loadTiers, so it re-runs every render → setTiers/setLoadingTiers → re-render → loop. For an admin viewing an active assignment the page spins/hangs on mount.

Automated review — PR #108 final pass.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Fixed in 04cc048. useInlineStatus now returns a useMemo'd object (stable identity unless status changes — the methods were already useCallback'd), so detailStatus in loadTiers's deps no longer changes every render and the mount effect runs once instead of looping. typecheck/lint/build green.

aria-label="Budget alerts"
className="px-5 pt-6 sm:px-8"
>
<div aria-live="polite" className="sr-only">
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[high · a11y] Live region is unmounted, so a dismiss→reappear transition isn't announced.

The component early-returns null when there are no alerts or the fingerprint is dismissed, removing the aria-live region from the DOM. When new alerts later appear the region is freshly inserted with text — screen readers don't announce the initial content of a newly-added live region (only later mutations). Keep the live region always mounted.

Automated review — PR #108 final pass.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Fixed in 04cc048. The aria-live region is now always mounted (the early return null was removed; the banner is rendered conditionally while the sr-only live region persists), so a none→alerts transition reads as a content change and is announced.

toast.success("Invoice overwritten successfully.");
}
// Success navigates to /invoices — navigation is the feedback.
router.push("/invoices");
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[high · lost feedback] Overwrite drops the budget-link warning.

overwriteInvoice returns linkWarning (e.g. "No active budget period covers this invoice date. Previous budget link was removed.") and linkedPeriodLabel. The old code surfaced these via toast; the new path just router.push("/invoices"), so the admin is never told the invoice lost / has no budget-period link.

Automated review — PR #108 final pass.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Fixed in 04cc048. handleOverwriteDuplicate now surfaces result.linkWarning on the page-level status (info, no auto-clear) and stays on the page when there's a caveat; a clean overwrite still navigates to /invoices.

toast.success("Invoice saved to archive.");
}
// Success navigates to /invoices — navigation is the feedback.
router.push("/invoices");
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[high · lost feedback] Save drops the budget-link / filter warning.

saveInvoice returns linkWarning / filterWarning on success; the new onSubmit ignores them and navigates immediately, so budget-linking caveats are silently lost (the inline status is never set on the success path).

Automated review — PR #108 final pass.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Fixed in 04cc048. onSubmit now surfaces result.linkWarning ?? result.filterWarning inline (info, persistent) and stays on the page; a clean save navigates (navigation = feedback).

@@ -313,16 +310,10 @@ export function InvoiceUploadForm() {

const result = await saveInvoice(submitData);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[high · UX] No success confirmation before navigation.

After saveInvoice succeeds, onSubmit calls router.push with no inline feedback. On a slow network the user can't tell a pending save from a failed one. (Partly mitigated: the submit button is disabled while isSubmitting.)

Automated review — PR #108 final pass.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

↪︎ Mitigated / partly addressed (04cc048). The submit button is disabled while isSubmitting, so a double-submit can't occur, and the meaningful non-clean case (a budget-link caveat) now shows inline and keeps the user on the page. For a clean save, navigation to /invoices — where the new row appears — is the intended success feedback; an inline [SAVED] would unmount before it's read. Leaving the clean-success path as navigation.

try {
await persistPreferences(value);
} catch {
// Revert the optimistic theme change; the visual revert is the
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[high · feedback] Theme persist failure is swallowed.

On a failed updatePreferences the hook reverts the theme but surfaces no error — the returned error is dropped. Other surfaces show status.error(...); here the only signal is the theme visually snapping back.

Automated review — PR #108 final pass.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

↪︎ Acknowledged — intentional. The optimistic theme change + visual revert is the feedback (the toggle snaps back), and a persist failure is rare. Surfacing it as inline status would mean threading an error channel from the hook into every toggle consumer (sidebar + appearance page); that's out of scope for this presentation-only redesign (whose documented stance is no toasts) and noted as possible follow-up.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[medium · UX] Duplicate dialog can't be dismissed via ESC / backdrop.

onOpenChange={(open) => { if (!open) return; ... }} blocks the close transition, so only the explicit Skip/Overwrite buttons dismiss it. (Note: this block predates this PR — flagging for awareness.)

Automated review — PR #108 final pass.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

↪︎ Acknowledged — pre-existing, out of scope here. This onOpenChange guard isn't part of this PR's diff; it predates the redesign and intentionally forces an explicit Skip/Overwrite choice for the duplicate decision. Could be relaxed (ESC = skip) in a follow-up.

const result = await archiveBudget({ id });
if (result.success) {
toast.success("Budget archived");
router.refresh();
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[medium · feedback] Archive success has no inline confirmation.

The success path calls router.refresh() without archiveStatus.ok(), so StatusText stays idle. (The AlertDialogAction also closes the dialog on click, so a status set here wouldn't render — the list refresh is the de-facto feedback.)

Automated review — PR #108 final pass.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

↪︎ No change — AlertDialogAction closes the dialog on click, so a status.ok() set here wouldn't render. The list router.refresh() (the archived budget leaving the active list) is the de-facto confirmation; errors are still surfaced via archiveStatus.error() in the footer.

status.error(result.error);
}
}, [router]);
}, [router, status]);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[medium · perf] handleRevoke deps include the whole status object.

Because useInlineStatus() returns a fresh object each render, handleRevoke (and the memoized columns) is recreated every render, defeating the memo. Fix at the hook (memoize its return) rather than per call site.

Automated review — PR #108 final pass.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Fixed at the source in 04cc048useInlineStatus now returns a memoized object, so status is stable across renders (changes only when its value changes). handleRevoke and the memoized columns no longer churn every render.

status.pending("Starting sync");
try {
const result = await triggerSync("invoice_period_matching");
if (result.success) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[medium · consistency] Apply-from-dry-run leaves pending status unresolved.

handleApplyFromDryRun sets status.pending(...) but never calls ok/clear before router.refresh(). The reload masks it, but it's inconsistent with the other handlers.

Automated review — PR #108 final pass.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Fixed in 04cc048handleApplyFromDryRun now calls status.ok("Sync started") before router.refresh(), resolving the pending status consistently with the other handlers.

- useInlineStatus now returns a memoized object (stable identity unless status
  changes). Fixes an infinite re-render loop on the assignment detail page
  (detailStatus in loadTiers deps -> effect -> loop) and the columns-memo churn
  in assignments-client; the methods were already useCallback'd.
- invoice-upload-form: restore the dropped budget-link/filter warnings — save
  and overwrite now surface result.linkWarning / filterWarning inline (info)
  and stay on the page instead of silently navigating away.
- alert-banner: keep the aria-live region always mounted so a none->alerts
  transition is announced (a freshly inserted live region's initial content
  isn't reliably announced).
- scheduled-jobs apply-from-dry-run: resolve the pending status before refresh.

typecheck + lint + build green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@studert studert merged commit 763ead3 into main Jun 2, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants