|
|
@@ -0,0 +1,122 @@
|
|
|
+# MovieDice UI Fixes — Parallel Batch Plan
|
|
|
+
|
|
|
+## Context
|
|
|
+
|
|
|
+User QA pass on MovieDice (Phase 4 complete, awaiting manual sign-off) surfaced 9 UX defects across the movie-list view, the logged-in home page, and the landing carousel. Each fix is small and largely isolated, so they parallelize well across git worktrees.
|
|
|
+
|
|
|
+Confirmed with user:
|
|
|
+- **"More Info"** = a **new** button on the expanded poster detail that fetches TMDB details and shows them in a modal (the existing `/api/tmdb/movie/[id]` proxy at `src/app/api/tmdb/movie/[id]/route.ts` already returns what's needed).
|
|
|
+- **Back button** = `router.back()` (browser history), fallback to `/` if `history.length <= 1`.
|
|
|
+- **Verification** = each worker runs `npm test` and `npx tsc --noEmit` on its branch; manual QA happens after merge.
|
|
|
+
|
|
|
+## What each worker needs to know (shared conventions)
|
|
|
+
|
|
|
+- **Stack**: Next.js 16 App Router, React 19, TanStack Query, Tailwind v4, Vitest + jsdom. Supabase self-hosted for data.
|
|
|
+- **Posters**: native `<img loading="lazy">` with alt text — never `next/image`. Get URLs via `getTMDBImageUrl(path, size)` from `src/types/tmdb.ts`.
|
|
|
+- **A11y**: min tap target 44×44, `aria-live="polite"` for result announcements, `prefers-reduced-motion` respected on animations. Reference pattern: `src/components/dice/roll-animation.tsx`, `src/components/movies/delete-button.tsx`.
|
|
|
+- **Test location**: `src/__tests__/{components,hooks,...}/*.test.(ts|tsx)`. Current count: 10 files / 71 tests green.
|
|
|
+- **Genre filter**: use `filterByGenresAndEmotionsStructured({ genreIds, moodKeys }, movies)` from `src/lib/dice/genre-filter.ts`. `movies.genres` stores TMDB genre **names** (e.g. "Action"), not IDs.
|
|
|
+- **No backwards-compat shims**: remove unused props/functions rather than deprecating.
|
|
|
+- **Keep comments sparse**. Don't add JSDoc bloat.
|
|
|
+
|
|
|
+## Work Units (9)
|
|
|
+
|
|
|
+### U1 — Genre tags non-clickable
|
|
|
+**Files**: `src/components/movies/genre-tag.tsx`, `src/components/movies/expanded-panel.tsx`, `src/components/movies/poster-grid.tsx`, `src/components/movies/movie-list-client.tsx`.
|
|
|
+**Change**: Convert `<GenreTag>` from `<button>` to `<span>` (visual chip only). Remove `onSelect` prop and the `selectedGenre` state plumbing through PosterGrid / ExpandedPanel / MovieListClient — per project convention, delete dead code rather than keep. U2 introduces the replacement filter mechanism via a dropdown, so in-panel click-to-filter can go.
|
|
|
+**Tests**: update `expanded-panel` tests if present; confirm `npm test` stays green.
|
|
|
+
|
|
|
+### U2 — Genre/mood filter dropdown under search bar
|
|
|
+**Files**: new `src/components/movies/genre-filter-dropdown.tsx`; edits to `src/components/movies/movie-list-client.tsx`, `src/components/movies/poster-grid.tsx`.
|
|
|
+**Change**: New collapsed-by-default dropdown rendered immediately below `<SearchBar>` inside `MovieListClient`. Two sections — Genres (from `TMDB_GENRE_MAP`) and Moods (from `CANONICAL_MOODS` — same source `GenreRollModal` uses). Multi-select chips. State lifted to `MovieListClient`; pass `{ genreIds: number[], moodKeys: string[] }` down to `PosterGrid`, which calls `filterByGenresAndEmotionsStructured` against its unwatched/watched split. Model the chip UX on `src/components/dice/genre-roll-modal.tsx` (but inline, not modal; no max-5 cap; no "Roll" CTA — this is a filter, not a roller). Empty selection = no filter.
|
|
|
+**Tests**: add a vitest for the filter integration (mock movies → select genre → filtered count). Keep GenreRollModal untouched (U6 keeps the RollBar).
|
|
|
+
|
|
|
+### U3 — Add-to-list dismisses search results, keeps query
|
|
|
+**Files**: `src/components/movies/movie-list-client.tsx`.
|
|
|
+**Change**: After `addMovie.mutate()` resolves successfully in `handleAdd` (lines 82–93), clear the `searchQuery → searchResults` rendering while keeping the text in `<SearchBar>`. Simplest approach: add a `searchDismissed: boolean` state (reset to `false` on next `handleSearch` call), gate the results list render on `!searchDismissed && searchQuery.length >= 2`. Do **not** clear `searchQuery`.
|
|
|
+**Tests**: add a vitest simulating `handleAdd` → results hidden but query preserved.
|
|
|
+
|
|
|
+### U4 — Watched button: green + shake + double-click to confirm
|
|
|
+**Files**: `src/components/movies/watched-button.tsx`. Reference: `src/components/movies/delete-button.tsx` (the pattern to mirror), `src/app/globals.css` (existing `animate-shake` utility).
|
|
|
+**Change**: When currently-unwatched, first click sets `confirming=true` and applies `animate-shake` + green background; second click (within some reset window, match DeleteButton's behavior) calls `onToggle`. When already watched, one click un-watches (no confirm needed — symmetric with how delete works). Reuse `animate-shake` utility — don't redefine it.
|
|
|
+**Tests**: extend or add vitest covering single-click unwatch, double-click-to-watch, timeout auto-reset if the delete button has one.
|
|
|
+
|
|
|
+### U5 — "More Info" button + TMDB modal
|
|
|
+**Files**: new `src/components/movies/more-info-modal.tsx`, new `src/hooks/use-tmdb-movie-details.ts`, edit `src/components/movies/expanded-panel.tsx`.
|
|
|
+**Change**:
|
|
|
+1. New hook `useTmdbMovieDetails(tmdbId)` using TanStack Query: `GET /api/tmdb/movie/{tmdbId}` (route already exists, `TMDBMovieDetails` type already exists in `src/types/tmdb.ts`). `staleTime: 1h` to match upstream cache.
|
|
|
+2. New `<MoreInfoModal>` overlay component: centered, focus-trap (mirror `GenreRollModal` focus-trap pattern at `src/components/dice/genre-roll-modal.tsx` lines 60–104), ESC closes, click-outside closes. Shows poster (w500), title+year, runtime, vote_average, overview, genres. On loading → skeleton; on error → friendly message.
|
|
|
+3. In `ExpandedPanel`, add a `More Info` button next to Trailer; opens the modal. Movies in the DB have `tmdb_id` (confirm column exists in `Database` types).
|
|
|
+**Tests**: vitest for the hook (mocked fetch), vitest for the modal (renders loading/success/error), vitest for the button wiring in ExpandedPanel.
|
|
|
+
|
|
|
+### U6 — Roll buttons vertical stack at top of list, above search
|
|
|
+**Files**: `src/components/movies/movie-list-client.tsx`, `src/components/dice/roll-bar.tsx`.
|
|
|
+**Change**: In `MovieListClient`'s JSX (currently SearchBar → SearchResults → RollBar → …), move `<RollBar>` to be the **first** child above the search bar. Edit `RollBar` to render buttons in a vertical stack (`flex flex-col gap-2`) instead of horizontal row (`flex gap-3`) — keep a11y-safe `min-h[44px] min-w[44px]`, full width buttons (`w-full`). Leave the `<RollAnnouncer>`, `<RollAnimation>`, `<RollResultCard>` where they are. Do NOT move announcer/result — only the button bar relocates.
|
|
|
+**Tests**: existing roll-bar test (if any) should still pass; add a snapshot or order assertion for `MovieListClient`.
|
|
|
+
|
|
|
+### U7 — Back button top-left on every page (browser history)
|
|
|
+**Files**: new `src/components/ui/back-button.tsx`, edits to `src/app/(app)/layout.tsx`, `src/app/(app)/list/[id]/page.tsx`, `src/app/(auth)/layout.tsx` (create if absent — check first), `src/app/(public)/layout.tsx`, `src/app/admin/layout.tsx` (check existence).
|
|
|
+**Change**: `<BackButton>` client component: calls `router.back()` on click; if `window.history.length <= 1`, navigates to `/` instead. 44×44 min, left-arrow SVG, `aria-label="Back"`. Insert into each layout's header left-slot. On the list page, the existing sticky header already owns the left slot (currently just `<h1>{group.name}</h1>`) — prepend the BackButton before the title.
|
|
|
+**Scope guard**: the logout and root-redirect routes may not need a back button — skip any page that is itself a fall-through redirect (`src/app/page.tsx`).
|
|
|
+**Tests**: vitest for the component — mock `next/navigation` `useRouter`, assert `router.back()` called, fallback to `router.push('/')` when history empty.
|
|
|
+
|
|
|
+### U8 — Center logged-in homepage content
|
|
|
+**Files**: `src/app/(app)/home/page.tsx`, `src/components/home/roll-section.tsx` (only if buttons need centering).
|
|
|
+**Change**: The page has `mx-auto max-w-3xl` (centered horizontally), but internal content (`<h1>Your Lists</h1>`, the RollSection buttons, the "+ Create List" button) is left-aligned. Apply `text-center` on the `<section>` and `items-center` on the flex wrapper so the whole hero block centers inside the container. ListGrid below stays as a grid. In `RollSection`, the `<div className="flex flex-wrap gap-3">` button row should switch to `justify-center`.
|
|
|
+**Tests**: snapshot or className assertion in a new `home-page.test.tsx`.
|
|
|
+
|
|
|
+### U9 — Carousel always visible during / after spin
|
|
|
+**Files**: `src/components/landing/carousel-animation.tsx`.
|
|
|
+**Change**: Root cause — during the `"spinning"` phase (lines 96–120), `scrollOffsetRef.current += FAST_SPEED` accumulates up to ~3780px over 3500ms **without** the auto-scroll wrap (`scrollOffsetRef.current -= SET_WIDTH` at line 92–94). Once past `SET_WIDTH` it's still within the tripled strip visually, but there's no guard against running off the third copy. And after settle, offset is never normalized back into the first-to-second copy range, so subsequent spins extend further.
|
|
|
+**Fix**: Apply the modulo wrap inside the spinning branch too: after each `scrollOffsetRef.current += delta`, do `if (scrollOffsetRef.current >= SET_WIDTH) scrollOffsetRef.current -= SET_WIDTH`. On settle, normalize: `scrollOffsetRef.current = scrollOffsetRef.current % SET_WIDTH` (or snap-to-poster math that already targets the middle copy). This keeps the visible window always inside the middle copy of the tripled set without requiring more DOM — memory footprint unchanged.
|
|
|
+**Do NOT** drop back to fewer posters or re-introduce virtualization. The user explicitly asked to avoid that.
|
|
|
+**Tests**: unit test for a helper function extracted from the animation loop: `wrapOffset(offset, setWidth) → number in [0, setWidth)`. Keep the rAF loop untested (jsdom can't do it meaningfully).
|
|
|
+
|
|
|
+## E2E Verification Recipe
|
|
|
+
|
|
|
+Per user decision: **unit tests + manual QA**. Each worker must:
|
|
|
+
|
|
|
+1. Run `npm test` — must stay green.
|
|
|
+2. Run `npx tsc --noEmit 2>&1 | grep -v "__tests__" | grep -v "\.test\." | grep -v ".next/types"` — no NEW errors in `src/` (pre-existing `.next/types/validator.ts` error is known and unrelated).
|
|
|
+3. **Do not** attempt to boot a dev server in-worktree — the parent coordinator already has `npm run dev` on localhost:3000 for manual QA post-merge.
|
|
|
+4. Before committing, invoke `Skill` tool with `skill: "simplify"` to review the diff.
|
|
|
+5. If tests or typecheck fail, fix and re-run before creating the PR.
|
|
|
+
|
|
|
+Manual QA after merge will be performed by the coordinator against localhost:3000.
|
|
|
+
|
|
|
+## Worker Instruction Template (shared by all units)
|
|
|
+
|
|
|
+Each worker gets the shared conventions above + its unit spec + the boilerplate from `/batch` phase 2:
|
|
|
+
|
|
|
+```
|
|
|
+After you finish implementing the change:
|
|
|
+1. Simplify — Invoke the Skill tool with skill: "simplify".
|
|
|
+2. Run unit tests — `npm test`. Fix any failures.
|
|
|
+3. Typecheck — `npx tsc --noEmit`. Ignore pre-existing errors in .next/types and __tests__ dirs; fix anything new in src/.
|
|
|
+4. Do NOT boot a dev server — coordinator does manual QA post-merge.
|
|
|
+5. Commit and push — clear message, then `gh pr create`.
|
|
|
+6. Report — end with `PR: <url>` or `PR: none — <reason>`.
|
|
|
+```
|
|
|
+
|
|
|
+## Files to modify — reference map
|
|
|
+
|
|
|
+| Unit | Primary files |
|
|
|
+|------|---------------|
|
|
|
+| U1 | `src/components/movies/genre-tag.tsx`, `expanded-panel.tsx`, `poster-grid.tsx`, `movie-list-client.tsx` |
|
|
|
+| U2 | new `src/components/movies/genre-filter-dropdown.tsx`; `movie-list-client.tsx`, `poster-grid.tsx` |
|
|
|
+| U3 | `src/components/movies/movie-list-client.tsx` |
|
|
|
+| U4 | `src/components/movies/watched-button.tsx` |
|
|
|
+| U5 | new `src/components/movies/more-info-modal.tsx`, new `src/hooks/use-tmdb-movie-details.ts`, `expanded-panel.tsx` |
|
|
|
+| U6 | `src/components/movies/movie-list-client.tsx`, `src/components/dice/roll-bar.tsx` |
|
|
|
+| U7 | new `src/components/ui/back-button.tsx`, `src/app/(app)/layout.tsx`, `src/app/(app)/list/[id]/page.tsx`, `(auth)/layout.tsx`, `(public)/layout.tsx`, `admin/layout.tsx` |
|
|
|
+| U8 | `src/app/(app)/home/page.tsx`, `src/components/home/roll-section.tsx` |
|
|
|
+| U9 | `src/components/landing/carousel-animation.tsx` |
|
|
|
+
|
|
|
+## Merge conflict hotspots
|
|
|
+
|
|
|
+`movie-list-client.tsx` is touched by **U1, U2, U3, U6**. These conflicts are predictable and resolvable by hand at merge time — each unit's edits are in different regions of the file (U1 removes genre-select plumbing; U2 adds dropdown below SearchBar; U3 adds `searchDismissed` state in handleAdd; U6 reorders JSX so RollBar is first). Expect to merge U6 first (pure reorder), then U3 (tiny state add), then U2 (new component + prop plumbing), then U1 (removal of what U2 replaces).
|
|
|
+
|
|
|
+## Out of scope
|
|
|
+
|
|
|
+- Axe browser scan, VO/Orca testing, and the Phase 4 E2E recipe — tracked in `research/PHASE4_TEST_MATRIX.md` and owned by the human QA pass.
|
|
|
+- Real-time `supabase-realtime` container restart loop (pre-existing).
|