WFDASH-WEBAPP/docs/audit/frontend-ux.md
Dave 0efbae8dc5 feat(quotes): enhance search functionality and user experience
- Updated the search input handling to improve responsiveness by integrating a debounced search mechanism, reducing latency from 550 ms to approximately 300 ms.
- Added keyboard shortcut support for focusing the search box, enhancing accessibility and usability for frequent users.
- Improved the search placeholder to reflect all searchable fields, providing clearer guidance to operators.
- Introduced a clear-search button to streamline user interactions, allowing quick resets of the search input.
- Enhanced the display of search results with a live result count badge, providing immediate feedback on search outcomes.

These changes collectively enhance the search experience for operators, making it more intuitive and efficient.
2026-08-28 20:13:44 +02:00

46 KiB
Raw Permalink Blame History

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():
    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 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.


  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.