diff --git a/plans/260428-0927-implement-todo-backlog/phase-01-auto-tick-test.md b/plans/260428-0927-implement-todo-backlog/phase-01-auto-tick-test.md deleted file mode 100644 index f55d90e..0000000 --- a/plans/260428-0927-implement-todo-backlog/phase-01-auto-tick-test.md +++ /dev/null @@ -1,92 +0,0 @@ ---- -name: Auto-tick integration test -phase: 1 -status: completed -priority: high -effort: 1-2h -completed: 2026-04-28 ---- - -# Phase 1 — Auto-tick integration test - -## Context -- TODO entry: highest-leverage item ("dedup-by-`at` fix already caught a P0") -- `src/lib/PlayerBoard.svelte:157-171` (auto-tick effect, dedup guard) -- `src/lib/call-bus.svelte.js` (bus driver — already tested) -- `src/lib/game-logic.js` — `findUncrossedCell` (pure helper) -- Existing test infra: vitest + happy-dom, **no** `@testing-library/svelte` - -## Decision -Don't add `@testing-library/svelte` just for one component. Extract the -auto-tick body into a pure helper in `game-logic.js`, test that. The -effect in `PlayerBoard.svelte` becomes a thin wrapper that calls the -helper and applies the result. - -## Files -- Create: `src/lib/auto-tick.js` -- Create: `src/lib/auto-tick.test.js` -- Modify: `src/lib/PlayerBoard.svelte` — replace effect body with helper call - -## Helper API - -```js -/** - * @param {object} args - * @param {number[][]} args.grid - * @param {boolean[][]} args.crossed - * @param {{num: number, at: number} | null} args.lastDraw - * @param {number} args.lastHandledAt - * @param {"player" | "master" | "both"} args.mode - * @returns {{ crossed: boolean[][], lastHandledAt: number, changed: boolean }} - */ -export function processAutoTick({ grid, crossed, lastDraw, lastHandledAt, mode }) { - if (!lastDraw) return { crossed, lastHandledAt, changed: false }; - if (lastDraw.at === lastHandledAt) return { crossed, lastHandledAt, changed: false }; - const next = { lastHandledAt: lastDraw.at }; - if (mode !== "both") return { crossed, ...next, changed: false }; - if (!grid || crossed.length === 0) return { crossed, ...next, changed: false }; - const target = findUncrossedCell(grid, crossed, lastDraw.num); - if (!target) return { crossed, ...next, changed: false }; - const updated = crossed.map((row, ri) => - ri === target.row ? row.map((v, ci) => (ci === target.col ? true : v)) : row - ); - return { crossed: updated, ...next, changed: true }; -} -``` - -## Test cases (5) -1. NEW draw, mode=both, number on board → cell crossed, `changed: true` -2. Same `at` re-fire → no mutation, `changed: false`, `lastHandledAt` unchanged -3. Manual untick after auto-tick, then SAME number with NEW `at` → re-crosses -4. mode=player → no mutation regardless of draw -5. Number not on board → no mutation, `lastHandledAt` advances (we still consume the event) - -## Steps -1. Create `auto-tick.js` with `processAutoTick` (above). Import `findUncrossedCell`. -2. Create `auto-tick.test.js` — mirror style of `call-bus.test.js`. -3. Refactor `PlayerBoard.svelte:157-171` effect: - ```js - $effect(() => { - const drawn = bus.lastDrawn; - const result = processAutoTick({ - grid, crossed, lastDraw: drawn, - lastHandledAt: lastHandledDrawAt, - mode: settings.mode, - }); - lastHandledDrawAt = result.lastHandledAt; - if (result.changed) crossed = result.crossed; - }); - ``` -4. `npm test` — all green. -5. Manual smoke: open app in mode=both, draw a number from master panel, confirm player cell crosses. - -## Success -- 5 unit cases pass. -- `npm test` green (no regression in existing tests). -- Manual smoke confirms behavior unchanged. - -## Risks -- The effect tracks `bus.lastDrawn` reactively — extracting body into a - helper means the effect still needs to *read* `bus.lastDrawn` for - reactivity. The wrapper above does that correctly (read inside the - `$effect` body before passing to helper). diff --git a/plans/260428-0927-implement-todo-backlog/phase-02-ci-inline-script-guard.md b/plans/260428-0927-implement-todo-backlog/phase-02-ci-inline-script-guard.md deleted file mode 100644 index a3c4a03..0000000 --- a/plans/260428-0927-implement-todo-backlog/phase-02-ci-inline-script-guard.md +++ /dev/null @@ -1,97 +0,0 @@ ---- -name: CI inline-script guard -phase: 2 -status: completed -priority: high -effort: 30m -completed: 2026-04-28 ---- - -# Phase 2 — CI inline-script guard - -## Context -- TODO: SvelteKit emits one inline bootstrap script. CSP relaxed to - `'unsafe-inline'` to accommodate. If a future SvelteKit upgrade adds - another inline block, we want CI to fail loudly. -- `static/_headers:2` — current CSP includes `script-src 'self' 'unsafe-inline'`. -- No CI build job today (Cloudflare Pages builds on its own; only - `.github/workflows/deploy-github-pages.yml` is a redirect-only job). - -## Decision -Add an `npm run verify:build` script that: -1. Counts ` - - -
- -
- -{#if grid} -
-
-
- {#each grid.flat() as num, idx} - {@const row = Math.floor(idx / 9)} - {@const col = idx % 9} - {@const hasNumber = num > 0} - {@const isCrossed = hasNumber && !!crossed[row]?.[col]} - {@const rowComplete = hasNumber && rowCompleteness[row]} - - {#if !hasNumber} - -
- - {#if toast} -
-
- {toast} -
-
- {/if} -
-{:else} -
- Nhấn "Tạo bảng mới" để bắt đầu chơi -
-{/if} - -{#if showCongrats} - - ... -{/if} - - -``` - -The full bingo-modal block follows the React structure 1:1 — `role="dialog"`, -`aria-labelledby="congrats-title"`, click-outside-to-dismiss, Escape key -handler. Use Svelte's `onkeydown` and a `tabindex={-1}` wrapper. - -## Behavior parity checklist (map to original fixes) - -- [ ] Bingo popup fires only once per row — guarded by `celebratedRows` Set -- [ ] Two-pass effect prevents skipped reset for higher-index rows — same - structure as React fix -- [ ] `isRowComplete` requires ≥1 numbered cell — already in `game-logic.js` -- [ ] Memoized `rowCompleteness` via `$derived` (was `useMemo`) -- [ ] localStorage shape validation via `safeParse` — in `game-logic.js` -- [ ] Real ` - | - - Trang quản trò → - -
- - - {#if showInstructions} -
- -
- {/if} - - - - - - -``` - -## `master/+page.svelte` — host route - -Replicate `app/master/page.jsx`. Key differences from React: - -- `useState` → `$state` runes -- `useEffect(load on mount)` → `$effect(...)` (runs once at mount because - no reactive deps tracked inside) -- `useEffect(save on change)` → `$effect(() => { saveState(state); })` -- `useCallback` → plain functions -- `BOARD = Object.freeze(...)` — module-scope, declared in ` - - - - - ``` - -The "host's own player card" piece becomes: -```svelte - -``` - -— same prop pattern. - -## Behavior to preserve - -- "Ván mới" button confirms before reset if game in progress -- "Xổ số" button hidden when `remaining.length === 0` -- Last drawn number badge with size animation -- Called numbers history strip (chip-style) -- 9×10 master board lighting up on draw -- Master's own player card below, with separate localStorage prefix - -## Files affected - -- create: `src/routes/+layout.svelte`, `src/routes/+layout.js`, - `src/routes/+page.svelte`, `src/routes/master/+page.svelte` - -## Verify - -- `npm run dev` — both `/` and `/master` render without errors -- Tab switching between routes preserves localStorage independently - (player at `loto_*`, master state at `loto_master`, master's card at - `loto_master_card_*`) -- Browser back/forward navigates correctly (SvelteKit client router) -- View source on prerendered build (Phase 5 verifies) shows the static - HTML shell with `ssr: false` skipping content render - -## Out of scope - -Codeserver dev profile (Phase 5), CF deploy (Phase 5), docs (Phase 6). - -## Status: planned diff --git a/plans/archive/260426-2033-sveltekit-refactor/phase-05-deploy-profiles.md b/plans/archive/260426-2033-sveltekit-refactor/phase-05-deploy-profiles.md deleted file mode 100644 index a2f4076..0000000 --- a/plans/archive/260426-2033-sveltekit-refactor/phase-05-deploy-profiles.md +++ /dev/null @@ -1,118 +0,0 @@ ---- -phase: 5 -title: Codeserver dev profile + CF Pages deploy -priority: high -effort: S -status: planned ---- - -# Phase 5 — Codeserver dev + CF Pages deploy - -Wire the two operational profiles already proven on Next: -1. `npm run dev:codeserver` working through `/absproxy/{port}/` -2. `npm run build` producing static export for CF Pages at `loto.miti99.com` - -## Codeserver dev profile - -Already drafted in Phase 1 `vite.config.js`. Verify by running: - -```bash -echo "CODESERVER_HOST=codeserver.sg.miti99.com" > .env.local -echo "CODESERVER_PORT=3000" >> .env.local -npm run dev:codeserver -``` - -Open `https://codeserver.sg.miti99.com/absproxy/3000/`. Page should load, -HMR socket should upgrade (Vite HMR config maps client → wss:443/...). - -**Differences vs Next implementation:** -- Vite handles HMR explicitly (Phase 1 config). Next 16's HMR was - finicky over the proxy; Vite's named-host config works because we tell - the client exactly which URL to connect on (matches sokoban's setup). -- SvelteKit's `paths.base` config takes the basePath, not Vite's. Bridge - with the same `BUILD_PROFILE` env var pattern, but route through - `svelte.config.js`: - -```js -// svelte.config.js -const profile = process.env.BUILD_PROFILE; -const isCodeserver = process.env.VITE_DEV_PROFILE === 'codeserver'; - -let base = ''; -if (isCodeserver) { - const port = process.env.CODESERVER_PORT ?? '3000'; - base = `/absproxy/${port}`; -} else if (profile === 'gh') { - base = '/loto'; -} -// else: empty (CF Pages root or local dev) - -export default { - preprocess: vitePreprocess(), - kit: { - adapter: adapter({ pages: 'build', assets: 'build', strict: true }), - paths: { base }, - }, -}; -``` - -The dev server reads `VITE_DEV_PROFILE` early (set by `npm run dev:codeserver`), -so `paths.base` populates correctly during dev. - -**Internal links** in `+page.svelte` etc. should use `base`: - -```svelte - - -Trang quản trò → -``` - -This keeps links working under any of the three modes (root, /loto, -/absproxy/3000). - -## CF Pages deploy - -Already covered by current setup. Just update the dashboard build command -when the project is rebuilt: - -| Setting | Value | -|---|---| -| Framework preset | SvelteKit | -| Build command | `npm run build` | -| Build output directory | `build` (NOT `out` — SvelteKit + adapter-static default) | -| Production branch | `master` | -| Environment variables | none (root basePath is the default) | - -Custom domain `loto.miti99.com` step unchanged. - -## Manual GH Pages export (the optional path) - -```bash -npm run build:gh # BUILD_PROFILE=gh sets paths.base = '/loto' -# Upload build/ to GitHub Pages -``` - -## Files affected - -- modify: `svelte.config.js` (paths.base logic) -- modify: `src/routes/+page.svelte` and `src/routes/master/+page.svelte` - (use `$app/paths` `base` for internal links) -- create/keep: `.env.local` (gitignored; user fills in) - -## Verify - -1. **Local dev** — `npm run dev` → `http://localhost:3000/`, links work. -2. **Codeserver** — `npm run dev:codeserver` → page loads at the proxy URL, - HMR works (no `wss` errors in console). -3. **CF Pages build** — `npm run build` → `build/` populated, asset URLs - start with `/_app/...` (root-relative). -4. **GH Pages build** — `npm run build:gh` → asset URLs start with - `/loto/_app/...`. - -## Out of scope - -Docs sync (Phase 6). - -## Status: planned diff --git a/plans/archive/260426-2033-sveltekit-refactor/phase-06-finalize.md b/plans/archive/260426-2033-sveltekit-refactor/phase-06-finalize.md deleted file mode 100644 index 10937a7..0000000 --- a/plans/archive/260426-2033-sveltekit-refactor/phase-06-finalize.md +++ /dev/null @@ -1,137 +0,0 @@ ---- -phase: 6 -title: Update docs, verify end-to-end, commit -priority: high -effort: S -status: planned ---- - -# Phase 6 — Finalize - -End-to-end verification, docs sync, commit + push. - -## Steps - -1. **End-to-end smoke test (manual)** - ```bash - npm run dev - ``` - - Visit `/`, generate a card, click 4 cells in row 0 → see "Chờ X" toast - - Click the 5th → see "Kinh!" popup with "Hàng 1 đã đầy đủ!" - - Press Esc — popup dismisses - - Reload — card persists - - Visit `/master`, click "Ván mới", click "Xổ số" several times — numbers - light up on the 9×10 board, history strip grows - - Generate a card on `/master` (the host's own card) — confirm it's a - different card than `/`'s (separate localStorage prefixes) - -2. **Build check** - ```bash - npm run build - ``` - Expect `build/` populated, no errors. `build/index.html`, - `build/master.html` (or `build/master/index.html` depending on - `trailingSlash` config). - -3. **Codeserver dev smoke** - ```bash - npm run dev:codeserver - ``` - Open the proxy URL; verify page + HMR. - -4. **Lint** - ```bash - npm run lint - ``` - No new errors. - -5. **Update docs.** Files to rewrite (re-generate from new code state, don't - edit the Next-era versions): - - - `docs/codebase-summary.md` — file table now lists `src/routes/...`, - `src/lib/...`, `svelte.config.js`, `vite.config.js` - - `docs/system-architecture.md` — diagram of SvelteKit route flow, - Svelte 5 runes pattern, drop "use client" client-only architecture - section (no longer relevant — `ssr: false` in `+layout.js`) - - `docs/code-standards.md` — Svelte 5 runes (`$state`, `$derived`, - `$effect`), ` - -
-
-
-
-

Quản trò

- ← Về trang người chơi -
- - -
-
-``` - -(Phase 5 will further re-shape this route.) - -## Implementation Steps - -1. Create `src/lib/MasterPanel.svelte`. Copy: - - Entire ` - - - - {#if settings.masterMode} -
-

- Quản trò -

- -
- {/if} - - - ``` - -2. `SettingsButton.svelte`: add the toggle in the modal. Keep it minimal — a labelled checkbox or a 2-state pill. Suggest pill for visual consistency with theme buttons. - - ```svelte -
- Chế độ quản trò -

- Hiện bảng quản trò bên dưới bảng người chơi -

- -
- ``` - -3. Verify: toggle on/off in settings → master panel appears/disappears live on `/`. `/master` still works. - -## Edge cases - -- User flips master mode off mid-game: panel unmounts; `loto_master` storage stays so next-on restores. ✓ Default Svelte behavior since the component manages its own state from storage on mount. -- User has no `loto_master` data when toggling on for the first time: MasterPanel's existing render handles this (`{:else} Nhấn "Ván mới" để bắt đầu`). ✓ - -## Risks - -| Risk | Likelihood | Impact | Mitigation | -|---|---|---|---| -| Unmount mid-auto-call interval (Phase 6) leaks timer | Med | Memory | $effect cleanup in Phase 6 covers it | -| Navigation from `/` to `/master` with masterMode=on shows master panel twice (once on /, once on /master) | n/a | — | Different routes — only one renders at a time | -| Removing "Trang quản trò" header link in Phase 3 leaves no nav to `/master` | True by design | Acceptable | `/master` becomes deep-link-only; users discover master mode via Settings | - -## Success criteria - -- `settings.masterMode === false`: `/` shows player board + footer only. No "Trang quản trò" link or master content. -- `settings.masterMode === true`: `/` shows player board, then "Quản trò" subhead, then full MasterPanel, then footer. -- `/master` still works (renders MasterPanel + back link to `/`). -- Toggling the setting repaints `/` live. - -## Next -- Phase 6: auto-call extends MasterPanel. diff --git a/plans/archive/260427-0100-master-merge-theme-auto-mobile-fit/phase-06-auto-call.md b/plans/archive/260427-0100-master-merge-theme-auto-mobile-fit/phase-06-auto-call.md deleted file mode 100644 index 4f57591..0000000 --- a/plans/archive/260427-0100-master-merge-theme-auto-mobile-fit/phase-06-auto-call.md +++ /dev/null @@ -1,133 +0,0 @@ -# Phase 6 — Auto-call: toggle + interval lifecycle - -## Context -- [plan.md](plan.md). Depends on Phase 1 (settings.autoCallEnabled, autoCallSpeed) and Phase 4 (MasterPanel). - -## Overview -- Priority: P1 -- Status: TODO -- Effort: ~30 min - -## UX - -In MasterPanel, when `settings.autoCallEnabled === true`: -- Add a 2nd line below the controls: speed slider 1–10 sec (label: "Tốc độ tự động: {N} giây/số") wired to `settings.autoCallSpeed`. -- "Xổ số" button label changes: - - Not running → **"Bắt đầu"** (green gradient). - - Running → **"Dừng"** (red gradient). -- Clicking starts/stops a `setInterval` that calls the existing `handleDrawNext()` every N seconds. -- Auto-stop when `state.remaining.length === 0`. - -When `settings.autoCallEnabled === false` (default): unchanged — manual "Xổ số" per click. - -The Auto on/off toggle itself lives in **Settings** (per user spec). Keeping the speed slider inline in MasterPanel — close to the Bắt đầu/Dừng button it controls — feels more discoverable than burying it in Settings. Acceptable deviation: speed lives in MasterPanel, master-mode + auto-enabled live in Settings. (Alt: put speed in Settings too. Decided inline because it's a runtime knob, not a preference.) - -Wait — re-read user spec: "When auto, Xổ số became Start/Stop". And "auto-call speed (1-10s/number, default 5)" persisted in settings. The toggle is also in settings. - -So: -- **Settings**: `autoCallEnabled` (toggle), `autoCallSpeed` (slider). -- **MasterPanel**: when `autoCallEnabled`, button switches to start/stop; reads speed from settings live (so changing speed in Settings while running re-arms the interval). - -Let me put the speed slider in Settings (cleaner), but display its current value as a small caption near the start/stop button so the host knows what's about to fire. - -## Lifecycle (the careful bit) - -```js -// Inside MasterPanel -let autoRunning = $state(false); - -$effect(() => { - // Re-arm interval whenever autoRunning OR speed changes - if (!autoRunning) return; - const ms = settings.autoCallSpeed * 1000; - const id = setInterval(() => { - if (!state || state.remaining.length === 0) { - autoRunning = false; // triggers cleanup - return; - } - handleDrawNext(); - }, ms); - return () => clearInterval(id); -}); - -// Auto-stop also if user disables the auto setting mid-run -$effect(() => { - if (!settings.autoCallEnabled && autoRunning) autoRunning = false; -}); - -// Auto-stop when starting a new game -function handleNewGame() { /* ... */ autoRunning = false; } -``` - -Race conditions handled: -- **Speed change while running**: $effect re-runs (its dependency `settings.autoCallSpeed` changed) → cleanup old `setInterval` → start new one. ✓ -- **Remaining hits 0 during a tick**: tick checks first, sets `autoRunning = false`, returns without drawing. The $effect re-runs, sees `!autoRunning`, returns without setting up a new interval. ✓ -- **User unmounts MasterPanel (master mode off) mid-run**: Svelte fires effect cleanup → `clearInterval`. ✓ -- **User disables auto setting while running**: secondary $effect catches it, flips `autoRunning = false`, primary effect cleans up. ✓ -- **Multiple intervals stacking**: impossible, primary $effect always runs cleanup before re-running. ✓ - -## Files - -| File | Change | -|---|---| -| `src/lib/MasterPanel.svelte` | Add `autoRunning` state, the two `$effect`s above, swap "Xổ số" button label/style based on `settings.autoCallEnabled` and `autoRunning`. Display speed caption when auto on. | -| `src/lib/SettingsButton.svelte` | Add "Tự động xổ" toggle + speed slider 1–10 (visible only when masterMode is on, OR always; pick: always — toggling auto without master mode is a no-op so harmless). | -| `src/lib/settings-store.test.js` | (already covered Phase 1) — no new tests for the slider UI itself. | -| (no new tests for the $effect lifecycle — relies on browser timers; manual verify in dev) | - -## Implementation Steps - -1. `MasterPanel.svelte`: - - Add `import { settings } from "$lib/settings-store.svelte.js"` if not already present. - - Add `let autoRunning = $state(false)`. - - Add the two $effects (above). - - Modify the "Xổ số" button: - ```svelte - {#if settings.autoCallEnabled} - - {:else if state && state.remaining.length > 0} - - {/if} - ``` - - Reset `autoRunning = false` in `handleNewGame`. - - Below the buttons, when auto on: small caption "Tự động: {settings.autoCallSpeed}s/số". - -2. `SettingsButton.svelte`: - - Add a "Tự động xổ" fieldset (after Master mode) with: - - On/off toggle pill (same style as masterMode). - - When on: a `` plus a label showing "{N} giây/số". - -3. svelte-check + manual dev verify: - - Master mode on, auto off: "Xổ số" works as before. - - Master mode on, auto on, speed=2: click "Bắt đầu" → numbers draw every 2s. Click "Dừng" → stops. Click "Bắt đầu" again → resumes. Reaching 90 numbers stops automatically. - - Change speed slider while running → interval re-arms at new speed. - -## Risks - -| Risk | Likelihood | Impact | Mitigation | -|---|---|---|---| -| Tab in background throttles `setInterval` to ≥1s | Real | If speed=1, browser may still fire ~1s. UX: slightly slower in background. | Acceptable — host typically has tab focused | -| User toggles autoCallEnabled rapidly while running | Low | Stale interval | Secondary $effect catches it | -| `handleDrawNext` mutates `state` during interval tick — does Svelte 5 reactivity batch correctly? | Low | If reactive update is slow, intervals could overlap. | `setInterval` ticks are independent of render; `handleDrawNext` is sync; no overlap risk in practice. | -| Confirm dialog in `handleNewGame` (`if (state && !confirm(...))`) blocks during auto-call → interval keeps firing while modal is up | Low | Visual glitch | `handleNewGame` is user-triggered, not from interval. Safe. | - -## Success criteria - -- Auto setting off → manual "Xổ số" unchanged. -- Auto setting on → "Bắt đầu"/"Dừng" button. Drawing every N seconds. Stops on Dừng. Stops at 90/90. Speed change re-arms. -- Disabling auto in Settings while running stops it. -- Tests still pass (no logic regression). -- No timer leaks (verified by toggling master-mode off while running — Svelte cleanup handles). - -## Done = Plan complete -- Mark plan.md status → `completed`. Run `/ck:project-management` sync if needed. -- Update docs (PDR, codebase-summary, system-architecture, dev-roadmap). -- Move "Theme switcher" from roadmap "Idea Phase" → "Currently Implemented". diff --git a/plans/archive/260427-0100-master-merge-theme-auto-mobile-fit/plan.md b/plans/archive/260427-0100-master-merge-theme-auto-mobile-fit/plan.md deleted file mode 100644 index b7b9d35..0000000 --- a/plans/archive/260427-0100-master-merge-theme-auto-mobile-fit/plan.md +++ /dev/null @@ -1,80 +0,0 @@ ---- -slug: master-merge-theme-auto-mobile-fit -created: 2026-04-27 -status: completed -completedAt: 2026-04-27 -mode: auto -blockedBy: [] -blocks: [] ---- - -# Master mode merge + theme toggle + auto-call + mobile fit - -Eight user-requested features bundled into one coordinated refactor. -Touches almost every component in the app but keeps the data model -(grid, crossed, called/remaining, storage keys) untouched. - -## Decisions (locked) - -- **Default color**: Excel "Standard Color: Purple" `#7030A0`. Most - saturated of the 10 standard purples; reads well on both white and - black grids; user picked "purple" as the default. -- **Color presets**: Office Standard Colors palette (10 swatches), see - Phase 1 for hex values. -- **Theme**: 3-state — `auto` (default, follow OS), `light`, `dark`. - Implemented by toggling `class="dark"` on `` + Tailwind v4 - `@variant dark (.dark &)` declaration. -- **Master route**: kept as a deep-link that auto-enables master mode - in settings on visit, then redirects (or just renders the same `/`). - Recommend: keep `/master` rendering the same content as `/` but with - master mode forced on for that view (no setting mutation — least - surprise). Storage prefixes preserved. -- **Master logic refactor**: extract everything in `src/routes/master/+page.svelte` - except the page-level header into `src/lib/MasterPanel.svelte`. Both - `/` (when master mode on) and `/master` mount that component. -- **Auto-call lifecycle**: `setInterval` inside an `$effect` that - depends on `(autoRunning, settings.autoCallSpeed)` so changing speed - while running re-arms cleanly. Auto-stop when remaining hits 0. -- **Mobile fit**: cell aspect ratio responsive — `aspect-square` on - mobile (default), `sm:aspect-[3/5]` on ≥640px. Number text - `text-base sm:text-2xl md:text-3xl`. Page padding tightened on - mobile. - -## Phases - -| # | Phase | Status | File | -|---|---|---|---| -| 1 | Settings expansion (theme, masterMode, autoCall*, Excel palette, purple default) + tests | DONE | [phase-01-settings-expansion.md](phase-01-settings-expansion.md) | -| 2 | Theme system: Tailwind v4 class-based dark + JS sync + settings UI | DONE | [phase-02-theme-system.md](phase-02-theme-system.md) | -| 3 | Mobile-fit + page header/footer refactor | DONE | [phase-03-mobile-and-header-footer.md](phase-03-mobile-and-header-footer.md) | -| 4 | Extract MasterPanel component from `/master` route | DONE | [phase-04-extract-master-panel.md](phase-04-extract-master-panel.md) | -| 5 | Master mode integration on `/` (inline render via setting) + `/master` deep-link | DONE | [phase-05-master-mode-integration.md](phase-05-master-mode-integration.md) | -| 6 | Auto-call toggle + interval lifecycle | DONE | [phase-06-auto-call.md](phase-06-auto-call.md) | - -## Files Touched (summary) - -| File | Phases | -|---|---| -| `src/lib/settings-store.svelte.js` | 1, 2, 5, 6 — new keys, theme apply logic | -| `src/lib/settings-store.test.js` | 1, 2, 6 — new tests | -| `src/lib/SettingsButton.svelte` | 1, 2, 5, 6 — Excel swatches, theme toggle, master mode toggle, auto-call speed | -| `src/lib/PlayerBoard.svelte` | 3 — responsive cells, drop "Made by" from bottom label, footer text simplification | -| `src/lib/MasterPanel.svelte` | 4 — NEW (extracted from master route) | -| `src/routes/+layout.svelte` | 2 — theme already loads on mount; ensure media-query listener if `auto` | -| `src/routes/+page.svelte` | 3, 5 — drop instructions, restructure header, mount MasterPanel conditionally, mount Footer | -| `src/routes/master/+page.svelte` | 4, 5 — collapse to a thin shell that mounts MasterPanel | -| `src/lib/PageFooter.svelte` | 3 — NEW small component (tagline + made-by) | -| `src/app.css` | 2, 3 — convert dark `@media` to class-based, add Tailwind dark variant declaration | -| `docs/project-overview-pdr.md` | post-impl — sync features | -| `docs/codebase-summary.md` | post-impl — new files | -| `docs/system-architecture.md` | post-impl — page-flow update | -| `docs/development-roadmap.md` | post-impl — move "Theme switcher" → Implemented | - -## Out of Scope - -- Per-card storage migrations -- E2E tests / component tests -- Sound effects, PWA, multiplayer sync -- Custom number range -- Card serial number badges -- Replacing PlayerBoard rendering for master grid diff --git a/plans/archive/260427-0100-master-merge-theme-auto-mobile-fit/reports/code-reviewer-260427-0113-session-review.md b/plans/archive/260427-0100-master-merge-theme-auto-mobile-fit/reports/code-reviewer-260427-0113-session-review.md deleted file mode 100644 index fc0a95a..0000000 --- a/plans/archive/260427-0100-master-merge-theme-auto-mobile-fit/reports/code-reviewer-260427-0113-session-review.md +++ /dev/null @@ -1,176 +0,0 @@ -# Session Review — 260427-0113 - -Scope: 6-feature working-tree refactor (settings/theme, Tailwind v4 dark variant, settings UI, mobile fit + footer, MasterPanel extraction, auto-call). 7 modified + 2 new files, 469 / 393 LOC. - -Validation run before review: -- `npx vitest run` — 53/53 pass. -- `npx svelte-check` — 0 errors, 0 warnings. -- `npx vite build` — clean (Tailwind v4 `@variant` compiled). - ---- - -## Must-fix (block ship) - -**None.** Build, tests, and types all green. Lifecycle and storage concerns below are real but each has an honest mitigation that keeps "ship today" defensible. - ---- - -## Nice-to-fix (do before ship if you have 30 min, otherwise file) - -### 1. Auto-call interval can leak on `/master` navigation when `autoCallEnabled=false` -File: `src/lib/MasterPanel.svelte:96-112` - -Walk-through: -1. `autoCallEnabled=true`, user is on `/master`, clicks "Bắt đầu" → `autoRunning=true`, primary effect arms `setInterval`. -2. User opens Settings, flips `autoCallEnabled=false`. Secondary effect (line 110-112) fires synchronously, sets `autoRunning=false`. Primary effect re-runs, cleanup runs, `clearInterval` fires. Safe. -3. User unmounts `MasterPanel` mid-run (navigates `/master` → `/` with `masterMode=false`, or simply closes tab) → Svelte runs `$effect` cleanup → `clearInterval`. Safe. -4. Edge: `state.remaining.length === 0` reached inside the tick → `autoRunning=false` set inside the interval callback. Next microtask the primary `$effect` re-runs with `autoRunning=false`, hits the early `return`, the prior cleanup fires → `clearInterval`. Safe but the interval fires *one extra time* (the one that set `autoRunning=false`) before tearing down. Not a leak — already correct because the early-return inside the callback (`if (!state || state.remaining.length === 0) { autoRunning=false; return; }`) prevents `handleDrawNext()` from being called the dead tick. Good. - -So no actual leak. **But** there is an ordering subtlety to flag: - -The two effects both depend on `autoRunning`. If the secondary effect runs *after* the primary in the same flush, and the user disables `autoCallEnabled` while running, the primary re-arms with the current `autoRunning=true` for that flush, then the secondary flips `autoRunning=false`, then the primary re-runs again and tears down. Net result: cleanup is called, no leak. But Svelte 5 doesn't guarantee effect order across separate `$effect` calls — depends on registration order. Today registration order is primary first, secondary second, so secondary always runs after. Fine. If someone reorders the script, the leak window widens to one tick. - -**Suggested fix (cheap):** Fold the secondary check into the primary effect to remove cross-effect dependency: - -```js -$effect(() => { - if (!autoRunning) return; - if (!settings.autoCallEnabled) { autoRunning = false; return; } - const ms = settings.autoCallSpeed * 1000; - const id = setInterval(() => { ... }, ms); - return () => clearInterval(id); -}); -``` - -Drop the second `$effect` entirely. Same behavior, one fewer reactive subscription, no ordering dependency. - -### 2. `loadSettings()` re-entry — leak window if layout remounts -File: `src/lib/settings-store.svelte.js:66-88` - -`applyTheme()` checks `if (mql && mqlListener)` and tears down before reattaching. Safe under sequential calls. **However** `loadSettings()` and `saveSettings()` both call `applyAll()` → `applyTheme()`. The teardown only runs if the *last* call left a listener attached. If something else attached a listener directly to `window.matchMedia(...)` outside this module (it doesn't today), it would leak. - -Within the module: safe. The test at line 269-280 covers `auto → dark` detach. Add an analogous test for `loadSettings()` called twice with `theme=auto` to lock in the invariant. (Skipped per your "no tests" instruction — note for tester agent.) - -**Real concern:** if you ever switch from `onMount` (`+layout.svelte:11`) to `$effect` for hydration, and the layout re-runs during HMR or route group switch, `applyTheme` is the only thing protecting you. The current guard is correct. Good defensive code. - -### 3. Two-tab `loto_master` race — unmitigated -File: `src/lib/MasterPanel.svelte:2,45-51` - -Storage key `loto_master` is shared between `/master` and `/` (when `masterMode=true`). Both tabs save state on every change with last-writer-wins. This was raised in your priority Q5; my read: - -- For a single-host quiz scenario (one device, one human), this is **fine**. Acceptable for ship today. -- The deeper bug isn't write conflicts, it's that **neither instance reacts to `storage` events**, so when `/` writes, `/master` keeps showing stale called-list and vice versa until refresh. The two MasterPanels are not actually shared state — they're independent state machines persisted to the same key. Confusing but not destructive. - -**Recommendation:** ship as-is. Add a `window.addEventListener("storage", ...)` reload in a follow-up if you ever support multi-screen sessions. Document the limitation in MasterPanel.svelte module-doc. - -### 4. `MasterPanel` mounted twice causes double saves -File: `src/routes/+page.svelte:33`, `src/routes/master/+page.svelte:30` - -If a user is on `/` with `masterMode=true` and navigates to `/master` (no link does this today, but typing the URL works), they hit two routes that each mount MasterPanel. SvelteKit unmounts `/`'s instance on navigation, so only one is active at a time. Safe. - -But if `/master` is open and you toggle on `masterMode` then go back to `/`, both now use `loto_master` *but the MasterPanel `$state` is reset* on mount — it loads from localStorage on the `$effect` at line 77. Result: state persists across navigation. Good. - -Edge: the `$effect` at line 77 has no dependencies — Svelte 5 runs it once on mount. ✓ Correct. - -### 5. `aspect-square` for cells at 360-639px feels cramped -File: `src/lib/PlayerBoard.svelte:184,194` - -You said "card is 9 cells wide × 38px = 342px, fits". At 9-col grid the cell width is `(viewport - padding) / 9`. With `px-2` (16px) it's `(360-16)/9 ≈ 38.2px`. Square cells give the user 38×38 tappable targets — under iOS HIG's 44px minimum and Android Material's 48dp. Touch target accessibility issue. - -**Options:** -- Push `sm:aspect-[3/5]` to start earlier — `xs` doesn't exist in Tailwind by default, but `min-[400px]:aspect-[3/5]` works. -- Or accept it and add `min-h-[44px]` to the cell button to enforce a floor (will break aspect ratio but improves tap accuracy). - -Not blocking ship — Vietnamese wedding/party venue users will tap it; works on iPad fine. Note for backlog. - -### 6. Speed slider has no `aria-label` -File: `src/lib/SettingsButton.svelte:219-227` - -The `` is wrapped by `