From ad6291e924fbe1e6e32b489cfb18967cad1ea503 Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Mon, 27 Apr 2026 18:57:14 +0700 Subject: [PATCH] fix: address full-project review findings (P0/P1/P2 + security) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P0: - PlayerBoard auto-tick: track lastDrawn.at non-reactively so the effect re-firing on crossed/grid changes (manual untick, clear, regen) no longer re-marks the latest drawn number. - Toast moved above the grid — was overlaying middle cells for 5s. - Bingo modal Escape now uses a window listener (matches settings modal pattern); the inline onkeydown rarely fired. - Dark winning-row contrast: bg-emerald-900/60 + text-emerald-200 to clear WCAG AA. New .cell-crossed-win class flips the slash to emerald so completed rows read as "win" not "marked off". - master-only mode: keep "Quản trò" h2 visible so the page has context when no player board sits above. P1: - Reset bus on PlayerBoard handleClear/handleGenerate. - Master mode picker now shows a per-mode hint line. - Hide "Quản trò đọc số" toggle entirely in mode=player (no effect there). Indent auto-call slider with the same nested-border treatment as voice waiting. - Hero number scales smaller on ≤375px (w-32 / border-6). - aria-live=polite on hero (was assertive — informational not urgent). - MasterPanel auto-stop early-return without redundant write. - Drop in-card "Made by miti99" — duplicates the page footer. - Replace biased Math.random sort shuffle with Fisher-Yates in randomNumbersInCol. - Reduced-motion gates: vibrate, smooth scroll, all keyframe animations short-circuit when prefers-reduced-motion: reduce. - History pill border-2 (was border-3, cramped 88s). Security: - static/_headers: CSP, X-Content-Type-Options, Referrer-Policy, Permissions-Policy, X-Frame-Options for Cloudflare Pages. - safeParse / loadSettings / MasterPanel.loadState: payload-size cap before JSON.parse + reviver strips __proto__ and constructor keys as defense-in-depth. - voice.clipUrl: encodeURIComponent on voice id and clip name. Tests: +findUncrossedCell unit tests (covers the auto-tick path that exposed the P0), +voice.js cancellation/chaining tests (playWaiting honours voiceWaitingNumber). 115/115 pass. Skipped (need new assets / deeper scope): - Vietnamese-supporting condensed @font-face (needs font upload). - Full master empty-state hero illustration. - Confetti emoji variety, color picker redesign, mode picker icons, brand-mood overhaul of the header subline. --- .../code-reviewer-260427-1151-full-project.md | 77 ++++++++++ .../security-260427-1151-full-audit.md | 67 ++++++++ .../ui-ux-designer-260427-1151-full-audit.md | 118 ++++++++++++++ src/app.css | 29 +++- src/lib/MasterPanel.svelte | 41 +++-- src/lib/PlayerBoard.svelte | 144 +++++++++--------- src/lib/SettingsButton.svelte | 21 ++- src/lib/game-logic.js | 42 ++++- src/lib/game-logic.test.js | 38 +++++ src/lib/settings-store.svelte.js | 11 +- src/lib/voice.js | 4 +- src/lib/voice.test.js | 93 +++++++++++ src/routes/+page.svelte | 12 +- static/_headers | 6 + 14 files changed, 606 insertions(+), 97 deletions(-) create mode 100644 plans/reports/code-reviewer-260427-1151-full-project.md create mode 100644 plans/reports/security-260427-1151-full-audit.md create mode 100644 plans/reports/ui-ux-designer-260427-1151-full-audit.md create mode 100644 src/lib/voice.test.js create mode 100644 static/_headers diff --git a/plans/reports/code-reviewer-260427-1151-full-project.md b/plans/reports/code-reviewer-260427-1151-full-project.md new file mode 100644 index 0000000..af70ea8 --- /dev/null +++ b/plans/reports/code-reviewer-260427-1151-full-project.md @@ -0,0 +1,77 @@ +# Code review — full project (260427-1151) + +Scope: `src/` SvelteKit + Svelte 5. Skip `node_modules`/`.svelte-kit`/`build`. +Focus: runes correctness, bus/state coordination, voice cancellation, edge cases, JSDoc, tests, anti-patterns. +Overall: code is clean, well-commented, JSDoc-typed throughout. One real auto-tick re-mark bug; rest are minor. + +## P0 — blocking + +**1. Auto-tick re-marks cells the user cleared / unticked / regenerated** — `src/lib/PlayerBoard.svelte:136-151`. +The auto-tick `$effect` reads `bus.lastDrawn` AND `crossed` AND `grid` reactively. So it re-fires when `crossed` changes (manual untick, "Xoá đánh dấu", "Tạo bảng mới") even though no new draw happened. With `bus.lastDrawn` still set, it scans for that number and re-marks it. Reproductions in "both" mode: +- Master draws 42. User taps 42 to untoggle → effect re-runs, sees `!crossed[r][c]`, re-marks. User cannot ever untick the latest draw. +- User taps "Xoá đánh dấu" → all marks clear → effect re-fires → 42 re-marked instantly. +- User taps "Tạo bảng mới" → new `crossed` matrix → effect re-fires → 42 marked on the new card. +Fixes: track `lastDrawn.at` (or the bus reference) in a non-reactive ref and early-exit when unchanged; OR call `resetBus()` from `handleClear`/`handleGenerate` AND skip the effect on the very first run after generate. Same hazard exists conceptually with the comment "auto-tick can't fight manual taps" — comment is wrong. + +## P1 — high + +**2. `handleClear` / `handleGenerate` don't reset `bus.lastDrawn`** — same file, lines 153-177. Even after fixing #1, the bus stays sticky across player-card resets. Master `handleNewGame` does reset it (`MasterPanel:139`); player resets should too in "both" mode (or at minimum, snapshot `lastDrawn.at` so post-clear effect knows to ignore stale draws). + +**3. `handleClear` doesn't reset `congratsRow`/`celebrationTier` and leaves `showCongrats`** — `PlayerBoard.svelte:167-177` sets `showCongrats = false` but if a celebration was mid-animation the state is fine. However `congratsRow`/`celebrationTier` aren't reset; harmless until next bingo overwrites. P2. + +**4. Multiple-row simultaneous bingo silently swallowed** — `PlayerBoard.svelte:102-112`. Pass 1 `break`s after first new completion, but pass 2 won't surface another. Not reachable today (one cell-toggle = one row at most). But auto-tick + cross-row coincidence + future logic changes could expose it. Add a short queue or track "pending celebrations" if you ever batch-mark. + +**5. Bingo modal Escape only works when backdrop has focus** — `PlayerBoard.svelte:393-402`. `onkeydown={onModalKeydown}` is on the backdrop `