Files
collective-lists/docs/history/fase-9-polish-dark-mode.md
Oier Bravo Urtasun 6cb13146f5 docs(fase-9): history doc + CLAUDE.md status + plan closed
- docs/history/fase-9-polish-dark-mode.md captures the final palette
  (hex + contrast ratios), the four sub-fases (dark mode / reading
  width / sync_conflicts / WebKit specs), the test-count delta
  (259 → 300 + 3 skipped, +41 specs), the upstream-bug audit for
  Realtime presence (#1617 still open as of 2026-05-18, latest stable
  v2.86.3), and the prod-migration note (existing sessions do not
  need a re-login because users.theme is queried, not in the JWT).
- CLAUDE.md project-status line flips Fase 9 to complete, bumps the
  test totals to 300 green / 3 skipped, and adds a new "Testing"
  gotcha for the WebKit project gate (RUN_WEBKIT=1 + libicu74 deps).
- plan/fase-9 marks the phase  with a one-liner about new test counts.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-18 01:53:02 +02:00

12 KiB
Raw Permalink Blame History

Fase 9 — Polish + Dark Mode (MVP2 · 1/6)

First slice of the MVP2 cycle. Feature flagship: dark mode. Plus three pulidos heredados (max-w-2xl, sync_conflicts, swipe-gesture E2E) cleared off the backlog without touching domain features.

Status: complete. 65 pgTAP assertions + 26 unit + 144 integration + 65 Playwright E2E = 300 tests green; 3 skipped (2 upstream Realtime presence + 1 gated rate-limit). WebKit specs (Fase 9.4) are gated behind RUN_WEBKIT=1 and counted separately.


9.1 — Dark mode

Three-mode preference (light | dark | system, default system for new users) persisted to public.users.theme and mirrored to localStorage for FOUC-free refreshes.

Final palette (CSS triplets, RGB)

Token Light Dark Contrast on background
--background 247 249 251 (#f7f9fb) 2 6 23 (slate-950)
--surface-container-low 241 245 249 (slate-100) 15 23 42 (slate-900) subtle grouping
--surface 255 255 255 (white) 30 41 59 (slate-800) base card
--surface-raised 248 250 252 (slate-50) 51 65 85 (slate-700) sidebar / popped card
--text-primary 15 23 42 (slate-900) 248 250 252 (slate-50) 14.7:1 (AAA)
--text-secondary 100 116 139 (slate-500) 148 163 184 (slate-400) 5.8:1 (AA)
--text-muted 148 163 184 (slate-400) 100 116 139 (slate-500)
--destructive 185 28 28 (red-700) 239 68 68 (red-500) tuned for the surface

All tokens stay as space-separated RGB triplets per the repo gotcha — consumed via rgb(var(--token) / <alpha>), never hsl(). Tailwind's darkMode: ['selector', '[data-theme="dark"]'] lets existing dark: utility pairs (bg-slate-100 dark:bg-slate-800, etc.) keep resolving from the same attribute the inline anti-FOUC script in app.html writes.

Plumbing

  • Migration 015 adds users.theme text NOT NULL DEFAULT 'system' CHECK (theme IN ('light','dark','system')). Existing rows pick up 'system' via the default; JWTs cache auth.users, not public.users, so adding the column does not invalidate active sessions — clients read it on demand.
  • src/app.html inline anti-FOUC script reads localStorage.theme (fallback 'system') and matchMedia('(prefers-color-scheme: dark)').matches synchronously, then writes <html data-theme=...> before the first paint. Works on every route including /logged-out (outside the (app) group), so RP-initiated logout never paints a light flash.
  • src/lib/theme.ts owns the JS-side stores (themePreference, resolvedTheme, derived isDark) plus a matchMedia('change') listener that re-resolves when the OS preference flips while the user is on system. Storage-agnostic — no Supabase coupling — so unit tests run against jsdom.
  • src/lib/components/ThemeToggle.svelte is a three-way radiogroup. Calls setThemePreference() synchronously and the optional onChange prop for caller-side persistence.
  • /settings wires the toggle through an onChange that fires an UPDATE on public.users.theme. Best-effort: a failed UPDATE does not roll the UI choice back.
  • Root +layout.svelte calls initTheme() once on mount to hydrate the JS stores from localStorage (the inline script already painted the correct theme).
  • (app)/+layout.svelte has a one-shot effect that reads public.users.theme after auth resolves and adopts the remote value when it differs from local. Query is deferred via setTimeout(..., 0) per the existing Supabase auth-lock gotcha.

Tests added

  • Unit (theme.test.ts): 8 specs — default 'system', set Light/Dark/System, OS dark resolution, OS-change re-resolution, invalid value rejection, localStorage round-trip.
  • Playwright (theme.test.ts): T-01 toggle persists across reload; T-02 system follows emulated colorScheme; T-03 anti-FOUC on refresh (also checks /logged-out, asserts getComputedStyle(body).backgroundColor === 'rgb(2,6,23)').
  • pgTAP (009_user_theme.sql): 8 assertions — column exists / type / not-null / default, three valid transitions, invalid value rejected via 23514.

Risk #1 outcome (manual palette work)

The existing components already use slate-X dark:slate-Y pairs everywhere they hardcode a color. Tailwind's darkMode: ['selector', '[data-theme="dark"]'] re-routes those pairs through the same attribute the anti-FOUC script writes — no per-component migration was needed. T-03 verifies the body background flips correctly from light to dark.

Migration note for prod

Users already in active sessions when migration 015 lands do NOT need to log out and back in. The JWT minted by GoTrue caches auth.users, not public.users — clients read theme via PostgREST SELECT ... WHERE id = auth.uid(). New rows pick up 'system' from the column default, so onboarding flows do not need a special case either.


9.2 — Desktop reading width

(app)/+layout.svelte wraps content detail routes (/lists/[id], /tasks/[id], /notes/[id]) in md:mx-auto md:w-full md:max-w-2xl. Mobile (<md) stays full-width, and /lists/[id]/session is explicitly excluded so the immersive shopping mode still occupies the whole viewport.

The decision sits in a new readingRoutePattern regex, kept independent from the existing detailRoutePattern so the two concerns (hide-top-bar vs. cap-width) stay separately testable.

Tests: RW-01 measures the bounding box of [data-testid="reading-column"] at 1280×900 and asserts width ≤ 672 + 4px slack; RW-02 at 390×800 asserts > 380; RW-03 asserts the wrapper is absent on /session.


9.3 — sync_conflicts wiring

Append-only audit log of last-write-wins conflicts surfaced by the offline mutation queue.

Migration 016

public.sync_conflicts (id, user_id, collective_id NULL, entity_type, entity_id, local_version jsonb, remote_version jsonb, resolution check IN ('remote_won','local_won'), created_at). Indexed (user_id, created_at DESC) for the "last 20 of mine" query.

RLS: owners can SELECT + INSERT their own rows; UPDATE / DELETE have no policy (silently denied — append-only at the API surface, not just the schema). collective_id is nullable for future user-scoped conflicts (theme, language) that have no collective context.

Queue.ts changes (apps/web/src/lib/sync/queue.ts)

  • New QueueContext (currentUserId, collectiveId) settable via setSyncContext() from the (app) layout's $effect whenever auth or active collective resolves.
  • UpdateOp gains an optional preImage field (the row state the client believed when it queued the UPDATE, restricted to the fields being patched).
  • Before sending an UPDATE: fetch the remote row (limited to patched fields) via .select(cols).match(...).maybeSingle(), proceed with the UPDATE (last-write-wins), then fire-and-forget INSERT into sync_conflicts when the remote diverged from preImage. Failure to fetch or log never blocks the UPDATE.

Debug panel

SyncConflictsPanel.svelte renders the last 20 conflict rows for the current user with an "Export JSON" button. Gated on import.meta.env.DEV (or a forceShow prop for an eventual secret toggle) so it never ships in the production bundle. Slotted into /settings between Language and Account.

Tests added

  • Unit (queue.test.ts): SC-01..SC-03 — conflict logging on divergence, no log when remote matches, conflict-log failure does not block the UPDATE.
  • Integration (packages/test-utils/tests/sync-conflicts.test.ts): SC-INT-01..SC-INT-04 — self-insert + read, cross-user read denied, user_id impersonation rejected, UPDATE/DELETE return 0 rows.
  • pgTAP (010_sync_conflicts.sql): 12 assertions — structure, RLS reads, RLS writes scoped to auth.uid(), append-only enforcement.

Wiring note

The queue itself is not yet plugged into the actual store mutation paths (stores/lists.ts etc.) — that's been pending since Fase 2b.2 and is out of Fase 9 scope. The conflict-logging path is in place: whenever the queue gets adopted by a real call site, conflicts will start flowing into sync_conflicts without further changes.


9.4 — Swipe gesture E2E in WebKit

History rewrite: the plan referenced two .skip swipe-to-delete tests in apps/web/tests/e2e/mobile.test.ts. Neither the file nor the skipped tests exist — Fase 5.10 repurposed the swipe gesture from delete → check-toggle (delete moved to the double-tap overlay), and the obsolete file was removed. The fase-9 scope is reinterpreted as "cover the current swipe-toggle gesture under a real WebKit engine, since Chromium's touch emulation breaks the pointer-event sequence".

Playwright config

playwright.config.ts gains a second project named webkit (devices['iPhone 13']) gated behind RUN_WEBKIT=1 so just test-all stays green on hosts without the GTK/WebKit system libs (libicu74, libxml2, libmanette-0.2-0, libwoff1 — installable via sudo pnpm exec playwright install-deps). The chromium project uses testIgnore: /\.webkit\.test\.ts$/ so the same spec never runs in both.

New tests

tests/e2e/swipe-toggle.webkit.test.ts:

  • SW-01: unchecked item + swipe right → marked checked.
  • SW-02: checked item + swipe left → marked unchecked (after a setup swipe-right).

The swipe handlers in lists/[id]/+page.svelte listen to pointer* events (unified pointer model), so the helper synthesises a pointerdown → 8× pointermove → pointerup sequence with pointerType:'touch' directly via page.evaluate(dispatchEvent). Works regardless of the project's hasTouch setting.

lists/[id]/+page.svelte was extended with data-checked and data-name on each item-row so the spec can robustly identify rows without fragile text-content queries.

Justfile

just playwright-install installs chromium + webkit; just test-webkit runs the gated suite (sets RUN_WEBKIT=1).

Sandbox environment note

The agent host (Ubuntu 25.04, "questing") ships libicu76 instead of the libicu74 Playwright's WebKit binary requires. Installing libicu74 needs root, which the agent cannot perform. The WebKit specs are therefore committed but unrun in this environment; any standard CI image that runs playwright install-deps picks them up.


9.Z — Verification + Realtime presence upstream audit

Test counts (before → after)

Category Baseline Fase 9 Delta
pgTAP assertions 45 65 +20 (8 user_theme + 12 sync_conflicts)
Vitest integration 140 144 +4 (SC-INT-01..04)
Vitest unit 15 26 +11 (8 theme + 3 SC-01..03)
Playwright E2E 59 65 +6 (3 theme T-01..03 + 3 reading-width RW-01..03)
Skipped 3 3 unchanged (2 realtime presence + 1 rate-limit gated)
Total green 259 300 +41

WebKit specs (2 new) are out of the default count — gated on RUN_WEBKIT=1.

Realtime presence upstream audit (parked work)

The two .skip Realtime presence tests in packages/test-utils/tests/realtime-presence.test.ts remain blocked by supabase/realtime#1617handle_out/3 is undefined + Presence channel crashes. Latest stable as of 2026-05-18 is supabase/realtime v2.86.3 (2026-04-21); no release between v2.76.5 (when we pinned in Fase 4) and v2.86.3 includes the fix. Issue has been open since 2025-11-14 with no PR linked. Revisit when the upstream issue closes.

The presence-of-avatars-in-shopping-mode polish (#5 in the original Fase 9 brainstorm) is therefore still parked, with this audit documented so the next person who finds the skipped tests doesn't redo the investigation.