Skip to content

Commit ec5b958

Browse files
committed
Fix Colors picker selection metadata layout and disabled swatches - PR_26159_026-colors-picker-metadata-layout
1 parent 1ad83b4 commit ec5b958

5 files changed

Lines changed: 468 additions & 194 deletions

File tree

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
# PR_26159_026 Colors Picker Metadata Layout Report
2+
3+
## Executive Summary
4+
PASS. The Colors picker regression is fixed and packaged. Picker swatches are selectable, selected/pinned swatches retain the generator settings that created them, metadata tooltips are present for generated and selected swatches, and the requested layout/control cleanup is implemented without page-local CSS, inline scripts, inline styles, or inline handlers.
5+
6+
## Requirement Checklist
7+
| Requirement | Status | Evidence |
8+
| --- | --- | --- |
9+
| Read project instructions first | PASS | Read `docs_build/dev/PROJECT_INSTRUCTIONS.md` before implementation. |
10+
| Use PR_26159_024 and PR_26159_025 as base context | PASS | Changes build on existing Colors curated palette/generator and restored selection/tag/sort behavior in `toolbox/colors/colors.js`, `toolbox/colors/index.html`, and `tests/playwright/tools/PaletteToolMockRepository.spec.mjs`. |
11+
| Every visible picker swatch selectable unless documented disabled reason exists | PASS | `renderPaletteGeneratorPreview` renders enabled picker buttons; Playwright clicks all visible picker swatches and verifies selection count. |
12+
| Remove red/not-allowed mouse behavior from selectable picker swatches | PASS | No live `not-allowed` cursor code remains in Colors files; Playwright asserts picker swatch cursors are not `not-allowed`. |
13+
| Store picker settings when picker swatch is selected or pinned | PASS | `currentPickerSettings`, `generatedSwatchFromTile`, and repository `clonePickerSettings` preserve Theme Collection, Palette Type, Variant, Colors, Steps, Contrast, Saturation, Hue Shift, Sort field/direction, Active tags, and Swatch size. |
14+
| Add way to return to selected/pinned swatch picker settings | PASS | Added `data-palette-restore-picker-settings` button and `applyPickerSettings`; Playwright verifies changing settings away and restoring from selected swatch. |
15+
| Hover tooltip for picker/selected/pinned swatches includes requested metadata | PASS | `pickerTooltipText` and `swatchTooltipText` include color, name, Theme Collection, Palette Type, Variant, Colors, Steps, Contrast, Saturation, Hue Shift, and tags; Playwright checks generated/selected title metadata. |
16+
| Add deterministic generated color names when curated name is missing | PASS | `colorFamilyName` and `generatorSwatchName` derive stable names from generator settings, color family, row, and column. |
17+
| Rename center section Selected Swatches to Palette | PASS | Center H2 in `toolbox/colors/index.html` is now `Palette`; Playwright asserts the heading. |
18+
| Move pinned/selected swatches and controls into top accordion named Selected Swatches | PASS | Top accordion remains `Selected Swatches` and contains active palette controls/list; generated preview grid moved outside that accordion. |
19+
| Put Undo and Redo side by side | PASS | Undo/Redo are wrapped in an `action-group`; Playwright compares their positions. |
20+
| Remove Symbol UI and Symbol-specific code | PASS | Visible Symbol fields/hooks removed from `toolbox/colors/index.html` and UI JS; repository keeps only hidden compatibility key generation for existing palette contracts. Static scan found no live Symbol UI hooks. |
21+
| Theme Collection, Palette Type, and Variant side by side | PASS | Selector controls now render in a 3-column Theme V2 grid; Playwright verifies matching row positions. |
22+
| Remove Variant options 64/128/256 colors | PASS | `PALETTE_VARIANTS` and numeric variant counts exclude 64/128/256; Playwright and static scan verify options are absent. |
23+
| Keep requested smaller Variant options | PASS | Full, 32, 16, 8, 4, High Contrast, Color Blind Safe, Grayscale, Print Friendly, Day, Night, Dawn, Dusk, Winter, and Summer remain in `PALETTE_VARIANTS`. |
24+
| Contrast, Saturation, Hue Shift side by side with sliders below labels | PASS | Slider controls now use a 3-column Theme V2 grid with each range input inside its label group; Playwright verifies same row placement. |
25+
| Restore sorting display/control order | PASS | Sort/size controls render as Hue ^, Sat, Brit, Name, Tag, Small, Medium, Large; Playwright asserts exact order. |
26+
| Preserve tag checkbox behavior | PASS | Tag filtering remains in `renderUserPalette`; Playwright checks tag checkbox filtering/marking behavior in the batch tag test. |
27+
| Preserve no spacing between picker grid swatches | PASS | Existing generated grid gap-free styling/behavior preserved; Playwright validates picker interactions after layout changes. |
28+
| Preserve Admin unfiltered toolbox list and non-admin filtered list | PASS | `ToolboxRoutePages.spec.mjs` passed, including Admin all-tools and creator filtering. |
29+
| Preserve Colors on toolbox/index.html | PASS | `ToolboxRoutePages.spec.mjs` passed for toolbox route registration. |
30+
| No console errors | PASS | Targeted Palette Tool Playwright lane passed without console error failures. |
31+
| Playwright impacted | PASS | Targeted Playwright lanes were run and passed. |
32+
| Produce repo-structured ZIP | PASS | Created under `tmp/PR_26159_026-colors-picker-metadata-layout_delta.zip`. |
33+
34+
## Validation Evidence
35+
| Lane | Status | Evidence |
36+
| --- | --- | --- |
37+
| Changed-file syntax | PASS | `node --check toolbox/colors/colors.js`; `node --check src/dev-runtime/persistence/tool-repositories/palette-workspace-repository.js`; `node --check tests/playwright/tools/PaletteToolMockRepository.spec.mjs`. |
38+
| Diff/static validation | PASS | `git diff --check` passed. Git reported LF/CRLF warnings only. |
39+
| Symbol/variant/cursor static scans | PASS | `rg` found no live Symbol UI hooks, no live 64/128/256 variant options, and no live not-allowed swatch behavior in active Colors files. |
40+
| Palette Tool runtime/UI lane | PASS | `npx playwright test tests/playwright/tools/PaletteToolMockRepository.spec.mjs --timeout=180000` -> 6 passed. |
41+
| Toolbox registration/navigation lane | PASS | `npx playwright test tests/playwright/tools/ToolboxRoutePages.spec.mjs --timeout=180000` -> 2 passed. |
42+
43+
## Skipped Lanes
44+
| Lane | Reason |
45+
| --- | --- |
46+
| Full samples validation | Skipped by request. This PR only touches Colors tool UI/runtime, the palette workspace repository compatibility path, and targeted Colors/toolbox Playwright coverage. |
47+
| Broad workspace smoke | Not run because no shared sample loader/framework or workspace shell code changed. Targeted toolbox route validation covered Colors visibility and role filtering. |
48+
49+
## Changed Files
50+
- `toolbox/colors/index.html`
51+
- `toolbox/colors/colors.js`
52+
- `src/dev-runtime/persistence/tool-repositories/palette-workspace-repository.js`
53+
- `tests/playwright/tools/PaletteToolMockRepository.spec.mjs`
54+
- `docs_build/dev/reports/colors-picker-metadata-layout-report.md`
55+
- `docs_build/dev/reports/codex_review.diff`
56+
- `docs_build/dev/reports/codex_changed_files.txt`
57+
58+
## Notes
59+
- The visible Symbol UI and Symbol-specific interactions were removed. A hidden generated palette key remains inside the repository because the existing palette persistence contract still needs a unique swatch key for updates, deletes, and legacy payload validation.
60+
- The picker settings metadata is copied into selected/pinned swatches so restoration and tooltip metadata survive normal repository cloning and UI rerenders.

src/dev-runtime/persistence/tool-repositories/palette-workspace-repository.js

Lines changed: 56 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,40 @@ const SOURCE_ID_RENAMES = Object.freeze({
5555
});
5656
const SYMBOL_CANDIDATES = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789!@$%^&*()-+=[]{};:,.?";
5757

58-
function cloneSwatch(swatch) {
58+
function clonePickerSettings(settings) {
59+
if (!settings || typeof settings !== "object" || Array.isArray(settings)) {
60+
return null;
61+
}
5962
return {
63+
activeTags: Array.isArray(settings.activeTags) ? [...settings.activeTags] : [],
64+
colors: settings.colors,
65+
contrast: settings.contrast,
66+
hueShift: settings.hueShift,
67+
paletteType: settings.paletteType,
68+
saturation: settings.saturation,
69+
sortDirection: settings.sortDirection,
70+
sortField: settings.sortField,
71+
steps: settings.steps,
72+
swatchSize: settings.swatchSize,
73+
themeCollection: settings.themeCollection,
74+
variant: settings.variant,
75+
variantValue: settings.variantValue
76+
};
77+
}
78+
79+
function cloneSwatch(swatch) {
80+
const cloned = {
6081
symbol: swatch.symbol,
6182
hex: swatch.hex,
6283
name: swatch.name,
6384
source: swatch.source,
6485
tags: [...swatch.tags]
6586
};
87+
const pickerSettings = clonePickerSettings(swatch.pickerSettings);
88+
if (pickerSettings) {
89+
cloned.pickerSettings = pickerSettings;
90+
}
91+
return cloned;
6692
}
6793

6894
function cloneSourcePaletteRow(row) {
@@ -217,25 +243,28 @@ export function normalizePaletteSwatchInput(input = {}, options = {}) {
217243
const hex = normalizeHex(source.hex);
218244
const name = normalizeText(source.name);
219245
const tags = normalizeTags(source.tags);
246+
const pickerSettings = clonePickerSettings(source.pickerSettings);
220247

221-
return {
248+
const swatch = {
222249
symbol,
223250
hex,
224251
name,
225252
source: normalizeSource(source.source || options.source),
226253
tags
227254
};
255+
if (pickerSettings) {
256+
swatch.pickerSettings = pickerSettings;
257+
}
258+
return swatch;
228259
}
229260

230261
export function validatePaletteSwatchInput(input = {}, existingSwatches = [], options = {}) {
231262
const swatch = normalizePaletteSwatchInput(input, options);
232-
const excludeSymbol = normalizeText(options.excludeSymbol);
263+
const excludeSymbol = normalizeText(options.excludeSymbol || options.excludeKey);
233264
const issues = [];
234265

235-
if (!swatch.symbol) {
236-
issues.push(createIssue("symbol", "Symbol", "Enter a symbol for this swatch."));
237-
} else if (!isOneCharacter(swatch.symbol)) {
238-
issues.push(createIssue("symbol", "Symbol", "Symbol must be exactly one character."));
266+
if (options.requireSymbol && !swatch.symbol) {
267+
issues.push(createIssue("symbol", "Palette Key", "Palette key is missing."));
239268
}
240269

241270
if (!swatch.hex) {
@@ -303,7 +332,7 @@ export function validatePaletteWorkspacePayload(payload = {}) {
303332
const normalizedSwatches = [];
304333
swatches.forEach((candidate, index) => {
305334
const normalized = normalizePaletteSwatchInput(candidate);
306-
const validation = validatePaletteSwatchInput(normalized, normalizedSwatches);
335+
const validation = validatePaletteSwatchInput(normalized, normalizedSwatches, { requireSymbol: true });
307336
if (validation.issues.length) {
308337
validation.issues.forEach((issue) => {
309338
issues.push({
@@ -569,7 +598,17 @@ function nextAvailableSymbol(existingSwatches, seedText = "") {
569598
const used = new Set(existingSwatches.map((swatch) => swatch.symbol));
570599
const seedCandidates = Array.from(normalizeText(seedText).toUpperCase().replace(/[^A-Z0-9]/g, ""));
571600
const candidates = [...seedCandidates, ...Array.from(SYMBOL_CANDIDATES)];
572-
return candidates.find((candidate) => !used.has(candidate)) || "";
601+
const compactCandidate = candidates.find((candidate) => !used.has(candidate));
602+
if (compactCandidate) {
603+
return compactCandidate;
604+
}
605+
let index = existingSwatches.length + 1;
606+
let candidate = `swatch-${index}`;
607+
while (used.has(candidate)) {
608+
index += 1;
609+
candidate = `swatch-${index}`;
610+
}
611+
return candidate;
573612
}
574613

575614
function generatedHarmonyNameRoot(name) {
@@ -652,6 +691,7 @@ export function createProjectWorkspacePaletteRepository(options = {}) {
652691
.map((row) => ({
653692
hex: row.hex,
654693
name: row.name,
694+
pickerSettings: clonePickerSettings(row.pickerSettings) || undefined,
655695
source: row.source,
656696
symbol: row.symbol,
657697
tags: [...row.tags]
@@ -681,6 +721,7 @@ export function createProjectWorkspacePaletteRepository(options = {}) {
681721
hex: swatch.hex,
682722
id: `${projectId}-palette-color-${index + 1}`,
683723
name: swatch.name,
724+
pickerSettings: clonePickerSettings(swatch.pickerSettings) || undefined,
684725
projectId,
685726
source: swatch.source,
686727
symbol: swatch.symbol,
@@ -770,7 +811,11 @@ export function createProjectWorkspacePaletteRepository(options = {}) {
770811
function saveSwatch(input = {}, optionsForSave = {}) {
771812
const projectId = activeProjectId();
772813
const swatches = getActiveSwatches();
773-
const validation = validatePaletteSwatchInput(input, swatches, {
814+
const inputWithKey = {
815+
...input,
816+
symbol: normalizeText(input.symbol) || optionsForSave.excludeSymbol || nextAvailableSymbol(swatches, `${input.name || ""} ${input.hex || ""}`)
817+
};
818+
const validation = validatePaletteSwatchInput(inputWithKey, swatches, {
774819
excludeSymbol: optionsForSave.excludeSymbol,
775820
source: optionsForSave.source || PALETTE_SOURCE_USER
776821
});
@@ -833,6 +878,7 @@ export function createProjectWorkspacePaletteRepository(options = {}) {
833878
return saveSwatch(
834879
{
835880
...input,
881+
symbol: selectedSwatch.symbol,
836882
source: selectedSwatch.source || PALETTE_SOURCE_USER,
837883
tags: [...selectedSwatch.tags]
838884
},

0 commit comments

Comments
 (0)