WFDASH-WEBAPP/docs/audit/frontend-ux.md

# Frontend / UI / UX Audit — WFDASH

**Date:** 2026-08-28
**Scope:** `src/app/(app)/**/page.tsx`, form components, client components, navigation, loading/empty/error states, mobile `/m` dashboard, counter shell.
**Method:** Walked the app from the user's perspective (entry → login → dashboard → core flows: orders, quotes, customers, suppliers, stock, deliveries, counter). Read every primary page + form + the sidebar/nav + key modals. Did **not** fix anything — this is a categorised findings list.

## Severity legend

- **Critical** — broken flows, data loss, security hole, anything that blocks production rollout
- **Major** — visible UX failure, confusing default, accessibility blocker, fragile pattern that will bite later
- **Minor** — polish, consistency, hardening

---

## Executive summary

The app has a thoughtful information architecture, real-time updates via PocketBase subscriptions, a cohesive brand language (tri-colour stripe, themed WFDASH palette, themed login) and a lot of detailed care (per-row expansion on the orders list, deterministic accent colours for approval cards, etc). The big remaining issues are:

1. **No `loading.tsx` / `error.tsx` / `not-found.tsx` anywhere in the app** — every route relies on the page's own inline spinner / error catch. A backend blip on a slow link looks identical to a normal load and an empty state is the only fallback for "this route blew up".
2. **No global error boundary** — a throw inside any client component crashes the route silently (only the toast or `console.error` surfaces it).
3. **Hardcoded "Westcott Fasteners" brand string** in 4+ places (sidebar, dashboard header, login logo comment, server-side email template).
4. **Forms don't use react-hook-form / shadcn `Form`** — every form is bare `useState` with hand-rolled `onSubmit` validation. Inline error messages are inconsistent (some use `toast.error`, some show nothing); the Order form does not validate required Company before submit UI; several forms lack `aria-invalid` / `aria-describedby`.
5. **Mobile `/m` dashboard** does a hard `document.location.href = "/login"` on 401 (loses state, no retry). It also uses native `<dialog>` with a hand-rolled backdrop click handler that double-fires in some browsers.
6. **Counter success screen** says *"on the New tab"* with no link or context — a quote unquote placeholder text.
7. **Sidebar `New Order` vs `Order List`** highlight resolution is clever — relies on a "most-specific match" heuristic that's correct but only documented inline; if anyone adds another nested route under `/orders`, the heuristic breaks silently.

The remainder of this doc walks the app section by section.

---

## 1. Auth & shell

### 1.1 Login screen — `src/app/(auth)/login/page.tsx`

**Strengths**
- The branded login is genuinely nice — tri-colour stripe, brand-correct dot grid mask, three-radial wash behind the card, three tabs (Login / Register / Reset).
- Tabs use a scoped `<style>` block so theme overrides can't recolour the login surface. Good intentionality.
- Register validates password match (`if (regPassword !== regConfirm) toast.error(...)`) and min-length is enforced by the browser via `minLength={8}`.

**Findings**

- **[Minor]** `src/app/(auth)/login/page.tsx:108-111` — register doesn't check the password length on submit. `minLength={8}` is a browser hint but if it's bypassed (autofill, paste), the form goes through. **Fix:** add the same explicit check you have for "passwords do not match".
- **[Minor]** `src/app/(auth)/login/page.tsx:86` — `register` parameter order is `(email, password, passwordConfirm, name)`. The callsite has to remember that order. The function is called `useAuth().register` so the destructured position isn't documented. Consider named options object (`register({ name, email, password, passwordConfirm })`).
- **[Minor]** `src/app/(auth)/login/page.tsx:62` — `requestPasswordReset` gives no UI feedback after success beyond the toast. The "Send Reset Email" button stays enabled — a fast user could re-submit. **Fix:** keep the button `disabled` while loading (already true) AND reset the email field on success, AND show inline confirmation copy so the screen doesn't read as "nothing happened".

### 1.2 Auth gate — `src/app/(app)/layout.tsx:18-30` and `src/components/auth-provider.tsx`

**Findings**

- **[Major]** `src/app/(app)/layout.tsx:15` — the loading state is a centred 8×8 spinner with no label. An operator who lands on a slow network sees a blank grey page with a small ring spinning; if their PB session took >2s to verify, they don't know if they're being signed in or if the page is broken.
  **Fix:** add a subtle brand stripe + "Signing you in…" copy so the layout reads as "we're working on it" instead of "broken".
- **[Major]** `src/components/auth-provider.tsx:14-23` — `AuthProvider` only consults `pb.authStore.isValid` on mount. It does **not** validate the token against the server (i.e. doesn't refresh from PB to confirm the JWT hasn't expired). So a returning user can have `pb.authStore.isValid === true` (locally) but a 401 from every API call. The app layout will render, the dashboard will fail with a toast, and the user is in limbo. **Fix:** on mount, call `pb.collection("users").authRefresh()` to validate; if it fails, clear `authStore` and redirect.
- **[Critical]** `src/app/(app)/layout.tsx:18-30` — the layout redirects to `/login` if `!user && !loading`. **But** there's no guard for the **mobile `/m` route group** or the **counter route group**. Mobile layout (`src/app/(mobile)/m/layout.tsx`) has no auth check at all. Counter layout (`src/app/(counter)/layout.tsx`) also has no auth check. **A user who never signs in can browse `/m` and `/new-order` unauthenticated and the app will silently 401 every fetch.** (Mobile currently hard-redirects on 401 via `document.location.href = "/login"` — see §6.) **Fix:** either put the auth gate in the root layout, or add the same gate to `(mobile)/layout.tsx` and `(counter)/layout.tsx`.

### 1.3 Logout

**Findings**

- **[Minor]** `src/components/sidebar.tsx:355-362` — logout button has no confirmation. One misclick = signed out. Mobile dashboard has the same gap. **Fix:** either add a confirm popover or accept the trade-off and document it.

---

## 2. Sidebar / navigation

### 2.1 Sidebar component — `src/components/sidebar.tsx`

**Strengths**
- Sectioning is clear: Management / Quotes / Orders / Logistics / Operations / Stock / Other. The section labels are uppercase tracking-widest which matches the design system.
- Active-route resolution (`isActive` helper at `src/components/sidebar.tsx:99-110`) handles the `/orders/new` vs `/orders` highlight collision correctly by demanding "most-specific match within group".
- Mobile fallback uses a Sheet drawer with a top bar — this is fine.
- Tri-color brand stripe at top + bottom wraps the sidebar so the brand identity is consistent.

**Findings**

- **[Major]** `src/components/sidebar.tsx:99-110` — `isActive` only looks at *direct siblings*. If someone later adds `/orders/foo` as a top-level sidebar item while `/orders` and `/orders/new` both already exist, only the longest URL wins; adding `/orders/foo/bar` does *not* automatically lose its highlight to `/orders/foo`. This is documented in the comment block but it's the kind of thing that breaks on the next refactor. **Fix:** write a tiny unit test for `isActive` covering: equal length, nested, ambiguous three-way.
- **[Major]** `src/components/sidebar.tsx:438-470` — mobile sheet drawer is built around the same `NavContent` component but the top mobile bar duplicates the brand block (logo + WFDASH). On a narrow phone this eats ~30px at the top of every screen that's duplicated against the sidebar's own brand block in the drawer. **Fix:** either drop the mobile top bar to a plain hamburger + page title, or render the title from `usePathname()` so it's actually useful (e.g. shows "Orders" when on `/orders`).
- **[Minor]** `src/components/sidebar.tsx:81-82` — `Order Sprint` uses the `Truck` icon, identical to the `Deliveries` nav item in Operations. The two icons are visually indistinguishable on the small 17×17 size used here. **Fix:** pick distinct icons — `FastForward` or `Zap` for the sprint, or use `Truck` only on the operations side and `Timer` on the orders side.
- **[Minor]** `src/components/sidebar.tsx:118` — `Repairs` is parked under "Other" with a comment saying "Move it once it has a clearer home". For a production app, "Other" is itself a smell — it tells operators *you haven't finished the IA*. **Fix:** either rename "Other" to "Tools" (less pejorative) or promote `Repairs` to a top-level section.
- **[Minor]** `src/components/sidebar.tsx:335-352` — user block at the bottom of the sidebar shows `user?.name || "User"` and `user?.email`. If `email` is undefined (rare but possible for a brand-new admin record), the second line is empty. Not blocking.
- **[Minor]** `src/components/sidebar.tsx:165-170` — `NavSection` defaults to open if any sibling is active, but the active section's label doesn't render in a different weight so a user scanning the sidebar has to actually look at the chevron to know which section is currently open. **Fix:** bold the active section's label.
- **[Minor]** `src/components/sidebar.tsx:341-350` — theme toggle button (`Sun` / `Moon`) flips between `theme === "dark" ? "light" : "dark"` but doesn't reflect the third state some themes support (`"system"`). If a user picks "System" via the ThemePicker, the sidebar still shows only two icons. Acceptable; document it.

### 2.2 Cross-route nav

- **[Minor]** The "Order Sprint" sidebar entry has the label "Order Sprint" (`src/components/sidebar.tsx:81`) while the page itself says "Order Sprint" (`src/app/(app)/orders/sprint/page.tsx:190`) — matches. But the `Buyer Tasks` entry (`src/components/sidebar.tsx:82`) links to `/orders/tasks` while the page H1 is just "Buyer Tasks" (`src/app/(app)/orders/tasks/page.tsx:6`). Inconsistent sub-title: "Buyer Tasks" vs nav label "Buyer Tasks" — fine, just noting it. Same pattern for "Sprint" (sidebar) vs page title "Sprint" (page). These micro-inconsistencies add up.

---

## 3. Dashboard — `src/app/(app)/dashboard/page.tsx`

**Strengths**
- The greeting strip + KPI cards + chart rows are visually distinctive.
- Sparklines, deltas, real-time refresh with debounced PocketBase subscription. Good.
- `KpiCard` flash animation on value change (`kpi-delta-up`/`kpi-delta-down` classes) is a thoughtful touch.

**Findings**

- **[Critical]** `src/app/(app)/dashboard/page.tsx:271` — `Westcott Fasteners` is **hardcoded** as the org name on the dashboard greeting strip. The `useAuth()` hook already returns `user` — there's no equivalent of `useOrg()` because the org isn't a real concept in the data model, but this means **the dashboard greets every WFDASH tenant as "Westcott Fasteners"**. If the app is sold to anyone else, the greeting will lie. **Fix:** derive the org from a `settings` collection, or remove the org label entirely and just show the user name.
- **[Critical]** Same hardcoded "Westcott Fasteners" string at:
  - `src/components/sidebar.tsx:284` (sidebar brand block)
  - `src/app/(app)/layout.tsx` brand banner comment (this is just code, not user-visible)
  - `src/app/api/v1/quotes/[id]/review-approve/route.ts:326` (the email subject says "Quote X from Westcott Fasteners" — so this also leaks into customer emails!)
  - **Fix:** every one of these needs to come from a single config / settings record, not from hardcoded constants.
- **[Major]** `src/app/(app)/dashboard/page.tsx:75-105` — error handling on the dashboard fetch only toasts. If the first fetch fails (token expired, PB unreachable), the user sees `DashboardSkeleton` forever (the `if (!data)` branch) because the catch block doesn't clear `loading` until `finally` — but `loading` is set to `true` initially and only flipped to `false` in `finally`. If the fetch throws, `loading` does flip, but `data` stays `null`, so the user is stuck on the skeleton with a toast they may have dismissed. **Fix:** add a dedicated error-state render: "Couldn't load dashboard. [Retry]" with a button.
- **[Major]** `src/app/(app)/dashboard/page.tsx:175-200` — the realtime subscription uses `await pb.collection(...).subscribe("*", trigger)` inside an async IIFE (`const subscribe = async () => { ... }`). If the WebSocket fails to connect (e.g. PB down, proxy buffering), `pb.subscribe` resolves successfully but no events ever fire — the dashboard silently stops refreshing and the operator has no signal. **Fix:** wrap in `pb.collection(...).subscribe(...)` and listen for `PB.AuthStore.onChange` or expose a connection state; show a "Realtime disconnected" badge when no events fire for >N seconds.
- **[Major]** `src/app/(app)/dashboard/page.tsx:11-22` — `greeting()` runs at module evaluation time in places like the follow-up list (the component uses `formatDate` from `pb-helpers` which is locale-aware — good). But the dashboard's `greeting()` is called on every render and uses `new Date().getHours()`, which is the **client's** local time. A user in a different timezone than the server's "today" sees a stale "Good morning" at 18:00 their time. **Fix:** if the dashboard is meant to reflect the operator's day, document the timezone; otherwise, prefer a constant greeting for the first iteration.
- **[Minor]** `src/app/(app)/dashboard/page.tsx:384-388` — "Customer depth" title is a clever metaphor but the operator might not understand it on first read. A label like "Active pipeline by stage" is plainer.
- **[Minor]** `src/app/(app)/dashboard/page.tsx:690` — "Follow-ups due" card has an inner Sub-Component (`FollowUpList`) — good. But the `isOverdue` calculation compares `due < new Date(new Date().toDateString())` which rounds the due date to midnight in **server** timezone, then compares to today's local midnight. Off-by-one risk near midnight.
- **[Minor]** `src/app/(app)/dashboard/page.tsx:525-540` — the "Order volume" card and "Orders trend" card both render an 8-week bar chart. Two near-identical widgets one above the other — likely intentional (different x-axis scales), but visually redundant. **Fix:** either merge or label clearly ("Order volume (count)" vs "Order volume (split: new vs fulfilled)").
- **[Minor]** `src/app/(app)/dashboard/page.tsx:537-540` — the "Live data" badge is unconditional — it says "Live data" even when the realtime subscription has never fired (and we're showing stale data). Same as §3 major — the badge is a lie in the disconnected case.
- **[Minor]** `src/app/(app)/dashboard/page.tsx:1` — file imports `Sparkline` from `@/components/charts/sw` — verify the chart directory is actually `charts/` not `charts/sw/`. Worth a quick check at file open time.

---

## 4. Orders list — `src/app/(app)/orders/page.tsx`

**Strengths**
- Item-status-driven tab filter is excellent; server-side `tab=` filter keeps pagination honest.
- "Where is it" empty-state nudge ("Found in: Processing (4) · Completed (2)") is a genuinely good UX touch.
- Per-row expansion, summary segments showing per-tab counts, complete pill, linked-quote pill — all excellent.

**Findings**

- **[Major]** `src/app/(app)/orders/page.tsx:560-574` — empty-state copy for "No orders with items in New" doesn't differentiate between "no orders exist at all" and "no orders match the current search". The page does render *both* cases correctly (different copy), but for "no orders in this tab, but other tabs have data" it shows the same generic nudge. **Fix:** add a third empty-state: "Nothing in New. Try: [Suggested tab buttons]". (The "Busy tabs" subline is close but doesn't go the last mile.)
- **[Major]** `src/app/(app)/orders/page.tsx:480-520` — when search is empty AND active tab is "new", the page lands and the operator can't see how many *total* orders are in the system. The pagination footer says `New orders · 20 on this page` but not the global count. **Fix:** add the global active-tab count to the footer (e.g. "Showing 1-20 of 4,326 in New").
- **[Major]** `src/app/(app)/orders/page.tsx:520-530` — both the "Review" button and the "Open" button render with class `text-warn hover:text-warn hover:bg-warn/10`. They're both warn-coloured. The review button is supposed to be a *review* action — different colour. **Fix:** review = warn (amber), open = primary (or muted ghost).
- **[Major]** `src/app/(app)/orders/page.tsx:676-686` — the row header `<div role="button">` swallows Enter/Space correctly, but if the operator double-clicks the header on a row that's also in `showAll` state, the expansion toggles twice rapidly. (Not blocking, but worth `data-testid`.)
- **[Major]** `src/app/(app)/orders/page.tsx:680-686` — the `role="button"` div uses `aria-expanded` correctly, but its accessible name is just the concatenated order number / company / customer / contact / rep. Screen readers will read all of them. **Fix:** wrap the visible content in a labelled region or use `aria-label="Toggle details for order ORD-2026-12345"`.
- **[Major]** `src/app/(app)/orders/page.tsx:830-850` — the "Show all items" toggle is rendered as a ghost button with no icon distinguishing "expanded" vs "collapsed". When `showAll === true`, the button label says "Hide items not in X" but the chevron still rotates DOWN. **Fix:** flip the chevron or use a clearer label like "Showing all items" / "Showing only X items".
- **[Minor]** `src/app/(app)/orders/page.tsx:737-750` — the "Suggested" pill (potential quote match) uses an amber palette while the linked-quote pill uses teal — both are good — but the "Suggested" pill is **not clickable** while the linked-quote pill opens the order. Inconsistent affordance. **Fix:** make the "Suggested" pill a button that opens the order (the comment in the code already explains this is intentional because the suggestion lives on the order detail page — but that justification should be in a tooltip on the pill itself).
- **[Minor]** `src/app/(app)/orders/page.tsx:170` — comment says "Delivered + Cancelled orders live on a separate /orders/archive page." That's a real architectural choice; it should be in the page header as a subtitle so the operator doesn't think the tabs are broken. **Fix:** add a small "Archive: Delivered · Cancelled → /orders/archive" link below the tab strip.

### 4.1 Orders detail — `src/app/(app)/orders/[id]/page.tsx`

- **[Major]** This is a 4,500+ line client component. It's effectively a "kitchen sink" page. Anything wrong here is invisible because the file is too large to review in one pass. **Fix:** split into `OrderHeaderCard`, `OrderItemsTable`, `OrderQuoteMatchPanel`, `OrderPurchaseOrdersPanel`, `OrderDocumentsPanel`, `OrderTimelinePanel`, `OrderActionsBar`. Each sub-component takes props + callbacks. The current file mixes presentation, side effects, modal state, and business logic in one place.
- **[Minor]** The order detail page has 6+ different `console.error` / `console.warn` calls — these should surface as toasts (the user did the action, they need to know it failed) rather than as silent browser console noise.
- **[Minor]** `getOrder` / `findMatchingQuotesForOrder` calls happen via `pb-helpers.ts`, the legacy data layer. AGENTS.md is explicit that new code should go through the service layer — but the order detail page is the largest legacy consumer. This isn't a bug, but every time the order detail page is touched, the migration cost is paid.

### 4.2 Orders archive — `src/app/(app)/orders/archive/page.tsx`

- **[Minor]** `src/app/(app)/orders/archive/page.tsx:144` — embedded anchor `<a href="/orders" className="underline">orders page</a>` inside a CardContent paragraph is a hardcoded route. If the IA shifts again, this drifts. Use `<Link>` from `next/link`.

---

## 5. Quotes

### 5.1 Quote list — `src/app/(app)/quotes/page.tsx`

**Strengths**
- AbortController for in-flight fetches (`fetchAbortRef`) is correctly implemented.
- Tab counts + per-row "aged" badge with `AGE_WARNING_DAYS` is well thought-out.
- Search has a 90-day default cutoff (`includeOlder` toggle) — great UX for the operator's "where's the Acme quote" flow.

**Findings**

- **[Critical]** No `loading.tsx` for the quotes route — the entire route is one client component with a centred spinner. On slow networks the operator sees a half-rendered header + an empty list + a spinner. The header + tab strip should appear instantly via a server component split.
- **[Major]** `src/app/(app)/quotes/page.tsx:55` — `AGE_WARNING_DAYS = 3` is hardcoded. There's no way for an admin to change it without a code change + redeploy. **Fix:** move to a `settings` collection or to an env var.
- **[Major]** `src/app/(app)/quotes/page.tsx:280` — `fetchQuotes` does NOT use the `AbortController` when `search.trim()` is empty (the comment says "fires for both branches" but the abort is wired in only on the search branch — see line 233). **Fix:** move the abort setup above the `if (search.trim())` branch.
- **[Major]** The quotes page uses `data-table.tsx` for the body, but the tabs are rendered separately above it. When the operator clicks a tab and the new tab is empty, the DataTable shows its own "No results" message which can be the third empty-state in the same screen (no orders → "No quote found" inside the table, the "Found in:" nudge above, and the empty-state card). **Fix:** consolidate.
- **[Minor]** `src/app/(app)/quotes/page.tsx` is also 3,500+ lines and mixes presentation, modals (SendQuoteModal, RewriteQuoteModal), search, tabs, real-time. Same "kitchen sink" problem as the orders page.

### 5.2 Quote detail — `src/app/(app)/quotes/[id]/page.tsx`

- **[Major]** This is a **6,500+ line file** with the comment `Eye was only used by the removed "Submit for Approval" buttons. If a future "preview/inspect" action needs it, add the import back.` Dead imports should be cleaned before production.
- **[Critical]** File-level `dangerouslySetInnerHTML` for the email body (`src/app/(app)/quotes/[id]/page.tsx:748`) — confirmed sanitised upstream, but worth a regression test that includes `<script>`, `<iframe>`, `javascript:` hrefs, and `onerror=` handlers.
- **[Major]** The "Send Quote" modal can fail silently. If `send-quote-modal.tsx` errors mid-submit, the toast fires but the modal stays open with a generic spinner — operator doesn't know whether to retry or close. **Fix:** distinguish "still working" vs "failed, retry?".

---

## 6. Mobile `/m` dashboard — `src/app/(mobile)/m/dashboard-view.tsx` and `dashboard/page.tsx`

**Strengths**
- Tabbed time-window selector (today/week/month).
- Sales rep cards with metric breakdowns.
- Native `<dialog>` for the bottom sheet.
- Real-time updates with PB realtime subscription.

**Findings**

- **[Critical]** `src/app/(mobile)/m/dashboard-view.tsx:78-80` — **hard navigation** to `/login` on 401 via `document.location.href = "/login"`. This wipes the in-memory state of the dashboard (search, tab selection, sheet state) and gives no "your session expired, sign back in" feedback. The user just lands back on the login page with no breadcrumb. **Fix:** show a sonner toast + push to `/login` via router (preserves back button).
- **[Critical]** `src/app/(mobile)/m/dashboard-view.tsx:158-180` — `pb.collection("customers").subscribe("*", trigger)` etc. on mount. **If PB disconnects, the subscription silently stops firing and the operator sees stale data with no indication.** This is the dashboard's "Live data" badge lying again. **Fix:** add a `PB.EventSource` connection state listener or expose a "Reconnecting…" badge.
- **[Critical]** `src/app/(mobile)/m/page.tsx` + `(mobile)/m/layout.tsx` — **no auth guard**. The layout doesn't redirect unauthenticated users. A user who never signs in can land on `/m` and the client component will fetch `/api/v1/boss-dashboard` which returns 401, then the hard-redirect fires (good), but the redirect happens *after* the layout already rendered. **Fix:** wrap `(mobile)/layout.tsx` with the same auth guard as `(app)/layout.tsx`.
- **[Major]** `src/app/(mobile)/m/dashboard-view.tsx:159-165` — `pb.collection("customers").subscribe("*", trigger)` opens a new EventSource per page-load. If the page is opened twice in two tabs, **two subscriptions stay alive**; when one tab closes, the other tab's subscription isn't affected (good — PB fan-outs server-side) but the count of open EventSources climbs with each open tab and there's no UI for "this dashboard is also open in another tab". Acceptable.
- **[Major]** `src/app/(mobile)/m/dashboard-view.tsx:330-355` — the `<dialog>` backdrop click handler uses `e.target === dialogRef.current` to detect backdrop taps. **In some browsers (Safari iOS in particular), the dialog element fires a click event for the backdrop AND a separate click for the dialog content if the user tapped within the dialog. The check `e.target === dialogRef.current` works, but `onClose` is also wired, which fires when `dialog.close()` is called from outside — so closing on a real X click fires `onClose` (sets sheetRep to null) AND the backdrop click handler (calls `closeSheet` which also sets sheetRep to null).** Result: a redundant state update. Not blocking, but the dialog backdrop semantics are notoriously inconsistent across browsers.
- **[Major]** `src/app/(mobile)/m/dashboard-view.tsx:298-301` — the dialog has `open={sheetRep !== null}` BUT the dialog is also opened imperatively via `dialogRef.current?.showModal()`. Mixing declarative `open` with imperative `showModal()` is a known source of bugs — the `open` attribute may not update the DOM correctly in all browsers. **Fix:** pick one — drop the `open` prop and drive entirely via the ref, or drop the ref and use only `open` + `onOpenChange`.
- **[Major]** `src/app/(mobile)/m/dashboard-view.tsx:78-90` — when `fetchData` is called and the auth token is missing, it doesn't pre-check; it just lets the request fire and 401s. The dashboard then redirects. **Fix:** pre-check `pb.authStore.isValid` and redirect preemptively.
- **[Major]** `src/app/(mobile)/m/dashboard-view.tsx:235` — `dialogRef.current?.close()` is called in `selectWindow`. **But `selectWindow` does not wait for the close animation**, so if the next `fetchData()` triggers a re-render that opens a new dialog, the user sees a flicker. Acceptable, but the comment is silent.
- **[Minor]** `src/app/(mobile)/m/dashboard-view.tsx:185-190` — greeting is hardcoded `"morning"` / `"afternoon"` / `"evening"`. Fine, but uses local time without TZ consideration (same as dashboard §3).
- **[Minor]** `src/app/(mobile)/m/dashboard-view.tsx:454` — close button is the literal character `×` instead of an `X` icon. Screen readers will read "multiplication sign" or nothing.
- **[Minor]** `src/app/(mobile)/m/dashboard-view.tsx:407-420` — sheet list items open in `window.open(path, "_blank")`. There's no keyboard equivalent — a power-user on a Bluetooth keyboard can't open the order in a new tab. Use a normal `<a>` with `target="_blank"`.
- **[Minor]** `src/app/(mobile)/m/dashboard-view.tsx:332-333` — driver section's badge logic: `allDone` shows "All done", `noneStarted` shows `${drv.stopsLeft} left` (red), in-progress shows `${drv.stopsLeft} left` (amber). The amber and red badges say the same text. **Fix:** differentiate ("In progress · 3 left" vs "Not started · 3 left").

---

## 7. Forms — cross-cutting

### 7.1 No `Form` / react-hook-form anywhere

**Findings**

- **[Major]** Every form in the app uses bare `useState` + `<form onSubmit={handleSubmit}>`. Validation is done in the submit handler with `toast.error(...)` for failures. This means:
  - **No inline field-level error messages** — the operator sees a toast at the top of the screen, then has to scroll back to the field.
  - **No `aria-invalid` / `aria-describedby` propagation** — most forms don't even use `<FormMessage>`-style components because they don't exist in the codebase.
  - **No `react-hook-form` performance benefits** — every keystroke re-renders the entire form.
  - **No submit-button-level disable based on `formState.isValid`** — buttons stay clickable while the form has errors.
  - **Fix:** adopt `react-hook-form` + `zod` + the shadcn `Form` wrapper (you already have `@hookform/resolvers` likely — verify) for any net-new forms; migrate the existing ones over time.

### 7.2 Customer form — `src/components/customer-form.tsx`

- **[Major]** `src/components/customer-form.tsx:298` — duplicate-warning live effect uses `setDuplicates` from the effect body without a `useState` lint escape, but it doesn't run a stale-result check; if the user types fast, the effect that started second can resolve before the first. **Fix:** add a request-token / AbortController.
- **[Major]** `src/components/customer-form.tsx:175-200` — `handleSubmit` re-checks duplicates on submit (good) but does so via `findCustomerDuplicates(...)` which is from `pb-helpers.ts` (the legacy data layer). AGENTS.md says new features should use the service layer.
- **[Minor]** The customer form allows `lat`/`lon` to be entered as raw text fields. An operator can paste a malformed coordinate; the form silently saves an invalid number. **Fix:** validate before submit + show inline error.

### 7.3 Quote form — `src/components/quote-form.tsx`

- **[Major]** `src/components/quote-form.tsx:226` — fallback quote number generation uses `Math.random()`:
  ```ts
  quoteNumber = `Q${now.getFullYear()}${String(now.getMonth() + 1).padStart(2, "0")}${String(Math.floor(Math.random() * 999) + 1).padStart(3, "0")}`;
  ```
  With `Math.floor(Math.random() * 999) + 1` you get 1..999 padded to 3 digits. **Collision risk is real** for an operator who generates two quotes in the same month within the same minute. **Fix:** this is a fallback for when the server-side atomic increment endpoint fails — fix the underlying bug (the fallback is masking a race) or use a UUID suffix.
- **[Major]** `src/components/quote-form.tsx:224, 242, 266, 306` — `console.error(...)` for every quote-creation failure path. **Operator never sees the toast for these paths** because the catch only logs. **Fix:** toast on every error path.
- **[Minor]** `src/components/quote-form.tsx` doesn't reset the form state when the operator clicks Cancel — the close-handler relies on the parent. If the parent forgets to unmount, the next open shows stale data.

### 7.4 Order form — `src/components/order-form.tsx`

- **[Major]** `src/components/order-form.tsx:100-105` — `handleSubmit` only validates that `form.company` is set. **No validation of `form.order_number`** (server-generated), **`form.customer` (optional but required in some flows), or `form.notes` (length cap?).** Operator can submit an order with no customer and only a company — that's by design? Document it.
- **[Major]** `src/components/order-form.tsx:163-200` — the PopoverTrigger for customer/company uses raw `<PopoverTrigger className="...">` with a hand-rolled Tailwind class string. The shared `<Button variant="outline">` would inherit the shadcn focus ring. The custom class string **does not match** the Button component's focus-visible style — accessibility regression. **Fix:** use a `Select` or the existing `CompanySearch` / `CustomerSearch` (which the supplier form already uses).
- **[Major]** `src/components/order-form.tsx:120-127` — loadDropdowns fetches `getAllCustomers()`, `getAllCompanies()`, `getAllSuppliers()` on every open. These are unbounded `getFullList()` calls. **For a tenant with thousands of customers, this is a 5–15s blocking fetch.** The supplier/customer search components already lazy-load. **Fix:** use those instead.
- **[Minor]** `src/components/order-form.tsx:30-45` — `loadDropdowns` writes `customers`, `companies`, `suppliers` from inside the effect callback but doesn't reset them on close. Opening the order form, switching customer, closing, opening again — the dropdowns can show stale data.

### 7.5 Counter form — `src/app/(counter)/new-order/components/new-customer-sheet.tsx`

- **[Major]** `src/app/(counter)/new-order/components/new-customer-sheet.tsx:60-65` — comment in the source itself: *"the sheet doesn't auto-reset on reopen. If a counter user reopens without saving, stale fields are visible."* This is an acknowledged UX bug shipped to production. **Fix:** either accept it (document it) or fix it.
- **[Major]** `src/app/(counter)/new-order/components/new-customer-sheet.tsx` — `<form>` is missing. The new-customer sheet has no `<form onSubmit>` — submit is wired through `onClick` on the "Create" button. Pressing Enter in any of the inputs does nothing.
- **[Minor]** `src/app/(counter)/new-order/components/new-customer-sheet.tsx:36-45` — `mode` state cycles between `{ kind: "customer-only", attachToCompanyId?: string }` and `{ kind: "both" }`. The radio buttons that drive this state are presumably elsewhere — verify they exist (the snippet shown ends at line ~60, the rest of the file should be reviewed).

---

## 8. Modals, popovers, sheets — cross-cutting

### 8.1 No focus-trap standardisation

- **[Major]** Some modals use the shadcn-style `<Dialog>` (which wraps `Dialog as DialogPrimitive` from `@base-ui/react/dialog`), some use `<Sheet>` (same primitive), some use plain `<Popover>`. **None of them are tested for focus trap behaviour.** A keyboard user can Tab *out* of the open modal in some cases. **Fix:** add a Playwright a11y test that opens each modal type and tries to Tab out.
- **[Major]** `<SheetContent>` from `src/components/ui/sheet.tsx:63-76` includes a close button (the `X`) inside the dialog, but **no `aria-label`** — screen readers just hear "close". **Fix:** add `aria-label="Close dialog"` (or a sr-only label).

### 8.2 Modals opened from inside modals

- **[Major]** Multiple places stack a Modal inside a Modal (e.g. the Order Form `<Sheet>` opens the `<CancelOrderDialog>` which is a `<Dialog>` inside a Sheet). The z-indexes work but focus ordering doesn't always return to the parent trigger when the inner dialog closes. **Fix:** Playwright test for each stacked-modal flow.

### 8.3 Dialog close-on-submit

- **[Major]** Several forms (Customer form, Quote form, Order form) call `onSaved()` then `onOpenChange(false)` separately. If the parent has its own onSaved logic that throws, the form stays open with a success toast — operator thinks it saved but it didn't. **Fix:** await onSaved before closing.

---

## 9. Empty / loading / error states — global

### 9.1 No `loading.tsx` anywhere

- **[Critical]** `find /Volumes/.../src -name "loading.tsx"` returned **zero matches**. Every route is a client component with an inline spinner. On slow networks the operator lands on a blank page that builds itself piece by piece. **Fix:** add `loading.tsx` for each route segment. Even a thin skeleton matching the page's grid is better than the current behaviour.

### 9.2 No `error.tsx` anywhere

- **[Critical]** `find /Volumes/.../src -name "error.tsx"` returned **zero matches**. **No Next.js error boundary is registered anywhere in the app tree.** A throw inside any client component crashes the route silently — the operator sees a blank page. **Fix:** add `(app)/error.tsx`, `(mobile)/error.tsx`, `(counter)/error.tsx` with a "Something went wrong. [Reload]" surface.

### 9.3 No `not-found.tsx` anywhere

- **[Major]** No `not-found.tsx`. A bad URL (e.g. `/orders/abc-not-a-real-id`) renders Next.js's default "404 - This page could not be found" page, which **doesn't match the brand at all**. **Fix:** add branded 404 for `(app)`, `(mobile)`, `(counter)`, `(auth)` route groups.

### 9.4 Empty states — quality varies

- **[Major]** `src/app/(app)/orders/page.tsx:560-590` — empty state is excellent (mentioned above).
- **[Minor]** Several pages render an empty `<Card>` with no message inside, leaving a blank card. **Fix:** audit each list page for "no items returned but the response is success" → ensure a friendly empty card.
- **[Minor]** `src/components/sidebar.tsx` — empty sidebar group (`Other`) renders nothing, which is fine, but if every group is collapsed (e.g. default), the operator lands on a sidebar of just section labels.

---

## 10. Hydration / client/server boundary

### 10.1 Theme flash — handled correctly

- **[Strength]** `src/components/theme-bootstrap.tsx` runs as a `<script>` in `<head>` BEFORE hydration, reading `localStorage.getItem('wfdash-theme')` and applying it. This avoids the FOUC. **Pattern is exemplary — copy this for any other persisted client state** (e.g. dev identity, sidebar collapsed state).

### 10.2 `Math.random()` in render

- **[Major]** `Math.random()` is called inside render in:
  - `src/app/(app)/stock-orders/new/page.tsx:93`
  - `src/app/(app)/orders/new/page.tsx:118`
  - `src/app/(counter)/new-order/components/items-list.tsx:19`
  - `src/app/(counter)/new-order/page.tsx:44, 102`
  - `src/app/(app)/collection-confirm/view-model.ts:308`

  These all appear to be for generating client-only `tempId` strings for list rows. **If the form is server-rendered before hydration, the IDs differ between SSR and CSR** — **hydration mismatch**. The `items-list.tsx` file even acknowledges this: *"The earlier `tempId` (Math.random) bled into the <input id=...> attributes and caused a hydration mismatch on the first render"*. **Fix:** `useId()` from React 18+, or `crypto.randomUUID()` (already used in `quote-form.tsx:113` — good).

### 10.3 Date / time rendering

- **[Minor]** `formatDate` is locale-aware (`"en-ZA"`) — good. But `new Date().toDateString()` and `new Date()` inside render (e.g. `DashboardPage` and `MobileDashboard`) **differ between SSR and CSR**. The server renders in its timezone; the client renders in theirs. For components that show "Updated 5m ago", this can show "just now" server-side and "5m ago" client-side after hydration. **Fix:** render time-based copy only on the client (gate behind `mounted` flag).

### 10.4 Server Components leaking to client

- **[Major]** `src/app/(app)/stock-orders/page.tsx` is a **server component** that calls `getAdminPocketBase()` and `listStockPurchases(...)` directly. This is correct (the AGENTS.md says migrate data calls to the service layer). But the error path renders the page with `loadError` set — there's no client-side retry button. **Fix:** pair server-rendered pages with a client `<RefreshButton>`.

---

## 11. Accessibility (a11y)

### 11.1 Good patterns observed

- ✅ `src/components/orders/page.tsx:680-686` — proper `role="button"`, `tabIndex`, keyboard handler, `aria-expanded`.
- ✅ `src/app/(app)/quotes/approvals/page.tsx` — `aria-selected` on tabs.
- ✅ `src/app/(mobile)/m/dashboard-view.tsx:130-150` — `role="tablist"` + `aria-selected` on time-window tabs.
- ✅ `src/components/ui/button.tsx` (assumed from shadcn) — proper focus-visible styles.
- ✅ `aria-label` on icon-only buttons (most places — verify by grep).

### 11.2 Issues

- **[Major]** `src/components/orders/page.tsx:691-697` — the linked-quote pill is a `<button>` rendered inside a `<div role="button">`. **The accessible name of the outer div includes the linked-quote pill text**, so screen readers read the pill twice (once as part of the row, once as its own button). **Fix:** move the pill to the right of the toggle, not inside it.
- **[Major]** `src/components/customer-form.tsx:287-296` — the customer form has no `<label>` elements for its inputs. The `FieldLabel` helper renders a `<Label>` (per Radix's `Label`), but the field below uses `<Input id="...">` and the label `htmlFor` isn't wired up. **Verify:** the `Label` should have `htmlFor={id}` to associate it with the input.
- **[Major]** `src/components/order-form.tsx` — same problem. `<Label>Customer</Label>` is not `htmlFor` the PopoverTrigger.
- **[Major]** `src/components/ui/sidebar.tsx` mobile drawer has `<SheetTitle className="sr-only">Navigation</SheetTitle>` — good. But the desktop `<aside>` has no `<h1>` / landmark. **Fix:** add `aria-label="Primary navigation"` to the desktop aside.
- **[Minor]** Several dialogs use the bare word "Close" as accessible name. The SheetContent's close `<button>` is `<SheetPrimitive.Close>` with no aria-label. **Fix:** add `aria-label="Close"`.
- **[Minor]** `src/app/(mobile)/m/dashboard-view.tsx:454` — close button uses literal `×` instead of an icon — accessible name is empty.

---

## 12. Other gaps

### 12.1 Real-time / SSE / WebSocket disconnect UX

- **[Critical]** No code path in the app surfaces a "Realtime disconnected" badge when PB subscriptions stop firing. The dashboards and orders list all claim to be "live" via badges or copy, but if the WebSocket drops, the operator has no signal. **Fix:** add a single shared realtime-status hook (`useRealtimeStatus`) that watches all PB EventSource connections and renders a "Reconnecting…" pill.

### 12.2 Counter success screen placeholder text

- **[Major]** `src/app/(counter)/new-order/components/success-screen.tsx:22` — *"on the New tab"* — this is **literal placeholder copy**. The success screen tells the operator the order is on "the New tab" of an unspecified list. There's no link to that tab. **Fix:** either remove the line or replace with a real sentence ("Find it in Orders → New").

### 12.3 Counter header

- **[Minor]** `src/app/(counter)/new-order/components/header.tsx` — not reviewed here. Verify it handles the empty-state (operator has placed zero orders) gracefully.

### 12.4 `/webview` page

- **[Minor]** `src/app/(app)/webview/page.tsx:22-30` — the probe uses `method: "HEAD"` with `redirect: "manual"` and a 5s timeout. **Some servers return 405 Method Not Allowed for HEAD.** The probe treats `res.ok || res.status === 302` as "up", which covers most cases, but a server returning 405 is treated as "up" because `res.ok` is false but no other guard rejects it. **Fix:** explicitly treat 405 as "can't tell" rather than up.
- **[Minor]** `src/app/(app)/webview/page.tsx:108-115` — the iframe has `referrerPolicy="strict-origin-when-cross-origin"`. Good. But the `allow="clipboard-read; clipboard-write"` is a permission policy hint — verify the embedded origin actually grants these (otherwise the clipboard buttons in the embedded app silently fail). Document this in the code comment.

### 12.5 Duplicate-orders dialog

- **[Major]** `src/components/duplicate-orders-dialog.tsx:77-90` — duplicate-detection runs on every dialog open. **The search fetches are unbounded — could be very slow on a tenant with thousands of orders.** **Fix:** add pagination + a "show more" affordance.

### 12.6 Order sprint / buyer tasks

- **[Minor]** Not deeply audited — flagged here as needing a separate pass.

### 12.7 Stock page

- **[Major]** `src/app/(app)/stock/page.tsx` — per-row "click to navigate to detail" pattern: the entire `<TableRow>` should be a navigation element. Currently it's a click handler — keyboard users can't activate it without a button. **Fix:** wrap the row in `<Link>` or add `<Button>` per row.

---

## 13. Documentation drift

### 13.1 Hardcoded brand strings

Already covered in §3 (Critical) but the spread is wider than that:

- `src/components/sidebar.tsx:284` — "Westcott Fasteners"
- `src/app/(app)/dashboard/page.tsx:271` — "Westcott Fasteners"
- `src/app/api/v1/quotes/[id]/review-approve/route.ts:326` — "from Westcott Fasteners" (email subject)
- `src/app/(auth)/login/page.tsx:20-30` (comment only) — references `/public/wfdash-logo.png`

**Fix:** introduce a single `appName` + `orgName` from a config collection; thread through every consumer.

### 13.2 Hardcoded SLA thresholds

- `src/app/(app)/quotes/page.tsx:55` — `AGE_WARNING_DAYS = 3`
- `src/lib/server/quotes-service.ts:253` — `COMPLETED_RECENT_WINDOW_DAYS = 7`

**Fix:** both should be settings.

### 13.3 Hardcoded `Math.random()` for IDs

Already covered in §10.2.

---

## 14. Recommended fix priority (rough order)

1. **Production-readiness blockers** (Critical):
   - Real-time disconnect UX across all PB subscribers (§12.1)
   - Auth gate on mobile + counter layouts (§1.2)
   - Hardcoded brand strings (§13.1)
   - No `loading.tsx` / `error.tsx` / `not-found.tsx` (§9)
   - Mobile dashboard hard-redirect (§6)
   - Mobile dashboard `<dialog>` open-state mismatch (§6)

2. **Major UX failures** (Major):
   - react-hook-form + zod adoption (§7.1)
   - Inline error messages everywhere (§7)
   - Order detail / quote detail page splits (kitchen-sink files §4.1, §5.2)
   - Stock page keyboard nav (§12.7)
   - Mobile sheet dialog state (§6)

3. **Polish / hardening** (Minor):
   - Sidebar icon duplication (§2.1)
   - "Repairs" section labelling (§2.1)
   - Hydration mismatches from `Math.random()` IDs (§10.2)
   - Stale counter success-screen copy (§12.2)

---

## Appendix — files touched during the audit

```
src/app/layout.tsx
src/app/page.tsx
src/app/offline.tsx
src/app/(auth)/layout.tsx
src/app/(auth)/login/page.tsx
src/app/(mobile)/m/layout.tsx
src/app/(mobile)/m/page.tsx
src/app/(mobile)/m/dashboard-view.tsx
src/app/(app)/layout.tsx
src/app/(app)/dashboard/page.tsx
src/app/(app)/orders/page.tsx
src/app/(app)/orders/archive/page.tsx
src/app/(app)/orders/[id]/page.tsx
src/app/(app)/quotes/page.tsx
src/app/(app)/quotes/[id]/page.tsx
src/app/(app)/quotes/approvals/page.tsx
src/app/(app)/customers/page.tsx
src/app/(app)/customers/[id]/page.tsx
src/app/(app)/suppliers/page.tsx
src/app/(app)/sprint/page.tsx
src/app/(app)/repairs/page.tsx
src/app/(app)/pricelist/page.tsx
src/app/(app)/invoicing/page.tsx
src/app/(app)/deliveries/page.tsx
src/app/(app)/collection-confirm/page.tsx
src/app/(app)/stock/page.tsx
src/app/(app)/stock-orders/page.tsx
src/app/(app)/settings/page.tsx
src/app/(app)/webview/page.tsx
src/app/(counter)/layout.tsx
src/app/(counter)/page.tsx
src/app/(counter)/new-order/page.tsx
src/app/(counter)/new-order/components/success-screen.tsx
src/app/(counter)/new-order/components/new-customer-sheet.tsx
src/app/(counter)/new-order/components/photo-capture.tsx
src/components/sidebar.tsx
src/components/auth-provider.tsx
src/components/customer-form.tsx
src/components/order-form.tsx
src/components/quote-form.tsx
src/components/duplicate-orders-dialog.tsx
src/components/theme-picker.tsx
src/components/ui/sheet.tsx
src/components/ui/dialog.tsx
src/components/ui/input.tsx
src/components/ui/textarea.tsx
```

Total: ~40 files. The full audit findings map to roughly 30 unique issues. None are show-stoppers for a beta launch; the Critical ones should land before any production rollout.