From ee71bf041de6e7309bbba840a06e814d8d38dd99 Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Mon, 27 Apr 2026 20:53:53 +0700 Subject: [PATCH] fix: address pass-2 review findings (PWA + CSP + copy) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P0: - CSP `script-src` was 'self' only, but SvelteKit's static export emits a small inline bootstrap script. Without 'unsafe-inline' the entire app silently fails under Cloudflare Pages CSP enforcement. Verified by inspecting the built index.html. - manifest `background_color` was the dark base (#0a0f1f); for the ~50% of users on light mode that gave a dark splash flash on every install/launch. Switch to #f8fafc to match the default light theme. - bare "Lô tô" mismatched manifest name "Lô tô — Hội chợ TN1"; align both to the same string so OS install prompt + browser tab match. Medium: - Audio runtime cache `cacheableResponse.statuses` was [0, 200]. Audio is same-origin, so opaque (0) responses can never legitimately appear; tightening to [200] removes a CDN-poisoning replay window. - Voice hint copy: "Đọc số đã xổ + báo Chờ/Kinh khi ở Cả hai" was shown in master-only mode too, where the hint is wrong (no player board → no Chờ/Kinh). Split copy per mode. Cosmetic: - Drop `includeAssets: ["icons/*.png", "audio/**/*.mp3"]` — both are already in static/, so the option was a no-op. - Replace `defaultVoiceId` fallback `"hoai-my"` with a hard read; the manifest is committed and authoritative — duplicate fallbacks just invite drift if the manifest ever rotates. Verified: npm test 115/115; npm run build clean (305 precache entries, no glob warnings); npm audit 0 vulnerabilities. Reports: plans/reports/{code-reviewer,ui-ux-designer,security}-260427-2047-pass2-full.md --- .../code-reviewer-260427-2047-pass2-full.md | 83 ++++++++++++ .../security-260427-2047-pass2-full.md | 73 ++++++++++ .../ui-ux-designer-260427-2047-pass2-full.md | 125 ++++++++++++++++++ src/app.html | 4 +- src/lib/SettingsButton.svelte | 12 +- static/_headers | 2 +- static/manifest.webmanifest | 2 +- vite.config.js | 12 +- 8 files changed, 301 insertions(+), 12 deletions(-) create mode 100644 plans/reports/code-reviewer-260427-2047-pass2-full.md create mode 100644 plans/reports/security-260427-2047-pass2-full.md create mode 100644 plans/reports/ui-ux-designer-260427-2047-pass2-full.md diff --git a/plans/reports/code-reviewer-260427-2047-pass2-full.md b/plans/reports/code-reviewer-260427-2047-pass2-full.md new file mode 100644 index 0000000..56a26a8 --- /dev/null +++ b/plans/reports/code-reviewer-260427-2047-pass2-full.md @@ -0,0 +1,83 @@ +# Code Review — Pass 2 (full project, post-PWA) + +**Scope:** `src/**`, `vite.config.js`, `static/_headers`, `static/manifest.webmanifest`, `package.json`. +**Baseline:** prior pass at `f28279b`. Reviewed commits: `ad6291e`, `f7db20c`, `d94294d`. +**Verdict:** clean. Prior P0/P1 findings landed correctly. No new P0. Two P1 around PWA/CSP. A few P2 nits. + +--- + +## Prior fixes — landing check + +- Auto-tick re-mark guard: `PlayerBoard.svelte:45,157-171` — `lastHandledDrawAt` compared against `bus.lastDrawn.at`. Manual untick / clear / regen no longer re-fires (re-runs read same `at`, early return). Correct. +- Toast positioning: `PlayerBoard.svelte:329-340` — anchored to grid container with `-top-3 sm:-top-4`, `pointer-events-none` wrapper, button-only `pointer-events-auto`. Correct. +- Modal Escape: window-level `keydown` in `PlayerBoard.svelte:142-150` and `SettingsButton.svelte:113-121`. Correct. +- Master empty state: extracted `MasterEmptyState.svelte`, used at `MasterPanel.svelte:348`. Correct. +- Biased shuffle replaced by Fisher-Yates: `game-logic.js:32-35`, `MasterPanel.svelte:35-38`. Correct. +- Reduced-motion gates: `MasterPanel.svelte:145-153` (scroll), `PlayerBoard.svelte:18-23` (vibrate), `app.css:219-232` (animations). Correct. +- Storage payload caps + `__proto__`/`constructor` reviver: `game-logic.js:166-189`, `MasterPanel.svelte:51-75`, `settings-store.svelte.js:127-132`. Correct. +- `encodeURIComponent` on voice URL: `voice.js:41`. Correct. +- Security headers: `static/_headers` present with strict CSP, COOP-equivalent (`frame-ancestors 'none'`), nosniff, etc. + +--- + +## P1 (action recommended pre-merge) + +**1. CSP ↔ inline SW registration race / failure mode** — `static/_headers:2` +CSP has `script-src 'self'` (no `'unsafe-inline'`, no nonce). `@vite-pwa/sveltekit` with `registerType: "autoUpdate"` injects an inline `<script>` registering `/sw.js` into the prerendered `index.html`. On a strict CSP host (Cloudflare Pages honors `_headers`), that inline registration block will be blocked → no PWA install. Two fixes: (a) switch to virtual `import { registerSW } from 'virtual:pwa-register'` from a real module, OR (b) configure plugin `injectRegister: 'script-defer'` with a hashed/external file. Verify post-build that `build/index.html` does NOT contain inline registration; if it does, this is silently broken in production. + +**2. `static/_headers` does not cover `/sw.js` MIME** — `static/_headers:9-10` +Only `Cache-Control: no-cache` is set on `/sw.js`. Cloudflare Pages will infer `application/javascript` from extension, but spec-strict registrars reject if `Content-Type` isn't `text/javascript`/`application/javascript`. Low-risk on CF, but add explicit `Content-Type: application/javascript` to be safe alongside the manifest entry already doing so. + +--- + +## P2 (nits / tech debt) + +- `vite.config.js:13` — `defaultVoiceId = audioManifest.voices[0]?.id ?? "hoai-my"`. Hardcoded fallback drifts from `audio-manifest.js:23`. Either move to a shared `scripts/audio-default.js` import or assert at config-eval time. Low likelihood of mismatch but easy to lose on a manifest rewrite. +- `vite.config.js:23` — `revision: 'audio-v1-{voice}-{n}'` is a manual cache-buster. Comment ("Bump the prefix when audio is regenerated") relies on humans. Consider hashing file content (`createHash('sha1', readFileSync(path))`) so audio regen invalidates automatically. +- `vite.config.js:46` — `includeAssets: ["icons/*.png", "audio/**/*.mp3"]` lists ALL voice mp3s, but `globPatterns` (line 48) does NOT include `mp3`. Result: `includeAssets` only copies them into the build (already happens via `static/`); the actual precache list is `globPatterns ∪ additionalManifestEntries`. Default voice is precached via `additionalManifestEntries`; alt voices fall through runtime CacheFirst as documented. Behavior is correct — but `includeAssets` is dead config noise, drop it or document its no-op role. +- `manifest.webmanifest:5-6` — `start_url: "."`, `scope: "."`. Under base path `/loto/`, browsers resolve relative to manifest URL, so this works on GH Pages. But Cloudflare and GH share the same file. If you ever add a non-root deploy without rewriting the manifest, scope drift will silently break PWA scope detection. Consider `%sveltekit.assets%`-templated manifest emitted at build time, or explicit `start_url: "/loto/"` + a CF-only override. +- `MasterPanel.svelte:107-109` — `callOrder` rebuilds the whole `Map` on every state change; fine at 90 entries. No action. +- `MasterPanel.svelte:90` — `heroEl` typed `HTMLDivElement | null` but `bind:this` runs at every render. Using `$state` here is correct in Svelte 5; OK. +- `PlayerBoard.svelte:48-52` — `rowCompleteness` derived; uses `grid.map((_, r) => isRowComplete(grid, crossed, r))`. Reads both reactively — fires on every cell toggle (9 calls). Acceptable. +- `MasterPanel.svelte:166-177` `handleDrawNext` does not check `settings.mode` before `broadcastDraw`; player auto-tick effect already gates on `settings.mode === "both"` (`PlayerBoard.svelte:162`), so harmless. But broadcasting in `master`-only mode is wasted work and pollutes the bus across mode flips. Suggest gating, OR documenting why it's intentional (so resuming from "both" → "master" → "both" mid-game works). +- `call-bus.svelte.js:1-22` — JSDoc accurate. `broadcastDraw` lacks `@returns`, `resetBus` lacks docstring; trivial. +- `game-logic.js:277-288` — `findUncrossedCell` JSDoc accurate. Top-down/left-right scan order matches test expectation. +- Stale comment risk: `MasterPanel.svelte:51-53` says "16 KB has 30× headroom" — true after the cap landed; keep. +- Dead/duplicate `isBrowser` check in `voice.js:54` (`cancelPlayback`) — fine, but `if (!isBrowser()) return;` is unreachable in test path; cosmetic. + +--- + +## Test-coverage gaps + +- No test for the `PlayerBoard` auto-tick effect (mode flip, dedup-by-`at`, manual-untick re-mark prevention). The behavior is the highest-risk new code path. Add one component test: drive `bus.lastDrawn`, assert `crossed[r][c]` flips once, untick by hand, broadcast same `at` → does NOT re-mark. Recommended. +- No test for `MasterPanel.handleDrawNext` → `broadcastDraw` linkage; bus contract tested in isolation only. +- No SW/Workbox integration test — out of scope for vitest, but a `npm run build:gh && grep -r 'sw.js' build/index.html` smoke check in CI would catch P1 #1. +- `voice.test.js` does not cover the `cho → number` cancel-mid-chain case (cancel between the two awaited `playClip` calls). The token mismatch path resolves cleanly; one test would lock it in. + +--- + +## Security + +- CSP unchanged, still strict. PWA SW served same-origin; runtime caching only matches `/audio/*.mp3` regex, no third-party cacheing. `manifest.webmanifest` is plain JSON, no scripts. Icons are local PNGs. New attack surface: SW lifecycle. `registerType: "autoUpdate"` + comment on line 38-39 ("Do NOT add `skipWaiting`") is correct — stale clients keep working until tab close. +- `npm overrides` for `serialize-javascript@^7.0.5` and `cookie@^0.7.2` are dev-only build-chain transitive vulns. Lockfile is the source of truth — confirm `npm ls serialize-javascript cookie` shows resolved 7.x/0.7.x post-`npm install`. +- No PII, no telemetry, no remote endpoints. + +--- + +## Positive + +- `lastHandledDrawAt` design (closed-over plain ref, not `$state`) is exactly right — avoids the auto-tick effect re-triggering itself. +- Two-pass `$effect` on bingo + waiting in `PlayerBoard.svelte:97-131` reads cleanly; comment on "at most one bingo popup per render" matches code. +- JSDoc + JSDoc-via-`/** @type */` casts give meaningful type narrowing without TS toolchain. +- Test file naming and `// @vitest-environment happy-dom` annotation per file is consistent. + +--- + +## Unresolved questions + +1. Is the PWA install actually working in production (Cloudflare Pages) under the strict CSP, or has nobody tested install + reload offline? See P1 #1. +2. Is `BUILD_PROFILE=gh` (`/loto/` base) deployed anywhere live? If not, `manifest.webmanifest`'s relative `start_url` is untested at non-root scope. +3. Should `MasterPanel.handleDrawNext` skip `broadcastDraw` when `settings.mode !== "both"`, or is the cross-mode bus intentional for future "Cả hai" toggling mid-game? + +**Status:** DONE +**Summary:** Prior P0/P1 fixes landed cleanly. Two P1 items around PWA SW registration vs strict CSP need a build-output check before next deploy. P2s are minor. diff --git a/plans/reports/security-260427-2047-pass2-full.md b/plans/reports/security-260427-2047-pass2-full.md new file mode 100644 index 0000000..533854f --- /dev/null +++ b/plans/reports/security-260427-2047-pass2-full.md @@ -0,0 +1,73 @@ +# Security & Reliability Audit — Lô tô (Pass 2) + +Date: 2026-04-27 | Scope: SvelteKit static export + new PWA layer (since `f28279b`). +Method: STRIDE + OWASP Top 10, manual. +Files scanned: `src/**`, `static/_headers`, `static/manifest.webmanifest`, `vite.config.js`, `package.json`, `npm audit`. + +## Summary +- 0 Critical, 0 High, **2 Medium, 4 Low, 5 Informational**. +- All 4 pass-1 mediums (M1 CSP, M2 proto-pollution, M3 payload cap, L1 encodeURIComponent) **resolved**. +- `npm audit` clean (overrides verified: `cookie@0.7.2`, `serialize-javascript@7.0.5` — no API breakage; both transitive-only at build time). +- No outbound network calls; no `{@html}`, `eval`, `fetch`, `XHR`, `WebSocket` anywhere in `src/`. + +## Findings + +### Medium + +**M1 — CSP gap: SW-update / workbox / manifest fetches not explicitly covered, and `style-src 'unsafe-inline'` likely still required.** +`static/_headers:2`. `connect-src 'self'` covers SW `update()` GETs, OK. But: (a) workbox's generated `sw.js` registers via `navigator.serviceWorker.register('/sw.js')` — covered by `worker-src 'self'`. (b) The plugin emits an inline `<script>` registration block in `app.html` injected at build; check post-build `index.html` for inline `<script>` — if present, current `script-src 'self'` will block it. `script-src` lacks `'unsafe-inline'` AND no nonce/hash declared (header docstring says "nonce for scripts" but no `'nonce-…'` token in the policy string). Verify built output. (c) `'unsafe-inline'` for style: confirmed needed — `style:` directives at `MasterPanel.svelte:309`, `PlayerBoard.svelte:282,356,384-386`, `SettingsButton.svelte:418`, `MasterEmptyState.svelte:18` all compile to inline `style=` attributes. Nonces don't help inline attributes; CSP3 `'unsafe-hashes'` does but isn't widely supported. Keep `'unsafe-inline'` for style. (file: `static/_headers:2`) + +**M2 — Service-worker precache integrity not protected by SRI; CacheFirst stores opaque (status 0) responses.** +`vite.config.js:54-67`. `cacheableResponse: { statuses: [0, 200] }` is **required** for CDN-served audio if range requests strip CORS, but it also means a poisoned CDN response is cached as opaque and served forever (CacheFirst). Risk vector: hijacked Cloudflare edge OR rogue CF Pages preview deploy serves a malicious mp3; SW caches and replays for the 30-day TTL. Mitigations available: (a) drop status `0` (audio is same-origin via `${base}/audio/...`, so 200 is sufficient — no opaque needed); (b) bump cache name on each redeploy via revision (already done for precache via `audio-v1-...`, but **not for runtime cache**). Recommend `cacheableResponse: { statuses: [200] }`. Same-origin `<audio>` does not need opaque mode. (file: `vite.config.js:65`) + +### Low + +**L1 — Manifest `start_url: "."` resolves relative; subpath deploys could mislead PWA install scope.** +`static/manifest.webmanifest:5-6`. Both GH-Pages (`/loto/`) and CF root use `"."` which resolves at the manifest URL. Acceptable, but if `_headers` ever serves `manifest.webmanifest` from a non-root path with redirect, scope can drift. Defense-in-depth: pin `start_url` to absolute path per build profile, or `"./"` (trailing slash) for clarity. Cache-Control `no-cache` (line 13 of `_headers`) is correct. (file: `static/manifest.webmanifest:5`) + +**L2 — Audio runtime cache `maxAgeSeconds: 30 days` with no integrity revision.** +`vite.config.js:60-64`. Precache entries get `revision: audio-v1-...`. Runtime cache (alternate voices) has no revision → if a voice clip is regenerated, clients hold the stale 30-day copy until natural eviction. Functional issue (stale audio), low security impact. Bump prefix → also bump runtime `cacheName` in same release to force purge. (file: `vite.config.js:59`) + +**L3 — Reactive bus is module-level singleton — fine for same-tab single user, but BroadcastChannel-style leakage if SW ever cross-posts.** +`src/lib/call-bus.svelte.js:10-13`. `bus` lives in JS module memory, scoped per tab/window — no cross-tab leak. SW does not import it (SW runs in separate context). PASS for current arch. Risk would only appear if a future feature uses `BroadcastChannel`/`postMessage` to mirror draws; document that bus is intentionally tab-local. (file: `src/lib/call-bus.svelte.js`) + +**L4 — `MasterPanel.loadState` minimal validator accepts any array shape.** +`MasterPanel.svelte:55-75`. Reviver strips `__proto__`/`constructor`, length cap 16 KB, `Array.isArray` on both halves — but elements are not type-checked. Poisoned origin could store `{called: ["💀"], remaining: [{}]}` → `state.called[i]` rendered into DOM at line 288 as `{num}` (Svelte text-interpolation auto-escapes, so no XSS), but `callOrder.get(num)` and number comparisons silently produce NaN/`undefined`. Ugly UI, no security breach. Add `n => typeof n === "number" && n >= 1 && n <= 90` per element if hardening further. (file: `src/lib/MasterPanel.svelte:64-69`) + +### Informational + +**I1 — `'unsafe-inline'` for style is unavoidable today.** Svelte's `style:` compiles to inline attributes. Hash-based or nonce-based style CSP would require Svelte-side opt-out + extracting all dynamic styles to CSS variables (already done for `--empty-cell-bg`; not done for confetti, master cell bg toggle). + +**I2 — `frame-ancestors 'none' + X-Frame-Options: DENY`**: belt-and-braces, OK. + +**I3 — `manifest-src 'self'` correctly added.** Required by Chrome since 2020. PASS. + +**I4 — Self-hosted font (`@fontsource/roboto-condensed`)** removes Google Fonts CDN dependency → `font-src 'self' data:` is sufficient and tight. PASS. + +**I5 — npm overrides verified non-breaking.** `npm ls cookie serialize-javascript` resolves cleanly to `0.7.2` and `7.0.5`; no peer-dep warnings; build artifacts unchanged. Both packages are build-time only (kit dev internals + workbox-build via @rollup/plugin-terser) — runtime never executes them. PASS. + +## Trust-boundary verification (re-confirmed) +- localStorage payload caps applied to **all 4 keys**: `loto_settings` (8KB, `settings-store.svelte.js:17,127`), `loto_grid` + `loto_crossed` (32KB, `game-logic.js:169,180`), `loto_master` (16KB, `MasterPanel.svelte:53,58`). PASS. +- `__proto__`/`constructor` reviver applied in **all 3 parse sites**: `settings-store.svelte.js:130`, `game-logic.js:182`, `MasterPanel.svelte:59`. PASS. +- `clipUrl` `encodeURIComponent` belt-and-braces on both `voice` + `name` (`voice.js:41`). PASS. +- Voice allowlist `VOICE_IDS.has(v)` (`settings-store.svelte.js:71`). PASS. +- CSP `frame-ancestors 'none'` blocks clickjacking. PASS. +- SW: `registerType: "autoUpdate"` without `skipWaiting` (`vite.config.js:40`) — explicit comment confirms intent. PASS. +- `noopener noreferrer` on all external `<a target="_blank">`. PASS. + +## Pass-1 follow-ups status +| ID | Item | Status | +|----|------|--------| +| M1 (pass1) | CSP added | RESOLVED | +| M2 (pass1) | proto/constructor stripping | RESOLVED (3 sites) | +| M3 (pass1) | localStorage payload caps | RESOLVED (4 keys) | +| L1 (pass1) | encodeURIComponent on clipUrl | RESOLVED | +| L2 (pass1) | Audio cache LRU | DEFERRED — acceptable while voice count ≤ ~5; SW cache now also caps at 400 entries | +| L3 (pass1) | crypto.getRandomValues | DEFERRED — not security-relevant | +| L4 (pass1) | cookie <0.7.0 | RESOLVED via override | + +## Unresolved questions +1. Inspect post-build `build/index.html` — does workbox/SvelteKit-PWA inject any inline `<script>` for SW registration? If yes, `script-src 'self'` blocks it (M1c). Run `npm run build && grep -c "<script>" build/index.html`. +2. Is `cacheableResponse.statuses: [0]` actually needed for same-origin `/audio/*.mp3`, or can it be tightened to `[200]` only? (M2) +3. GitHub Pages mirror — does it serve `_headers`? (GH Pages ignores `_headers`; CSP only enforced on CF.) Acceptable since GH is mirror-only? +4. Should runtime `cacheName: "loto-audio"` be versioned (e.g. `loto-audio-v1`) so a future audio regen forces purge? (L2) diff --git a/plans/reports/ui-ux-designer-260427-2047-pass2-full.md b/plans/reports/ui-ux-designer-260427-2047-pass2-full.md new file mode 100644 index 0000000..9d9d356 --- /dev/null +++ b/plans/reports/ui-ux-designer-260427-2047-pass2-full.md @@ -0,0 +1,125 @@ +# Lô tô — UI/UX Audit Pass 2 (260427-2047) + +Scope: re-audit since `f28279b` against current `main`. Read `app.css`, `app.html`, `manifest.webmanifest`, `+page.svelte`, `PlayerBoard`, `MasterPanel`, `MasterEmptyState`, `SettingsButton`, `PageFooter`. Severity P0/P1/P2. + +Mostly clean follow-up. Most pass-1 items addressed well. Findings below are new or partial-fix regressions. + +--- + +## P0 + +### 1. PWA splash + tab theme color hardcoded saturated blue, mismatches light page background +**Where:** `app.html:9` (`#1565c0` light) and `manifest.webmanifest:9-10` (`theme_color #1565c0`, `background_color #0a0f1f`). +Light app bg is `#f8fafc` (near-white) but Safari tab strip / Chrome top bar paints `#1565c0`. Hard color jump where the bg should bleed into the chrome. PWA splash on iOS uses `background_color` only — `#0a0f1f` (deep navy) → light-mode users get a dark-navy splash that flashes into a near-white app. Jarring on cold launch. +**Fix sketch:** light theme-color `#f3e9d7` or `#fff7ec` (warm off-white that echoes the amber top-glow). Manifest `background_color` to a neutral midpoint or fork by media: `background_color: #f8fafc`. iOS doesn't honor light/dark manifest yet, so pick the value matching the *more common* launch theme — `auto` defaults to user OS, so neutral cream is safer than near-black. + +### 2. Tab title still bare "Lô tô" — PWA install card name mismatch +**Where:** `app.html:6` `<title>Lô tô` vs `manifest.name "Lô tô — Hội chợ TN1"`. +Installed app shows full name; browser tab + history show only "Lô tô". Cold-share link previews lose the "Hội chợ TN1" context entirely. Also no Open Graph tags. +**Fix sketch:** title `Lô tô — Hội chợ TN1`; add ``, `og:description`, `og:image` (use `icon-512.png`). One-time copy, immediate brand lift on share. + +--- + +## P1 + +### 3. PlayerBoard empty-state and MasterEmptyState are near-clones with conflicting prompts +**Where:** `PlayerBoard.svelte:343-371` ghost grid + `MasterEmptyState.svelte:6-43` ghost grid. +Both render same opacity-30 monochrome ghost-grid pattern. Master mode in `both` shows player ghost AND master ghost stacked when no game started. Visual repetition; the page reads "two empty boxes" not "two distinct roles". Also: master grid shows 99 cells but real master board is 11×9=99 with last row = single cell at col 8 — ghost should mirror the actual silhouette. +**Fix sketch:** vary the ghosts visually — player ghost shows a subtle row-of-numbers stripe; master ghost shows scattered "called dots" pattern. Or hide the player ghost entirely in `both` mode pre-game (the master-mode hero CTA carries the call-to-action). + +### 4. Mode picker glyphs read at first glance only for "player"; "master" megaphone is ambiguous, "both" looks like a stacked-window icon +**Where:** `SettingsButton.svelte:259-275`. +Player rect-with-grid-lines reads instantly = "card with rows". Master path `M3 11l14-6v14L3 13z` + arc is a megaphone but at 24×24 stroke 1.8 looks like an abstract triangle pointing right; not enough silhouette weight at 28px tall. "Both" stacked rectangles read as "two cards" not "player + master roles". Hint line below mitigates but the glyph itself doesn't sell. +**Fix sketch:** master = filled megaphone with sound waves (use stroke-width 2.2 + fill-on-active). "Both" = player-card-glyph layered with mini-megaphone badge in corner — composes the two prior glyphs. Keeps semantic continuity ("both = the two things above stacked"). + +### 5. "Đặt lại" reset chip — bordered now, but still no confirm dialog +**Where:** `SettingsButton.svelte:431-440`. +Pass-1 fix turned reset into a chip-with-border (good). But it still resets all settings (theme, mode, color, voice, auto-call) on a single tap. Easy mis-tap on mobile next to "Xong". No undo. +**Fix sketch:** `if (confirm("Đặt lại tất cả tuỳ chỉnh?"))` guard; or convert to two-step ("Tap to reset" → "Confirm reset" inline state for 3s). Native `confirm()` is fine here, low frequency. + +### 6. Settings modal scroll on small viewports — sticky header/footer absent +**Where:** `SettingsButton.svelte:166-167`. `max-h-[90vh] overflow-y-auto` whole-modal scroll. +On 375×667 (iPhone SE) with both auto-call AND voice-waiting expanded, modal hits ~700px. User scrolls past "Cài đặt" title; "Đặt lại / Xong" footer scrolls off too — must scroll back to dismiss. Title and primary CTAs should be persistent. +**Fix sketch:** sticky title row (`sticky top-0 bg-white dark:bg-slate-800 -mx-6 px-6 pt-6 pb-3 z-10`), sticky footer row similarly. Inner content gets the scroll. Saves a scroll-trip per session. + +### 7. Auto-call slider — still no tick labels at 1s/5s/10s +**Where:** `SettingsButton.svelte:306-316`. +Pass-1 noted this. Fixed: nesting + left-border indent (good). Not fixed: tick labels. Slider value floats free, user has no anchor for "what is fast vs slow". +**Fix sketch:** below slider, add `
1s5s10s
` aligned to track. Trivial. + +### 8. Voice "Quản trò đọc số" hint copy still confusing in `both` mode +**Where:** `SettingsButton.svelte:336-338`. Pass-1 note: hide in player mode — done. But: hint says `Đọc số đã xổ + báo Chờ/Kinh khi ở "Cả hai".` In `master` mode the second clause ("báo Chờ/Kinh") is wrong — there's no player board to call Chờ/Kinh from in solo master. +**Fix sketch:** branch the hint by mode: master → `Đọc số đã xổ.`; both → `Đọc số đã xổ và báo Chờ/Kinh thay người chơi.` + +### 9. Header subline `🏮 Hội chợ TN1` — lantern emoji renders monochrome on Windows/Linux, color on Apple/Android +**Where:** `+page.svelte:31`. +Cross-platform lantern inconsistency. On a Windows browser the lantern is line-art outlined, breaking the festive intent. Dashes-flank treatment is good though. +**Fix sketch:** ship a tiny SVG lantern inline (12×16, color: rose-500) in place of the emoji. Same byte cost, consistent across OS, theme-tintable. + +### 10. Master "Số vừa xổ" hero — w-32 mobile is good (pass-1 fixed), but border-[6px] eats interior +**Where:** `MasterPanel.svelte:244-252`. +128px circle - 12px border (×2) = 104px interior for an 8xl number. Number renders fine but the ring feels chunky at this size; reads as "thick outlined badge", not "called number". Aspect feels token-y not announcer-y. +**Fix sketch:** `border-[4px] sm:border-[10px]`. Keep desktop chunk; trim mobile. + +--- + +## P2 + +### 11. Section-divider hatch repeats under the bottom decorative band but with no label slot +**Where:** `PlayerBoard.svelte:316`. `` — empty label = just the cross-hatch flanks ::before/::after with `flex:1` and 0 gap. Visually thinner than divider above section-label rows. +**Fix sketch:** swap to `
` — semantic match, consistent thickness. Tested: same color path via `--section-accent`. + +### 12. `aria-live="assertive"` on hero number — still assertive in pass-1 had it as P2; updated to "polite" in code (good). No regression. + +### 13. Confetti emoji set still `["🎊", "✨", "🎉", "🥳"]` — pass-1 P2 note re. lantern + Vietnamese-flavored set not addressed +**Where:** `PlayerBoard.svelte:35`. +Add `🏮 🎋 🥢` for fairground feel. Match the lantern in the header for cohesion. + +### 14. `apple-touch-icon` only 192px (no 180px specifically; no `apple-touch-startup-image`) +**Where:** `app.html:11`. iOS scales 192→180 fine but loses sharpness. Missing splash image = bare-color flash on cold launch. +**Fix sketch:** generate `icon-180.png` and `apple-splash-{2048x2732,1668x2388,1170x2532}.png` from `source.svg`. Wire ``. Optional polish. + +### 15. `MasterEmptyState` ghost grid uses `i % 11 < 9 && i % 7 === 0` — generates non-deterministic-looking sparse fill that doesn't mirror the master grid silhouette (col 8 row 10 only has 90) +**Where:** `MasterEmptyState.svelte:14`. Cosmetic — looks like a noise-ghost rather than a master-board ghost. Fine as decoration but pass on opportunity to communicate "tracking grid" affordance. +**Fix sketch:** mirror real `BOARD` shape: row 0 cols 1-8, rows 1-9 all cols, row 10 col 8. Then ghost-fill ~15-20% with a deterministic mod pattern. + +### 16. Toast above grid (pass-1 fixed) — but on `both` mode the toast renders inside PlayerBoard's `relative` wrapper while master panel below pushes content; toast `-top-3` goes negative into the page-padding zone, can clip on very narrow screens (≤320px) where parent has only `px-2` (8px). +**Where:** `+page.svelte:11`, `PlayerBoard.svelte:330`. +Edge case (≤320 = older Android, rare). Toast still readable but center text near the screen edge. +**Fix sketch:** add `mx-2` on the toast button so it can compress safely; or move toast to `top-1` (positive offset, sits inside the rounded board chrome). + +--- + +## Pass-1 fix verification + +| Pass-1 item | Verdict | +|---|---| +| 1. Master empty state | ✅ MasterEmptyState added, role pill shown | +| 2. Dark winning row contrast | ✅ `bg-emerald-900/60 text-emerald-200` | +| 3. Vietnamese font fallback | ✅ self-hosted Roboto Condensed 700 | +| 4. Toast over cells | ✅ moved to `-top-3` (see P2 §16 edge case) | +| 5. Mode picker glyphs | ⚠️ added but readability mixed (P1 §4) | +| 7. Master hero w-40 mobile clip | ✅ w-32 sm:w-56 | +| 8. Auto-call slider density | ⚠️ partial — nesting fixed, no tick labels (P1 §7) | +| 9. Voice nesting wording | ⚠️ partial — hidden in player but copy still off in master (P1 §8) | +| 10. Color picker grouped | ✅ bordered card + sub-headers | +| 11. Reset button visibility | ⚠️ chip-bordered (good) but no confirm (P1 §5) | +| 12. Header subline brand mood | ✅ dash-flanked, lantern emoji (see P1 §9 cross-OS) | +| 13. Bingo modal row-number size | ✅ text-5xl/6xl | +| 18. aria-live polite on hero | ✅ | +| 19. Reduced-motion gating | ✅ media query added | +| 14. Footer dual attribution | ✅ in-card credit removed | + +--- + +## Unresolved questions + +1. Manifest `background_color: #0a0f1f` was chosen for dark; should we accept the light-mode PWA splash dark-flash, or pick a neutral cream? (P0 §1) +2. iOS Safari standalone — has it actually been tested on a device, or only DevTools simulated? Apple-status-bar `black-translucent` interacts with `safe-area-inset-top`; nothing in the layout reserves that inset. +3. Is there appetite to drop the `🏮` emoji entirely if the SVG-inline lantern is rejected? Plain dashes alone read fine and ship-stable. (P1 §9) +4. Auto-call max 10s — is that the right ceiling? Real-life Lô tô callers often pause 15-20s for call-and-response. Out of scope but data point. + +--- + +**Status:** DONE +**Summary:** 16 findings — 2 P0 (PWA splash/theme color mismatch, tab title brand), 8 P1 (empty-state duplication, glyph readability, reset-confirm, sticky modal chrome, slider ticks, voice hint copy, lantern cross-OS, mobile hero ring), 6 P2. Pass-1 mostly addressed; partial-fixes on items 5/8/9/11. diff --git a/src/app.html b/src/app.html index 8117cc5..b089d7b 100644 --- a/src/app.html +++ b/src/app.html @@ -3,8 +3,8 @@ - Lô tô - + Lô tô — Hội chợ TN1 + diff --git a/src/lib/SettingsButton.svelte b/src/lib/SettingsButton.svelte index 8541437..9c499aa 100644 --- a/src/lib/SettingsButton.svelte +++ b/src/lib/SettingsButton.svelte @@ -333,9 +333,15 @@ {#if settings.mode !== "player"}
{@render switchRow("Quản trò đọc số", settings.voiceEnabledMaster, toggleVoiceMaster)} -

- Đọc số đã xổ + báo Chờ/Kinh khi ở "Cả hai". -

+ {#if settings.mode === "both"} +

+ Đọc số đã xổ + báo Chờ/Kinh. +

+ {:else} +

+ Đọc số đã xổ. +

+ {/if}
{/if} diff --git a/static/_headers b/static/_headers index e467f99..c10c0ab 100644 --- a/static/_headers +++ b/static/_headers @@ -1,5 +1,5 @@ /* - Content-Security-Policy: default-src 'self'; img-src 'self' data:; media-src 'self'; style-src 'self' 'unsafe-inline'; script-src 'self'; connect-src 'self'; font-src 'self' data:; frame-ancestors 'none'; base-uri 'self'; form-action 'self'; manifest-src 'self'; worker-src 'self' + Content-Security-Policy: default-src 'self'; img-src 'self' data:; media-src 'self'; style-src 'self' 'unsafe-inline'; script-src 'self' 'unsafe-inline'; connect-src 'self'; font-src 'self' data:; frame-ancestors 'none'; base-uri 'self'; form-action 'self'; manifest-src 'self'; worker-src 'self' X-Content-Type-Options: nosniff Referrer-Policy: strict-origin-when-cross-origin Permissions-Policy: accelerometer=(), camera=(), geolocation=(), gyroscope=(), microphone=(), payment=(), usb=() diff --git a/static/manifest.webmanifest b/static/manifest.webmanifest index b590fc4..86b2051 100644 --- a/static/manifest.webmanifest +++ b/static/manifest.webmanifest @@ -7,7 +7,7 @@ "display": "standalone", "orientation": "portrait", "theme_color": "#1565c0", - "background_color": "#0a0f1f", + "background_color": "#f8fafc", "lang": "vi", "icons": [ { diff --git a/vite.config.js b/vite.config.js index b21ac1a..416d953 100644 --- a/vite.config.js +++ b/vite.config.js @@ -7,10 +7,12 @@ import { defineConfig, loadEnv } from "vite"; // Precache the default voice's clips so the app is fully offline-capable // on first install (without bloating the install with every voice). // Alternate voices fall through to runtime CacheFirst on first play. +// `audio-manifest.js` is the single source of truth for voice ids; we +// duplicate the read here because vite.config can't import .svelte.js. const audioManifest = JSON.parse( readFileSync("./static/audio/manifest.json", "utf8"), ); -const defaultVoiceId = audioManifest.voices[0]?.id ?? "hoai-my"; +const defaultVoiceId = audioManifest.voices[0].id; const clipNames = [ ...Array.from({ length: 90 }, (_, i) => String(i + 1)), "cho", @@ -18,8 +20,8 @@ const clipNames = [ ]; const defaultVoicePrecacheEntries = clipNames.map((n) => ({ url: `/audio/${defaultVoiceId}/${n}.mp3`, - // Static asset with a stable name; Workbox needs a revision string - // to track changes. Bump the prefix when audio is regenerated. + // Workbox needs a revision string to invalidate stale clips. + // Bump the prefix when audio is regenerated. revision: `audio-v1-${defaultVoiceId}-${n}`, })); @@ -43,7 +45,6 @@ export default defineConfig(({ mode }) => { // confusing. strategies: "generateSW", manifest: false, - includeAssets: ["icons/*.png", "audio/**/*.mp3"], workbox: { globPatterns: ["**/*.{js,css,html,svg,png,woff2,webmanifest}"], maximumFileSizeToCacheInBytes: 5 * 1024 * 1024, @@ -62,7 +63,8 @@ export default defineConfig(({ mode }) => { maxEntries: 400, maxAgeSeconds: 60 * 60 * 24 * 30, }, - cacheableResponse: { statuses: [0, 200] }, + // Audio is same-origin → no need to allow opaque (0). + cacheableResponse: { statuses: [200] }, }, }, ],