From 68a808ec7ec978403e47ff2cd32fad9099cbff4b Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Mon, 27 Apr 2026 21:15:46 +0700 Subject: [PATCH] chore: drop orphan Phaser-era assets bg.png (295 KB) and logo.png (24 KB) had zero references in src, index.html, or any CSS, but matched the workbox precache glob and shipped to every install. Removing them drops the precache from 555 KB to 235 KB (-58%). Update the docs note. Also add the review-fixes plan to plans/. --- docs/codebase-summary.md | 2 +- .../phase-01-dependabot-prs.md | 68 +++++++ .../phase-02-modal-a11y-and-hygiene.md | 167 ++++++++++++++++++ .../phase-03-pwa-cleanup.md | 102 +++++++++++ .../plan.md | 58 ++++++ public/assets/bg.png | Bin 302554 -> 0 bytes public/assets/logo.png | Bin 24692 -> 0 bytes 7 files changed, 396 insertions(+), 1 deletion(-) create mode 100644 plans/260427-2055-review-fixes-and-dep-prs/phase-01-dependabot-prs.md create mode 100644 plans/260427-2055-review-fixes-and-dep-prs/phase-02-modal-a11y-and-hygiene.md create mode 100644 plans/260427-2055-review-fixes-and-dep-prs/phase-03-pwa-cleanup.md create mode 100644 plans/260427-2055-review-fixes-and-dep-prs/plan.md delete mode 100644 public/assets/bg.png delete mode 100644 public/assets/logo.png 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 + +... +