Skip to content

Commit c6a71aa

Browse files
committed
Analyze Colors differences between PR_025 and current implementation - PR_26159_033-colors-pr025-diff-analysis & Clean up Colors picker duplicate handling and Symbol validation - PR_26159_034-colors-picker-duplicate-cleanup
1 parent 2c761c1 commit c6a71aa

6 files changed

Lines changed: 3218 additions & 102 deletions

File tree

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
# PR_26159_034 Colors Picker Duplicate Cleanup Report
2+
3+
## Summary
4+
5+
Implemented the Colors picker duplicate cleanup:
6+
- Removed the bottom `Already in Project (#)` picker section.
7+
- Removed the `Include already in Project swatches` checkbox and explanatory paragraph.
8+
- Kept already-in-project picker swatches visible in their original grid positions and true colors.
9+
- Removed the pin/add marker from already-in-project picker swatches only.
10+
- Kept duplicate add-click blocked without native disabled state, gray-out, opacity changes, or not-allowed cursor.
11+
- Moved the available count label below the picker grid.
12+
- Preserved key-based, Symbol-free active Colors validation.
13+
14+
Playwright impacted: Yes.
15+
16+
## PR_025 Diff Findings Summary
17+
18+
From `docs_build/dev/reports/colors_pr025_vs_current.md`:
19+
- PR_025 used Symbol exhaustion to disable generated cells.
20+
- The available/unavailable split was introduced later, not in PR_025.
21+
- Uneven columns came from filtering available and unavailable arrays before rendering rows.
22+
- The Symbol regression is stale/remaining validation state or active validator drift, not current scoped Colors source.
23+
24+
## Requirement Checklist
25+
26+
| Requirement | Status | Evidence |
27+
| --- | --- | --- |
28+
| Read `docs_build/dev/PROJECT_INSTRUCTIONS.md` first | PASS | Read before edits. |
29+
| Use PR_26159_033 diff report as source context | PASS | Report findings summarized above and used to remove the later split renderer instead of restoring Symbol-era PR_025 behavior. |
30+
| Remove `Already in Project (#)` section and supporting bottom-section code | PASS | `toolbox/colors/colors.js` removes `createPickerGroupLabel`, `createPickerGridPlaceholder`, `appendPickerGroupRows`, unavailable group rendering, and `data-palette-picker-group` rows. |
31+
| Remove `Include already in Project swatches` checkbox and explanatory text | PASS | `toolbox/colors/index.html` removes `data-palette-include-already-project-swatches` and `data-palette-picker-disabled-reason`; `toolbox/colors/colors.js` removes related state/listener. |
32+
| Keep already-in-project swatches visible in Picker Swatches | PASS | Single grid renderer now renders every generated swatch in-place through `appendPickerRows`. |
33+
| Remove pin/add marker only for already-in-project swatches | PASS | `createGeneratorPreviewInput` appends `createPinIndicator` only when `options.available !== false`; Playwright verifies duplicate swatch has no `[data-palette-pin-indicator]`. |
34+
| Do not gray out already-in-project or duplicate swatches | PASS | Duplicate swatches are not disabled; Playwright verifies opacity remains `1`. |
35+
| Do not alter displayed color | PASS | Playwright verifies duplicate swatch color input value matches the generated hex. |
36+
| Do not show red dot/not-allowed cursor | PASS | Duplicate swatches have no pin indicator and Playwright verifies cursor is not `not-allowed`. |
37+
| Disable only add click for already-in-project/duplicate swatches | PASS | Click handler blocks when `data-palette-generator-unavailable="true"`; Playwright verifies count does not increase after duplicate click. |
38+
| Move `Available Picker Swatches (#)` label under picker grid | PASS | `toolbox/colors/index.html` keeps `data-palette-generator-preview-status` below the grid; `renderPaletteGeneratorPreview` writes `Available Picker Swatches (${availableCount})`. |
39+
| Allow `palette-generator-preview-row` to use available horizontal and vertical space | PASS | Existing Theme V2 flex row is preserved; Playwright verifies row width matches preview width and row heights fill preview height. |
40+
| Permanently fix active Symbol validation/state/schema/seed references | PASS | Active Colors/palette-browser scoped grep found no `Enter a symbol`, `requireSymbol`, `swatchSymbol`, `paletteGeneratorSymbol`, `data-palette-symbol`, `palette-symbol`, or `"symbol":` in active Colors repository/schema/seed/API paths. |
41+
| Reseed/fresh reload must not restore Symbol requirement | PASS | Targeted Playwright opens fresh pages/server state and repository test validates symbol-free payloads and lifecycle add/update/remove/pin/unpin. |
42+
| Add/remove/pin/unpin/update/clear work without Symbol | PASS | `PaletteToolMockRepository.spec.mjs` repository lifecycle and UI tests passed. |
43+
| Validate no `Already in Project` section exists | PASS | Playwright asserts unavailable group labels are absent. |
44+
| Validate no Include checkbox/text exists | PASS | Playwright asserts checkbox and disabled-reason paragraph are absent. |
45+
| Validate already-in-project swatches remain visible with true color | PASS | Playwright verifies duplicate swatch remains in grid and color value matches generated hex. |
46+
| Validate available swatches still add | PASS | Playwright adds available picker swatches and verifies Project Swatches count increases. |
47+
| Validate no console errors | PASS | Targeted Playwright uses `expectNoPageFailures`; all tests passed. |
48+
| Do not run full samples validation | PASS | Full samples skipped; no sample/shared loader changed. |
49+
50+
## Symbol Cleanup Notes
51+
52+
Active Colors and palette-browser validation are key-based:
53+
- `toolbox/colors/colors.js`
54+
- `toolbox/colors/index.html`
55+
- `src/dev-runtime/persistence/tool-repositories/palette-workspace-repository.js`
56+
- `src/shared/schemas/tools/palette-browser.schema.json`
57+
- `src/dev-runtime/guest-seeds/palette-source-mock-db.js`
58+
- `src/dev-runtime/persistence/mock-db-store.js`
59+
- `src/dev-runtime/server/mock-api-router.mjs`
60+
- `tests/playwright/tools/PaletteToolMockRepository.spec.mjs`
61+
62+
Known excluded legacy/reference Symbol hits:
63+
- `src/engine/paletteList.js` contains legacy symbol-coded palette lists and is not active Colors `tools.palette-browser.swatches` validation.
64+
- `tests/tools/PaletteManagerV2Baseline.test.mjs` covers the older Palette Manager V2 baseline, not current Colors.
65+
- Archived V1/V2 and historical report files contain Symbol references by design and were not modified.
66+
67+
## Validation
68+
69+
Commands run:
70+
- `node --check toolbox/colors/colors.js`
71+
- `node --check toolbox/colors/palette-api-client.js`
72+
- `node --check tests/playwright/tools/PaletteToolMockRepository.spec.mjs`
73+
- `node --check src/dev-runtime/persistence/tool-repositories/palette-workspace-repository.js`
74+
- `npx playwright test tests/playwright/tools/PaletteToolMockRepository.spec.mjs`
75+
- `git diff --check`
76+
- Static grep checks for removed picker split/checkbox references and active Symbol validation drift.
77+
78+
Results:
79+
- PASS: JS syntax checks passed.
80+
- PASS: Palette Tool targeted Playwright passed, 7/7 tests.
81+
- PASS: `git diff --check` passed.
82+
- PASS: Active Symbol validation drift scan passed.
83+
84+
Notes:
85+
- Playwright emitted Node experimental SQLite warnings and seed-only audit fallback diagnostics; these are existing diagnostic output and did not fail the targeted lane.
86+
87+
## Skipped Lanes
88+
89+
| Lane | Status | Reason |
90+
| --- | --- | --- |
91+
| Full samples validation | SKIPPED | No samples, shared sample loader, or sample framework changed. |
92+
| Full Playwright suite | SKIPPED | Requested scope affects Colors picker and palette-browser validation only; targeted Palette Tool Playwright covers impacted behavior. |
93+
| V8 coverage report | SKIPPED | Not requested for this PR. |
94+

0 commit comments

Comments
 (0)