Skip to content

Commit 0bbff01

Browse files
ToolboxAidToolboxAid
committed
Update Colors picker preview pins duplicates and grid behavior - PR_26159_038-colors-picker-preview-behavior & Polish Colors sorting duplicate grid and picker controls - PR_26159_039-colors-sorting-duplicate-grid-polish
1 parent a009fe3 commit 0bbff01

8 files changed

Lines changed: 260 additions & 120 deletions

File tree

‎assets/theme-v2/css/forms.css‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,10 @@ input[type="range"] {
354354
padding: var(--space-0)
355355
}
356356

357+
.tool-column[class*="tool-group-"] input[type="range"] {
358+
accent-color: var(--tool-group-color)
359+
}
360+
357361
textarea {
358362
min-height: var(--size-textarea-min);
359363
resize: vertical

‎docs_build/dev/reports/colors-sorting-duplicate-grid-polish-report.md‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
# PR_26159_039-colors-sorting-duplicate-grid-polish
2+
3+
Generated: 2026-06-08
4+
Playwright impacted: Yes
5+
Full samples validation: Skipped
6+
7+
## Summary
8+
9+
Implemented the Colors picker polish requested in PR_26159_039:
10+
11+
- Theme Collection and Palette Type options render in alphabetical/natural order.
12+
- Variant options render with `Full` first/default, then sorted remaining variants.
13+
- Grid Colors is now a number field defaulted to `8`.
14+
- Picker Swatches sliders render as label + slider rows.
15+
- Duplicate picker cells remain in the grid; unchecked `Show duplicates` makes duplicate cells transparent while preserving layout, checked shows duplicate colors, and duplicates never receive pins.
16+
- User Defined Swatch `Add` now uses the same button styling class as `Update` and `Clear`.
17+
18+
## Requirement Checklist
19+
20+
| Requirement | Status | Evidence |
21+
| --- | --- | --- |
22+
| Read `docs_build/dev/PROJECT_INSTRUCTIONS.md` first | PASS | Read before edits. |
23+
| Hard stop if current git branch is not `main` | PASS | `git branch --show-current` returned `main`; `git status -sb` showed `## main...origin/main` before edits. |
24+
| Sort all Theme Collection options alphabetically | PASS | `toolbox/colors/colors.js` adds sorted collection rendering; Playwright asserts sorted Theme Collection options. |
25+
| Sort all Palette Type options alphabetically | PASS | `toolbox/colors/colors.js` adds sorted type rendering; Playwright asserts sorted type options for Nature, Sci-Fi, and all collections. |
26+
| Sort Variant options alphabetically with `Full` first/default | PASS | `toolbox/colors/colors.js` renders `Full` first and defaults to `full`; Playwright asserts the sorted Variant option list. |
27+
| Picker Swatches sliders are label + slider on same line | PASS | `toolbox/colors/index.html` uses Theme V2 form-table rows for Contrast, Saturation, and Hue Shift; Playwright checks label/control row alignment. |
28+
| Show duplicates unchecked renders transparent duplicate cells with preserved spacing | PASS | `toolbox/colors/colors.js` keeps duplicate cells in the row map and sets SVG fill to `transparent`; Playwright checks row column count remains 8 and duplicate fill is transparent. |
29+
| Show duplicates checked renders duplicate colors with no pins | PASS | `toolbox/colors/colors.js` exposes duplicate cells with real fill but unavailable/no pin; Playwright checks true color and no pin. |
30+
| Duplicates never get a pin | PASS | `createGeneratorPreviewInput()` only adds a pin when available; duplicate cells are unavailable. Playwright checks duplicate no-pin behavior. |
31+
| Grid Colors is a number field defaulted to 8 | PASS | `toolbox/colors/index.html` changes Colors to `<input type="number" value="8">`; Playwright asserts type, step, and default value. |
32+
| Sliders use same group color styling as Colors container | PASS | `assets/theme-v2/css/forms.css` sets range `accent-color: var(--tool-group-color)` for tool-group columns; Playwright compares slider accent color to the resolved group token. |
33+
| User Defined Swatch Add button matches Update/Clear styling | PASS | `toolbox/colors/index.html` changes Add from `btn primary` to `btn`; Playwright asserts Add/Update/Clear all use `btn`. |
34+
| Preserve red pin add / green pin remove / no-pin duplicate | PASS | Existing pin behavior preserved; targeted Playwright validates red add, green remove, and no-pin duplicate click blocking. |
35+
| Preserve no Picker Preview selection box | PASS | Existing no-selection-box assertions still pass in targeted Playwright. |
36+
| Preserve Add/Update/Clear without Symbol validation | PASS | Targeted Playwright validates Add/Update/Clear and no Symbol validation; active scan found no Symbol validation hits. |
37+
| Validate no console errors | PASS | Targeted Playwright `expectNoPageFailures()` passed for all page tests. |
38+
| Produce required reports | PASS | This report, `testing_lane_execution_report.md`, `playwright_v8_coverage_report.txt`, `codex_review.diff`, and `codex_changed_files.txt`. |
39+
| Produce repo-structured ZIP under `tmp/` | PASS | `tmp/PR_26159_039-colors-sorting-duplicate-grid-polish_delta.zip`. |
40+
41+
## Validation Evidence
42+
43+
| Lane | Result |
44+
| --- | --- |
45+
| `node --check toolbox/colors/colors.js` | PASS |
46+
| `node --check tests/playwright/tools/PaletteToolMockRepository.spec.mjs` | PASS |
47+
| `npx playwright test tests/playwright/tools/PaletteToolMockRepository.spec.mjs` | PASS, 8 passed |
48+
| `git diff --check` | PASS, line-ending warnings only |
49+
| Active Symbol validation scan | PASS |
50+
51+
## Skipped Lanes
52+
53+
| Lane | Reason |
54+
| --- | --- |
55+
| Full samples validation | Safe to skip because no samples, sample launcher, or shared sample framework files changed. |
56+
| Broad Playwright suite | Safe to skip because the targeted Palette Tool lane covers the impacted Colors runtime/UI behavior and console checks. |
57+
58+
## Notes
59+
60+
- Variant sorting uses natural alphabetical ordering through `Intl.Collator` with numeric comparison, so numeric variant labels sort `4`, `8`, `16`, `32` after `Full`.
61+
- Hidden duplicates retain their true Hex in datasets/tooltips and only hide the visible SVG fill while `Show duplicates` is unchecked.

‎docs_build/dev/reports/coverage_changed_js_guardrail.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ Missing changed runtime JS files are WARN, not FAIL.
66
Source: Playwright/Chromium built-in V8 coverage from the active Playwright run.
77

88
Changed runtime JS files considered:
9-
(85%) toolbox/colors/colors.js - executed lines 1981/1981; executed functions 169/200
9+
(86%) toolbox/colors/colors.js - executed lines 2012/2012; executed functions 177/206
1010

1111
Guardrail warnings:
1212
(100%) none - no changed runtime JS coverage warnings

‎docs_build/dev/reports/playwright_v8_coverage_report.txt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,24 +12,24 @@ Note: entry percentages use function coverage when available, otherwise line cov
1212
Note: coverage entries are aggregated across every page/tool where coverageReporter.start(page) and coverageReporter.stop(page) ran.
1313

1414
Exercised tool entry points detected:
15-
(84%) Toolbox Index - exercised 3 runtime JS files
15+
(85%) Toolbox Index - exercised 3 runtime JS files
1616
(0%) Tool Template V2 - not exercised by this Playwright run
1717
(64%) Theme V2 Shared JS - exercised 2 runtime JS files
1818

1919
Changed runtime JS files covered:
20-
(85%) toolbox/colors/colors.js - executed lines 1981/1981; executed functions 169/200
20+
(86%) toolbox/colors/colors.js - executed lines 2012/2012; executed functions 177/206
2121

2222
Files with executed line/function counts where available:
2323
(58%) src/engine/api/server-api-client.js - executed lines 159/159; executed functions 11/19
2424
(64%) assets/theme-v2/js/gamefoundry-partials.js - executed lines 525/525; executed functions 28/44
2525
(64%) assets/theme-v2/js/tool-display-mode.js - executed lines 201/201; executed functions 9/14
2626
(78%) toolbox/tool-registry-api-client.js - executed lines 148/148; executed functions 21/27
27-
(85%) toolbox/colors/colors.js - executed lines 1981/1981; executed functions 169/200
27+
(86%) toolbox/colors/colors.js - executed lines 2012/2012; executed functions 177/206
2828
(100%) toolbox/colors/palette-api-client.js - executed lines 19/19; executed functions 4/4
2929

3030
Uncovered or low-coverage changed JS files:
3131
(100%) none - no low-coverage changed runtime JS files
3232

3333
Changed JS files considered:
3434
(0%) tests/playwright/tools/PaletteToolMockRepository.spec.mjs - changed JS file not collected as browser runtime coverage
35-
(85%) toolbox/colors/colors.js - changed JS file with browser V8 coverage
35+
(86%) toolbox/colors/colors.js - changed JS file with browser V8 coverage

‎docs_build/dev/reports/testing_lane_execution_report.md‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,38 @@
11
# Testing Lane Execution Report
22

3-
PR: PR_26159_031-colors-picker-viewer-restore
3+
PR: PR_26159_039-colors-sorting-duplicate-grid-polish
44
Generated: 2026-06-08
55
Full samples validation: SKIPPED
66

77
## Summary
88

9-
PASS: 7
10-
WARN: 0
9+
PASS: 6
10+
WARN: 1
1111
FAIL: 0
1212
SKIP: 2
1313

1414
## Executed Lanes
1515

1616
| Lane | Status | Command | Evidence |
1717
| --- | --- | --- | --- |
18-
| Colors runtime syntax | PASS | `node --check toolbox/colors/colors.js` | Colors picker grouping, ROYGBIV data, and unavailable handling parse. |
19-
| Colors API helper syntax | PASS | `node --check toolbox/colors/palette-api-client.js` | Validation API contract forwarding parses. |
20-
| Palette Playwright syntax | PASS | `node --check tests/playwright/tools/PaletteToolMockRepository.spec.mjs` | Updated Palette tests parse. |
21-
| Palette / Colors Playwright | PASS | `npx playwright test tests/playwright/tools/PaletteToolMockRepository.spec.mjs` | 6 passed. Validates available/unavailable picker groups, unavailable click blocking, no graying/not-allowed cursor, ROYGBIV, Add/Update without Symbol validation, and no console errors. |
22-
| Runtime V8 coverage | PASS | Palette Playwright afterAll coverage reporter | `docs_build/dev/reports/playwright_v8_coverage_report.txt` includes `toolbox/colors/colors.js` and `toolbox/colors/palette-api-client.js`. |
23-
| Static whitespace validation | PASS | `git diff --check` | No whitespace errors; line-ending warnings only. |
24-
| Targeted string scans | PASS | Active `rg` scans for Symbol validation/control strings and old global picker checkbox strings | No active Colors Symbol validation/control hits and no old global picker checkbox strings. |
18+
| Branch gate | PASS | `git branch --show-current` | Returned `main`; implementation proceeded. |
19+
| Colors runtime syntax | PASS | `node --check toolbox/colors/colors.js` | Colors picker sorting and duplicate-grid logic parse. |
20+
| Palette Playwright syntax | PASS | `node --check tests/playwright/tools/PaletteToolMockRepository.spec.mjs` | Updated targeted assertions parse. |
21+
| Palette / Colors Playwright | PASS | `npx playwright test tests/playwright/tools/PaletteToolMockRepository.spec.mjs` | 8 passed. Covers sorted selectors, numeric Grid Colors, slider layout/group color, transparent duplicate cells, duplicate no-pin behavior, Symbol-free Add/Update/Clear, and no console errors. |
22+
| Runtime V8 coverage | PASS | Palette Playwright afterAll coverage reporter | `docs_build/dev/reports/playwright_v8_coverage_report.txt` includes changed runtime JS coverage for `toolbox/colors/colors.js`. |
23+
| Static whitespace validation | PASS | `git diff --check` | No whitespace errors; Git reported line-ending warnings only. |
24+
| Active Symbol validation scan | PASS | `rg -n "Symbol: Enter a symbol|data-palette-symbol|palette.*symbol|symbol" toolbox/colors src/dev-runtime/persistence/tool-repositories/palette-workspace-repository.js tests/playwright/tools/PaletteToolMockRepository.spec.mjs` | No active Symbol validation hits. Remaining matches are Symbol-free test fixture names. |
2525

26-
## Skipped Lanes
26+
## Notes
2727

28-
| Lane | Status | Reason |
28+
| Item | Status | Detail |
2929
| --- | --- | --- |
30-
| Full samples validation | SKIP | This PR does not touch sample JSON, sample launcher code, or shared sample framework behavior. |
31-
| Broad Playwright suite | SKIP | Targeted Palette Playwright covers the changed Colors runtime/UI behavior and console-error assertions. |
30+
| Initial Playwright run | WARN | First run failed only because the test expected the old variant order. The implementation produced natural alphabetical/numeric order, the assertion was corrected, and the full targeted lane was rerun successfully. |
31+
| SQLite test-server output | WARN | Local targeted Playwright still emits Node SQLite experimental warnings and seed-only audit fallback diagnostics from existing test infrastructure. Assertions passed. |
3232

33-
## Notes
33+
## Skipped Lanes
3434

35-
- The unavailable picker swatch rule is normalized RGB duplicate detection against Project Swatches via `pinnedSwatchForHex(hex)`.
36-
- Unavailable picker swatches are not disabled buttons, so their color and cursor remain normal while click handling prevents duplicate adds.
37-
- Existing local test-server output still prints SQLite experimental warnings and seed-only audit fallback diagnostics; targeted Playwright assertions passed.
35+
| Lane | Status | Reason |
36+
| --- | --- | --- |
37+
| Full samples validation | SKIP | This PR does not touch samples, sample loader/runtime code, or shared sample framework behavior. |
38+
| Broad Playwright suite | SKIP | The existing targeted Palette Tool lane covers the changed Colors UI/runtime behavior and includes console-error checks. |

0 commit comments

Comments
 (0)