diff --git a/docs/codebase-summary.md b/docs/codebase-summary.md index 55ddc87..19a4001 100644 --- a/docs/codebase-summary.md +++ b/docs/codebase-summary.md @@ -25,7 +25,7 @@ src/ public/ ├── style.css # Legacy file — theme now lives in src/app.css ├── favicon.png -└── assets/ # bg.png, logo.png (unused, reserved) +└── assets/ # qr.jpg (donate VietQR) ``` ## Data flow diff --git a/plans/260427-2055-review-fixes-and-dep-prs/phase-01-dependabot-prs.md b/plans/260427-2055-review-fixes-and-dep-prs/phase-01-dependabot-prs.md new file mode 100644 index 0000000..bbcd8f2 --- /dev/null +++ b/plans/260427-2055-review-fixes-and-dep-prs/phase-01-dependabot-prs.md @@ -0,0 +1,68 @@ +# Phase 01 — Triage & Merge Dependabot PRs + +**Priority:** High +**Status:** pending +**Effort:** ~S (3 PRs, all transitive devDeps, all MERGEABLE) + +## Context + +3 open dependabot PRs against `tiennm99/sokoban` main: + +| # | Bump | Fixes | +|---|------|-------| +| 7 | postcss 8.5.3 → 8.5.12 | GHSA-qx2v-qp2m-jg93 | +| 5 | rollup 4.40.0 → 4.60.2 | GHSA-mw96-cpmx-2vgc | +| 4 | picomatch 4.0.2 → 4.0.4 | GHSA-c2c7-rcm5-vvqj + GHSA-3v7f-55p6-f55p | + +All transitive devDeps. None ship to browser. Each PR touches `package-lock.json` only. + +## Risks + +- **Lockfile rebase**: when we regenerated `package-lock.json` during the earlier rebase, our local versions may already be ≥ the dependabot target. In that case dependabot will auto-close on push, or `gh pr merge` will succeed as a no-op. Either is fine. +- **Conflict with our lockfile**: possible since we just touched it. PR mergeable status is reported as YES (snapshot 20:55), but verify per-PR before merge. +- **Build break**: rollup major bump (4.40 → 4.60) is the highest risk; verify `npm run build` after each merge. + +## Implementation Steps + +1. Snapshot current versions: + ```bash + npm ls postcss rollup picomatch 2>&1 | head -20 + ``` +2. For each PR (in order: #4 → #5 → #7, smallest blast radius first): + - `gh pr view --json mergeable,mergeStateStatus` + - If mergeable + clean: `gh pr merge --squash --auto` (or `--merge` if user prefers; squash keeps history clean for transitive bumps) + - If conflict: `gh pr comment --body "Conflicts with current lockfile after recent rebase. Closing — local has acceptable version."` then close +3. After each merge: `git pull --rebase`, then `npm run build`, expect green. +4. After all PRs: `npm audit` → confirm postcss/rollup/picomatch chains are gone. Note any remaining vulns (expected: serialize-javascript via @rollup/plugin-terser via workbox-build via vite-plugin-pwa). + +## Decision tree per PR + +``` +Is PR mergeable? +├── YES + clean +│ └── gh pr merge --squash → pull → build → next +├── MERGEABLE but lockfile-stale +│ └── Local already at target version → close PR with comment +└── CONFLICT + └── Close PR with comment; rely on next dependabot run +``` + +## Todo + +- [ ] Snapshot current versions of postcss, rollup, picomatch +- [ ] Triage PR #4 (picomatch) +- [ ] Triage PR #5 (rollup) +- [ ] Triage PR #7 (postcss) +- [ ] Pull main after each merge +- [ ] Build verification after each merge +- [ ] Final `npm audit` — note residual vulns + +## Success Criteria + +- All 3 dependabot PRs are either merged or closed-with-comment (not stuck) +- `npm run build` passes after each merge +- `npm audit` shows reduced or unchanged vuln count, never increased + +## Next + +- Phase 02: modal a11y + simplifier hygiene wins diff --git a/plans/260427-2055-review-fixes-and-dep-prs/phase-02-modal-a11y-and-hygiene.md b/plans/260427-2055-review-fixes-and-dep-prs/phase-02-modal-a11y-and-hygiene.md new file mode 100644 index 0000000..9e3e657 --- /dev/null +++ b/plans/260427-2055-review-fixes-and-dep-prs/phase-02-modal-a11y-and-hygiene.md @@ -0,0 +1,167 @@ +# Phase 02 — Modal A11y + Simplifier Hygiene Wins + +**Priority:** Medium +**Status:** pending +**Effort:** ~M (touches 4-5 files, ~30 LOC saved net) + +## Context + +- Reviewer C3: DonateModal lacks auto-focus on open and focus-restore on close. +- Simplifier #12: `.overlay`/`.dialog` CSS duplicated between GameView and DonateModal — extract to `app.css`. +- Simplifier #13: `touch-action: manipulation` + `-webkit-tap-highlight-color: transparent` repeated; could move to global `button { }` in `app.css`. +- Simplifier #1: `level-parser.js` has `key as cellKey` re-export — only used inside `board-model.js`; alias is purely cosmetic. Drop or pick one name. +- Simplifier #3: `BoardModel.isSolved` has `if (this.boxes.length === 0) return false;` — Microban guarantees ≥1 box, but the guard is also cheap to keep. Inline the early-return into the return expression for one-line clarity. +- Simplifier #4: `LevelSelectView` declared `completedCount` as `$state` but never reassigns it — should be `const`. +- Simplifier #7: `Board.svelte` redeclares `DIRS` array inside `$derived.by` — move to module scope. + +## Out of scope + +- `LevelSelectView.completedCount` not refreshing on return-from-game — separate bug, not a simplification (would belong in a future bugfix plan). +- DonateModal extraction into a store (only 2 callsites). + +## Architecture + +``` +app.css + .overlay / .dialog shared classes + + global button { touch-action; -webkit-tap-highlight-color } + +GameView.svelte remove .overlay / .dialog scoped CSS, use shared +DonateModal.svelte remove .overlay / .dialog scoped CSS, use shared + + auto-focus CLOSE on open, restore on close +MobileControls.svelte remove now-redundant button styles +AppButton.svelte remove now-redundant touch-action / tap-highlight + +level-parser.js drop `key as cellKey` re-export +board-model.js import key directly (rename usages) +board-model.js inline isSolved guard +LevelSelectView.svelte $state completedCount → const completedCount +Board.svelte hoist DIRS to module scope +``` + +## Implementation Steps + +### A. Modal a11y — DonateModal focus management + +```svelte + +... +