Fix Colors Symbol validation and picker grid columns - PR_26159_032-colors-symbol-grid-fix

Author: ToolboxAid

docs_build/dev/reports/colors-symbol-grid-fix-report.md

@@ -0,0 +1,76 @@
+# PR_26159_032-colors-symbol-grid-fix Report
+
+Playwright impacted: Yes
+
+## Summary
+
+Fixed the Colors picker grid so available and unavailable picker sections preserve the generated column count when swatches move between sections. Added regression coverage proving active `tools.palette-browser.swatches` validation and Colors add/remove/pin/unpin/update/clear flows work without Symbol.
+
+## Root Cause: Symbol
+
+Active `tools.palette-browser.swatches` validation is owned by `src/dev-runtime/persistence/tool-repositories/palette-workspace-repository.js` through `validatePaletteWorkspacePayload()` and `validatePaletteSwatchInput()`. That path requires `key`, `hex`, and `name`; it does not require Symbol. The blocking regression was not covered by a direct no-Symbol payload/lifecycle assertion, so stale or adjacent validation could reappear unnoticed.
+
+Exact fixed file:
+- `tests/playwright/tools/PaletteToolMockRepository.spec.mjs`
+
+Evidence:
+- Added direct `validatePaletteWorkspacePayload()` coverage for a `tools.palette-browser.swatches` payload with no Symbol field.
+- Added repository lifecycle coverage for add, update, remove, pin, unpin, and clear with no Symbol field.
+- Active Symbol scan found no active Colors `Symbol: Enter a symbol`, `data-palette-symbol`, `requireSymbol`, `swatchSymbol`, or `paletteSwatchSymbol` requirement. The only remaining hit is `tests/tools/PaletteManagerV2Baseline.test.mjs`, which is a separate Palette Manager V2 baseline test and not active Colors `palette-browser` validation.
+
+## Root Cause: Picker Grid
+
+`appendPickerGroupRows()` previously rendered rows from filtered group arrays. When one generated swatch moved from Available to Already in Project, that cell disappeared from the Available row, so rows could collapse below the selected Colors column count.
+
+Exact fixed file:
+- `toolbox/colors/colors.js`
+
+Fix:
+- `appendPickerGroupRows()` now receives the full generated swatch set, filters per section for counts, and inserts placeholder cells for positions owned by the other section.
+- Available and unavailable rows now keep `settings.colors` cells per rendered row.
+- Unavailable swatches remain true color, are not disabled, do not use a not-allowed cursor, and still block add-clicks through the existing availability guard.
+
+## Requirement Checklist
+
+| Requirement | Status | Evidence |
+|---|---|---|
+| Remove remaining active Symbol requirement from `tools.palette-browser.swatches` validation | PASS | `validatePaletteWorkspacePayload()` no-Symbol assertion in `tests/playwright/tools/PaletteToolMockRepository.spec.mjs`; targeted Symbol scan |
+| Add works without Symbol | PASS | Playwright `Palette Tool adds, updates, validates, and shows project-owned swatches` |
+| Remove works without Symbol | PASS | Repository lifecycle assertion for `removeSwatch()` |
+| Pin works without Symbol | PASS | Repository lifecycle assertion for `pinSourceSwatch()` and Playwright picker add |
+| Unpin works without Symbol | PASS | Repository lifecycle assertion removing a pinned source swatch |
+| Update works without Symbol | PASS | Playwright update flow and repository `updateSelectedSwatch()` assertion |
+| Clear works without Symbol | PASS | Playwright Clear control and repository `clearProjectData()` assertion |
+| Search active source/schemas/validators/fixtures for Symbol | PASS | `rg` scan; only out-of-scope Palette Manager V2 baseline hit remains |
+| Colors = 8 renders exactly 8 columns per picker row | PASS | New Playwright test `Palette Tool preserves eight-column picker rows when swatches are unavailable` |
+| Missing/unavailable/already-in-project swatches do not collapse grid | PASS | Placeholder cells in `toolbox/colors/colors.js`; Playwright row child-count assertions |
+| Available and unavailable sections preserve column count | PASS | Playwright asserts 8-column and 16-column available/unavailable rows |
+| Unavailable swatches keep true color and occupy cells | PASS | Playwright opacity/value assertions |
+| No red dot/not-allowed cursor for unavailable swatches | PASS | Playwright cursor assertions |
+| Unavailable swatches do not add when clicked | PASS | Playwright count remains unchanged and log reports not added |
+| Available swatches add when clicked | PASS | Playwright count increases after available picker click |
+| Preserve PR025 available top/unavailable bottom viewer | PASS | Existing Available/Already in Project labels and sections preserved |
+| Preserve current controls/options and ROYGBIV | PASS | Existing selector/ROYGBIV Playwright assertions still pass |
+| Preserve Project Swatches and Picker Swatches accordions | PASS | Existing accordion assertions still pass |
+| No console errors | PASS | `expectNoPageFailures()` across targeted Colors Playwright lane |
+
+## Validation
+
+| Lane | Status | Evidence |
+|---|---|---|
+| Changed JS syntax | PASS | `node --check toolbox/colors/colors.js`; `node --check tests/playwright/tools/PaletteToolMockRepository.spec.mjs` |
+| Targeted Colors Playwright | PASS | `npx playwright test tests/playwright/tools/PaletteToolMockRepository.spec.mjs` -> 7 passed |
+| Changed-file static validation | PASS | `git diff --check -- toolbox/colors/colors.js tests/playwright/tools/PaletteToolMockRepository.spec.mjs` |
+| Active Symbol scan | PASS | No active Colors Symbol requirement found; out-of-scope Palette Manager V2 baseline hit documented |
+
+## Skipped Lanes
+
+Full samples validation was skipped because this PR only changes the Colors tool runtime and its targeted Playwright coverage. No sample loader, shared sample framework, or production game sample path changed.
+
+Full repo Playwright was skipped because the impacted lane is the Colors/Palette Tool lane and no shared Theme V2, server API, DB adapter, or cross-tool integration behavior changed.
+
+## Artifacts
+
+- Review diff: `docs_build/dev/reports/codex_review.diff`
+- Changed files: `docs_build/dev/reports/codex_changed_files.txt`
+- ZIP: `tmp/PR_26159_032-colors-symbol-grid-fix_delta.zip`

docs_build/dev/reports/coverage_changed_js_guardrail.txt

@@ -6,7 +6,7 @@ Missing changed runtime JS files are WARN, not FAIL.
 Source: Playwright/Chromium built-in V8 coverage from the active Playwright run.
 
 Changed runtime JS files considered:
-(85%) toolbox/colors/colors.js - executed lines 1938/1938; executed functions 165/194
+(85%) toolbox/colors/colors.js - executed lines 1953/1953; executed functions 165/194
 (100%) toolbox/colors/palette-api-client.js - executed lines 19/19; executed functions 4/4
 
 Guardrail warnings:

docs_build/dev/reports/playwright_v8_coverage_report.txt

@@ -17,23 +17,21 @@ Exercised tool entry points detected:
 (64%) Theme V2 Shared JS - exercised 2 runtime JS files
 
 Changed runtime JS files covered:
-(85%) toolbox/colors/colors.js - executed lines 1938/1938; executed functions 165/194
+(85%) toolbox/colors/colors.js - executed lines 1953/1953; executed functions 165/194
 (100%) toolbox/colors/palette-api-client.js - executed lines 19/19; executed functions 4/4
 
 Files with executed line/function counts where available:
 (58%) src/engine/api/server-api-client.js - executed lines 159/159; executed functions 11/19
 (64%) assets/theme-v2/js/gamefoundry-partials.js - executed lines 525/525; executed functions 28/44
 (64%) assets/theme-v2/js/tool-display-mode.js - executed lines 201/201; executed functions 9/14
 (78%) toolbox/tool-registry-api-client.js - executed lines 148/148; executed functions 21/27
-(85%) toolbox/colors/colors.js - executed lines 1938/1938; executed functions 165/194
+(85%) toolbox/colors/colors.js - executed lines 1953/1953; executed functions 165/194
 (100%) toolbox/colors/palette-api-client.js - executed lines 19/19; executed functions 4/4
 
 Uncovered or low-coverage changed JS files:
 (100%) none - no low-coverage changed runtime JS files
 
 Changed JS files considered:
-(0%) assets/theme-v2/js/account-achievements.js - changed JS file not collected as browser runtime coverage
-(0%) tests/playwright/account/AchievementsPage.spec.mjs - changed JS file not collected as browser runtime coverage
 (0%) tests/playwright/tools/PaletteToolMockRepository.spec.mjs - changed JS file not collected as browser runtime coverage
 (85%) toolbox/colors/colors.js - changed JS file with browser V8 coverage
 (100%) toolbox/colors/palette-api-client.js - changed JS file with browser V8 coverage

tests/playwright/tools/PaletteToolMockRepository.spec.mjs

@@ -2,7 +2,8 @@ import { expect, test } from "@playwright/test";
 import {
   PALETTE_WORKSPACE_PATH,
   PALETTE_TOOL_TABLES,
-  createProjectWorkspacePaletteRepository
+  createProjectWorkspacePaletteRepository,
+  validatePaletteWorkspacePayload
 } from "../../../src/dev-runtime/persistence/tool-repositories/palette-workspace-repository.js";
 import { startRepoServer } from "../../helpers/playwrightRepoServer.mjs";
 import { clearPlaywrightStorage, installPlaywrightStorageIsolation } from "../../helpers/playwrightStorageIsolation.mjs";
@@ -145,11 +146,19 @@ async function addUserSwatch(page, swatch) {
 }
 
 async function currentPreviewHexes(page, row) {
-  return page.locator(`[data-palette-generator-preview-row='${row}'] [data-palette-generator-color]`).evaluateAll((swatches) => (
+  return page.locator(`[data-palette-picker-group='available'][data-palette-generator-preview-row='${row}'] [data-palette-generator-color]`).evaluateAll((swatches) => (
     swatches.map((swatch) => swatch.value.toUpperCase())
   ));
 }
 
+async function expectPickerRowsToHaveColumnCount(page, group, columns) {
+  const counts = await page.locator(`[data-palette-picker-group='${group}']`).evaluateAll((rows) => (
+    rows.map((row) => row.children.length)
+  ));
+  expect(counts.length).toBeGreaterThan(0);
+  expect(counts).toEqual(counts.map(() => columns));
+}
+
 async function visiblePaletteNames(page) {
   return page.locator("[data-palette-user-list] [data-palette-swatch-key]").evaluateAll((swatches) => (
     swatches.map((swatch) => swatch.dataset.paletteSwatchName)
@@ -218,6 +227,39 @@ test("Palette repository owns project swatches and protects invalid payloads", a
   expect(repository.findSwatch("reference-red")).toMatchObject({ name: "Reference Red", tags: ["batch"] });
   expect(repository.findSwatch("reference-green")).toBeNull();
 
+  const noSymbolPayloadValidation = validatePaletteWorkspacePayload({
+    tools: {
+      "palette-browser": {
+        swatches: [
+          { key: "symbol-free-swatch", hex: "#224466", name: "Symbol Free Swatch", tags: [] }
+        ]
+      }
+    }
+  });
+  expect(noSymbolPayloadValidation.valid).toBe(true);
+  expect(noSymbolPayloadValidation.normalized.tools["palette-browser"].swatches[0]).toMatchObject({
+    hex: "#224466",
+    key: "symbol-free-swatch",
+    name: "Symbol Free Swatch"
+  });
+
+  const lifecycleRepository = createProjectWorkspacePaletteRepository({ sourcePaletteRows });
+  const lifecycleAdd = lifecycleRepository.addSwatch({ hex: "#224466", name: "Lifecycle Blue" });
+  expect(lifecycleAdd.ok).toBe(true);
+  const lifecycleKey = lifecycleAdd.snapshot.selectedSwatch.key;
+  expect(lifecycleRepository.updateSelectedSwatch({ hex: "#224477", name: "Lifecycle Updated" }).ok).toBe(true);
+  expect(lifecycleRepository.findSwatch(lifecycleKey)).toMatchObject({ hex: "#224477", name: "Lifecycle Updated" });
+  expect(lifecycleRepository.removeSwatch(lifecycleKey).ok).toBe(true);
+  expect(lifecycleRepository.findSwatch(lifecycleKey)).toBeNull();
+  const lifecycleSourceSwatch = lifecycleRepository.listSourceSwatches({ sourceId: "reference" })[0];
+  expect(lifecycleRepository.pinSourceSwatch(lifecycleSourceSwatch).ok).toBe(true);
+  expect(lifecycleRepository.findSwatch(lifecycleSourceSwatch.key)).toMatchObject({ name: lifecycleSourceSwatch.name });
+  expect(lifecycleRepository.removeSwatch(lifecycleSourceSwatch.key).ok).toBe(true);
+  expect(lifecycleRepository.findSwatch(lifecycleSourceSwatch.key)).toBeNull();
+  lifecycleRepository.addSwatch({ hex: "#667788", name: "Clearable Swatch" });
+  lifecycleRepository.clearProjectData();
+  expect(lifecycleRepository.getTables().palette_colors).toEqual([]);
+
   const invalidPayload = {
     tools: {
       "palette-browser": {
@@ -507,6 +549,44 @@ test("Palette Tool renders curated swatch selector controls and live preview", a
   }
 });
 
+test("Palette Tool preserves eight-column picker rows when swatches are unavailable", async ({ page }) => {
+  const failures = await openRepoPage(page, "/toolbox/colors/index.html");
+
+  try {
+    await page.locator("[data-palette-generator-colors]").selectOption("8");
+    await page.locator("[data-palette-generator-steps]").selectOption("2");
+    await expectPickerRowsToHaveColumnCount(page, "available", 8);
+    const firstAvailableEightColumnSwatch = page.locator("[data-palette-generator-swatch][data-palette-generator-availability='available']").first();
+    const eightColumnHex = await firstAvailableEightColumnSwatch.getAttribute("data-palette-generator-hex");
+    await firstAvailableEightColumnSwatch.click();
+    await expect(page.locator("[data-palette-count]")).toHaveText("1");
+
+    await expectPickerRowsToHaveColumnCount(page, "available", 8);
+    await expectPickerRowsToHaveColumnCount(page, "unavailable", 8);
+    const unavailableEightColumnSwatch = page.locator(`[data-palette-generator-swatch][data-palette-generator-hex='${eightColumnHex}'][data-palette-generator-availability='unavailable']`);
+    await expect(unavailableEightColumnSwatch).toHaveAttribute("data-palette-generator-unavailable-reason", "Already in Project");
+    const unavailableEightColumnVisual = await unavailableEightColumnSwatch.evaluate((swatch) => ({
+      cursor: getComputedStyle(swatch).cursor,
+      opacity: getComputedStyle(swatch).opacity,
+      value: swatch.querySelector("input[type='color']").value
+    }));
+    expect(unavailableEightColumnVisual.cursor).not.toBe("not-allowed");
+    expect(unavailableEightColumnVisual.opacity).toBe("1");
+    expect(unavailableEightColumnVisual.value.toUpperCase()).toBe((eightColumnHex || "").slice(0, 7).toUpperCase());
+
+    await unavailableEightColumnSwatch.click();
+    await expect(page.locator("[data-palette-count]")).toHaveText("1");
+    await expect(page.locator("[data-palette-log]")).toContainText("Already in Project picker swatch was not added");
+    await page.locator("[data-palette-generator-swatch][data-palette-generator-availability='available']").first().click();
+    await expect(page.locator("[data-palette-count]")).toHaveText("2");
+
+    expectNoPageFailures(failures);
+  } finally {
+    await workspaceV2CoverageReporter.stop(page);
+    await failures.server.close();
+  }
+});
+
 test("Palette Tool generated grid swatches can be selected, pinned, and refreshed", async ({ page }) => {
   test.setTimeout(120000);
   const failures = await openRepoPage(page, "/toolbox/colors/index.html");
@@ -595,6 +675,8 @@ test("Palette Tool generated grid swatches can be selected, pinned, and refreshe
     await expect(page.locator("[data-palette-picker-group-label='available']")).toHaveText("Available Picker Swatches (63)");
     await expect(page.locator("[data-palette-picker-group-label='unavailable']")).toHaveText("Already in Project (1)");
     await expect(page.locator("[data-palette-generator-swatch][data-palette-generator-unavailable='true']")).toHaveCount(1);
+    await expectPickerRowsToHaveColumnCount(page, "available", 16);
+    await expectPickerRowsToHaveColumnCount(page, "unavailable", 16);
     const unavailableVisual = await unavailablePickerSwatch.evaluate((swatch) => ({
       cursor: getComputedStyle(swatch).cursor,
       opacity: getComputedStyle(swatch).opacity,

toolbox/colors/colors.js

@@ -989,7 +989,20 @@ function createPickerGroupLabel(group, count) {
   return label;
 }
 
-function appendPickerGroupRows(fragment, group, items, settings) {
+function createPickerGridPlaceholder(row, column) {
+  const placeholder = document.createElement("span");
+  placeholder.className = "palette-generator-preview-swatch";
+  placeholder.dataset.paletteGeneratorPlaceholder = "true";
+  placeholder.dataset.paletteGeneratorRow = String(row);
+  placeholder.dataset.paletteGeneratorColumn = String(column);
+  placeholder.setAttribute("aria-hidden", "true");
+  return placeholder;
+}
+
+function appendPickerGroupRows(fragment, group, allSwatches, settings) {
+  const items = allSwatches.filter((item) => (
+    group === "available" ? item.available : !item.available
+  ));
   fragment.append(createPickerGroupLabel(group, items.length));
   if (!items.length) {
     const message = document.createElement("p");
@@ -1004,28 +1017,33 @@ function appendPickerGroupRows(fragment, group, items, settings) {
 
   const rows = new Map();
   items.forEach((item) => {
-    const rowItems = rows.get(item.row) || [];
-    rowItems.push(item);
+    const rowItems = rows.get(item.row) || new Map();
+    rowItems.set(item.column, item);
     rows.set(item.row, rowItems);
   });
-  [...rows.entries()].forEach(([row, rowItems]) => {
-    const rowElement = document.createElement("div");
-    rowElement.className = "palette-generator-preview-row";
-    rowElement.setAttribute("role", "row");
-    rowElement.dataset.paletteGeneratorPreviewRow = String(row);
-    rowElement.dataset.palettePickerGroup = group;
-    rowItems.forEach((item) => {
-      rowElement.append(createGeneratorPreviewInput(item.hex, settings.paletteType.name, item.row, item.column, settings, {
-        available: item.available,
-        pinned: Boolean(item.pinnedSwatch),
-        pinnedSwatch: item.pinnedSwatch,
-        selected: item.selected,
-        unavailableReason: item.unavailableReason,
-        pickerSettings: currentPickerSettings(settings)
-      }));
+  [...rows.entries()]
+    .sort(([left], [right]) => left - right)
+    .forEach(([row, rowItems]) => {
+      const rowElement = document.createElement("div");
+      rowElement.className = "palette-generator-preview-row";
+      rowElement.setAttribute("role", "row");
+      rowElement.dataset.paletteGeneratorPreviewRow = String(row);
+      rowElement.dataset.palettePickerGroup = group;
+      for (let column = 0; column < settings.colors; column += 1) {
+        const item = rowItems.get(column);
+        rowElement.append(item
+          ? createGeneratorPreviewInput(item.hex, settings.paletteType.name, item.row, item.column, settings, {
+            available: item.available,
+            pinned: Boolean(item.pinnedSwatch),
+            pinnedSwatch: item.pinnedSwatch,
+            selected: item.selected,
+            unavailableReason: item.unavailableReason,
+            pickerSettings: currentPickerSettings(settings)
+          })
+          : createPickerGridPlaceholder(row, column));
+      }
+      fragment.append(rowElement);
     });
-    fragment.append(rowElement);
-  });
 }
 
 function paletteGeneratorSummary(settings) {
@@ -1076,11 +1094,9 @@ function renderPaletteGeneratorPreview(action = "Palette generator preview updat
     }
   }
 
-  const availableSwatches = allSwatches.filter((swatch) => swatch.available);
-  const unavailableSwatches = allSwatches.filter((swatch) => !swatch.available);
   const fragment = document.createDocumentFragment();
-  appendPickerGroupRows(fragment, "available", availableSwatches, settings);
-  appendPickerGroupRows(fragment, "unavailable", unavailableSwatches, settings);
+  appendPickerGroupRows(fragment, "available", allSwatches, settings);
+  appendPickerGroupRows(fragment, "unavailable", allSwatches, settings);
 
   elements.generatorPreview.replaceChildren(fragment);
   setText(elements.generatorPreviewStatus, paletteGeneratorSummary(settings));