Add Sticker Studio first playable and deploy workflow#2
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughIntroduces Sticker Studio, a complete browser-based puzzle game vertical slice where players place stickers on pages. Includes game engine with solver validation, campaign and daily level data, application UI with state management, comprehensive styling, GitHub Pages deployment automation, and test coverage. ChangesSticker Studio Full Implementation
Sequence DiagramsequenceDiagram
actor User
participant App as App (app.js)
participant Engine as Engine (engine.js)
participant Data as Data (data.js)
participant DOM as DOM & localStorage
User->>App: Click target or sheet
activate App
App->>Engine: evaluateSheet() or evaluatePlaceAction()
activate Engine
Engine->>Data: Retrieve level/target/sheet metadata
Data-->>Engine: Target/sheet details, prereqs
Engine->>Engine: Check constraints (clip, prereqs, state)
Engine-->>App: {ok, message} or error
deactivate Engine
alt Action valid
App->>Engine: applyAction() to update state
activate Engine
Engine->>Engine: Advance sheet index, update placed targets, clips
Engine->>Engine: Check for completion or failure
Engine-->>App: Updated state + outcome (pending/jam/cleared)
deactivate Engine
App->>App: syncSelectionAfterState()
App->>DOM: Update DOM, set document.title
App->>DOM: Save progress to localStorage
alt Level cleared
App->>Engine: solveLevel() validates solvability
activate Engine
Engine->>Engine: BFS solver traces final path
Engine-->>App: Solution sequence for future reference
deactivate Engine
App->>App: Show modal, transition to next/replay
end
else Action invalid
App->>App: Show error message
end
deactivate App
User->>DOM: Sees updated board and modal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Sticker Studio preview deployed. URL: https://NightProgrammers.github.io/games/previews/sticker-studio/pr-2/ If this is the first deployment, enable GitHub Pages once in repository settings and point it at the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
sticker-studio/src/app.js (1)
109-134: 💤 Low valueMinor: clamp
levelIndexto a valid range, not just an upper bound.
Math.min(levelIndex, HANDCRAFTED_LEVELS.length - 1)only protects against overshoot; a negativelevelIndex(e.g. from a future caller, or fromNumber(target.dataset.index)on malformed markup at Line 827) would indexHANDCRAFTED_LEVELS[-1]→undefined→ crash increateRuntime. A symmetric clamp keepsloadLeveldefensible against any future caller.♻️ Suggested clamp
- appState.levelIndex = Math.min(levelIndex, HANDCRAFTED_LEVELS.length - 1); + appState.levelIndex = Math.max( + 0, + Math.min(levelIndex, HANDCRAFTED_LEVELS.length - 1) + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sticker-studio/src/app.js` around lines 109 - 134, The current clamp for levelIndex in loadLevel only uses Math.min and allows negative indices, which can make HANDCRAFTED_LEVELS[appState.levelIndex] undefined and crash later in createRuntime; change the assignment that sets appState.levelIndex (currently using Math.min(levelIndex, HANDCRAFTED_LEVELS.length - 1)) to a symmetric clamp that also enforces a minimum of 0 (e.g. compute a clampedIndex = Math.max(0, Math.min(levelIndex, HANDCRAFTED_LEVELS.length - 1)) and assign it to appState.levelIndex) so all callers of loadLevel cannot produce out-of-range indexes when HANDCRAFTED_LEVELS is accessed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/sticker-studio-preview.yml:
- Around line 39-47: The workflow runs "Verify Sticker Studio" steps (step name
"Verify Sticker Studio") without installing dependencies first, causing failures
on clean runners; add an install step (e.g., run: npm ci with working-directory:
sticker-studio) immediately before each "run: npm run verify" invocation
(including the other occurrence around lines 150-157) so node_modules are
present when the verify script runs.
In `@sticker-studio/src/app.js`:
- Around line 55-57: Wrap the saveProgress function body in a try/catch so any
exceptions from localStorage.setItem (e.g., QuotaExceededError or platform write
failures) are swallowed or logged without throwing; inside saveProgress (which
currently calls localStorage.setItem(STORAGE_KEY,
JSON.stringify(appState.progress))) try JSON.stringify and setItem, and in catch
handle the error (e.g., console.warn or a no-op) so calls from recordWin won’t
propagate exceptions and abort the UI.
- Around line 840-846: The change handler bound on root (root.addEventListener
... data-action='daily-seed') updates appState.dailySeed but doesn't refresh the
UI; modify that handler to, after setting appState.dailySeed (or using
dailySeedFromDate()), call the same refresh logic used when opening daily:
invoke loadLevel(...) with the new seed and then call render() (or the existing
function that re-renders the active page) so the active daily page (title,
validation, art, reward) updates immediately when the date input changes.
In `@sticker-studio/styles.css`:
- Around line 214-263: Add a visible keyboard focus indicator by defining
:focus-visible styles for .rail-chip, .ghost-button, .primary-button,
.target-node, and .sheet-card (use the :focus-visible pseudo-class so mouse
focus is unaffected). Implement a high-contrast, rounded indicator that respects
existing border-radius (e.g., an outline or outer box-shadow with a 2–4px
thickness and accessible color from your palette) and ensure it doesn't change
layout (use outline or box-shadow, not borders). Apply the rule to those
selectors (e.g., ".rail-chip:focus-visible, .ghost-button:focus-visible,
.primary-button:focus-visible, .target-node:focus-visible,
.sheet-card:focus-visible") so keyboard users can clearly see which control has
focus.
---
Nitpick comments:
In `@sticker-studio/src/app.js`:
- Around line 109-134: The current clamp for levelIndex in loadLevel only uses
Math.min and allows negative indices, which can make
HANDCRAFTED_LEVELS[appState.levelIndex] undefined and crash later in
createRuntime; change the assignment that sets appState.levelIndex (currently
using Math.min(levelIndex, HANDCRAFTED_LEVELS.length - 1)) to a symmetric clamp
that also enforces a minimum of 0 (e.g. compute a clampedIndex = Math.max(0,
Math.min(levelIndex, HANDCRAFTED_LEVELS.length - 1)) and assign it to
appState.levelIndex) so all callers of loadLevel cannot produce out-of-range
indexes when HANDCRAFTED_LEVELS is accessed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d6057a75-2c2a-46a7-83aa-8cb90eef7322
📒 Files selected for processing (10)
.github/workflows/sticker-studio-preview.ymlsticker-studio/README.mdsticker-studio/index.htmlsticker-studio/package.jsonsticker-studio/src/app.jssticker-studio/src/daily.jssticker-studio/src/data.jssticker-studio/src/engine.jssticker-studio/styles.csssticker-studio/tests/engine.test.js
| - name: Set up Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 22 | ||
|
|
||
| - name: Verify Sticker Studio | ||
| working-directory: sticker-studio | ||
| run: npm run verify | ||
|
|
There was a problem hiding this comment.
Install dependencies before running verification.
Line 44 and Line 155 run npm run verify without an install step. On clean runners, this can fail due to missing node_modules, breaking both preview and stable deploys.
Suggested fix
- name: Set up Node.js
uses: actions/setup-node@v4
with:
node-version: 22
+ cache: npm
+ cache-dependency-path: sticker-studio/package-lock.json
+
+ - name: Install dependencies
+ working-directory: sticker-studio
+ run: npm ci
- name: Verify Sticker Studio
working-directory: sticker-studio
run: npm run verify
@@
- name: Set up Node.js
uses: actions/setup-node@v4
with:
node-version: 22
+ cache: npm
+ cache-dependency-path: sticker-studio/package-lock.json
+
+ - name: Install dependencies
+ working-directory: sticker-studio
+ run: npm ci
- name: Verify Sticker Studio
working-directory: sticker-studio
run: npm run verifyAlso applies to: 150-157
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/sticker-studio-preview.yml around lines 39 - 47, The
workflow runs "Verify Sticker Studio" steps (step name "Verify Sticker Studio")
without installing dependencies first, causing failures on clean runners; add an
install step (e.g., run: npm ci with working-directory: sticker-studio)
immediately before each "run: npm run verify" invocation (including the other
occurrence around lines 150-157) so node_modules are present when the verify
script runs.
| function saveProgress() { | ||
| localStorage.setItem(STORAGE_KEY, JSON.stringify(appState.progress)); | ||
| } |
There was a problem hiding this comment.
Wrap saveProgress in a try/catch.
localStorage.setItem can throw QuotaExceededError, and Safari's "Prevent cross-site tracking"/private mode and some embedded WebViews throw on every write. Since loadProgress already tolerates failures, saveProgress (called from recordWin after every clear) should match — otherwise a single failing write surfaces as an uncaught exception inside the click handler and aborts the win-modal render.
🛡️ Suggested defensive write
function saveProgress() {
- localStorage.setItem(STORAGE_KEY, JSON.stringify(appState.progress));
+ try {
+ localStorage.setItem(STORAGE_KEY, JSON.stringify(appState.progress));
+ } catch {
+ // Storage may be unavailable (private mode, quota); progress is still
+ // tracked in-memory for the current session.
+ }
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sticker-studio/src/app.js` around lines 55 - 57, Wrap the saveProgress
function body in a try/catch so any exceptions from localStorage.setItem (e.g.,
QuotaExceededError or platform write failures) are swallowed or logged without
throwing; inside saveProgress (which currently calls
localStorage.setItem(STORAGE_KEY, JSON.stringify(appState.progress))) try
JSON.stringify and setItem, and in catch handle the error (e.g., console.warn or
a no-op) so calls from recordWin won’t propagate exceptions and abort the UI.
| root.addEventListener("change", (event) => { | ||
| const target = event.target.closest("[data-action='daily-seed']"); | ||
| if (!target) { | ||
| return; | ||
| } | ||
| appState.dailySeed = target.value || dailySeedFromDate(); | ||
| }); |
There was a problem hiding this comment.
Daily seed change does not refresh the active daily page.
When the user is already in daily mode and edits the date input, this handler updates appState.dailySeed but neither calls loadLevel(...) nor render(). The foil page on screen continues to show the previous seed (level title, validation, page art, reward detail) until the user explicitly re-opens the daily, which is confusing because the seeded preview UX implies the page tracks the picker.
🪄 Suggested behavior
root.addEventListener("change", (event) => {
const target = event.target.closest("[data-action='daily-seed']");
if (!target) {
return;
}
- appState.dailySeed = target.value || dailySeedFromDate();
+ const nextSeed = target.value || dailySeedFromDate();
+ appState.dailySeed = nextSeed;
+ if (appState.mode === "daily") {
+ loadLevel({ mode: "daily", dailySeed: nextSeed, showStart: false });
+ }
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| root.addEventListener("change", (event) => { | |
| const target = event.target.closest("[data-action='daily-seed']"); | |
| if (!target) { | |
| return; | |
| } | |
| appState.dailySeed = target.value || dailySeedFromDate(); | |
| }); | |
| root.addEventListener("change", (event) => { | |
| const target = event.target.closest("[data-action='daily-seed']"); | |
| if (!target) { | |
| return; | |
| } | |
| const nextSeed = target.value || dailySeedFromDate(); | |
| appState.dailySeed = nextSeed; | |
| if (appState.mode === "daily") { | |
| loadLevel({ mode: "daily", dailySeed: nextSeed, showStart: false }); | |
| } | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sticker-studio/src/app.js` around lines 840 - 846, The change handler bound
on root (root.addEventListener ... data-action='daily-seed') updates
appState.dailySeed but doesn't refresh the UI; modify that handler to, after
setting appState.dailySeed (or using dailySeedFromDate()), call the same refresh
logic used when opening daily: invoke loadLevel(...) with the new seed and then
call render() (or the existing function that re-renders the active page) so the
active daily page (title, validation, art, reward) updates immediately when the
date input changes.
| .rail-chip, | ||
| .ghost-button, | ||
| .primary-button { | ||
| border: 0; | ||
| border-radius: 999px; | ||
| transition: transform 160ms ease, box-shadow 160ms ease, background-color 160ms ease; | ||
| } | ||
|
|
||
| .rail-chip { | ||
| min-height: 44px; | ||
| background: rgba(255, 251, 247, 0.84); | ||
| border: 1px solid rgba(98, 74, 48, 0.14); | ||
| color: var(--ink); | ||
| font-weight: 700; | ||
| } | ||
|
|
||
| .rail-chip.is-active, | ||
| .rail-chip:hover, | ||
| .ghost-button:hover, | ||
| .primary-button:hover, | ||
| .sheet-card:hover, | ||
| .target-node:hover { | ||
| transform: translateY(-1px); | ||
| } | ||
|
|
||
| .rail-chip.is-cleared { | ||
| box-shadow: inset 0 0 0 1px rgba(107, 150, 110, 0.36); | ||
| } | ||
|
|
||
| .rail-chip.is-locked { | ||
| opacity: 0.45; | ||
| } | ||
|
|
||
| .ghost-button, | ||
| .primary-button { | ||
| padding: 12px 18px; | ||
| font-weight: 700; | ||
| } | ||
|
|
||
| .ghost-button { | ||
| background: rgba(255, 251, 247, 0.86); | ||
| border: 1px solid rgba(98, 74, 48, 0.14); | ||
| color: var(--ink); | ||
| } | ||
|
|
||
| .primary-button { | ||
| color: white; | ||
| background: linear-gradient(135deg, #c97059, #af5847); | ||
| box-shadow: 0 14px 28px rgba(175, 88, 71, 0.24); | ||
| } |
There was a problem hiding this comment.
Add a visible :focus-visible indicator for the custom buttons.
.rail-chip, .ghost-button, .primary-button (and .target-node at Line 294, .sheet-card at Line 412) all set border: 0 / background: none and the stylesheet never defines a :focus-visible rule, so the browser's default focus outline is also suppressed on most surfaces. Since the entire UI is keyboard-driven through these buttons (level rail, target picking, park/undo/restart, modal actions), keyboard users currently have no way to see which control has focus. This is an accessibility blocker for keyboard navigation.
♿ Suggested minimal focus-visible rule
+.rail-chip:focus-visible,
+.ghost-button:focus-visible,
+.primary-button:focus-visible,
+.sheet-card:focus-visible,
+.target-node:focus-visible {
+ outline: 3px solid var(--accent-dark);
+ outline-offset: 2px;
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .rail-chip, | |
| .ghost-button, | |
| .primary-button { | |
| border: 0; | |
| border-radius: 999px; | |
| transition: transform 160ms ease, box-shadow 160ms ease, background-color 160ms ease; | |
| } | |
| .rail-chip { | |
| min-height: 44px; | |
| background: rgba(255, 251, 247, 0.84); | |
| border: 1px solid rgba(98, 74, 48, 0.14); | |
| color: var(--ink); | |
| font-weight: 700; | |
| } | |
| .rail-chip.is-active, | |
| .rail-chip:hover, | |
| .ghost-button:hover, | |
| .primary-button:hover, | |
| .sheet-card:hover, | |
| .target-node:hover { | |
| transform: translateY(-1px); | |
| } | |
| .rail-chip.is-cleared { | |
| box-shadow: inset 0 0 0 1px rgba(107, 150, 110, 0.36); | |
| } | |
| .rail-chip.is-locked { | |
| opacity: 0.45; | |
| } | |
| .ghost-button, | |
| .primary-button { | |
| padding: 12px 18px; | |
| font-weight: 700; | |
| } | |
| .ghost-button { | |
| background: rgba(255, 251, 247, 0.86); | |
| border: 1px solid rgba(98, 74, 48, 0.14); | |
| color: var(--ink); | |
| } | |
| .primary-button { | |
| color: white; | |
| background: linear-gradient(135deg, #c97059, #af5847); | |
| box-shadow: 0 14px 28px rgba(175, 88, 71, 0.24); | |
| } | |
| .rail-chip, | |
| .ghost-button, | |
| .primary-button { | |
| border: 0; | |
| border-radius: 999px; | |
| transition: transform 160ms ease, box-shadow 160ms ease, background-color 160ms ease; | |
| } | |
| .rail-chip { | |
| min-height: 44px; | |
| background: rgba(255, 251, 247, 0.84); | |
| border: 1px solid rgba(98, 74, 48, 0.14); | |
| color: var(--ink); | |
| font-weight: 700; | |
| } | |
| .rail-chip.is-active, | |
| .rail-chip:hover, | |
| .ghost-button:hover, | |
| .primary-button:hover, | |
| .sheet-card:hover, | |
| .target-node:hover { | |
| transform: translateY(-1px); | |
| } | |
| .rail-chip.is-cleared { | |
| box-shadow: inset 0 0 0 1px rgba(107, 150, 110, 0.36); | |
| } | |
| .rail-chip.is-locked { | |
| opacity: 0.45; | |
| } | |
| .ghost-button, | |
| .primary-button { | |
| padding: 12px 18px; | |
| font-weight: 700; | |
| } | |
| .ghost-button { | |
| background: rgba(255, 251, 247, 0.86); | |
| border: 1px solid rgba(98, 74, 48, 0.14); | |
| color: var(--ink); | |
| } | |
| .primary-button { | |
| color: white; | |
| background: linear-gradient(135deg, `#c97059`, `#af5847`); | |
| box-shadow: 0 14px 28px rgba(175, 88, 71, 0.24); | |
| } | |
| .rail-chip:focus-visible, | |
| .ghost-button:focus-visible, | |
| .primary-button:focus-visible, | |
| .sheet-card:focus-visible, | |
| .target-node:focus-visible { | |
| outline: 3px solid var(--accent-dark); | |
| outline-offset: 2px; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sticker-studio/styles.css` around lines 214 - 263, Add a visible keyboard
focus indicator by defining :focus-visible styles for .rail-chip, .ghost-button,
.primary-button, .target-node, and .sheet-card (use the :focus-visible
pseudo-class so mouse focus is unaffected). Implement a high-contrast, rounded
indicator that respects existing border-radius (e.g., an outline or outer
box-shadow with a 2–4px thickness and accessible color from your palette) and
ensure it doesn't change layout (use outline or box-shadow, not borders). Apply
the rule to those selectors (e.g., ".rail-chip:focus-visible,
.ghost-button:focus-visible, .primary-button:focus-visible,
.target-node:focus-visible, .sheet-card:focus-visible") so keyboard users can
clearly see which control has focus.
1 participant
Summary
Validation
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests