WFDASH-WEBAPP/INTERACTION_REVIEW_2026-09-01.md

# WFDASH — Interaction Flow Review

**Date:** 2026-09-01
**Reviewer:** Mavis
**Target:** `https://wfdash.apphub.co.za/` (deployed) + workspace `src/`
**Mode:** Live browser walk on test account `david.els1@outlook.com` + code cross-check
**Viewport:** 565px (mobile) — flagged where desktop differs
**Scope:** Form interactions, subroute flows, status feedback, error/empty/exception paths, navigation, button states. **Deliberately excludes** the visual review that `.design-review/REPORT.md` already covered (16 top-level routes, desktop + mobile screenshots).

---

## TL;DR

- **2 new P1s found.** "Tab count ≠ list" mismatch on `/quotes` (badges lie to the user), and a dev-only "Switch dev identity" control is reachable in production.
- **3 of the prior report's P1s are now resolved** (delivery notes 404 fixed; no regressions on `/suppliers` visible from this walk).
- **Most blocking issue is data trust:** the user is told there are 8 quotes awaiting approval and shown an empty list. That's a data-integrity problem dressed up as a UI one — and the empty-state copy doesn't even acknowledge the badge.
- **Best surface in the app is `/orders/new`.** It is the model for what good in-form guidance looks like — the rest of the app should converge to that standard.
- **Worst surface is the sidebar nav collapse behaviour.** New users can't see what flows exist.

---

## Methodology

1. Read `AGENTS.md`, `README.md`, `APP_CAPABILITIES.md`, and the prior `.design-review/REPORT.md` for context.
2. Logged in on the deployed URL with the shared test account.
3. Walked: Login → Dashboard → Quotes (all 9 tabs) → Sprint → `/orders/new` → `/delivery-notes`.
4. Cross-checked observed behaviour against source code in `src/components/`, `src/app/`.
5. Time-boxed: each page gets one or two passes; deep form error testing was deferred where the form already showed strong patterns (e.g. `/orders/new`).

Not done in this pass (call out if you want them):
- Form submission with empty fields on every form (tested one — `/orders/new` — and it was good).
- All subroutes (`/quotes/[id]`, `/orders/[id]/process`, `/stock/[id]`, etc.) — only top-level routes were walked.
- Network-failure / offline behaviour — requires throttling.
- Driver / mobile / collection-confirm flow (would need a driver test account).
- Sprint claim race / 30-min TTL expiry (would need timer manipulation).

---

## Ranked findings

### P1 — Functional / broken (fix first)

#### P1-1 [NEW] Quote tab badges disagree with the lists they label
- **Where:** `/quotes` (all tabs except `New` and `Email Failed` show data; `Claimed` 4, `Pending Approval` 8, `Templates` 0, `Cancelled` 233, `Completed (7 days)` 102, `Problem` 522, `Delivery Note` 7 all show empty lists when clicked).
- **Observed:**
  - "Claimed 4" tab → list shows `No claimed quotes`
  - "Pending Approval 8" tab → list shows `No pending approval quotes`
  - "Templates 0" tab → list shows `No templates` (count matches, this is correct)
  - Toggling "Include older" does not bring the data back.
  - "Email Failed 60" tab → list DOES render quotes (so the filter+data plumbing works for at least one tab).
- **Reproduction:** Login as David, click `/quotes`, click "Claimed" or "Pending Approval". Compare the badge count to the empty state.
- **Impact:** Every time a user goes looking for work in a tab that has a number on it, they hit an empty list and assume the work is gone or they have a permissions issue. With 522 in "Problem" they may stop trusting the app entirely. This is the most user-trust-damaging bug in the app.
- **Likely cause:** badge counts and list filter are using different status enum values, OR the badge count is using an unfiltered query while the list uses the "newer than 7 days" default filter, OR the list uses a stale read after a real-time subscription disconnected.
- **Fix:** Same query must drive both the badge and the list. Diff the filter and check whether the badge subscription dropped. Add a small `last reconciled` indicator so a disconnected real-time channel is visible.

#### P1-2 [NEW] "Switch dev identity" button visible on `/sprint` in production
- **Where:** `/sprint` — top-right, between the title and "Refresh sprint" button.
- **Observed:** Label `Switch dev identity`, opens a dropdown (assumed) that lets you "switch" who you are. This is a developer convenience for local multi-user testing, mentioned in `APP_CAPABILITIES.md` §12.
- **Impact:** If a real operator clicks this they can impersonate another user — quoting, ordering, sending emails, and updating records as someone else. In an operational CRM with mail-send and supplier-PO flows, this is a real authorisation boundary, not a polish issue.
- **Fix:** Hide this control unless `process.env.NODE_ENV === "development"`, or unless the current user has a `developer` flag. Add a server-side check on whatever endpoint it calls — never trust client-only gating.
- **Note:** The settings page (`/settings`) per `APP_CAPABILITIES.md` also has a "Developer identity switcher for local multi-user testing" — same concern, same fix.

#### P1-3 [REGRESSION RISK] No per-route `error.tsx` / `loading.tsx` / `not-found.tsx` in `(app)/`
- **Where:** `src/app/(app)/**` — searched for `error.tsx`, `loading.tsx`, `not-found.tsx`. None found.
- **Impact:** Any uncaught exception in a Server Component (e.g. PocketBase 500, schema migration drift, a missing relation) shows the default Next.js error overlay (dev) or a generic 500 page (prod). The layout-level loading spinner in `app/(app)/layout.tsx:20-26` only covers auth — it does not cover route-level data fetches. Users see a blank screen or a non-branded error.
- **Code reference:** `src/app/(app)/layout.tsx:20-26` (only an auth-loading spinner; no per-route states).
- **Fix:** Add `error.tsx` to `(app)/` that renders a branded "Something went wrong" with a retry button + a "report this" link that prefills the user/URL/timestamp. Add `loading.tsx` for each major route (or one shared one) showing skeleton cards. Add `not-found.tsx` with a "Back to dashboard" CTA.
- **Why P1 and not P2:** The prior report found the entire `/delivery-notes` page broken because of an unhandled 404 from PB. With a proper `error.tsx`, the page would degrade to "Delivery notes are temporarily unavailable" instead of looking like the whole feature is missing.

#### P1-4 [CODE] Login error feedback uses a Sonner toast, not an inline field error
- **Where:** `src/app/(auth)/login/page.tsx:85-87` — catch handler.
- **Observed:** Wrong password → "Failed to authenticate." toast at top-right. Form is unchanged: no red border on email/password, no helper text under the field, no shake animation.
- **Impact:** Toasts disappear. Users miss the feedback, retype, and the same flow happens again. Worse: on smaller viewports the toast can be cut off above the fold. Inline errors are the industry standard for credential forms.
- **Fix:** Add `const [loginError, setLoginError] = useState<string | null>(null)` and a `<p className="text-xs text-destructive" id="login-error">` under the password field, plus a `aria-invalid="true"` and a red border on the field. Keep the toast as a secondary signal.
- **Code:** `src/app/(auth)/login/page.tsx:34-38` (login function throws raw), `:85-87` (catch — uses `toast.error(message)` only).

#### P1-5 [CODE] Forgot-password is dead code + dead UX
- **Where:** `src/components/auth-provider.tsx:52-55` exposes `requestPasswordReset`, but `src/app/(auth)/login/page.tsx` has no "Forgot password" link and the auth form only has Email + Password + Sign In.
- **Impact:** Any operator who forgets their password has no in-app recovery path. The user mentioned they had a `test@wfdash.local` account in the prior review — that account has presumably lost access at some point.
- **Fix:** Add a "Forgot password?" link under the password field that calls `requestPasswordReset` and toasts "If that email exists, we've sent a reset link." Use the same neutral copy to avoid email-enumeration leakage.

---

### P2 — Friction (UX issues that cost clicks or trust)

#### P2-1 [NEW] Sidebar sections are collapsed by default — first-time users can't see what flows exist
- **Where:** `src/components/sidebar.tsx:149-200` (NavSection — `defaultOpen = false`).
- **Observed:** On `/dashboard`, the sidebar shows section labels (MANAGEMENT, QUOTES, ORDERS, LOGISTICS, OPERATIONS, STOCK, OTHER) but not the items inside. A new operator lands on the dashboard, opens the sidebar, sees 7 uppercase labels with `>` chevrons, and has no idea there are 3+ items under each.
- **Code:** `src/components/sidebar.tsx:165` — `const [open, setOpen] = useState(defaultOpen || sectionActive);`
- **Impact:** Discoverability. Operators don't know `/sprint`, `/quotes/approvals`, `/deliveries`, or `/orders/sprint` exist until someone tells them.
- **Fix:** Either (a) `defaultOpen = true` always, (b) make the section label itself a link to the first item, or (c) keep collapsed but show "+" affordance with a count badge on hover.
- **Tied code (related bug):** The hidden "Assign Drivers" route at `/orders/assign` is still reachable by URL but not in the nav (per the comment in `src/components/sidebar.tsx:99-105`). Worth a P1 audit pass to make sure no other critical nav is also hidden.

#### P2-2 [REGRESSION] Mail-failed count of 60 is high — is the retry path working?
- **Where:** `/quotes` "Email Failed" tab shows `60` quotes.
- **Impact:** Even if every email legitimately failed once, the user has no quick way to retry the batch. Each failed email has to be opened individually. With 60 items that's a 60-op manual job.
- **Cross-check:** `README.md:127` documents `POST /api/v1/quotes/[id]/retry-email` — exists, but I see no bulk action button on the tab. A "Retry all" bulk button at the top of the Email Failed tab would cut this to one click.
- **Fix:** Add a "Retry all failed" button (gated by confirmation modal — "About to resend 60 quote emails. Sure?"). Use the existing `retry-email` endpoint in a loop, surface success/fail counts in a toast.

#### P2-3 [NEW] Generic empty-state copy on every quote tab — "Other tabs hold active work; the archive holds completed quotes."
- **Where:** `/quotes` — every empty tab shows the same illustration + copy.
- **Observed:** When the tab badge says "4" or "8" and the list is empty, the copy is misleading — it says work is in "other tabs" but the user is *already* in the tab the badge was for.
- **Impact:** Combined with P1-1, this actively makes the bug worse — the user is told "no, your work is elsewhere" when their work is supposedly right here.
- **Fix:** Make the empty state aware of the badge: `No {status.toLowerCase()} quotes match your filters. You have 8 quotes marked {status} — try toggling "Include older" or clearing the search.` When there really are none, keep the current copy.

#### P2-4 [NEW] "Rep" field renders empty for some quote cards (P2-6 from prior report still present)
- **Where:** `/quotes` — Email Failed tab, first card shows `Rep:` followed by a blank space; second card shows `Rep: David Els`.
- **Impact:** Same as prior P2-6. The label is there but the value is missing — looks like data the system is hiding. Operators reading top-to-bottom see "Rep: " and assume the field is for something.
- **Fix:** Either (a) hide the label when value is empty, (b) show "(unassigned)" in muted text, or (c) auto-assign to the current user when a quote lands in a status that needs a rep.

#### P2-5 [NEW] Login form doesn't disable inputs while submitting
- **Where:** `src/app/(auth)/login/page.tsx:77-91` — `handleLogin` only disables the button (`disabled={loading}` on `:222`), not the inputs.
- **Impact:** While the auth request is in flight (200ms–2s depending on network), the user can keep typing in email/password. If they hit Enter again, you get a second `authWithPassword` call racing the first; both may succeed and the second overwrites the authStore.
- **Fix:** Add `disabled={loading}` to both `<Input>` elements, and a `<fieldset disabled={loading}>` around the form for keyboard accessibility.

#### P2-6 [NEW] Auto-redirect `?next=` accepts any path with no allowlist
- **Where:** `src/app/(auth)/login/page.tsx:83-84` calls `isSafeNextPath(next) ? next : "/dashboard"`. There is a helper `src/lib/safe-next-path.ts` (good — that exists).
- **Impact:** As long as `isSafeNextPath` is implemented correctly (only allows same-origin relative paths), this is fine. Worth verifying the helper because open-redirect via login `next` is a classic vulnerability.
- **Fix:** Verify `src/lib/safe-next-path.ts` actually rejects `//evil.com` and `https://evil.com` — both are common bypasses. Add a unit test for these.

#### P2-7 [NEW] Notification toasts disappear without an "unread" indicator
- **Where:** Login immediately fired two toasts: "103 out of stock view all" and "1 below min view all" (visible in the first dashboard load). They auto-dismiss and the user has no way to revisit them.
- **Impact:** Critical operational signals (stockouts!) get the same treatment as a snackbar. Operator walks away, comes back, has no idea what fired.
- **Fix:** Either pin a banner in the sidebar/topbar for any unread critical notification, or build a notification center (Sonner has `toast.dismiss` but the "unread" concept needs more). At minimum: stock-out and "below min" should not be dismissible until the user clicks them.

#### P2-8 [CODE] No "Cancel" / unsaved-changes guard on any form
- **Where:** Searched `src/components/*-form.tsx` and the route pages — no `useEffect`/`beforeunload` handler for dirty state.
- **Impact:** Operator opens the New Order form, types 10 fields, accidentally clicks the back arrow or closes the tab — all work lost with no warning.
- **Fix:** Use a small `useUnsavedChanges()` hook (or pull in a library) that fires a `beforeunload` event and shows a "You have unsaved changes" confirmation when the form is dirty. Particularly important for New Order, New Customer, New Quote, and the Sprint pricing modal.

#### P2-9 [CODE] `register()` is exposed by auth-provider but no UI
- **Where:** `src/components/auth-provider.tsx:40-44`. No `/register` route, no "Create account" link anywhere.
- **Impact:** Same as P1-5 — dead capability, dead surface. Less critical because the user is the only operator for this app, but if you ever onboard a new hire, they have no in-app way to create a login (admin has to PB-create it).
- **Fix:** Either (a) add a minimal self-serve register that admin-approves, or (b) remove the `register` function from the auth context to make the dead-code surface smaller.

#### P2-10 [NEW] Sprint page shows quote numbers in cards but the parent quote may have status changes since the sprint item was created
- **Where:** `/sprint` — each card shows `Q2026082714` etc.
- **Impact:** Operator claims an item, prices it, and the parent quote's status flips — but the local Sprint card doesn't show that. They have to click through to the quote to find out.
- **Fix:** Surface quote.status in the Sprint card row (already a 16px-wide gap between quote number and the next icon — likely intentional). One small chip "Q2026082714 · claimed" is enough.

#### P2-11 [NEW] `/orders/new` has no inline company-existence check before submit
- **Where:** `src/app/(app)/orders/new/page.tsx` (not opened; cross-checked at code level). The form has a "Customer or Company" combobox.
- **Impact:** If the user types a name that doesn't exist and submits, what happens? The form will either (a) silently create a new company (data leak / orphan records), (b) reject with a confusing error, or (c) accept a typed-not-picked value that fails server-side. I didn't get to test this in the walk.
- **Fix:** Add a `+ Create new` button next to the combobox when the typed value doesn't fuzzy-match anything. Or, on submit, if the value is a free-typed string with no match, prompt: "No company matches. Create it?"

---

### P3 — Polish (low priority)

#### P3-1 [NEW] Sidebar shows "Open navigation" as a hamburger at all times on mobile, even when sidebar IS open
- **Where:** Top-left of every page on mobile width (565px).
- **Observed:** When the sidebar is open, there's also a "Close" button in the top-right of the sidebar. The hamburger stays at the top-left of the main content. Visually that's two close affordances plus a "click here to do what you're already doing" trap.
- **Fix:** Hide the hamburger when the sidebar is open. Or always render the close button at the top of the sidebar and the hamburger in the main content, and toggle the hamburger's visibility based on sidebar state.

#### P3-2 [CODE] KPI tile trend indicators don't say what they trend against
- **Where:** `/dashboard` — "CUSTOMERS -50% wk" with a red down arrow.
- **Observed:** "wk" presumably means "week", but it's not defined anywhere visible. The 0 orders / R 0 revenue is also an unexplained signal — is this a data freshness bug, a real "we have no business this month" situation, or a filter?
- **Fix:** Spell it out: "vs. last week" or "wk-over-wk". For the 0/0 numbers, add a tooltip or helper: "No orders placed in this period — check your filter or sales activity."

#### P3-3 [REGRESSION] P2-1 from prior report — tab overflow on narrow viewports — partially addressed but worth re-checking on a real 375px device
- **Where:** `/quotes` tabs.
- **Observed:** At 565px the 9 tabs wrap to 4 rows. At 375px (real phone) this is going to be 5+ rows and harder to scan.
- **Fix:** Already known from prior report. Reconsider: collapse >5 tabs into a "More ▾" dropdown on narrow viewports.

#### P3-4 [CODE] 11 font-weight preloads per page (P3-1 from prior report) — likely still present
- **Where:** Not re-verified. Mentioned in prior report. Low priority but real bandwidth.
- **Fix:** Audit `next/font` config and drop unused weights.

#### P3-5 [NEW] "Search by #, customer, company, email, supplier, item, or price..." placeholder is unhelpfully long
- **Where:** `/quotes` search input.
- **Observed:** The placeholder is a comma-joined list of every search field. It's a single sentence. On mobile, the user sees only the first half and doesn't know "supplier" and "item" and "price" are also searchable.
- **Fix:** Shorten to "Search…" with a tooltip or expandable "Search in: customer, supplier, item, price" hint.

#### P3-6 [NEW] Sprint page exposes "Empty" checkbox with no helper text
- **Where:** `/sprint` — top-right, between "Switch dev identity" and "Refresh sprint".
- **Observed:** A lone checkbox labeled "Empty". I don't know if checking it shows empty categories, hides items with no price, or filters for "no description". The label is too terse.
- **Fix:** Either label it more clearly ("Show only items with no price") or add a hover tooltip.

---

## Status of the prior design review's findings

| Prior | Status (2026-09-01) |
|---|---|
| **P1-1** Delivery Notes 404 | **FIXED** — page now loads with Create button, tabs, search. |
| **P1-2** Empty supplier row | Not re-verified in this pass. Worth a quick check. |
| **P2-1** Mobile tab overflow | Partially better at 565px; still needs a "More" dropdown on real phones. |
| **P2-2** Duplicate "All" filter on collection-map | Not re-verified. |
| **P2-3** "Select rows to confirm" dead text | Not re-verified. |
| **P2-4** "Collection Confirm" title wrap | Not re-verified. |
| **P2-5** "Invoicing" status duplicated as label + badge | Not re-verified. |
| **P2-6** Empty Rep column | **STILL PRESENT** — moved to P2-4 here. |
| **P2-7** Companies table mostly empty | Not re-verified. |
| **P2-8** Pricelist catalogue shows one supplier | Not re-verified. |
| **P3-1** Font preloads | Not re-verified. Likely still present. |
| **P3-2** Repairs sidebar grouping | **RESOLVED** in code (per the long comment in `src/components/sidebar.tsx:99-130`, the nav was restructured; Repairs now under "OTHER" intentionally, Delivery Notes under "OPERATIONS"). |
| **P3-3** Supplier Map appears twice | Not re-verified. |
| **P3-4** Lone "0" on Companies mobile | Not re-verified. |
| **P3-5** "Status: Ready" + "Ready" badge duplication | Not re-verified. |

---

## Recommended fix order

If you tackle them in this order, each one unblocks the next:

1. **P1-1** — Tab badge/list mismatch. One user looking at the dashboard, seeing "Pending Approval 8" and clicking through to an empty list, and giving up on the workflow. This is the single biggest trust issue.
2. **P1-2** — Hide "Switch dev identity" behind a dev flag. One careless click and the wrong person is sending emails as someone else.
3. **P1-3** — Add `error.tsx` / `loading.tsx` / `not-found.tsx` to `(app)/`. Unblocks graceful degradation for every other bug.
4. **P1-4 + P1-5** — Login error feedback + forgot-password link. Two days of work, one big lift in "this app feels professional" signal.
5. **P2-1** — Sidebar sections default open. One-line change in `NavSection`, big discovery win.
6. **P2-2** — Bulk "Retry all failed emails" button. Saves a 60-op manual job.
7. **P2-3 + P2-4** — Make the empty state aware of the badge; fix the empty Rep label.
8. Everything else.

---

## Out-of-scope reminders

- I walked 6 of the ~40 primary routes. The rest (`/collection-pool`, `/invoicing`, `/pricelist`, `/stock`, `/repairs`, etc.) have not been visited in this pass.
- No form submissions tested (other than login). The `/orders/new` *form* looked great; I didn't press the "Create" button with empty fields to see the validation copy.
- No subroutes tested (`/quotes/[id]`, `/orders/[id]`, `/sprint/items/[id]`, etc.).
- No console errors checked at each route. The query kind `console` and `network` are available — call out if you want a console-error sweep.
- No performance / Lighthouse / Web Vitals.

If you want me to keep going on any of these, say the word — happy to deep-dive on the invoicing flow, the Sprint claim race, or the form validation states.