Prechádzať zdrojové kódy

[Docs] Phase 3+5 implementation plan

Plan for tasks 3.5 (infinite scroll polish), 3.8 (watched-section
collapse), 3.9 (realtime subscription completion), 5.5 (empty states),
5.6 (network-error UX). Notes that 3.5 and 3.9 are largely already
wired; remaining work is integration polish, reconnect triggers,
status surfacing, and harmonized empty-state component usage.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
User 1 mesiac pred
rodič
commit
16e4fc0297
1 zmenil súbory, kde vykonal 198 pridanie a 0 odobranie
  1. 198 0
      research/PLAN-PHASE-3-5.md

+ 198 - 0
research/PLAN-PHASE-3-5.md

@@ -0,0 +1,198 @@
+# PLAN — Phase 3 & 5 (tasks 3.5, 3.8, 3.9, 5.5, 5.6)
+
+## Status discovery (relevant to all tasks)
+
+- Infinite scroll (3.5) is **already wired** end-to-end: `useGroupMovies` is `useInfiniteQuery` with `MOVIES_PER_PAGE=12`, `getNextPageParam` keyed on page index; `PosterGrid` uses `useInfiniteScroll` (IntersectionObserver, `rootMargin: 200px`) on a sentinel div. **Task 3.5 is largely complete**; remaining work is mostly polish + interaction with realtime/persistence.
+- Watched-section collapse (3.8) is **partially in place**: `PosterGrid` has a local-state toggle (`watchedOpen`), `aria-expanded`, rotate caret. Missing: persistence, reduced-motion, animation polish, default state contract, layout positioning.
+- Realtime (3.9) is **mostly working**: `useRealtimeMovies` handles INSERT/UPDATE/DELETE with cache surgery; `useRealtimeChannel` has `ReconnectionManager` w/ exponential backoff + jitter (1s→30s). Missing: presence-of-self skip, page-window vs detail-row coherence with infinite scroll, status surface to UI, online/visibility-change reconnect.
+- Empty states (5.5): `src/components/shared/empty-state.tsx` already has presets (`emptyList`, `noSearchResults`, `noGenreMatches`, `newUser`). Surfaces use ad-hoc `<p>` text. `home/empty-state.tsx` is the polished anchor.
+- Network-error UX (5.6): `OfflineBanner` exists but is **not mounted anywhere**; `ErrorBoundary` exists but isn't applied to route trees; `ErrorMessage` has presets but unused. No toast system.
+
+---
+
+## Task 3.5 — Infinite scroll (polish + integration)
+
+**Files to modify**
+
+- `src/hooks/use-group-movies.ts` — add `getNextPageParam` guard against undefined, consider `maxPages` cap when persistence is on
+- `src/components/movies/poster-grid.tsx` — sentinel position, accessible "Load more" button fallback
+- `src/hooks/use-infinite-scroll.ts` — accept `enabled` flag (disable while a roll-carousel is in flight to avoid layout shift)
+- `src/components/providers.tsx` — when `persistQueryClient` lands, set `maxPages` on infinite queries (otherwise IndexedDB grows unbounded)
+
+**Implementation order**
+
+1. Add `enabled` to `useInfiniteScroll`; pass `false` while `rollState !== "idle"` (prevents fetching while carousel animation runs).
+2. Add a visible **"Load more"** fallback button rendered alongside the sentinel — covers prefers-reduced-motion users and keyboard-only users; click triggers `fetchNextPage()`. Hide when `!hasNextPage`.
+3. Cap `maxPages` to ~10 (120 movies) once persistence is enabled, with `getPreviousPageParam` so older pages can refetch when scrolled back. (Open question — see below.)
+4. Verify sentinel placement: today the sentinel is **between unwatched grid and Watched section** — acceptable, but means scrolling to Watched also triggers loads. Move sentinel inside unwatched grid wrapper, after the last item.
+
+**Trade-offs**
+
+- IntersectionObserver vs scroll-event: IO is already chosen and correct — keep.
+- `maxPages` cap vs unbounded: capped is safer for IndexedDB persistence and realtime cache mutation cost (each INSERT iterates pages). **Recommend cap=10** unless lists routinely exceed 120 movies.
+- Page size 12 fits 4-col × 3-row on desktop and 2-col × 6-row on mobile — good. Keep.
+
+**Risks / open questions**
+
+- Realtime INSERT prepends to page 0 only; if cap is set, it can drop the oldest page on overflow — confirm desired behavior.
+- When persistence rehydrates a stale cache with N pages already, do we refetch page 0 only or all pages? Recommend `refetchOnMount` for page 0 + `refetchInterval` off; rely on realtime for fresh.
+
+---
+
+## Task 3.8 — Watched-section collapse
+
+**Files to modify**
+
+- `src/components/movies/poster-grid.tsx` — replace `useState` with persisted hook, add reduced-motion-aware animation, tighten layout
+- `src/hooks/use-persisted-state.ts` _(new)_ — generic `localStorage`-backed `useState` with SSR-safe lazy init
+- `src/lib/constants.ts` — add `WATCHED_COLLAPSE_KEY = "moviedice:watched-open"` (per-list key shape)
+
+**Implementation order**
+
+1. Create `usePersistedState<T>(key, initial)` — lazy init from `localStorage` on mount (avoid hydration mismatch by reading inside `useEffect` and reconciling).
+2. Key by `groupId`: `moviedice:watched-open:{groupId}`. **Default: collapsed** (matches "Watched" being de-prioritized; reduces visual noise on big lists).
+3. Replace `setWatchedOpen` in `PosterGrid` with persisted state.
+4. Animate the disclosure with CSS grid-rows trick (`grid-template-rows: 0fr` → `1fr`) gated by `@media (prefers-reduced-motion: reduce)` → instant.
+5. Toggle position: keep inline below unwatched grid (current location) — predictable, doesn't compete with the sticky header. Add `data-testid="watched-toggle"` for tests.
+
+**Trade-offs**
+
+- localStorage vs server: localStorage is per-device and sufficient (UI preference, not a shared decision). Server persistence would require schema, RLS, mutations — overkill. **Recommend localStorage.**
+- Per-list vs global key: per-list lets a user keep different lists in different states. **Recommend per-list.**
+- Animation library: avoid framer-motion for one disclosure; CSS grid-rows works in evergreen browsers and doesn't ship JS.
+
+**Risks / open questions**
+
+- Should an empty Watched section render the toggle disabled, or hide entirely? (Current code hides — recommend keep.)
+- Should the count badge live next to the chevron or as a pill? Current: inline `(N)` — fine.
+
+---
+
+## Task 3.9 — Realtime subscriptions (finish)
+
+**Files to modify**
+
+- `src/hooks/use-realtime-movies.ts` — INSERT page-window placement, self-event skip
+- `src/hooks/use-realtime-channel.ts` — reconnect on `visibilitychange` + `online` events
+- `src/components/movies/movie-list-client.tsx` — surface `status` (banner when `error`/`disconnected` >5s)
+- `src/lib/realtime/subscription-manager.ts` — already complete
+
+**Implementation order**
+
+1. **INSERT placement w/ infinite scroll**: New rows ordered by `added_at DESC` always belong on page 0. Current code prepends to `pages[0]` — correct. But if the cache has been `maxPages`-evicted from the head, the new row may overlap with a future fetch. Mitigation: keep the prepend, and dedupe in `fetchGroupMovies` via `select` + `id` set check, or simpler — leave dedupe in the INSERT handler (already present) and accept occasional duplicate page entries collapsed by id.
+2. **Self-event echo**: `useAddMovie`/`useToggleWatched`/`useDeleteMovie` already optimistically update; the realtime echo is idempotent for UPDATE/DELETE but INSERT can briefly double-show. Add `payload.new.added_by === currentUser.id && createdWithinLastNms` skip OR (preferred) make optimistic INSERT use the real DB id from the mutation response (mutations should already return the row) — then realtime INSERT is a no-op via existing dedupe. Audit `useAddMovie` to confirm.
+3. **Reconnect triggers**: in `useRealtimeChannel`, add `window.addEventListener("online", resubscribe)` and `document.addEventListener("visibilitychange", () => document.visibilityState === "visible" && resubscribe())`. Reset backoff attempt on each. Also call `queryClient.invalidateQueries(["group-movies", groupId])` on reconnect to catch missed events during the gap.
+4. **Status surface**: pass `status` from `useRealtimeMovies` up; show a tiny "Reconnecting..." pill when `status === "error"` for >3s. Don't surface transient `connecting`.
+5. **Single-list invariant**: `useRealtimeMovies` is called only in `MovieListClient` (per-list page). Confirmed compliant with CLAUDE.md "one list at a time."
+
+**Trade-offs**
+
+- Realtime + infinite scroll cache mutation cost: O(pages × pageSize). With 10-page cap × 12 = 120 entries, negligible.
+- Per-row UPDATE updates whichever page contains the row — current implementation handles this.
+- DELETE doesn't shift pages; acceptable (pages can shrink), and a subsequent `fetchNextPage` loads from current `from/to` which may double-count one row at the boundary. Open question — see below.
+
+**Risks / open questions**
+
+- Pagination boundary drift after DELETE (range pagination + mutating dataset): a deleted row before the next-page boundary causes a row to shift across pages, missing on next fetch. Mitigation options: (a) cursor pagination by `added_at`, (b) accept the rare miss, (c) `queryClient.invalidateQueries` on DELETE. **Recommend (c)** — full refetch is cheap at 12/page and removes the entire class of bug.
+- Should we add presence/typing indicators? Not in scope.
+
+---
+
+## Task 5.5 — Empty states (audit + apply)
+
+**Surfaces and treatments**
+
+| Surface                              | File                                                   | State today                 | Plan                                                                                                                        |
+| ------------------------------------ | ------------------------------------------------------ | --------------------------- | --------------------------------------------------------------------------------------------------------------------------- |
+| Home grid (no lists)                 | `src/components/home/list-grid.tsx`                    | Uses `home/empty-state.tsx` | **Done** — keep                                                                                                             |
+| List detail (no movies)              | `src/components/movies/poster-grid.tsx` line 106       | Plain `<p>`                 | Replace with `EmptyState` preset `emptyList` + CTA "Search to add a movie" that focuses the search bar via shared ref/event |
+| Search results (no matches)          | `src/components/movies/search-results.tsx` line 43     | Plain `<p>`                 | Replace with `EmptyState` preset `noSearchResults` (compact variant)                                                        |
+| Genre roll (no matches)              | `src/components/movies/movie-list-client.tsx` line 152 | Yellow banner               | Keep banner — transient roll outcome, not an empty state                                                                    |
+| Watched section (none watched)       | `poster-grid.tsx` line 149                             | Section hidden              | Keep hidden                                                                                                                 |
+| Roll pool empty (`poolEmpty`)        | `RollBar`                                              | Disabled                    | Verify `RollBar` shows tooltip/aria-label "Add movies to roll"                                                              |
+| All-user-movies (cross-list profile) | `use-all-user-movies.ts`                               | Unknown                     | Audit; add `EmptyState` if surfaced                                                                                         |
+
+**Files to create/modify**
+
+- `src/components/shared/empty-state.tsx` — add `compact` variant (smaller padding, no large icon)
+- `src/components/movies/poster-grid.tsx` — swap plain text for `<EmptyState>`
+- `src/components/movies/search-results.tsx` — swap plain text
+- `src/components/home/empty-state.tsx` — keep as-is (anchor design); harmonize `shared/empty-state.tsx` styling on theme tokens (currently `gray-900`/`gray-500` on shared, `foreground` tokens on home)
+
+**Implementation order**
+
+1. Harmonize `shared/empty-state.tsx` styling to use theme tokens matching `home/empty-state.tsx`.
+2. Add `compact` prop and an icon prop convention.
+3. Replace each surface's inline text with the component.
+4. Wire "Search to add" CTA in list-detail empty state — emit a custom event or lift a ref; simplest: scroll to top + focus query selector for the search input.
+
+**Trade-offs**
+
+- Centralized presets vs per-surface custom copy. **Recommend presets** for consistency, override `description` per surface where needed.
+
+**Risks / open questions**
+
+- Visual style mismatch between `shared` (light-mode-only colors) and `home` (theme tokens) — confirm dark-mode-first design and consolidate.
+
+---
+
+## Task 5.6 — Network-error UX
+
+**Files to create/modify**
+
+- `src/components/providers.tsx` — set `QueryCache` global `onError`, expand `defaultOptions` (retry policy, mutation `onError`)
+- `src/components/shared/toast.tsx` _(new)_ — minimal toast/portal singleton (`announce(message, variant)`)
+- `src/hooks/use-toast.ts` _(new)_ — thin wrapper
+- `src/app/layout.tsx` (or `(app)/layout.tsx`) — mount `<OfflineBanner />` and `<ToastViewport />`
+- `src/app/(app)/layout.tsx` — wrap children in `<ErrorBoundary>` with `ErrorMessage` fallback
+- `src/components/movies/poster-grid.tsx` — replace inline "Failed to load movies" with `ErrorMessage type="network"` + retry button calling `refetch()`
+- `src/components/home/list-grid.tsx` — same upgrade
+- Mutation hooks (`use-delete-movie`, `use-toggle-watched`) — add `onError` toast (the `useAddMovie` fix is independent)
+
+**Implementation order**
+
+1. Mount `<OfflineBanner />` in root layout (offline can be detected pre-auth too).
+2. Build a tiny toast system: portal to body, top-right, role="status", dismiss after 5s, variants `info|error|success`. ~80 lines, no dependency.
+3. Wire `QueryCache({ onError })` and `MutationCache({ onError })` in `Providers` to fire toasts globally for unhandled errors. Local handlers can opt out by setting `meta: { silent: true }`.
+4. Add `<ErrorBoundary>` around `(app)` layout content with a fallback that includes a "Reload" button.
+5. Per-query inline errors: convert plain `<p>`s to `<ErrorMessage>` + retry button (reuses TanStack `refetch`).
+6. Mutation errors: in each mutation hook's `onError`, call `toast.error(...)`. Roll back optimistic updates.
+
+**Toast vs inline guidance**
+
+- **Inline** (`ErrorMessage` + retry): query-level failures where data can't render — ListGrid, PosterGrid, search results.
+- **Toast**: mutation-level failures where the UI is otherwise fine — add/delete/toggle-watched, copy-invite-link, etc.
+- **Banner**: connection state — `OfflineBanner` (already built).
+- **ErrorBoundary**: render-time crashes only.
+
+**Trade-offs**
+
+- Custom toast vs `sonner`/`react-hot-toast`: a stripped-down custom toast keeps the dependency footprint clean. **Recommend custom**, ~80 LOC.
+- Global `QueryCache.onError` vs per-call: global catches unhandled, per-call (`meta.silent`) opts out. Best of both.
+- Retries: `retry: 1` is set globally. Mutations don't retry by default — keep that (idempotency not guaranteed for INSERT).
+
+**Risks / open questions**
+
+- `navigator.onLine` lies (says online when on a captive portal). Recommend trusting it for the banner only; rely on actual fetch failures for retry logic.
+- Should toasts persist across route changes? React state will survive within `(app)` layout — fine.
+- Does Sentry need a breadcrumb on toast errors? Probably yes — `Sentry.addBreadcrumb` from the toast helper.
+
+---
+
+## Cross-cutting concerns
+
+1. **3.5 ↔ 3.9 (infinite scroll + realtime)**: The DELETE/INSERT cache mutations must iterate all loaded pages (already do). Recommend invalidating `["group-movies", groupId]` on realtime DELETE to avoid pagination boundary drift; INSERT is fine via prepend+dedupe. If a `maxPages` cap is set (3.5 step 3), realtime INSERT must respect it (drop tail) — implement via `slice(0, maxPages)` after prepend.
+2. **3.5 ↔ persistence (post-MVP `persistQueryClient`)**: Cap `maxPages` to bound IndexedDB size; set `gcTime` ≥ persistence rehydrate window.
+3. **3.9 ↔ 5.6**: Realtime status `error` should NOT spam toasts (it backs off automatically). Surface only as a small inline pill after >3s, never as a toast.
+4. **3.8 ↔ 5.5**: When the unwatched section is empty but watched has items, show `EmptyState` for unwatched ("All caught up — pick something to watch next") and keep Watched expandable below. Worth a dedicated preset.
+5. **5.5 ↔ 5.6**: Errors are not empty states. PosterGrid currently conflates them slightly (separate `isError` and `length===0` branches — already correct). Document the distinction in `shared/empty-state.tsx` JSDoc.
+
+---
+
+## Critical Files for Implementation
+
+- `/home/user/moviedice/src/components/movies/poster-grid.tsx`
+- `/home/user/moviedice/src/hooks/use-realtime-movies.ts`
+- `/home/user/moviedice/src/hooks/use-realtime-channel.ts`
+- `/home/user/moviedice/src/components/providers.tsx`
+- `/home/user/moviedice/src/components/shared/empty-state.tsx`