Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions assets/theme-v2/css/gamefoundrystudio.css
Original file line number Diff line number Diff line change
Expand Up @@ -1195,6 +1195,10 @@ body.tool-focus-mode .tool-column:last-of-type {
opacity: 1
}

.sprite-canvas-cell.is-painted {
background: var(--text)
Comment on lines +1198 to +1199

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Move painted-pixel CSS into a loaded stylesheet

The new drawing code only makes painted pixels visible through this .is-painted rule, but /toolbox/sprites/index.html links only assets/theme-v2/css/theme.css, and theme.css does not import gamefoundrystudio.css (repo-wide search also found no runtime import of this file). In the actual Sprite Creator page, clicking Pencil/Fill will add the class and update the count, but the pixel background will not change, so the core drawing behavior is effectively invisible to users.

Useful? React with 👍 / 👎.

}

@media(max-width:980px) {

.grid.cols-4,
Expand Down
90 changes: 89 additions & 1 deletion assets/toolbox/sprites/js/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
const DEFAULT_GRID_SIZE = 16;
const SUPPORTED_GRID_SIZES = Object.freeze([16, 32]);
const DRAWING_TOOLS = Object.freeze(["pencil", "eraser", "fill"]);

const editorState = {
activeTool: "pencil",
gridSize: DEFAULT_GRID_SIZE,
paintedPixels: new Set(),
};

function gridLabel(size) {
return `Sprite Creator ${size} by ${size} pixel canvas`;
Expand All @@ -9,6 +16,67 @@ function buttonForSize(size) {
return document.querySelector(`[data-sprites-grid-size="${size}"]`);
}

function pixelKey(row, column) {
return `${row}:${column}`;
}

function draftStatusText() {
const count = editorState.paintedPixels.size;
if (count === 0) {
return "Unsaved editor state: empty draft.";
}
return `Unsaved editor state: ${count} draft pixel${count === 1 ? "" : "s"} painted.`;
}

function updateDraftStatus() {
const status = document.querySelector("[data-sprites-draft-status]");
if (status) {
status.textContent = draftStatusText();
}
}

function setActiveTool(toolName) {
if (!DRAWING_TOOLS.includes(toolName)) {
return;
}
editorState.activeTool = toolName;
document.querySelectorAll("[data-sprite-tool-button]").forEach((button) => {
const isActive = button.dataset.spriteToolButton === toolName;
button.classList.toggle("primary", isActive);
button.setAttribute("aria-pressed", String(isActive));
});
const status = document.querySelector("[data-sprites-tool-status]");
if (status) {
status.textContent = `${toolName[0].toUpperCase()}${toolName.slice(1)} is active. Drawing stays in unsaved editor state for this page session only.`;
}
}

function paintCell(cell) {
const key = pixelKey(cell.dataset.spritePixelRow, cell.dataset.spritePixelColumn);
if (editorState.activeTool === "eraser") {
editorState.paintedPixels.delete(key);
cell.classList.remove("is-painted");
} else {
editorState.paintedPixels.add(key);
cell.classList.add("is-painted");
Comment on lines +57 to +61

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Expose painted state to assistive tech

For screen-reader users, painting or erasing a grid cell only changes the CSS class and draft count; the cell's accessible name/state remains just Pixel row ..., column ..., so navigating the grid cannot distinguish painted pixels from empty ones. When adding drawing state, update each affected cell's label or an ARIA state such as aria-selected/aria-pressed in the paint/erase/fill paths so non-visual users can edit the sprite.

Useful? React with 👍 / 👎.

Comment on lines +59 to +61

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor Fill when it is the active canvas tool

After the Fill button is clicked, the toolbar and status leave fill as the active tool, but clicking canvas cells still follows this generic non-eraser branch and paints only the single clicked pixel. For example, if Fill is selected and the grid is then reset by changing canvas size, the UI still says Fill is active, yet the next canvas click does not fill the grid; either Fill should not remain an active tool after its button action or this path should invoke the fill behavior.

Useful? React with 👍 / 👎.

}
updateDraftStatus();
}

function fillGrid() {
const grid = document.querySelector("[data-sprites-pixel-grid]");
if (!grid) {
return;
}
editorState.paintedPixels.clear();
grid.querySelectorAll("[data-sprite-pixel-row]").forEach((cell) => {
const key = pixelKey(cell.dataset.spritePixelRow, cell.dataset.spritePixelColumn);
editorState.paintedPixels.add(key);
cell.classList.add("is-painted");
});
updateDraftStatus();
}

function setGridSize(size) {
const grid = document.querySelector("[data-sprites-pixel-grid]");
const status = document.querySelector("[data-sprites-grid-status]");
Expand All @@ -17,6 +85,8 @@ function setGridSize(size) {
}

grid.replaceChildren();
editorState.gridSize = size;
editorState.paintedPixels.clear();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid clearing the draft when reselecting current size

When a user has painted pixels and clicks the already-active 16x16/32x32 size button, the click still calls setGridSize with the same size, and this clear drops all session pixels and rebuilds the grid. Since the selected display-mode control remains enabled, reselecting the current mode should be a no-op or preserve the draft; otherwise an accidental click loses unsaved work.

Useful? React with 👍 / 👎.

grid.dataset.spritesGridSize = String(size);
grid.setAttribute("aria-label", gridLabel(size));

Expand All @@ -26,11 +96,11 @@ function setGridSize(size) {
const cell = document.createElement("button");
cell.className = "sprite-canvas-cell";
cell.type = "button";
cell.disabled = true;
cell.setAttribute("role", "gridcell");
cell.setAttribute("aria-label", `Pixel row ${row}, column ${column}`);
cell.dataset.spritePixelRow = String(row);
cell.dataset.spritePixelColumn = String(column);
cell.addEventListener("click", () => paintCell(cell));
grid.append(cell);
}

Expand All @@ -43,6 +113,7 @@ function setGridSize(size) {
if (status) {
status.textContent = `Canvas display mode: ${size}x${size}. No pixel data is saved.`;
}
updateDraftStatus();
}

function wireGridControls() {
Expand All @@ -55,5 +126,22 @@ function wireGridControls() {
});
}

function wireDrawingTools() {
document.querySelectorAll("[data-sprite-tool-button]").forEach((button) => {
const toolName = button.dataset.spriteToolButton;
if (!DRAWING_TOOLS.includes(toolName) || button.disabled) {
return;
}
button.addEventListener("click", () => {
setActiveTool(toolName);
if (toolName === "fill") {
fillGrid();
}
});
});
}

wireGridControls();
wireDrawingTools();
setGridSize(DEFAULT_GRID_SIZE);
setActiveTool(editorState.activeTool);
21 changes: 16 additions & 5 deletions dev/tests/playwright/tools/SpritesToolShell.spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@ const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);
const repoRoot = path.resolve(__dirname, "..", "..", "..", "..");
const SPRITE_TOOLBAR_PLACEHOLDERS = [
"Pencil",
"Eraser",
"Fill",
"Line",
"Rectangle",
"Circle",
Expand Down Expand Up @@ -228,10 +225,13 @@ test("Sprite Creator shell loads with visible tool, canvas, details, and status
await expect(page.getByRole("heading", { level: 2, name: "Pixel Work Area" })).toBeVisible();
await expect(page.getByRole("heading", { level: 2, name: "Sprite Details" })).toBeVisible();
await expect(page.locator("[data-sprites-toolbar]")).toBeVisible();
await expect(page.getByRole("button", { name: "Pencil tool" })).toBeEnabled();
await expect(page.getByRole("button", { name: "Eraser tool" })).toBeEnabled();
await expect(page.getByRole("button", { name: "Fill tool" })).toBeEnabled();
for (const toolName of SPRITE_TOOLBAR_PLACEHOLDERS) {
await expect(page.getByRole("button", { name: `${toolName} tool placeholder` })).toBeDisabled();
}
await expect(page.locator("main")).toContainText("Toolbar placeholders only");
await expect(page.locator("[data-sprites-tool-status]")).toContainText("Pencil is active");
await expect(page.locator("[data-sprites-pixel-grid]")).toBeVisible();
await expect(page.locator("[data-sprites-pixel-grid] [role='gridcell']")).toHaveCount(256);
await expect(page.locator("[data-sprites-grid-status]")).toContainText("Canvas display mode: 16x16");
Expand All @@ -240,7 +240,18 @@ test("Sprite Creator shell loads with visible tool, canvas, details, and status
await expect(page.locator("[data-sprites-pixel-grid] [role='gridcell']")).toHaveCount(1024);
await expect(page.locator("[data-sprites-grid-status]")).toContainText("Canvas display mode: 32x32");
await expect(page.getByRole("button", { name: "32x32" })).toHaveAttribute("aria-pressed", "true");
await expect(page.locator("[data-sprites-shell-status]")).toContainText("Shell ready");
const firstPixel = page.locator("[data-sprites-pixel-grid] [role='gridcell']").first();
await firstPixel.click();
await expect(firstPixel).toHaveClass(/is-painted/);
await expect(page.locator("[data-sprites-draft-status]")).toContainText("1 draft pixel painted");
await page.getByRole("button", { name: "Eraser tool" }).click();
await firstPixel.click();
await expect(firstPixel).not.toHaveClass(/is-painted/);
await expect(page.locator("[data-sprites-draft-status]")).toContainText("empty draft");
await page.getByRole("button", { name: "Fill tool" }).click();
await expect(page.locator("[data-sprites-pixel-grid] .is-painted")).toHaveCount(1024);
await expect(page.locator("[data-sprites-draft-status]")).toContainText("1024 draft pixels painted");
await expect(page.locator("[data-sprites-shell-status]")).toContainText("Editor ready");
await expect(page.locator("main")).toContainText("Palette/Colors keys only");
await expect(page.locator("main")).not.toContainText(/Not implemented yet|future rebuild work|Static wireframe only|Plan sprite creation/i);
await expect(page.locator("style, [style], script:not([src])")).toHaveCount(0);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# PR_26179_CHARLIE_025-sprites-basic-drawing

Team: CHARLIE
Workflow: stacked feature workflow
Base branch: PR_26179_CHARLIE_024-sprites-canvas-grid
Canonical ZIP path: dev/workspace/zip/PR_26179_CHARLIE_025-sprites-basic-drawing_delta.zip

## Summary

Implemented basic Sprite Creator drawing for Pencil, Eraser, and Fill on the visible pixel canvas. Drawing state is page-session editor state only and is clearly labeled as unsaved. No product save, browser storage, API, DB, schema, or publishing behavior was added.

## Branch Validation

PASS

- Current branch: PR_26179_CHARLIE_025-sprites-basic-drawing
- Based on: PR_26179_CHARLIE_024-sprites-canvas-grid
- No start_of_day files changed
- No DB/API/schema files changed
- No stale PR #219-#228 code copied

## Requirement Checklist

| Requirement | Status | Notes |
| --- | --- | --- |
| Implement Pencil | PASS | Clicking a pixel paints it in unsaved editor state. |
| Implement Eraser | PASS | Clicking a painted pixel clears it. |
| Implement Fill | PASS | Fill paints the current grid. |
| Page-session editor state only | PASS | State lives in the module runtime only. |
| Clearly marked unsaved | PASS | Status copy labels unsaved editor state. |
| No product save | PASS | No save/load/publishing contract added. |
| No browser-owned authoritative product data | PASS | No browser storage or persistence added. |
| No DB/API/schema changes | PASS | Only UI/JS/CSS/test/report files changed. |

## Validation Lane Report

Commands:

```text
node --check assets/toolbox/sprites/js/index.js
node --check dev/tests/playwright/tools/SpritesToolShell.spec.mjs
git diff --check -- toolbox/sprites/index.html assets/toolbox/sprites/js/index.js assets/theme-v2/css/gamefoundrystudio.css dev/tests/playwright/tools/SpritesToolShell.spec.mjs
rg --pcre2 -n -i "localStorage|sessionStorage|indexedDB|<style|style=|<script(?![^>]+src=)|on(click|change|submit|input|load|error)=|imageDataUrl|local-mem|fake-login|MEM DB" toolbox/sprites/index.html assets/toolbox/sprites/js/index.js dev/tests/playwright/tools/SpritesToolShell.spec.mjs
npx playwright test dev/tests/playwright/tools/SpritesToolShell.spec.mjs --workers=1 --reporter=list --output=<temp>
```

Results:

- Node syntax checks: PASS
- `git diff --check`: PASS
- Guard scan: PASS, no matches
- Targeted Playwright: PASS, 1 test passed

## Manual Validation Notes

1. Open `/toolbox/sprites/index.html` from the stacked branch.
2. Click a pixel with Pencil active and confirm it paints.
3. Choose Eraser, click the painted pixel, and confirm it clears.
4. Choose Fill and confirm the visible grid fills.
5. Confirm copy says the state is unsaved editor state.

## ZIP Path

`dev/workspace/zip/PR_26179_CHARLIE_025-sprites-basic-drawing_delta.zip`
2 changes: 1 addition & 1 deletion docs_build/dev/reports/codex_changed_files.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
assets/toolbox/sprites/js/index.js
assets/theme-v2/css/gamefoundrystudio.css
dev/tests/playwright/tools/SpritesToolShell.spec.mjs
docs_build/dev/reports/PR_26179_CHARLIE_024-sprites-canvas-grid.md
docs_build/dev/reports/PR_26179_CHARLIE_025-sprites-basic-drawing.md
docs_build/dev/reports/codex_changed_files.txt
docs_build/dev/reports/codex_review.diff
Loading
Loading