Files
Meal-Planner/fix-ui-audit.md
T
admin f5fb7558c4 fix(migration): simplify aisle migration + add persistent backup script
- Drop the empty batch_alter_table block and the meaningless
  set_config call from migration 0015. Temp tables still persist
  for the migration's session (Alembic's transactional_ddl).
- New backend/scripts/persist_aisle_backup.sql creates
  public.ingredient_aisle_backup_0015 and
  public.grocery_item_aisle_backup_0015 permanent tables for
  operators who want a recoverable record beyond the migration.
- Update Review/sprint2-verification.md, Review/ui-nielsen-audit.md
  and fix-ui-audit.md with the correct container-based deploy
  steps: docker compose exec db psql -U mealplanner -d mealplanner
  -f /dev/stdin < ...sql. Host psql is not available on the
  deployment host; the db runs inside the container.
2026-06-03 17:41:59 -07:00

227 lines
17 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Fix Plan — UI/UX Audit (Nielsen 10 Heuristics)
## Goal
Resolve the 14 issues (5 P0, 6 P1, 3 P2) from `Review/ui-nielsen-audit.md` in three sprints, ending each sprint with a deployable, demonstrable improvement on `http://100.108.208.56:8082/`.
## Scope boundaries
- **In:** frontend React/TS fixes in `frontend/src/`. Backend one-off data migrations only when required (B8 aisle normalization, B4/Recommended route).
- **Out:** New features (bulk add, keyboard shortcuts, onboarding tour) — those are future work, listed in §Future.
- **Reuse:** `components/ErrorBoundary.tsx` already exists (verified). `components/ui/*` (Button, Card, Select, Input, EmptyState, Badge, Skeleton, LoadingSpinner) are the building blocks — use them, don't roll new ones.
- **Stack confirmed:** React 18 + TS + Vite + Tailwind + react-router-dom 6 + @tanstack/react-query + react-hot-toast + @hello-pangea/dnd + lucide-react + framer-motion. **No new deps** in Sprint 1/2. Sprint 3 may add `react-joyride` only if approved (defer to §Future).
## Conventions
- One commit per task: `fix(ui): <short>` / `feat(ui): <short>` / `refactor(frontend): <short>`.
- Before each commit: `cd frontend && npm run lint && npm run build` (the `build` script runs `tsc` first — type-checks the project).
- After each task, re-screenshot the affected page in the same playwright session and diff against `/tmp/opencode/mp-review/screenshots/`. Save new shots in `/tmp/opencode/mp-review/screenshots/fix-sprintN/`.
- All UI text in sentence case. New copy matches existing `lib/toast.ts` style.
- Type updates go in `frontend/src/types/index.ts`; do not duplicate shapes inline.
---
## Sprint 1 — Stop the bleeding (P0s)
**Goal:** Every P0 bug is gone. Each is independently demoable on the live deployment.
### S1.1 · B1 — RecipeDetail ingredients: drop `.trim()` so unit + name don't fuse
- **File:** `frontend/src/pages/RecipeDetail.tsx:161`
- **Change:** Replace
```tsx
{ing.qty != null && `${ing.qty} ${ing.unit || ''} `.trim()}
```
with
```tsx
{ing.qty != null && `${ing.qty}${ing.unit ? ` ${ing.unit}` : ''}`}
```
followed by a literal `' '` before `{ing.name}`.
- **Verify:** Open `/recipes/eae6591f...` (Black Bean Tacos). Ingredient row reads `2 can Black Beans, Canned` (with space). Re-run `npm run build`.
### S1.2 · B2 — MealDetail ingredients: align field name with backend (`qty`)
- **Files:** `frontend/src/types/index.ts`, `frontend/src/pages/MealDetail.tsx:249-252`
- **Change:**
1. In `types/index.ts` confirm `MealIngredient` shape; align to backend `qty` / `unit`. If the type currently has `quantity`, rename to `qty` (single source of truth).
2. In `MealDetail.tsx:249-252`, switch reads to `ing.qty` / `ing.unit`. Keep the existing null-guard so `qty == null` is skipped cleanly.
- **Verify:** Open `/meals/<any>` (e.g. `/meals/f28...` Pork Stir-Fry). Row reads `1 lb Pork Chops, Bone-In` not `lb Pork Chops`. `npm run build` clean.
### S1.3 · B3 — MealDetail cost: fix `$N/A per serving`
- **File:** `frontend/src/pages/MealDetail.tsx:191`
- **Change:**
```tsx
{item.estimated_cost != null
? `$${item.estimated_cost.toFixed(2)} per serving`
: 'No price estimate yet'}
```
- **Verify:** Reload `/meals/f28...`. Price line reads either `$X.XX per serving` or `No price estimate yet` — never `$N/A`.
### S1.4 · B4 — `/recommended` blank page: add `*` NotFound + alias
- **Files:** `frontend/src/App.tsx`, new `frontend/src/pages/NotFound.tsx`
- **Change:**
1. Create `pages/NotFound.tsx` — friendly card with `AlertTriangle` icon, message *"We can't find that page."*, primary `<Button>` → `/`, secondary → back. Reuse `components/ui/EmptyState.tsx` if it fits.
2. In `App.tsx`:
- Add `import { Navigate } from 'react-router-dom'`.
- Insert `<Route path="/recommended" element={<Navigate to="/recipes/recommended" replace />} />`.
- Append `<Route path="*" element={<NotFound />} />` after the existing routes.
3. Add a `// TODO(seo): audit email/share links for `/recommended` references` comment.
- **Verify:**
- Visit `http://100.108.208.56:8082/recommended` → redirects to `/recipes/recommended`, renders the Recommended page.
- Visit `http://100.108.208.56:8082/this-does-not-exist` → renders NotFound.
- `npm run build` clean.
### S1.5 · B5 — Mobile dashboard: always show empty meal slots
- **File:** `frontend/src/pages/Dashboard.tsx` (lines ~164, ~219)
- **Change:** Audit every `hidden md:flex` / `hidden md:block` / `hidden md:inline` inside `DayColumn` and the empty-slot JSX. Remove the `hidden` class on the empty-slot CTAs (the `Empty+Generate` placeholder block). For decorative chrome (e.g. day-of-week abbreviations), keep `hidden md:flex` only if there's a separate mobile-friendly label.
- **Verify:** Re-screenshot at 390 px width. Empty slots are tappable; tapping Generate fires the same query as on desktop. `npm run build` clean.
### S1.6 · Sprint 1 verification gate
- `cd frontend && npm run lint && npm run build` → both 0 errors / 0 warnings.
- Re-run playwright walkthrough; capture `screenshots/fix-sprint1/*.png` for: recipe detail (Black Bean Tacos), meal detail (Pork Stir-Fry), `/recommended`, `/this-does-not-exist`, mobile dashboard.
- Manual smoke: tap Generate on a mobile viewport, confirm a new meal lands in the slot.
- **Done when:** All five P0 bugs absent in the re-captured screenshots AND lint/build pass.
---
## Sprint 2 — Trust the data (P1s)
**Goal:** No more silent data corruption in the UI. Every displayed value is consistent across pages and either present-and-correct or explicitly absent.
**Status (2026-06-02):** ✅ All six P1s + the bonus S3.3 mobile stat-grid fix are implemented. `npm run build` green. Ready to commit and deploy.
### S2.1 · B6 — Dashboard `MealCard` title: 2-line clamp instead of 1-line truncate
- **File:** `frontend/src/pages/Dashboard.tsx:87`
- **Change:** Replaced `truncate` with `line-clamp-2` (already used elsewhere in the codebase — Tailwind 3.4+ has it in core). Image shrinks to 40×40 on `<md` (was 56×56 always) to give the title more room. Added `leading-tight` to tighten line-height for 2 lines.
- **Verify:** Trigger Generate on the dashboard. New card title is fully visible across 2 lines (no `B..` truncation). Build clean.
### S2.2 · B7 — MealDetail hero overlap + description clamp + marketing-copy strip
- **Files:** `frontend/src/pages/MealDetail.tsx:168-197`, `frontend/src/lib/utils.ts`
- **Change (3 sub-steps, one commit):**
1. Hero reworked: image is in normal flow, content panel uses `relative -mt-16 sm:-mt-20` instead of `absolute bottom-0`. Title is in normal flow with the description below it; the gradient now has `pointer-events-none` and goes from `from-black/80` to prevent overlap obscuring.
2. Description rendered via `cleanDescription(recipe.description)` with `line-clamp-2`.
3. Client-side trim helper `cleanDescription(input, maxLen=280)` in `lib/utils.ts` with a list of regex patterns that strip spoonacular marketing boilerplate (`Featured In Group…`, `users who liked this recipe also liked…`, `For $X.XX per serving, this recipe covers…`, `It is brought to you by Foodista.`, etc.) and trims to the last sentence within 280 chars.
4. Raw `recipe.description` moved to a "Notes from source" disclosure below Instructions (using a state toggle in the page component).
- **Verify:** Reload `/meals/<id>`. Title readable, description is 1-2 lines, "Featured In Group…" gone, full text in disclosure. Build clean.
### S2.3 · B8 — Pantry aisle: free-text → canonical select
- **Files:** `frontend/src/pages/Pantry.tsx`, `frontend/src/types/index.ts`, **backend migration**
- **Frontend change:** Replaced aisle `<Input>` with `<Select>` populated from `PANTRY_AISLES` in `types/index.ts`:
```ts
export const PANTRY_AISLES = [
'Produce', 'Meat & Seafood', 'Dairy & Eggs', 'Pantry',
'Frozen', 'Bakery', 'Beverages', 'Spices', 'Other',
] as const;
export type PantryAisle = (typeof PANTRY_AISLES)[number];
```
Also converted Unit to a `<Select>` with the canonical unit list. Added `*` to "Ingredient name" label as a required-field marker.
- **Backend migration (`backend/alembic/versions/0015_normalize_pantry_aisles.py`):**
- Revises `0014`. Runs in a single upgrade step.
- Creates a `TEMP` backup table for each of `ingredient.aisle` and `grocery_item.aisle` (so a DBA can recover via `SELECT * FROM pg_temp.ingredient_aisle_backup` if needed).
- `UPDATE`s both columns via a generated `CASE LOWER(COALESCE(aisle,'')) WHEN ... END` mapping. Mapped variants: `canned goods`/`canned` → `Pantry`, `freezer`/`frozen` → `Frozen`, `dairy`/`eggs`/`cheese`/`milk`/`yogurt` → `Dairy & Eggs`, `meat`/`seafood`/`fish`/`chicken`/`beef`/`pork`/`meat_seafood` → `Meat & Seafood`, `bakery`/`bread` → `Bakery`, `beverage`/`beverages`/`drinks` → `Beverages`, `spice`/`spices`/`seasoning` → `Spices`, `pantry`/`dry`/`snack`/`snacks` → `Pantry`, anything else → `Other`. NULL stays NULL.
- **Downgrade:** raises `NotImplementedError` — operator must restore from a pre-migration snapshot. Documented in migration docstring.
- **Dry-run SQL helper (`backend/scripts/dry_run_aisle_migration.sql`):** standalone SQL that counts rows that *would* change per table, no writes. Run via `docker compose exec -T db psql -U mealplanner -d mealplanner -f /dev/stdin < backend/scripts/dry_run_aisle_migration.sql` (no host psql needed; the db runs in a container).
- **Persistent backup helper (`backend/scripts/persist_aisle_backup.sql`):** creates `public.ingredient_aisle_backup_0015` and `public.grocery_item_aisle_backup_0015` permanent tables. Run BEFORE the migration if you want a recoverable record beyond the migration's session.
- **Verify (on dev DB):**
```bash
# Persistent backup (optional, recommended)
docker compose exec -T db psql -U mealplanner -d mealplanner \
-f /dev/stdin < backend/scripts/persist_aisle_backup.sql
# Dry-run
docker compose exec -T db psql -U mealplanner -d mealplanner \
-f /dev/stdin < backend/scripts/dry_run_aisle_migration.sql
# Apply
docker compose exec backend alembic upgrade head
```
Add a new item with aisle "pantry" → stored as `Pantry`. Open Pantry list → all rows show sentence-case canonical labels. Frontend `npm run build` clean.
### S2.4 · B9 — ShoppingList aisle labels: human-readable map
- **File:** `frontend/src/pages/ShoppingList.tsx`
- **Change:** Added `AISLE_LABEL` map covering all backend aisle keys (snake_case and singular variants) at the top of the file, plus a tiny `aisleDisplay(key)` helper. Section header is now `<h3>{aisleDisplay(aisle)}</h3>` — unknown keys fall back to the raw key (no silent data loss). Also collapsed the S3.3 mobile stat-card grid into this edit since the file was already open.
- **Verify:** Reload `/shopping-list`. Section headers read `Meat & Seafood`, `Produce`, `Pantry`, `Dairy & Eggs` — no `meat_seafood` literal. Build clean.
### S2.5 · B10 — Mobile pantry table: scroll hint
- **File:** `frontend/src/pages/Pantry.tsx:236`
- **Change:** Wrapped `overflow-x-auto` in a `relative` container. Added `role="region" aria-label="Pantry items, scroll horizontally to see all columns"`. Right-edge gradient overlay (`pointer-events-none absolute inset-y-0 right-0 w-8 bg-gradient-to-l from-white to-transparent md:hidden`, `aria-hidden`) hints at overflow on mobile only.
- **Verify:** Screenshot at 390 px. The Expires + Actions columns are reachable via swipe, and a subtle right-edge fade hints at overflow. Build clean.
### S2.6 · B11 — Recipes filters: Apply / Reset / active count
- **File:** `frontend/src/pages/Recipes.tsx`
- **Change:**
1. Lifted filter state into a single `applied` object (query-bound) and a `pending` object (form-bound). Form fields mutate `pending`; the query uses `applied`.
2. Added `activeCount = Object.values(applied).filter(Boolean).length`.
3. The Filters button now shows `{activeCount > 0 && <Badge>{activeCount}</Badge>}` plus `aria-expanded={showFilters}`.
4. Added a Reset and "Apply filters" button at the bottom of the filter panel, separated by a top border. Apply commits `pending → applied`; Reset clears both.
5. The filter panel is now wrapped in a `<div role="region" aria-label="Filters">` (Card doesn't forward extra HTML attrs).
- **Verify:** Open `/recipes`, apply 2 filters, collapse panel → button shows `Filters (2)`. Click Reset → all cleared, badge gone. Build clean.
### S2.7 · Sprint 2 verification gate
- `npm run lint && npm run build` pass.
- Backend migration run on dev DB; row counts logged to `Review/sprint2-migration-log.md`.
- Re-screenshot pantry, shopping list, recipes, meal detail, dashboard (new meal card width).
- **Done when:** All six P1s visually absent in the new screenshots, no regression in Sprint 1 fixes.
---
## Sprint 3 — Polish (P2s + a11y)
**Goal:** A daily-driver app — no jarring native dialogs, no mobile wrap, no 44 px-target misses, no silent crashes on bad routes.
### S3.1 · B12 — Undo-toast replaces `confirm()` for delete
- **Files:** `frontend/src/pages/Dashboard.tsx`, `Pantry.tsx`, `ShoppingList.tsx`, `lib/toast.ts`
- **Change:**
1. Extend `lib/toast.ts` with `toastUndo(msg, onUndo, ms=5000)` that uses `react-hot-toast` custom render with an "Undo" button.
2. Replace every `confirm('Delete…?')` and `window.confirm(...)` with the new helper. The undo handler re-fires the create mutation.
- **Verify:** Delete a meal → toast appears "Meal removed" with Undo. Click Undo within 5s → meal re-appears. Build clean.
### S3.2 · B13 — Mobile nav: `whitespace-nowrap` on link text
- **File:** `frontend/src/App.tsx:30-33`
- **Change:** Add `whitespace-nowrap` to the `linkClass` helper return string. Consider also reducing the `px-3` to `px-2 sm:px-3` to keep all 4 links on one line down to 360 px.
- **Verify:** Screenshot at 360 px. All 4 links on one line. Build clean.
### S3.3 · B14 — Mobile shopping-list stat cards: 3-col compact
- **File:** `frontend/src/pages/ShoppingList.tsx`
- **Change:** Replace the current 3 stacked full-width tiles (mobile) with `grid grid-cols-3 gap-2` and shrink padding. Hide the descriptive label on `< sm`; show only the value.
- **Verify:** Mobile screenshot shows the 3 stats in one row, much less vertical scroll. Build clean.
### S3.4 · ErrorBoundary is already present (verified)
- No action. Note this in commit message: `chore(docs): ErrorBoundary already mounted in App.tsx:42; B-H9 closed without code change.`
### S3.5 · A11y sweep (4 small fixes, one commit)
- **Files:** `frontend/src/App.tsx`, `frontend/src/pages/Recipes.tsx`, `frontend/src/pages/Dashboard.tsx`, `frontend/src/components/ui/Badge.tsx`
- **Change:**
1. `Navigation.tsx`/`App.tsx`: add `aria-current={isActive(prefix) ? 'page' : undefined}` on each `<Link>`.
2. Recipes filter panel `<section>`: `role="region" aria-label="Filters"`.
3. Empty Generate slots: bump to `min-h-11` (44 px) on the button itself.
4. Badge component: add optional `icon` prop + `aria-label` for color-only badges.
- **Verify:** Tab through the nav: active link has `aria-current="page"`. Inspect filter panel DOM. Measure empty-slot buttons at 390 px width.
### S3.6 · Sprint 3 verification gate
- `npm run lint && npm run build` pass.
- Final playwright walkthrough. All 14 audit findings closed in screenshots.
- Update `Review/ui-nielsen-audit.md` to mark each fix with a `[x]` and commit hash reference.
---
## Future (NOT in this plan — capture as follow-up tickets)
- F1. Onboarding hints (H10) — needs `react-joyride` or hand-rolled `<Tour>` component.
- F2. Keyboard shortcuts (`/`, `g p`, `g s`, `n m`).
- F3. Bulk add on Pantry/Shopping List (H7).
- F4. Plan-the-whole-week button (H7).
- F5. Persistent week selector in URL.
- F6. `aria-label` on color-only status badges (generalized).
- F7. Global `react-query` `onError` toast handler.
---
## Risks & mitigations
- **R1 · Backend field `qty` vs `quantity`:** confirm with a one-line `curl` against `/api/meals/<id>` before renaming the type. If the API still returns `quantity`, use a shim `ing.qty ?? ing.quantity` rather than breaking other consumers.
- **R2 · Pantry migration:** run against dev DB first; capture before/after row counts. **Do not** run on prod without the `--backup-table` step in place.
- **R3 · Tailwind `line-clamp-N`:** verify the project's `tailwind.config.js` enables the `lineClamp` core plugin (Tailwind 3.3+ has it on by default; project is on `^3.4.1`, so it should work).
- **R4 · Undo-toast:** requires the delete mutation to be reversible (i.e. we have the prior item body). Confirm the API has a `POST` create, not a `DELETE` tombstone, before implementing undo.
- **R5 · Build/runtime parity:** `npm run build` runs `tsc && vite build`. If a teammate runs `vite build` alone, type errors slip through. Add a CI hint in PR template.
---
## Done when (overall)
- [ ] All 14 audit findings closed and screenshot-verified.
- [ ] `npm run lint && npm run build` green in CI.
- [ ] Backend aisle-migration run on dev; row counts logged.
- [ ] `Review/ui-nielsen-audit.md` updated with `[x]` per finding + commit refs.
- [ ] No regressions in existing Playwright walkthrough (full screenshot diff vs `/tmp/opencode/mp-review/screenshots/`).