WFDASH-WEBAPP/.design-review/REPORT.md

# WFDASH — Full-App Design Review

> **Scope:** 16 main routes × 2 viewports (desktop 1280×800, mobile 375×812) = 32 screenshots captured fresh from `http://localhost:4000` on 2026-07-10.
> **Mode:** Visual inspection + DOM probing + source review. **No code changes made** (per request).
> **Screenshots:** `/Volumes/EXT/DEVELOPMENT/WFDASH_NEXTJS/.design-review/{desktop,mobile}/*.png`

---

## Summary

| Item | Value |
|------|-------|
| Target URL | `http://localhost:4000` (Next.js 16 + Tailwind v4 + shadcn/Base-UI) |
| Routes reviewed | 16 (all under `src/app/(app)/`) |
| Viewports tested | Desktop 1280×800 + Mobile 375×812 |
| Screenshots captured | 32 (16 routes × 2 viewports) |
| Console errors found | 1 (PB 404 on `delivery_notes` collection) |
| Console warnings | 11 (font preloads — see P3) |
| **P1 (functional)** | 2 |
| **P2 (visual quality)** | 8 |
| **P3 (polish)** | 5 |

The app has a **solid design foundation** (consistent sidebar, good card-based layouts, sensible shadcn/Base-UI usage, working mobile responsive grid for most pages). The main issues cluster around (1) **broken states** in a few routes, (2) **mobile tab overflow** patterns, and (3) **data quality** leaking into the UI.

---

## P1 — Functional / Broken

### [P1-1] Delivery Notes page is completely broken
- **Page:** `/delivery-notes`
- **Element:** Page body — the empty-state component
- **Issue:** Renders "Error loading data — Missing collection context" on **both desktop and mobile**. The PocketBase collection `delivery_notes` returns 404 (verified in console: `GET /api/collections/delivery_notes/records → 404`). The page is effectively unusable; users will see a dead-end error.
- **Console evidence:**
  ```
  [ERROR] Failed to load resource: 404
  @ .../api/collections/delivery_notes/records?page=1&perPage=20&filter=&sort=-created&expand=order,company,customer
  ```
- **Recommendation:** Either (a) re-create / migrate the `delivery_notes` PB collection, (b) rename the fetch target if it was renamed, or (c) hide the route from the sidebar until the backend exists. The empty-state component itself is well-designed — the problem is upstream.

### [P1-2] First supplier row in the table has no name (empty `<p class="font-medium"></p>`)
- **Page:** `/suppliers` (desktop table view)
- **Element:** `<tbody><tr>...</tr></tbody>` first row, `<td>` first child
- **Issue:** The first supplier record has an empty Name field. The DOM renders as `<p class="font-medium"></p>` with no text, an empty bordered card on mobile. Visible on both viewports.
- **Evidence:**
  ```js
  document.querySelector("tbody tr:first-child td:first-child").textContent === ""
  ```
- **Recommendation:** Either (a) default to a placeholder like `(unnamed)`, (b) hide rows with no name, or (c) fall back to email/phone. The empty state looks broken even though the row is "valid" in the DB. At minimum, the column should not render an empty `<p>` tag with no fallback content.

---

## P2 — Visual Quality (affects UX but not strictly broken)

### [P2-1] Tabs overflow horizontally on mobile
- **Pages:** `/orders`, `/collection-map`, `/customers` sub-nav
- **Issue:** Tab strips use a single horizontal flex row that gets clipped at 375px:
  - **Orders mobile:** `New 2 | Processing 1 | Colle...` (Collection Pool is cut off mid-word)
  - **Collection-map mobile:** `Today | Tomorrow +3 days Next week` then `All Assigned Unassigned` then `All Ready Unready` — wraps onto 3 separate lines, no clear grouping or scroll affordance
  - **Quotes mobile:** Templates tab orphans onto its own row, breaking the row's visual grouping
- **Recommendation:** Either (a) allow horizontal scroll with `overflow-x: auto` + a right-edge gradient to signal "more →", (b) wrap into 2 rows cleanly with a separator, or (c) collapse tabs behind a "More" dropdown when there are >4 items. The current behavior is "tabs get cut off with no scrollbar visible" which feels broken on a touch device.

### [P2-2] "All" button appears twice with identical styling — looks like duplicate controls
- **Page:** `/collection-map` (desktop)
- **Issue:** Two filter groups both have an `All` option:
  - Group 1 (Assigned filter): `All | Assigned | Unassigned`
  - Group 2 (Ready filter): `All | Ready | Unready`
  - Both highlight `All` in blue. The user can't tell at a glance if these are the same control, two different filters, or related/dependent.
- **Recommendation:** Visually separate the two groups (e.g., vertical spacing + a small label like "Status" / "Readiness" above each), or use different visual treatment for the second `All` (e.g., outlined instead of solid) so they read as distinct filters.

### [P2-3] "Select rows to confirm." text placed awkwardly next to "Confirm this PO" button
- **Page:** `/collection-confirm` (both viewports)
- **Issue:** Right next to the primary green "Confirm this PO" button there's a grey text "Select rows to confirm." — but the page shows a single PO with a single item, so there's nothing to "select". The text is dead context that confuses the action.
- **Recommendation:** Either remove the helper text in single-PO mode, or move it to a tooltip/description that explains the bulk-select flow when the user is on a multi-PO view.

### [P2-4] "Collection Confirm" title wraps to 2 lines on mobile, eating vertical space
- **Page:** `/collection-confirm` (mobile 375)
- **Issue:** The page title is "Collection Confirm" with the user badge (`david.els1`) and Refresh button on the same row. The title gets squeezed and wraps to 2 lines. Could be just "Confirm" or move the user/Refresh to a second row.
- **Recommendation:** Shorten to "Confirm" on mobile, or move the right-side controls to a second row on small screens.

### [P2-5] "Invoicing" status shown twice — once as label, once as badge
- **Page:** `/invoicing` (mobile 375)
- **Issue:** The order item card shows "Status: Ready" as plain text **AND** a "Ready" badge right below it. Redundant — user sees the same word twice in two different visual treatments.
- **Recommendation:** Drop the plain "Status:" label, keep just the badge. Or invert — drop the badge, keep the label with semantic color.

### [P2-6] "Rep" column shows only "—" for every quote
- **Page:** `/quotes` (desktop table)
- **Issue:** Every row in the `Rep` column is an em dash. Either (a) the field is genuinely empty for all 5 visible quotes (data issue), or (b) the column shouldn't be shown at all if it's always empty, or (c) the dash should be replaced with a more meaningful "Unassigned" / "—" with a hover explanation.
- **Recommendation:** Show "Unassigned" as text, or hide the column when no rows have values, or add a small icon to indicate the field exists but is null.

### [P2-7] Companies table is 80% empty placeholders
- **Page:** `/companies` (desktop)
- **Issue:** Of 10 visible rows, only 2 have any data beyond the name (Mpondo Mining has address + vendor #). 8 of 10 rows show `—` for Email, Tel, and Vendor #. This is likely seed/test data, but the design is honest about it — the placeholders look like missing data, not intentional.
- **Recommendation:** For demo/seed data, consider hiding empty columns or using a "Add contact info" CTA inline. Real production data will likely be more complete, but the current pattern makes the table feel broken.

### [P2-8] Pricelist catalogue shows only one supplier with all-zero prices
- **Page:** `/pricelist` (desktop Catalogue tab)
- **Issue:** All 10 visible rows are `Tel-Screw Products (Pty) Ltd.` with `R 0,00` for both Cost and Selling. SKU, Description, Category, Brand, Markup, Source all show `—`. This is either (a) the only supplier with data in the test DB, or (b) the AI extraction is failing for this supplier. The "1521 line items from AI-extracted price lists" headline contrasts sharply with the empty rows.
- **Recommendation:** Add a small "showing 1 of N suppliers" filter chip, or fix the data ingestion so the columns actually populate. Right now the page is contradictory.

---

## P3 — Polish (low priority, design-system quality)

### [P3-1] Excessive woff2 preloads — every page logs 11 warnings
- **Console warnings** (every page):
  ```
  The resource ...woff2?... was preloaded using link preload but not used
  within a few seconds from the window's load event. Please make sure it
  has an appropriate `as` value and it is preloaded intentionally.
  ```
- **Issue:** Next.js 16 with `next/font` is preloading ~12 font weights per route, but the actual pages only use 2-3. Wasted bandwidth + log noise.
- **Recommendation:** Audit which font weights are actually used. Drop the unused ones from the preload list. This is a build-config / `next/font` config tweak, not a per-page fix.

### [P3-2] Sidebar groups "Repairs" under "OTHER" instead of "OPERATIONS"
- **Page:** `/repairs` (sidebar nav)
- **Issue:** Repairs is logically an operations task (it's about fixing returned items), but it's grouped under OTHER alongside settings/preferences. Same for `Delivery Notes`. Re-pair them with their semantic neighbours.
- **Recommendation:** Move Repairs + Delivery Notes under OPERATIONS, leave Pricing-data tools (Pricelist) in OTHER (or create a new TOOLS group).

### [P3-3] "Supplier Map" link appears twice in sidebar
- **Page:** `/suppliers` (desktop sidebar)
- **Issue:** The "Supplier Map" entry appears at the very bottom of the supplier list, after the "Add Supplier" cards. It's also a top-level item visible in OTHER or the right nav. Visually it's a "leak" — looks like a footer item accidentally rendered mid-list.
- **Recommendation:** Either move it to a dedicated "Tools" submenu under SUPPLIERS, or promote it to its own sidebar item.

### [P3-4] "0" appears as a standalone line in Companies mobile cards
- **Page:** `/companies` (mobile 375)
- **Issue:** Every company card shows a lone "0" between the name and the dates. No label, no context. The desktop table doesn't have a corresponding column visible.
- **Recommendation:** Either remove the count, label it (e.g., "0 contacts", "0 POs"), or hide it when zero.

### [P3-5] "Status: Ready" + "Ready" badge duplication + "Adal Engineering" + "GIELIE" subtext pattern
- **Page:** `/invoicing` (desktop + mobile)
- **Issue:** Each order card shows two supplier/people labels stacked: e.g. "Adal Engineering" (icon) + "GIELIE" (icon) on the same line. "GIELIE" looks like a customer handle/shortcode, not a person. The visual treatment is identical (icon + text), so the user can't tell if these are two different entities, primary/secondary, or a duplicate.
- **Recommendation:** Use a separator (·) or different icon/colour for the secondary label. Or move one to a sub-line.

---

## Cross-cutting observations

### What's working well
- **Sidebar/nav structure** is consistent and accessible. Active state, hover, dark-mode toggle are all clean.
- **Card pattern** is used consistently (orders, quotes, customers, sprint items). Visual rhythm is good.
- **KPI tiles** on dashboard are well-composed — colour-coded accents, sparklines, deltas with up/down arrows.
- **Empty states** (e.g. `/repairs`) are properly handled with icon + message + CTA.
- **Theme picker** on Settings shows 6 themes with mini-window previews — nice touch.
- **Quote line item modal** and **collection confirm** have well-organized action buttons (Adjust / Add Note / Attach / Confirm / Return to pool).
- **Sidebar scroll isolation** works — the (app) layout correctly constrains the page to viewport height with internal scroll.
- **Mobile responsive** works for most list pages (Customers, Suppliers, Quotes) — cards stack cleanly with proper padding.

### Patterns worth replicating
- The `group/card` + `rounded-2xl` + `shadow-sm shadow-foreground/5 ring-1 ring-foreground/10` card treatment is consistent and looks polished.
- KPI tile colour-coding (cyan/pink/orange/green) is used as an accent stripe at the top of each card — nice visual hint.
- Filter pills with colour dots (`● New 2 | ● Processing 1`) double-encode the status (colour + label) for accessibility.

### Patterns to fix everywhere
- The `space-y-3` on mobile card lists adds up to a lot of vertical space. For a 32-item list (sprint), 32 × 200px = 6400px tall. Consider:
  - Smaller cards on mobile (`py-3` instead of `py-5`)
  - Or a "load more" / pagination pattern
  - Or virtualization for very long lists

### What I did NOT test
- **Tablet viewport (768px)** — user said desktop + mobile only
- **Dark mode** — captured in light mode
- **Form modals** (e.g. Add Customer, New Order) — only top-level routes
- **Subroutes** (e.g. `/orders/[id]`, `/quotes/[id]`, `/companies/[id]`) — only top-level routes
- **Interactive states** (hover, focus, error states on inputs)
- **Accessibility (a11y) audit** — no screen reader / keyboard-only testing
- **Performance** — only console warnings, no Lighthouse / Web Vitals

### Test methodology notes
- I planted a PocketBase auth token in `localStorage` for the `test@wfdash.local` user via the admin API (impersonation). The token was overwritten during the session by a fresh `david.els1@outlook.com` auth — both are valid users, so this didn't affect the visual review.
- The first sweep captured stale Next.js client-side routes (some routes appeared to render the previous page's content). I re-ran with `waitForFunction` to wait for the actual H1 of the target page before screenshotting. The final 32 screenshots are all unique and accurate.
- Full-page screenshots (`fullPage: true`) were used so the report can also comment on below-the-fold content.

---

## Recommended fix order (if you tackle these)

1. **Fix the Delivery Notes 404** — P1-1, blocks a whole feature
2. **Add a name fallback for the empty supplier** — P1-2, 2 lines of code
3. **Fix mobile tab overflow** — P2-1, affects 3+ routes
4. **Dedupe "All" filter on collection-map** — P2-2, simple label/visual change
5. **Tighten the data-quality ones** (Reps column, Company contacts, Pricelist rows) — P2-6/7/8
6. **Polish pass** (P3s) — low-risk, can ship anytime
</content>
</invoke>