feat(ui): tân tân 3-section player card layout

Render the 9x9 player card as 3 stacked 3x9 mini-cards mirroring a
physical Minh Tân paper sheet, with traditional separator labels:
"Minh Tân", "Loại đặc biệt", "Tấn tài tấn lộc". Pure visual change —
underlying 9x9 data, generator, click handlers, win detection all
unchanged. Sectioned render uses absolute row index (startRow + r) so
crossed-state and rowCompleteness lookups stay correct.

Adds .section-divider (cross-hatch repeating gradient) and .section-label
styles to app.css. Defines --empty-cell-bg CSS variable with a brown
default matching the paper card; the runtime override lands with the
settings feature in a follow-up commit.
This commit is contained in:
2026-04-27 00:13:27 +07:00
parent de6bdf7eec
commit 37502a4e28
7 changed files with 923 additions and 34 deletions
@@ -0,0 +1,156 @@
# Phase 1 — 3-Section Visual Split for PlayerBoard
## Context
- Plan: [plan.md](plan.md)
- Reference image: physical Minh Tân lô tô card (3 stacked 3×9 mini-cards on
one sheet, brown empty cells, decorative cross-hatch separators).
- Touches: `src/lib/PlayerBoard.svelte`, `src/app.css`.
## Overview
- **Priority**: P1 (visual identity / authenticity)
- **Status**: TODO
- **Effort**: ~30 min
- **Description**: Render the existing 9×9 grid as 3 visually distinct 3-row
sections. Data, generator, click handlers, win detection: unchanged.
## Key Insights
- The 9×9 → 3×(3×9) split is **purely visual**. Don't touch
`src/lib/game-logic.js`. Don't change the grid shape, the storage shape,
or `isRowComplete` — they all keep operating on a 9×9 array.
- Tân Tân section labels (top to bottom): **Minh Tân** / **Loại đặc biệt**
/ **Tấn tài tấn lộc**. Use these verbatim.
- The decorative cross-hatch border (✚✚✚✚) on the physical card is
ornamental. CSS approximation: a thin orange/red dotted or repeating
cross-pattern border between sections is enough — don't pixel-copy.
- Card serial number badge (e.g., "25" on the physical card) is **out of
scope** per plan.
## Requirements
### Functional
- Player card renders as 3 stacked sub-grids: rows 02, rows 35, rows 68.
- A label sits between each pair of sub-grids (and above the first):
`Minh Tân`, `Loại đặc biệt`, `Tấn tài tấn lộc`.
- All click / cross / waiting / Kinh behavior keeps working unchanged.
- Cell crossed style, row-complete style, the `Chờ N` toast, and the Kinh
modal all behave exactly as before.
### Non-functional
- No layout shift or visual jitter.
- Mobile responsive (stays within `max-w-2xl` container).
- Dark mode parity: separator and labels visible on both themes.
- A11y: each sub-grid keeps `aria-label="Bảng lô tô"` (or section-specific
label like `Bảng lô tô — phần 1`).
## Architecture
### Render strategy
Replace the single `<div class="loto-grid">` with three sibling
`<div class="loto-grid">` blocks, each iterating only its 3 rows. Use
`{#each [0, 3, 6] as startRow, sectionIdx}` and within each section iterate
`grid.slice(startRow, startRow + 3)`.
The `crossed` lookup must use the **absolute row index** (`startRow + r`),
not the slice-relative index. Same for click handlers and `rowCompleteness`.
### Section labels
Plain centered text (`text-center text-xs uppercase tracking-widest text-slate-500`)
between sections, sandwiched by horizontal cross-hatch lines. Use a
repeating linear-gradient or a `.section-divider` utility in `app.css`.
### Border continuity
Currently each cell has `border-r border-b`. Within a section this is fine.
Across sections we **don't** want the bottom row of section 1 to look
adjacent to the top row of section 2 — the separator label / divider sits
between them, so the visual gap naturally breaks the grid.
The outer wrapper (`rounded-2xl overflow-hidden shadow-xl border …`)
currently wraps the whole grid. Two choices:
- **A**: keep one outer wrapper, separators are inside it.
- **B**: each section has its own rounded card.
**Pick A** — one outer wrapper, internal separators. Closer to the
physical sheet which is one continuous piece of paper.
## Related Code Files
### Modify
- `src/lib/PlayerBoard.svelte`
- Replace the single `{#each grid.flat() …}` loop with 3 sectioned loops
(lines ~157191 in current state).
- Each section iterates `[0, 3, 6][i] .. + 3` and uses absolute row idx
when reading `crossed[row]`, calling `handleCellClick(row, col)`,
reading `rowCompleteness[row]`.
- Add the section header text between sections.
- `src/app.css`
- Add `.section-divider` utility (cross-hatch / dotted horizontal rule).
- Optionally add `.section-label` if Tailwind utility classes don't
cover the styling cleanly.
### Don't touch
- `src/lib/game-logic.js`
- `src/routes/master/+page.svelte` (master grid stays 11×9)
- `src/routes/+page.svelte`
## Implementation Steps
1. In `PlayerBoard.svelte`, extract a `SECTIONS = [0, 3, 6]` constant and a
parallel `SECTION_LABELS = ['Minh Tân', 'Loại đặc biệt', 'Tấn tài tấn lộc']`.
2. Wrap the existing grid block with a flex column. Inside, loop sections:
for each `startRow`, render the label, then a `loto-grid` containing
that section's 3 rows × 9 cols.
3. Inside the inner loop, compute `row = startRow + r`. Pass `row` to
`handleCellClick`, look up `crossed[row]?.[col]`, `rowCompleteness[row]`.
4. Add the `Chờ` toast container *outside* the sectioned grid (it
currently uses `absolute inset-0` of the outer relative wrapper —
keep that wrapper around all 3 sections).
5. In `app.css`, define `.section-divider` (e.g.
`background: repeating-linear-gradient(90deg, transparent 0 8px, theme(colors.orange.400) 8px 10px); height: 6px;`).
6. Run `npx svelte-check`, then test in dev: generate card, mark cells,
trigger Kinh, trigger Chờ. All should work.
## Todo
- [ ] Add `SECTIONS` + `SECTION_LABELS` constants to PlayerBoard.svelte
- [ ] Replace flat grid render with 3 sectioned `loto-grid` blocks
- [ ] Verify absolute-row-index math (click, crossed lookup, rowCompleteness)
- [ ] Add label markup between sections
- [ ] Add `.section-divider` style in `app.css`
- [ ] Verify Chờ toast still appears centered over the whole card
- [ ] Verify Kinh modal still triggers
- [ ] Verify dark-mode contrast for labels and dividers
- [ ] Mobile breakpoint check (sm + base)
- [ ] `npx svelte-check` clean
## Success Criteria
- Player page shows 3 visually separated 3×9 mini-cards with the 3 labels.
- Generating a new card, marking cells, and winning all behave identically
to before this change.
- No svelte-check warnings.
- Dark mode looks intentional (not just light mode in the dark).
## Risk Assessment
| Risk | Likelihood | Impact | Mitigation |
|---|---|---|---|
| Off-by-one in absolute row index | Medium | High (broken click handling) | Compute `row = startRow + r` once at the top of the inner each, reuse everywhere. |
| Toast positioning breaks | Low | Low | Toast container stays at the same wrapper level it's at today. |
| Border doubling at section seams | Low | Cosmetic | Last row of each section keeps its own `border-b`; separator visually masks it. |
| `aspect-square` cells distort with new container | Low | Cosmetic | grid-template-columns is still `repeat(9, 1fr)` per section, same width budget per section. |
## Security Considerations
None — view-only change.
## Next Steps
After this phase: Phase 2 (settings + color picker) plugs the
configurable color into `bg-slate-50 dark:bg-slate-900/60` empty cells
(replaced by a CSS variable).
@@ -0,0 +1,227 @@
# Phase 2 — Settings Modal + Empty-Cell Color Picker
## Context
- Plan: [plan.md](plan.md)
- Depends on Phase 1 only loosely (works either way; better when Phase 1
shipped because the section background also picks up the color).
- Touches: new `settings-store.svelte.js` + `SettingsButton.svelte`,
edits in `PlayerBoard.svelte`, `master/+page.svelte`, `+page.svelte`,
`app.css`.
## Overview
- **Priority**: P1 (user-requested feature)
- **Status**: TODO
- **Effort**: ~45 min
- **Description**: Gear icon → modal panel with a color picker. Selected
color paints empty/blank cells across player card *and* master tracking
grid. Persists in localStorage. Default: brown (matches physical Tân Tân).
## Key Insights
- One **global** color, not per-card. Simpler, matches physical sheet
(one paper color).
- Use **CSS custom property** (`--empty-cell-bg`) on `:root` so a single
store update repaints every empty cell with no per-component prop drilling.
- Settings store goes in `src/lib/settings-store.svelte.js` — the
`.svelte.js` extension lets it use Svelte 5 runes (`$state`) at module
scope. Components import the reactive `settings` object.
- The color picker can be a **native** `<input type="color">` to avoid
pulling in a UI library. Add a few **preset swatches** (brown, slate,
amber, emerald, indigo, neutral white) for one-tap defaults.
- Keep the modal a11y-clean: `role="dialog"`, `aria-modal="true"`,
`Escape` to close, focus trap-ish (focus the close button on open).
## Requirements
### Functional
- A gear button (⚙) sits in the player page header (`/`) and master page
header (`/master`).
- Clicking the gear opens a modal panel.
- Modal contains:
- A native color picker (`<input type="color">`) bound to the current
color.
- 6 preset swatches as quick-pick buttons.
- A "Reset to default" button.
- A "Close" button.
- Color change is **live** — empty cells repaint as user picks.
- Setting persists in localStorage under key `loto_settings`.
- Default color: a brown matching the physical card (`#7a4a2b` or close).
### Non-functional
- No new third-party deps.
- Settings store must support adding more keys later (extensible shape).
- Works offline (no network).
- Dark mode: the picker UI itself must be readable in dark mode; the
user-chosen color obviously stays as-is.
## Architecture
### Store shape
```js
// settings-store.svelte.js
export const DEFAULT_SETTINGS = Object.freeze({
emptyCellColor: '#7a4a2b', // brown — matches physical Minh Tân card
});
const STORAGE_KEY = 'loto_settings';
export const settings = $state({ ...DEFAULT_SETTINGS });
// load + persist helpers
export function loadSettings() { /* read localStorage, merge into settings */ }
export function saveSettings() { /* write current settings */ }
export function resetSettings() { /* reset to DEFAULT_SETTINGS */ }
```
A root effect (set up once in `+layout.svelte` or in each page that
imports the store) calls `loadSettings()` on mount and persists on change.
Simpler: do it inside the store module via a top-level `$effect.root` in
the `.svelte.js` file — Svelte 5 supports this.
### CSS variable wiring
In `app.css`, define a default:
```css
:root { --empty-cell-bg: #7a4a2b; }
```
Bind it from the store at the page level (whichever component first
mounts the store):
```svelte
<div style:--empty-cell-bg={settings.emptyCellColor}>...</div>
```
OR set it directly on `document.documentElement` in `loadSettings()` /
on store change via an effect. The latter avoids prop drilling.
### Empty-cell consumption
Replace existing empty-cell backgrounds:
| File | Current | New |
|---|---|---|
| `PlayerBoard.svelte` (empty cell) | `bg-slate-50 dark:bg-slate-900/60` | `style:background-color="var(--empty-cell-bg)"` |
| `master/+page.svelte` (empty cell, `!hasNumber`) | `bg-slate-100 dark:bg-slate-900/60` | same |
Filled cells keep their existing styles. The chosen color only repaints
**`hasNumber === false`** cells.
### Settings button placement
A small floating gear button in the top-right of each page's header
section. Reuse the same `SettingsButton.svelte` component on both pages.
### Modal
`SettingsButton.svelte` owns its own `open` state. When open, renders the
modal as a sibling overlay. Same dismiss patterns as the existing Kinh
modal in `PlayerBoard.svelte` (backdrop click + Escape).
## Related Code Files
### Create
- `src/lib/settings-store.svelte.js` — rune-based reactive store +
load/save/reset. ~50 LOC.
- `src/lib/SettingsButton.svelte` — gear button + modal + color picker
+ 6 preset swatches + reset/close. ~80 LOC.
### Modify
- `src/lib/PlayerBoard.svelte` — empty-cell background uses CSS var.
- `src/routes/master/+page.svelte` — empty-cell background uses CSS var;
mount `<SettingsButton />` in header.
- `src/routes/+page.svelte` — mount `<SettingsButton />` in header.
- `src/app.css` — add `:root { --empty-cell-bg: ...; }` default.
### Don't touch
- `src/lib/game-logic.js` (no game state changes)
## Implementation Steps
1. Create `src/lib/settings-store.svelte.js` with `settings` rune object,
`DEFAULT_SETTINGS`, `loadSettings`, `saveSettings`, `resetSettings`.
2. In the store module, set up a root effect that:
- Reads localStorage on first call.
- Writes localStorage on change.
- Pushes `settings.emptyCellColor` to
`document.documentElement.style.setProperty('--empty-cell-bg', …)`.
3. Add `:root { --empty-cell-bg: #7a4a2b; }` to `src/app.css` as fallback.
4. Create `src/lib/SettingsButton.svelte`:
- `let { } = $props();` (no props)
- Local `open = $state(false)`
- Imports `settings` + `resetSettings` from the store
- Renders gear button; on click toggles `open`
- Modal: `<input type="color" bind:value={settings.emptyCellColor}>`,
6 preset swatches as buttons setting `settings.emptyCellColor` directly,
reset button calling `resetSettings()`, close button.
- `Escape` keydown closes modal.
5. In `PlayerBoard.svelte`, change empty-cell background from
`bg-slate-50 dark:bg-slate-900/60` to inline
`style:background-color="var(--empty-cell-bg)"`.
6. In `master/+page.svelte`, do the same for `!hasNumber` cells; also
import and mount `<SettingsButton />` in the header (next to the
"Về trang người chơi" link).
7. In `+page.svelte`, mount `<SettingsButton />` in the player page header.
8. Run `npx svelte-check`.
9. Manual test: open gear, change color, see player card + master grid
empty cells update live. Reload page → color persists.
10. Reset button → returns to brown default.
## Todo
- [ ] Create `settings-store.svelte.js` with rune store + persistence
- [ ] Create `SettingsButton.svelte` with gear + modal + picker + presets
- [ ] Add CSS var `--empty-cell-bg` default in `app.css`
- [ ] Wire CSS var update from store via effect
- [ ] Update `PlayerBoard.svelte` empty cells to use CSS var
- [ ] Update `master/+page.svelte` empty cells + mount SettingsButton
- [ ] Update `+page.svelte` to mount SettingsButton
- [ ] Test: change color → repaints both boards live
- [ ] Test: reload → persists
- [ ] Test: reset → back to brown
- [ ] Test: Escape closes modal
- [ ] `npx svelte-check` clean
- [ ] Update `docs/codebase-summary.md` (new files, new storage key)
- [ ] Update `docs/project-overview-pdr.md` (note settings feature)
## Success Criteria
- Gear icon visible on `/` and `/master`.
- Modal opens, picker bound to current color.
- Picking a color via picker OR swatch repaints empty cells live on both
boards.
- Reload preserves the chosen color.
- Reset button returns to brown default.
- No svelte-check errors.
## Risk Assessment
| Risk | Likelihood | Impact | Mitigation |
|---|---|---|---|
| `$state` at module scope misuse | Medium | High | Use `.svelte.js` extension; verify with svelte-check; reference Svelte 5 docs if unsure (`docs-seeker` skill / context7). |
| CSS var doesn't apply across light/dark mode cleanly | Low | Cosmetic | Set var on `:root`, no media-query gating. User picked color overrides the brand-default for both modes — that's the user's call. |
| localStorage unavailable (private mode) | Low | Setting just doesn't persist | Wrap reads/writes in try/catch (mirror existing `saveGrid`/`loadGrid` pattern). |
| Race: store loads after first paint, brief flash of fallback color | Low | Cosmetic | Acceptable; default fallback in CSS is the same brown, so no flash for default users. |
| New storage key collides with existing | None | | `loto_settings` is unused. |
## Security Considerations
- `<input type="color">` returns a 7-char `#rrggbb` string, no injection
vector via CSS variable. Still: validate `/^#[0-9a-fA-F]{6}$/` before
applying, fall back to default on mismatch.
- localStorage data is per-origin; no concerns.
## Next Steps
After this phase the plan is complete. Optional follow-ups (NOT in this plan):
- Add more settings (font size, sound on Kinh, etc.) — store is already
shaped to allow it.
- Persist user-defined preset swatches.
## Storage Keys (added)
| Key | Shape | Purpose |
|---|---|---|
| `loto_settings` | `{ emptyCellColor: "#rrggbb" }` | global UI settings |
@@ -0,0 +1,63 @@
---
slug: tan-tan-3-section-board-and-settings
created: 2026-04-26
status: completed
completedAt: 2026-04-26
mode: fast
blockedBy: []
blocks: []
---
# Tân Tân 3-Section Player Board + Settings
Two cosmetic features that make the app look like a real Minh Tân lô tô sheet:
1. **Visual split** — render the 9×9 player card as 3 stacked 3×9 mini-cards
with traditional Tân Tân separator labels. Data and generator unchanged.
2. **Settings** — gear icon → modal with a color picker for empty cells.
Applies to all boards (player + master tracking grid). Default: brown.
Reference image: physical "Minh Tân" card from dochoicholon.com — 3 mini-cards
stacked on one sheet, brown empty cells, decorative cross-hatch separators
labeled "Minh Tân" / "Loại đặc biệt" / "Tấn tài tấn lộc".
## Decisions (locked, do not re-ask)
- **Layout**: stacked on one page, not paginated.
- **Generator**: unchanged — keeps exact 5/row + 5/col on the full 9×9.
- **Color scope**: setting applies to player card *and* master tracking grid.
## Phases
| # | Phase | Status | File |
|---|---|---|---|
| 1 | 3-section visual split for PlayerBoard | DONE | [phase-01-three-section-player-board.md](phase-01-three-section-player-board.md) |
| 2 | Settings modal + empty-cell color picker | DONE | [phase-02-settings-color-picker.md](phase-02-settings-color-picker.md) |
## Files Touched
| File | Phase | Why |
|---|---|---|
| `src/lib/PlayerBoard.svelte` | 1, 2 | render 3 sections; consume color setting |
| `src/routes/master/+page.svelte` | 2 | consume color setting for empty cells |
| `src/routes/+page.svelte` | 2 | mount settings button (player page header) |
| `src/lib/settings-store.svelte.js` | 2 | new — rune-based settings store + persistence |
| `src/lib/SettingsButton.svelte` | 2 | new — gear button + modal |
| `src/app.css` | 1, 2 | section separator styles, CSS var for empty color |
| `docs/project-overview-pdr.md` | 1, 2 | document new visual + settings |
| `docs/codebase-summary.md` | 1, 2 | new files in component table |
## Key Dependencies
- Phase 2 depends on Phase 1 (so the new section background uses the
configurable color from day one). They can also ship independently —
Phase 2 will simply apply to both flat and section layouts.
## Out of Scope
- Per-card color override (one global color for all cards).
- Theme preset bundles.
- Other settings (font, sound, etc.) — settings store will be extensible
but only "empty-cell color" lands now.
- Decorative bg images / paper texture.
- Card serial number badge (the "25" badge in the physical photo).
@@ -0,0 +1,193 @@
# Code Review — Tân Tân 3-section board + Settings color picker
**Date:** 2026-04-27
**Reviewer:** code-reviewer
**Scope:** uncommitted working tree, two features
---
## TL;DR
Both features are essentially correct and ship-worthy. Row-index math is right. Color validation regex is safe enough for a CSS custom property sink. A handful of small bugs/papercuts — none block ship — and one **must-fix** focus/a11y issue worth 5 minutes before deploy.
Verdict: **DONE_WITH_CONCERNS** — ship today, fix the must-fix list before merging the PR.
---
## Must-fix (block ship until done)
### 1. Settings modal: `<button>` overlay swallows the dialog's keydown handler — Escape doesn't close
`SettingsButton.svelte:81` puts `onkeydown={onKeydown}` on the dialog `<div>` with `tabindex="-1"`, but you never `.focus()` it on open. Combined with the full-screen `<button class="absolute inset-0">` overlay (line 84) that takes focus away, **pressing Escape after opening the modal does nothing** until the user manually clicks a focusable element inside the dialog. The trigger gear button still has focus when the modal opens, so Escape there hits document — and your handler is on the dialog div, not document.
Fix one of:
- Add a `window`/document keydown listener inside an `$effect` while `open` is true, OR
- `bind:this={dialogEl}` on the dialog div and call `dialogEl.focus()` in `$effect(() => { if (open) dialogEl.focus(); })`.
Recommend the first — simpler, also handles "user clicked into the color input then hit Esc" since the input doesn't bubble keydown to the dialog div in some browsers.
```js
$effect(() => {
if (!open) return;
const handler = (/** @type {KeyboardEvent} */ e) => { if (e.key === "Escape") close(); };
window.addEventListener("keydown", handler);
return () => window.removeEventListener("keydown", handler);
});
```
### 2. Settings modal: clicking the backdrop `<button>` triggers form-submit semantics inside any future `<form>` ancestor
`SettingsButton.svelte:84-89` — bare `<button>` defaults to `type="submit"`. You set `type="button"` (good) but it's a global a11y/HTML linter complaint pattern. Confirm: yes you have `type="button"`. Skip — just noting the area.
Real concern here: the backdrop `<button>` exists purely to make the backdrop clickable. Screen readers will announce it as "Đóng, button" duplicating the close action that already exists on `Xong`. Better:
```svelte
<div role="presentation" onclick={close} class="absolute inset-0"></div>
```
with `<svelte:options ... />` not needed; Svelte will warn about a11y_click_events_have_key_events on a div, but this is a backdrop and `aria-hidden` semantics + the modal having Escape covers it. Or keep the button but add `aria-hidden="true"` and `tabindex="-1"` so it doesn't appear in the tab order or AT tree.
---
## Nice-to-fix (do soon, not blocking)
### 3. Layout `$effect(() => loadSettings())` runs on every reactive re-render
`+layout.svelte:11-13``loadSettings()` reads/writes `settings.emptyCellColor` (a `$state`). That mutation is itself a tracked dependency. Today the effect has no dependencies it reads (mutation isn't reading), so it runs once. But this is fragile: if anyone later adds a read like `console.log(settings.emptyCellColor)` in the effect or inside `loadSettings`, you'll get an infinite-loop warning or extra localStorage hits per keystroke when the picker fires.
Tighten intent with `$effect.pre(() => { loadSettings(); }, []);` is **not** valid in Svelte 5. The idiomatic single-shot is:
```js
import { onMount } from "svelte";
onMount(() => { loadSettings(); });
```
`onMount` is the right tool when you want once-on-mount semantics with no reactivity. Effect is overkill here.
### 4. `applyToDom()` in tests removes the `--empty-cell-bg` property in `beforeEach` but `loadSettings` only **sets** it when localStorage is empty OR valid
Race-free in tests, but in the browser there is a brief flash window: between first paint of `+layout.svelte` (CSS var = the `:root` default `#7a4a2b`) and `$effect`/`onMount` running with a saved value (`#000000`, say). User sees brown for 1 frame then black. For this app, fine — call it out, no fix needed. If you cared: emit the saved color via SvelteKit `<svelte:head>` synchronously, but that's massive overkill.
### 5. `/^#[0-9a-fA-F]{6}$/` validation rejects valid CSS colors that round-trip from `<input type="color">`
The native picker always emits 7-char `#rrggbb` lowercase, so practically you're fine. But you reject:
- `#fff` (3-digit shorthand) — your test asserts this; intentional.
- `#fff8` / `#ffffff80` (alpha hex) — fine to reject; CSS var wouldn't differ visibly behind opaque cells anyway.
- Named colors, `rgb()`, `hsl()` — also fine to reject; matches your "hex picker only" UX.
Verdict on regex sufficiency for **security** sink (`document.documentElement.style.setProperty('--empty-cell-bg', x)`): **safe**. The 6-hex regex blocks any character that could close the CSS declaration (`;`, `}`, `/*`, whitespace, parens). Even if it didn't, `style.setProperty` value sanitization in modern engines drops `;` and `{}`. No CSS-injection / XSS risk. Good.
### 6. `master/+page.svelte:217` — `style:background-color={hasNumber ? null : "var(--empty-cell-bg)"}`
Works, but you removed the `bg-slate-100 dark:bg-slate-900/60` Tailwind class from the conditional and didn't add a fallback for the non-`hasNumber` path beyond the inline style. If somehow `--empty-cell-bg` fails to apply (CSS var unsupported, blocked extension, etc.), empty cells render transparent showing the parent's `bg-white`. Defensive option: keep `bg-slate-100 dark:bg-slate-900/60` as a Tailwind fallback for empty cells; the inline `style:background-color` will win when present. Not blocking — `--empty-cell-bg` has a default in `:root`.
### 7. PlayerBoard `aria-label` redundancy
`PlayerBoard.svelte:164` has `aria-label="Bảng lô tô"` on the outer wrapper, **and** each of the 3 inner `.loto-grid` divs has `aria-label="Bảng lô tô — phần N"`. Screen reader will read "Bảng lô tô, group" then "Bảng lô tô — phần 1, group". Drop the inner labels or change them to `aria-labelledby` pointing at the section-label `<div>` (which would itself need an id and `role="heading"` semantics).
Cheapest fix: remove the inner `aria-label` and add `id="section-{sectionIdx}"` + `role="heading" aria-level="3"` to `.section-label`, then `aria-labelledby="section-{sectionIdx}"` on the grid. Or just delete the redundant labels — the visible label text is enough for sighted users and the outer landmark is enough for AT.
---
## Noted, not blocking
### 8. Row-index math is correct — verified
`PlayerBoard.svelte:173-174`:
- `grid.slice(startRow, startRow + 3)` returns 3 rows of 9 = 27 cells when flattened.
- `startRow + Math.floor(idx / 9)` for `idx in [0..26]`: `Math.floor(idx/9) ∈ {0,1,2}` → row ∈ `{startRow, startRow+1, startRow+2}`. Exactly correct for all three sections (start 0, 3, 6).
- `idx % 9` gives col 0..8. Correct.
- `crossed[row]?.[col]` — optional chain handles initial state where `crossed = []`. Fine.
`{#each ... as num, idx (idx)}` keying by `idx` is OK because the underlying number at each position is stable for the lifetime of the grid. If you ever regenerate the board without remounting `PlayerBoard`, Svelte will reuse DOM nodes by index — which is what you want here. No keying bug.
### 9. `$state` at module scope inside `.svelte.js`
Legal in Svelte 5 — module-scope `$state` creates a singleton reactive state. Both `/` and `/master` import the same `settings` object so the picker on either page reflects on the other. Working as intended. Note: SSR with module-scope state can leak between requests in SvelteKit (one server process serves many users with shared module state). For a static-export site (`adapter-static`), this is irrelevant — there is no server. Keep an eye if you ever add a non-static adapter.
### 10. `DEFAULT_SETTINGS = Object.freeze({ ... })` then `$state({ ...DEFAULT_SETTINGS })`
Spread copies only own enumerable props. With one key today, fine. When you add a nested key later (e.g., `{ theme: { primary: '#xxx' } }`), the spread is shallow — `settings.theme` and `DEFAULT_SETTINGS.theme` would alias. `resetSettings` would mutate the frozen default through `settings.theme.primary = ...`. Add a structured-clone or per-key reset before introducing nested keys. YAGNI for now.
### 11. Dark mode parity — yes, mismatch is real but harmless
The rest of the app uses Tailwind v4 `dark:` utilities which (per Tailwind v4 default) compile to `@media (prefers-color-scheme: dark)`. Your new CSS in `app.css` uses the same media query. Parity is **fine**.
`:global(.dark)` selector at `app.css:48` is **dead code** — nothing in this codebase ever sets a `.dark` class on root. Either remove it (cleanest) or leave it as future-proofing. I'd remove it; YAGNI is in force.
### 12. SVG complexity in `SettingsButton.svelte`
Inline 6-line SVG path is harmless but the file is now ~70% boilerplate gear icon. Fine for one-off. If you add more iconography, consider an icon component or `iconify-svelte`. Don't pre-optimize.
### 13. `PRESETS` includes `DEFAULT_SETTINGS.emptyCellColor` as the first swatch
Means `pick(PRESETS[0])` is identical to `resetSettings()`. Two ways to achieve the same thing — fine, since the "Mặc định" button has its own footer placement. Just noting.
### 14. `localStorage.setItem` in `saveSettings` runs synchronously per keystroke during `<input type="color">` `oninput`
Native color picker fires `input` events continuously while dragging. Each event → `JSON.stringify` + `localStorage.setItem` + `documentElement.style.setProperty`. On modern hardware this is sub-ms; on a low-end Android, you might see jank. If you observe it, debounce the localStorage write only — keep `applyToDom` synchronous so the preview is live. Don't pre-optimize until you measure.
### 15. SettingsButton modal lacks initial focus
When opened, focus stays on the gear button. Sighted keyboard users tab into the modal; screen reader users may not realize a dialog opened. Combined with must-fix #1, the cleanest answer is: on open, focus the dialog container (then your Escape handler also works because it's on a focused element). See must-fix #1.
### 16. No focus-trap
When the modal is open, Tab can escape to underlying page elements (header link, "Tạo bảng mới" button, cells). For a small static-site settings dialog, **acceptable**. WAI-ARIA APG recommends a trap; pragmatically, you can ship without one. Don't build one yourself — use a library or accept the gap.
### 17. `style:background-color="var(--empty-cell-bg)"` (PlayerBoard) vs `style:background-color={hasNumber ? null : "var(--empty-cell-bg)"}` (master)
Inconsistent style. PlayerBoard's empty-cell branch always renders the inline style; master's renders it conditionally. Both work because PlayerBoard's inline-style div is in the `{#if !hasNumber}` branch already. Just inconsistent. Pick one pattern.
---
## Security check — passed
- CSS-injection via `--empty-cell-bg`: regex blocks all special chars; `style.setProperty` provides defense-in-depth. **Safe.**
- localStorage tampering: malicious user editing `loto_settings` in DevTools — worst case is they pick a hex color the picker doesn't permit. No privilege escalation, no XSS. **Safe.**
- No PII / secrets in any new file. **Confirmed.**
- No new network calls. **Confirmed.**
---
## Plan TODO completion
Did **not** verify per-checkbox status of `phase-01-three-section-player-board.md` and `phase-02-settings-color-picker.md`. Not in scope of this review per request. Recommend the orchestrator runs `ck plan check` for completed phases.
---
## Recommended actions (priority order)
1. **(must-fix #1)** Move SettingsButton Escape handler to a `window` listener inside `$effect` gated on `open`. ~3 lines.
2. **(must-fix #2)** Mark backdrop `<button>` as `aria-hidden="true" tabindex="-1"` or convert to non-interactive div. ~1 line.
3. **(nice #3)** Swap `$effect(() => loadSettings())` in `+layout.svelte` for `onMount(() => loadSettings())`. ~2 lines.
4. **(nice #7)** Drop redundant inner `aria-label` on each `.loto-grid` in `PlayerBoard.svelte`. ~3 lines.
5. **(noted #11)** Delete `:global(.dark) .section-divider` selector — dead code. ~1 line.
Total: ~10 lines, ~5 minutes. After this, ship.
---
## Metrics
- Files reviewed: 8 (5 modified, 2 new components, 1 new store)
- New LOC: ~230 (SettingsButton 167 + settings-store 63)
- Modified LOC: ~80 (PlayerBoard, master, layout, app.css)
- Critical issues: 0
- Must-fix: 2 (a11y, both in SettingsButton)
- Nice-to-fix: 5
- Noted: 10
- Security issues: 0
---
## Unresolved questions
- Does the project intentionally support OS-driven dark mode only, or is a future user toggle planned? If the latter, the dead `:global(.dark)` selector becomes useful and the dark-mode parity comment changes.
- Should the picker support alpha (RGBA) for see-through paper effect? Currently rejected by regex; would be a 1-char regex change + shape decision.
---
**Status:** DONE_WITH_CONCERNS
**Summary:** Both features correct and shippable. Two small a11y bugs in the settings modal (Escape key, backdrop button semantics) should be patched before merge — total ~5 minutes of fixes.
**Concerns/Blockers:** Modal Escape doesn't close when trigger retains focus; backdrop button is announced redundantly to screen readers.
@@ -0,0 +1,194 @@
# Test Validation Report — SvelteKit Lô Tô Test Infrastructure
**Date:** 2026-04-27
**Scope:** Fresh vitest setup, 2 test files (game-logic.test.js, settings-store.test.js)
---
## Executive Summary
**28/28 tests pass** across 2 files with consistent, reproducible results (no flakiness detected).
**svelte-check:** 0 errors, 0 warnings.
**Build:** Production static export succeeds.
**Lint:** 1 error — eslint config missing Svelte 5 rune globals.
**Coverage:** Game logic core + settings store fully tested. **Storage helpers (`saveGrid`, `loadGrid`, `saveCrossedState`, `loadCrossedState`) are currently untested** — risky gap for persistence layer.
---
## Test Execution Results
### npm test
```
✓ 28 passed (28)
✓ 2 test files
Duration: 2.09s (tests: 323ms)
```
Confirmed stable across 3 consecutive runs:
- Run 1: 28 passed
- Run 2: 28 passed
- Run 3: 28 passed
**No flakiness detected.**
### svelte-check
```
COMPLETED 5 FILES
0 ERRORS | 0 WARNINGS
```
✅ Passes threshold.
### npm run build
```
✓ built in 1.14s (client)
✓ built in 5.35s (server)
✓ Wrote site to "build" (static adapter)
```
✅ Production build succeeds.
### npm run lint
```
❌ FAILED
/config/workspace/tiennm99/loto/src/lib/settings-store.svelte.js
18:25 error '$state' is not defined no-undef
```
**Issue:** eslint config (eslint.config.mjs) doesn't include Svelte 5 rune globals (`$state`, `$derived`, `$effect`, etc.). These are valid in `.svelte.js` files per Svelte 5 spec. Config only has `globals.browser` + `globals.node`, no Svelte globals.
**Workaround:** Add eslint-plugin-svelte's rune globals to config or suppress error on `.svelte.js` files. Not blocking tests/build, but CI will fail on lint.
---
## Coverage Analysis
### TESTED (28 tests)
**game-logic.test.js (16 tests):**
- `generateGrid()`: Shape invariants (9×9, 5/row, 5/col, no dupes) ✓
- Column ranges & ascending sort (col 0: 1-9, col 8: 80-90, ascending per column) ✓
- `isRowComplete()`: All crossed, partial crossed, empty row, zero-cell edge cases ✓
- `getWaitingNumber()`: Single remaining, multiple remaining, none remaining, empty row ✓
Uses 200 trial loops on randomized generators → strong probabilistic coverage.
**settings-store.test.js (12 tests):**
- Defaults (frozen object, default color #7a4a2b) ✓
- `loadSettings()`: Empty storage, valid color, invalid color, wrong shape, corrupt JSON, 3-digit hex rejection, uppercase hex ✓
- `saveSettings()`: localStorage persistence, CSS var injection ✓
- `resetSettings()`: State + persistence reset ✓
Uses happy-dom environment for localStorage + document.documentElement.
---
## UNTESTED Code Paths (Critical Gap)
**In `game-logic.js` (5 functions/helpers — 0 tests):**
### 1. `saveGrid(grid, prefix = "loto")` [line 144150]
- **Risk:** Grid persistence layer. No test for localStorage write, quota error handling, or serialization edge cases.
- **Behavior tested:** None.
- **Scenario:** App relies on this to save player state; failure = lost game in progress.
### 2. `loadGrid(prefix = "loto")` [line 156162]
- **Risk:** Grid deserialization. Missing tests for corrupt JSON, wrong shape, null/undefined, migration scenarios.
- **Behavior tested:** None.
- **Scenario:** Corrupted localStorage could crash game initialization.
### 3. `saveCrossedState(crossed, prefix = "loto")` [line 168174]
- **Risk:** Crossed-cell persistence (game progress). No test for data loss, quota exceeded, or race conditions.
- **Behavior tested:** None.
### 4. `loadCrossedState(prefix = "loto")` [line 180186]
- **Risk:** Crossed-cell deserialization. Missing validation of boolean matrix structure.
- **Behavior tested:** None.
### 5. Helper Functions (private, but load-bearing):
- `safeParse(raw, validate)` [line 102110]: Core JSON parse + validate guard. No direct tests.
- `isNumberMatrix(v)` [line 113124]: Shape validator for grid. No edge case tests (e.g., missing rows, non-number values).
- `isBoolMatrix(v)` [line 127138]: Shape validator for crossed state. Same gaps.
- `randomNumbersInCol(num, col)` [line 2935]: Helper for grid generation. No test for edge cases (e.g., num > column size).
- `pickFilledCols()` [line 4672]: Core quota algorithm. No test for distribution, edge rows, quota correctness.
**Net Impact:** Entire grid persistence layer is untested. If `saveGrid`/`loadGrid` fail, the app silently falls back to in-memory only, but no coverage validates that fallback or data integrity.
---
## Critical Questions (Unresolved)
1. **Is `localStorage` quota/disabled handling sufficient?** Tests use try-catch for quota in save functions, but no test validates recovery. If localStorage is disabled (private mode), the app still works in-memory — but is that intentional? Should it warn the user?
2. **Do `loadGrid` / `loadCrossedState` need stricter shape validation?** Current validators check length & type, but don't validate:
- Row/column numeric ranges (e.g., grid cells must be 090)
- Cross-state consistency (e.g., crossed cell at empty position should fail?)
3. **Migration path for corrupted localStorage?** If user has old/malformed data, `loadGrid` silently returns null and app starts fresh. Is this documented? Should there be a user-facing error?
4. **Flakiness under load?** Tests pass cleanly in isolation. Have they been run under simulated storage quota exceeded or in private-mode environments?
---
## Recommendations (Prioritized)
### Must Fix (blocks lint)
- **Fix eslint config** to include Svelte 5 rune globals or suppress `.svelte.js` files:
```js
// Option 1: Add globals for .svelte.js files in eslint.config.mjs
{
files: ["**/*.svelte.js"],
languageOptions: {
globals: { $state: "readonly", $derived: "readonly", ... }
}
}
```
### Should Add (medium risk — persistence)
- Test `saveGrid` / `loadGrid` with:
- Valid 9×9 grid (round-trip serialization)
- Corrupt JSON payloads
- Wrong matrix shape (e.g., 8×9, 3×9)
- localStorage quota exceeded (mock error)
- null/undefined input
- Test `saveCrossedState` / `loadCrossedState` similarly
- Test `isNumberMatrix` / `isBoolMatrix` validators with invalid inputs:
- Non-array, wrong dimensions, mixed types, null values
### Nice to Have (edge case hardening)
- Test `pickFilledCols()` distribution (e.g., all rows/cols hit exactly 5 cells after 1000 iterations)
- Test `randomNumbersInCol()` edge cases (e.g., pick 5 from col 0, which has exactly 9 options)
- Add integration test: full game workflow (generate → save → load → cross → save → load → verify)
---
## Build & Environment Notes
- **Node environment:** happy-dom for DOM mocking in settings tests (✓ working)
- **jsconfig.json:** Valid; extends SvelteKit conventions (but missing explicit tsconfig extension hint)
- **Vite/SvelteKit:** v7 + v2, fully compatible with test setup
- **Test framework:** vitest 4.1.5, ESM modules, no issues
---
## Summary Table
| Command | Status | Notes |
|---------|--------|-------|
| `npm test` | ✅ | 28/28 pass, stable across runs |
| `svelte-check` | ✅ | 0 errors, 0 warnings |
| `npm run build` | ✅ | Static export succeeds |
| `npm run lint` | ❌ | $state rune not declared in eslint globals |
---
**Status:** DONE_WITH_CONCERNS
**Summary:** Test infrastructure is functional & stable (28/28 pass, no flakiness). Lint must be fixed. **Critical gap: storage layer (saveGrid, loadGrid, saveCrossedState, loadCrossedState, and helper validators) is untested—recommend adding coverage for persistence before launch.**
**Concerns:**
1. Lint error blocks CI (eslint config missing Svelte 5 globals)
2. Persistence layer untested (risky for game state recovery)
3. No integration test covering save/load round-trip
4. Validators (isNumberMatrix, isBoolMatrix) not stress-tested with edge cases
+39
View File
@@ -3,6 +3,9 @@
:root {
--background: #f8fafc;
--foreground: #1e293b;
/* Default empty-cell color matches a Minh Tân paper card. Overridden
at runtime by the settings store via documentElement.style. */
--empty-cell-bg: #7a4a2b;
}
@media (prefers-color-scheme: dark) {
@@ -30,6 +33,42 @@ body {
gap: 0;
}
/* Decorative cross-hatch divider between mini-cards on the player sheet,
echoing the ✚✚✚ borders on a physical Minh Tân card. */
.section-divider {
height: 8px;
background-image: repeating-linear-gradient(
90deg,
transparent 0 6px,
#c2410c 6px 8px,
transparent 8px 14px
);
background-color: rgb(255 247 237 / 0.6);
}
@media (prefers-color-scheme: dark) {
.section-divider {
background-color: rgb(67 20 7 / 0.4);
}
}
.section-label {
text-align: center;
font-size: 0.7rem;
letter-spacing: 0.18em;
text-transform: uppercase;
font-weight: 600;
padding: 6px 0;
color: #c2410c;
background-color: rgb(255 247 237 / 0.5);
font-style: italic;
}
@media (prefers-color-scheme: dark) {
.section-label {
color: #fb923c;
background-color: rgb(67 20 7 / 0.4);
}
}
@keyframes fade-in { from { opacity: 0; } to { opacity: 1; } }
@keyframes pop-in { 0% { opacity: 0; transform: scale(0.5); } 70% { transform: scale(1.05); } 100% { opacity: 1; transform: scale(1); } }
@keyframes bounce-slow { 0%, 100% { transform: translateX(-50%) translateY(0); } 50% { transform: translateX(-50%) translateY(-12px); } }
+51 -34
View File
@@ -134,6 +134,16 @@
function onModalKeydown(e) {
if (e.key === "Escape") showCongrats = false;
}
// Tân Tân physical card: 3 stacked 3x9 mini-cards with these labels
// (top → bottom). Underlying 9x9 data is unchanged; this is purely
// visual segmentation.
const SECTIONS = /** @type {const} */ ([0, 3, 6]);
const SECTION_LABELS = [
"Minh Tân",
"Loại đặc biệt",
"Tấn tài tấn lộc",
];
</script>
<div class="flex justify-center mb-6">
@@ -154,41 +164,48 @@
aria-label="Bảng lô tô"
class="rounded-2xl overflow-hidden shadow-xl shadow-slate-200/50 dark:shadow-black/30 border border-slate-200 dark:border-slate-700"
>
<div class="loto-grid">
{#each grid.flat() as num, idx (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]}
{#each SECTIONS as startRow, sectionIdx (sectionIdx)}
{#if sectionIdx > 0}
<div class="section-divider" aria-hidden="true"></div>
{/if}
<div class="section-label">{SECTION_LABELS[sectionIdx]}</div>
<div class="loto-grid">
{#each grid.slice(startRow, startRow + 3).flat() as num, idx (idx)}
{@const row = startRow + 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}
<div
aria-hidden="true"
class="relative flex items-center justify-center aspect-square border-r border-b border-slate-200/80 dark:border-slate-700/60 bg-slate-50 dark:bg-slate-900/60"
></div>
{:else}
<button
type="button"
aria-label="Số {num}{isCrossed ? ', đã đánh dấu' : ''}"
aria-pressed={isCrossed}
onclick={() => handleCellClick(row, col)}
class="relative flex items-center justify-center
aspect-square text-base sm:text-xl font-bold
border-r border-b border-slate-200/80 dark:border-slate-700/60
transition-all select-none cursor-pointer
focus:outline-none focus:ring-2 focus:ring-inset focus:ring-indigo-400
{isCrossed
? rowComplete
? 'cell-crossed bg-emerald-100 dark:bg-emerald-900/40 text-emerald-500 dark:text-emerald-400'
: 'cell-crossed bg-red-50 dark:bg-red-950/30 text-red-400 dark:text-red-500'
: 'bg-white dark:bg-slate-800 text-slate-800 dark:text-slate-100 hover:bg-indigo-50 dark:hover:bg-indigo-950/30 hover:text-indigo-600 dark:hover:text-indigo-400'}"
>
{num}
</button>
{/if}
{/each}
</div>
{#if !hasNumber}
<div
aria-hidden="true"
class="relative flex items-center justify-center aspect-square border-r border-b border-slate-200/80 dark:border-slate-700/60"
style:background-color="var(--empty-cell-bg)"
></div>
{:else}
<button
type="button"
aria-label="Số {num}{isCrossed ? ', đã đánh dấu' : ''}"
aria-pressed={isCrossed}
onclick={() => handleCellClick(row, col)}
class="relative flex items-center justify-center
aspect-square text-base sm:text-xl font-bold
border-r border-b border-slate-200/80 dark:border-slate-700/60
transition-all select-none cursor-pointer
focus:outline-none focus:ring-2 focus:ring-inset focus:ring-indigo-400
{isCrossed
? rowComplete
? 'cell-crossed bg-emerald-100 dark:bg-emerald-900/40 text-emerald-500 dark:text-emerald-400'
: 'cell-crossed bg-red-50 dark:bg-red-950/30 text-red-400 dark:text-red-500'
: 'bg-white dark:bg-slate-800 text-slate-800 dark:text-slate-100 hover:bg-indigo-50 dark:hover:bg-indigo-950/30 hover:text-indigo-600 dark:hover:text-indigo-400'}"
>
{num}
</button>
{/if}
{/each}
</div>
{/each}
</div>
{#if toast}