chore(plans): add countdown indicator plan + review report

This commit is contained in:
tiennm99 committed 2026-04-30 19:32:09 +07:00
1 parent 7eb96808b0
commit 1e89137fa6
4 files changed
+400

No files matched your search

@@ -0,0 +1,110 @@
---
phase: 1
title: "Build AutoCountdown component"
status: completed
priority: P2
effort: "1h"
dependencies: []
---
# Phase 1: Build `AutoCountdown.svelte`
## Overview
Self-contained countdown component: shows seconds-remaining number with a
circular SVG ring that depletes from full → empty over each tick interval.
No knowledge of game state — pure visual driven by props.
## Requirements
**Functional**
- Display integer seconds remaining (e.g. `5 → 4 → 3 → 2 → 1`)
- Render circular progress ring (SVG) that depletes smoothly during the tick
- Reset to full whenever a new tick starts (parent signals via `tickKey` prop change)
- Pause/hide cleanly when `running === false`
**Non-functional**
- Smooth animation via `requestAnimationFrame` — no `setInterval` polling
- Respect `prefers-reduced-motion`: skip ring animation, show only number
- File ≤ 200 lines (KISS — keep visual logic only)
- No localStorage / no settings reads — props-driven only
## Architecture
**Props**
```js
{
running: boolean, // master is auto-calling
duration: number, // seconds per tick (settings.autoCallSpeed, 1..10)
tickKey: number, // changes on each draw — triggers ring reset
}
```
**Internal state**
- `tickStart` (`$state`, ms timestamp): set to `performance.now()` when
`tickKey` changes or `running` flips on
- `now` (`$state`, ms): updated by rAF loop while `running === true`
- `secondsRemaining` (`$derived`): `Math.ceil(duration - (now - tickStart) / 1000)`
clamped to `[0, duration]`
- `progress` (`$derived`): `(now - tickStart) / (duration * 1000)` clamped to `[0, 1]`
**rAF loop**
- Single `$effect` keyed on `running`: starts loop when true, cancels on cleanup
- Loop sets `now = performance.now()`, then `requestAnimationFrame(loop)`
- When `running === false` → no rAF active, render last frame statically
**SVG ring**
- Outer `<svg viewBox="0 0 100 100">` square, sized via wrapper class
- Background track: full circle, light stroke
- Progress arc: same circle, `stroke-dasharray = circumference`,
`stroke-dashoffset = circumference * progress` → arc shrinks as time elapses
- Rotated `-90deg` so depletion starts at 12 o'clock and goes clockwise
**Reduced motion fallback**
- Detect once via `window.matchMedia('(prefers-reduced-motion: reduce)')`
- If set → skip rAF loop; update `now` only on `tickKey` change (one frame)
- Number still updates per tick (jumps from `5 → 4 → 3 ...`); ring stays full
## Related Code Files
- Create: `src/lib/AutoCountdown.svelte`
## Implementation Steps
1. Scaffold `<script>` block with props (`running`, `duration`, `tickKey`)
2. Add `tickStart` / `now` `$state` and derived `secondsRemaining` / `progress`
3. Add `$effect` to (a) reset `tickStart` on `tickKey` or `running` rising edge
and (b) drive rAF loop while `running`
4. Wire reduced-motion check (one-time, module-scope or component-scope const)
5. Render SVG: track circle + progress arc with dynamic `stroke-dashoffset`
6. Center seconds number with `text-3xl font-black tabular-nums`
7. Match token color language: amber-50 background, sky/emerald rings —
pick **one** neutral color (slate or amber) since this isn't a number token
8. Self-test: `console.log` derived values briefly (remove before commit)
## Success Criteria
- [ ] `AutoCountdown.svelte` < 200 lines
- [ ] Renders nothing visually intrusive when `running === false`
- [ ] Ring depletes from full to empty over `duration` seconds
- [ ] Number ticks down: `duration → duration-1 → ... → 1`
- [ ] Resets cleanly when `tickKey` changes
- [ ] No memory leaks — rAF cancelled on `running=false` and component unmount
- [ ] Reduced-motion: ring static, number still updates
## Risk Assessment
- **rAF leak**: forgetting cleanup when `running` flips false → loop keeps
running invisibly. Mitigation: single `$effect` with `cancelAnimationFrame`
in cleanup; verify with DevTools Performance panel.
- **Off-by-one number flash**: `Math.ceil(0)` → 0 right at tick edge before
parent resets `tickKey`. Mitigation: clamp lower bound to 1 while
`running && progress < 1`, allow 0 only when stopped.
- **Drift from setInterval**: parent uses `setInterval`, this uses `rAF` →
microsecond drift over many ticks. Acceptable: ring is visual feedback,
not authoritative timer; parent's interval still fires on schedule.
## Notes
- Keep visual style consistent with `MasterPanel`'s "Số vừa xổ" hero —
reuse `border-[6px]`, `rounded-full`, `tabular-nums`, `font-black`
@@ -0,0 +1,130 @@
---
phase: 2
title: "Integrate into MasterPanel and verify"
status: completed
priority: P2
effort: "30m"
dependencies: [1]
---
# Phase 2: Integrate into MasterPanel and verify
## Overview
Mount `AutoCountdown` inside `MasterPanel.svelte`, drive it from the existing
auto-call `$effect`, and verify behavior in the browser. Replace (or augment)
the static "Tự động: Xs/số" caption with the live countdown.
## Requirements
**Functional**
- Countdown appears only when `settings.autoCallEnabled && autoRunning && state?.remaining.length > 0`
- Resets each time `handleDrawNext()` fires
- Disappears when host clicks "Dừng" or game runs out
**Non-functional**
- No regression to existing `setInterval` timing — countdown is decorative
- No additional re-renders on the master grid (3-cell wide affected area only)
- All existing 53 vitest tests still pass
## Architecture
**Tick key signaling**
- Add `let tickCount = $state(0)` to `MasterPanel`
- `handleDrawNext()` increments `tickCount` after `broadcastDraw`
- Pass `tickKey={tickCount}` to `AutoCountdown`
**Reset on toggle**
- When `toggleAuto()` flips `autoRunning` from false → true, also bump
`tickCount` so countdown starts immediately at full duration
- Cleanest: bump `tickCount` inside the auto-call `$effect` on the rising
edge of `autoRunning`
**Layout**
- Replace lines 220–226 (`{#if settings.autoCallEnabled && state && state.remaining.length > 0}` block)
with a flex row that contains the countdown when running, and falls back
to the static caption when not running
- Or simpler: keep the static caption, add countdown above the "Số vừa xổ"
hero only while `autoRunning` — less rewiring of existing layout
**Recommendation**: keep static caption, render `AutoCountdown` immediately
above the "Số vừa xổ" hero (line 229), gated by `autoRunning && state?.remaining.length > 0`.
Size around `w-20 h-20 sm:w-24 sm:h-24` (smaller than the hero so it doesn't
compete for visual attention).
## Related Code Files
- Modify: `src/lib/MasterPanel.svelte`
- Add import for `AutoCountdown`
- Add `tickCount` state
- Bump `tickCount` in `handleDrawNext` and on `autoRunning` rising edge
- Render `<AutoCountdown>` above the hero block
## Implementation Steps
1. Import `AutoCountdown` from `$lib/AutoCountdown.svelte`
2. Add `let tickCount = $state(0);` near `autoRunning`
3. In `handleDrawNext()`: append `tickCount++;` after `broadcastDraw(next)`
4. In the auto-call `$effect`: on the rising edge of `autoRunning` (i.e. when
the effect re-runs because `autoRunning` flipped to true), also `tickCount++`
so the ring resets immediately rather than waiting for the first interval tick
5. Add markup before line 229's hero block:
```svelte
{#if autoRunning && state && state.remaining.length > 0}
<div class="flex justify-center mb-3">
<AutoCountdown
running={autoRunning}
duration={settings.autoCallSpeed}
tickKey={tickCount}
/>
</div>
{/if}
```
6. Run `npm run lint` — fix any warnings
7. Run `npm test` — ensure all 53 tests still pass
8. Run `npm run dev`, open browser, manually verify:
- Enable master mode + auto-call in settings
- Click "Bắt đầu" → countdown appears, ring depletes, number ticks down
- Each new draw resets the countdown
- Click "Dừng" → countdown disappears
- Change `autoCallSpeed` mid-run → next tick uses new duration cleanly
- Toggle reduced-motion (DevTools → Rendering tab) → ring stays static, number still updates
## Success Criteria
- [ ] Countdown visible only during active auto-call
- [ ] Smooth ring animation on default-motion devices
- [ ] Number resets to `autoCallSpeed` value at each tick
- [ ] No console errors / no leaked rAF after stopping
- [ ] All 53 existing vitest tests pass
- [ ] `npm run lint` clean
- [ ] `npm run build` succeeds
## Risk Assessment
- **Speed change mid-run**: parent's `$effect` tears down + re-arms the
`setInterval` when `settings.autoCallSpeed` changes (already handled,
see line 116). The `AutoCountdown` `duration` prop will also flow
through, so its derived calculations re-base. Need to bump `tickCount`
on speed change too — otherwise ring shows wrong progress until next
natural tick. Mitigation: bump `tickCount++` inside the auto-call
`$effect` body so any re-arm (running, speed, enabled) resets the ring.
- **CSP impact**: SVG inline + Svelte-injected style attrs (e.g.
`stroke-dashoffset`) — already permitted by existing CSP setup
(see `scripts/inject-csp-hashes.mjs`). No new CSP work needed.
- **Build size**: one small SVG component, negligible.
## Verification Checklist (manual)
- [ ] `npm run dev` starts cleanly
- [ ] Settings → enable "Chế độ quản trò" + "Tự động xổ"
- [ ] Click "Bắt đầu", observe countdown
- [ ] Watch ≥3 ticks — verify smooth depletion + reset
- [ ] Change speed slider → ring re-bases without glitch
- [ ] DevTools → Rendering → "prefers-reduced-motion: reduce" → verify static fallback
- [ ] Stop, verify countdown unmounts; rAF count in DevTools idle
## Docs Impact
- Minor: add brief note to `docs/codebase-summary.md` about the new component
and to `docs/system-architecture.md` if it lists key UI components
@@ -0,0 +1,36 @@
---
title: "Auto-call countdown indicator"
status: completed
created: 2026-04-30
completed: 2026-04-30
slug: auto-call-countdown
---
# Auto-call countdown indicator
Show a visible countdown (number + shrinking circular ring) while the master
panel auto-calls numbers, so the host knows exactly when the next draw fires.
## Why
`MasterPanel.svelte` currently shows only a static "Tự động: Xs/số" line while
auto-call runs. Host has no per-tick feedback — UX feels dead between draws,
especially at slower speeds (5–10s).
## Phases
| # | Phase | Status |
|---|-------|--------|
| 1 | [Build `AutoCountdown.svelte`](phase-01-build-autocountdown.md) | completed |
| 2 | [Integrate into MasterPanel + verify](phase-02-integrate-and-verify.md) | completed |
## Key Files
- Create: `src/lib/AutoCountdown.svelte`
- Modify: `src/lib/MasterPanel.svelte`
## Out of Scope
- Sound/vibration on tick — voice already speaks when number is drawn
- Configurable countdown styling — match existing token visual language
- Player-side countdown (this is master-only)
@@ -0,0 +1,124 @@
---
title: Code review — AutoCountdown + MasterPanel integration
reviewer: code-reviewer
date: 2026-04-30
slug: auto-countdown
scope:
- src/lib/AutoCountdown.svelte (new)
- src/lib/MasterPanel.svelte (modified)
plan: plans/260430-1919-auto-call-countdown/
---
# Code Review — Auto-call countdown
## Summary
Implementation matches the plan. rAF cleanup, off-by-one clamp, reactivity, and
race conditions all look sound under Svelte 5 runes semantics. Two **minor**
items worth a follow-up; otherwise good to ship.
## Findings
### Critical
None.
### Major
None.
### Minor
**M1. Hidden coupling: `AutoCountdown` reset effect doesn't depend on `duration`.**
File: `src/lib/AutoCountdown.svelte:26-32`
```js
$effect(() => {
tickKey; // subscribe
if (running) {
tickStart = performance.now();
now = tickStart;
}
});
```
Reset only re-bases on `tickKey` or `running` rising edge. If a parent ever
changes `duration` without also bumping `tickKey`, the ring's progress jumps
mid-tick (because `totalMs` recomputes while `elapsedMs` is unchanged).
Today this is safe: `MasterPanel`'s auto-call `$effect` reads
`settings.autoCallSpeed`, so any speed change tears down + re-arms the effect
which bumps `tickCount` (line 129). The component contract is implicit, not
enforced.
Fix (cheap, robust): include `duration` in the reset effect:
```js
$effect(() => {
tickKey;
duration; // also re-baseline if duration changes without a tick bump
if (running) {
tickStart = performance.now();
now = tickStart;
}
});
```
**M2. `reduceMotion` keeps rAF loop alive needlessly.**
File: `src/lib/AutoCountdown.svelte:36-43`
When `reduceMotion === true`, `dashOffset` is forced to 0 (static ring), but
the rAF loop still runs at ~60Hz updating `now` purely to drive
`secondsRemaining`. A 1Hz `setInterval` (or skipping the loop and updating
`now` only on `tickKey` change as the plan suggested) would be cheaper and
match the plan spec verbatim. Not user-visible; pure CPU hygiene.
### Nits
**N1. `let tickStart = $state(performance.now())` at module top-level.**
Runs at component instantiation, not module import (Svelte 5 compiles `<script>`
into the component constructor), so there's no SSR concern despite `ssr: false`
in `+layout.js`. No action needed; flagged only because module-scope
`performance.now()` looks scary at a glance.
**N2. `tickCount++` inside `$effect` body (`MasterPanel.svelte:129`).**
Safe today because the effect doesn't *read* `tickCount`, so writing it can't
loop. This is a fragile invariant — if anyone later reads `tickCount` inside
that same effect (e.g. for logging/diagnostics), it becomes an infinite
self-trigger. A one-line comment ("not read in this effect — safe to write")
above line 129 would harden the intent.
## Concern Verification
| Concern raised | Verdict | Notes |
|---|---|---|
| rAF leak on `running=false` / unmount / rapid `tickKey` | Safe | Single `$effect` keyed on `running`; cleanup `cancelAnimationFrame(raf)` closes over the latest `raf` id (re-assigned each frame). `tickKey` change doesn't tear down rAF effect (not in deps), only re-bases `tickStart` via the other effect — correct, no churn. |
| Off-by-one number flash at tick edge | Safe | `Math.max(1, Math.ceil(duration - elapsedMs/1000))` clamps to ≥1 while `running`, and falls to `duration` (not 0) when stopped. Cannot render `0`. |
| Reactivity / runes correctness | Correct | Bare `tickKey;` reads the prop and registers as a dep (Svelte 5 tracks property reads inside effect bodies). `running` read via `if (running)` also registered. No infinite loop because no effect both reads and writes the same state. |
| Race: parent effect `tickCount++` vs `handleDrawNext` `tickCount++` | Safe | Parent's auto-call effect doesn't read `tickCount`, so its own write doesn't re-trigger it. Only `autoRunning`, `settings.autoCallEnabled`, `settings.autoCallSpeed` cause re-runs. Each re-arm bumps once; each draw bumps once; child sees a strictly increasing `tickKey`. |
| `currentColor` + `text-amber-500` SVG pattern | Idiomatic | Matches `SettingsButton.svelte:138` and Tailwind's recommended pattern. Per-`<circle>` `text-*` class sets `color`, which `stroke="currentColor"` resolves on that element. |
| `role="timer"` + `aria-live="off"` | Correct | A timer ticking once per second with `aria-live="polite"` would spam screen readers. `off` is the right call; the `aria-label` still exposes current value on focus/inspection. |
| Code style (kebab-case, JSDoc, comment density) | Matches | `@typedef Props`, JSDoc on props, comment style consistent with `MasterPanel.svelte` and `PlayerBoard.svelte`. File is 99 lines (well under 200 LOC limit). |
## Behavioral Checklist
- [x] Concurrency: no shared mutable state across components; only `tickCount` flows parent→child
- [x] Error boundaries: no exceptions thrown; `matchMedia` optional-chained for older clients
- [x] API contracts: `Props` JSDoc matches usage; `tickKey` semantics documented
- [x] Backwards compatibility: no exported interface change; component is purely additive
- [x] Input validation: not applicable — component is render-only, props are internally controlled
- [x] Auth/authz: not applicable — visual UI only
- [x] N+1 / query efficiency: not applicable — no I/O
- [x] Data leaks: not applicable — no PII surface
- [x] Fact-checked: paths and line numbers grep-verified against actual files
## Recommended Actions
1. (Minor) Add `duration` to the reset `$effect` deps in `AutoCountdown.svelte`
to make the reset contract explicit rather than relying on parent discipline.
2. (Minor) Drop the rAF loop on `reduceMotion` — replace with `setInterval(.., 1000)`
or update `now` only on `tickKey` change. Aligns code with phase-01 plan.
3. (Nit) One-line comment at `MasterPanel.svelte:129` documenting why
`tickCount++` inside the effect is loop-safe.
## Unresolved Questions
None.