Skip to content

Commit f8ea486

Browse files
committed
Fix Colors picker preview sizing pins and selection layout - PR_26159_036-colors-picker-preview-layout
1 parent caddcda commit f8ea486

8 files changed

Lines changed: 221 additions & 42 deletions

File tree

assets/theme-v2/css/accordion.css

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,3 +202,49 @@ body.tool-focus-mode .tool-center-panel>details.vertical-accordion[open]>.accord
202202
inset: var(--space-52) var(--space-0) var(--space-0);
203203
overflow: auto
204204
}
205+
206+
body:not(.tool-focus-mode) .tool-center-panel:has(>.accordion-fill-stack) {
207+
display: flex;
208+
flex-direction: column;
209+
height: calc(100vh - var(--space-20));
210+
min-height: var(--space-0);
211+
overflow: hidden
212+
}
213+
214+
body:not(.tool-focus-mode) .tool-center-panel>.accordion-fill-stack {
215+
display: flex;
216+
flex: 1 1 auto;
217+
flex-direction: column;
218+
min-height: var(--space-0)
219+
}
220+
221+
body:not(.tool-focus-mode) .tool-center-panel>.accordion-fill-stack>.accordion-fill-panel {
222+
flex: 0 0 auto;
223+
min-height: var(--space-0)
224+
}
225+
226+
body:not(.tool-focus-mode) .tool-center-panel>.accordion-fill-stack>.accordion-fill-panel[open] {
227+
flex: 1 1 var(--space-0);
228+
min-height: var(--space-0);
229+
position: relative
230+
}
231+
232+
body:not(.tool-focus-mode) .tool-center-panel>.accordion-fill-stack>.accordion-fill-panel[open]>summary {
233+
min-block-size: var(--space-52);
234+
min-height: var(--space-0)
235+
}
236+
237+
body:not(.tool-focus-mode) .tool-center-panel>.accordion-fill-stack>.accordion-fill-panel[open]>.accordion-scroll-body {
238+
position: absolute;
239+
inset: var(--space-52) var(--space-0) var(--space-0);
240+
display: flex;
241+
flex-direction: column;
242+
min-height: var(--space-0);
243+
overflow: hidden
244+
}
245+
246+
body:not(.tool-focus-mode) .tool-center-panel>.accordion-fill-stack>.accordion-fill-panel[open]>.accordion-scroll-body>.scroll-region {
247+
flex: 1 1 var(--space-0);
248+
min-height: var(--space-0);
249+
overflow: auto
250+
}

assets/theme-v2/css/forms.css

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -293,15 +293,14 @@ input[type="color"][data-palette-swatch-size="large"] {
293293
.palette-swatch-tile:focus-visible,
294294
.palette-generator-preview-swatch:focus-visible {
295295
outline: var(--border-width-sm) solid var(--gold);
296-
outline-offset: var(--space-3)
296+
outline-offset: calc(var(--space-3) * -1)
297297
}
298298

299299
.palette-swatch-tile[data-palette-selected="true"],
300300
.palette-generator-preview-swatch[data-palette-selected="true"] {
301301
outline: var(--border-width-sm) solid var(--gold);
302-
outline-offset: var(--space-3);
303-
box-shadow: 0 0 0 var(--space-3) var(--gold-soft), 0 0 var(--space-18) var(--orange-shadow-color);
304-
transform: scale(1.04)
302+
outline-offset: calc(var(--space-3) * -1);
303+
box-shadow: inset 0 0 0 var(--space-3) var(--gold-soft), 0 0 var(--space-18) var(--orange-shadow-color)
305304
}
306305

307306
.palette-swatch-pin {
@@ -323,6 +322,10 @@ input[type="color"][data-palette-swatch-size="large"] {
323322
background: var(--green)
324323
}
325324

325+
.palette-generator-preview-swatch[data-palette-generator-availability="available"] .palette-swatch-pin {
326+
background: var(--green)
327+
}
328+
326329
input[type="file"],
327330
select {
328331
min-height: var(--space-44)
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
# PR_26159_036 Colors Picker Preview Layout Report
2+
3+
## Summary
4+
5+
Playwright impacted: Yes.
6+
7+
This PR fixes the Colors Picker Preview header/status layout, makes Project Swatches and Picker Preview share the center column fill space, keeps selected Project Swatch outlines inside the tile, anchors Project Swatch size/restore controls to the right, makes addable picker pins green, and corrects duplicate picker Hex handling so only the top-most duplicate in a column remains addable.
8+
9+
Reusable Theme V2 CSS was required because the existing accordion fill behavior only applied under `body.tool-focus-mode`; normal local tool mode had no reusable fill rule for center-panel accordion stacks. The new CSS is in Theme V2 only and does not add page-local or tool-local CSS.
10+
11+
## Requirement Checklist
12+
13+
| Requirement | Status | Evidence |
14+
| --- | --- | --- |
15+
| Read `PROJECT_INSTRUCTIONS.md` first | PASS | Read before editing; relevant PR completion, CSS, Playwright, and ZIP sections reviewed. |
16+
| Use PR_26159_035 as base context | PASS | Created `PR_26159_036-colors-picker-preview-layout` from `PR_26159_035-colors-picker-layout-tags`. |
17+
| Move `Available Picker Swatches (xx)` to the right of `Picker Preview` in the accordion/header area | PASS | Header markup at `toolbox/colors/index.html`; Playwright header assertion in `tests/playwright/tools/PaletteToolMockRepository.spec.mjs`. |
18+
| Keep current font/style for the count | PASS | Count keeps existing `status` class and `role=status` in the summary. |
19+
| Picker Preview grid uses 100% available horizontal and vertical space inside the open accordion | PASS | Reusable fill CSS in `assets/theme-v2/css/accordion.css`; Playwright validates grid/body width and height fill ratios. |
20+
| Do not force a fixed accordion height | PASS | No fixed accordion height added; the center panel supplies available viewport space and open accordions flex/share it. |
21+
| Project Swatches and Picker Preview share available vertical space | PASS | Both accordions use `accordion-fill-panel`; Playwright validates both-open heights. |
22+
| Closing Project Swatches lets Picker Preview use available vertical space | PASS | Playwright closes Project Swatches and verifies Picker Preview grid height increases. |
23+
| Closing Picker Preview lets Project Swatches use available vertical space | PASS | Playwright closes Picker Preview and verifies Project Swatches scroll region height increases. |
24+
| Selected Project Swatch outline remains inside the tile and visible | PASS | Theme V2 outline offset moved inside in `assets/theme-v2/css/forms.css`; Playwright verifies selected outline offset is <= 0 and transform is removed. |
25+
| Small/Medium/Large/Restore Picker Settings anchored right | PASS | Controls use existing `grid cols-2` + `action-group--end` pattern; Playwright verifies right-side positioning. |
26+
| Addable Picker Preview swatches show a green pin marker | PASS | Theme V2 rule in `assets/theme-v2/css/forms.css`; Playwright compares pin background to `--green`. |
27+
| Duplicate/unaddable picker swatches have the pin removed | PASS | Runtime only renders pin when `available !== false`; Playwright verifies duplicate lower swatches have no pin. |
28+
| Duplicate detection compares Hex within the same column only | PASS | `duplicatePickerHexReasons` keys by `column:hex` in `toolbox/colors/colors.js`. |
29+
| Top-most duplicate keeps pin/add marker | PASS | `duplicatePickerHexReasons` uses the minimum row as the keeper; Playwright validates top duplicate is addable. |
30+
| Duplicates below top remain true color but cannot add | PASS | Playwright validates lower duplicate opacity/color/cursor and blocked click behavior. |
31+
| Unique Hex values keep pin/add marker | PASS | Existing generated swatch tests verify available unique swatches add to Project Swatches. |
32+
| Do not gray out duplicates | PASS | Playwright validates lower duplicate opacity remains `1`. |
33+
| Do not show red not-allowed marker/cursor | PASS | Playwright validates duplicate and already-in-project swatch cursor is not `not-allowed`; addable pin is green. |
34+
| Preserve Project Swatches accordion | PASS | Center Project Swatches accordion remains in `toolbox/colors/index.html` and is covered by Playwright. |
35+
| Preserve Picker Swatches left-column accordion | PASS | Left Picker Swatches accordion remains in `toolbox/colors/index.html` and is covered by Playwright. |
36+
| Preserve Picker Preview accordion | PASS | Picker Preview remains a center accordion and is covered by Playwright. |
37+
| Preserve ROYGBIV selector | PASS | Existing ROYGBIV assertions remain in Palette Tool Playwright coverage. |
38+
| Preserve Theme Collection / Palette Type / Variant options | PASS | Existing option assertions remain in Palette Tool Playwright coverage. |
39+
| Preserve Add/Update/Clear without Symbol validation | PASS | Existing Add/Update/Clear and Symbol-free validation assertions remain passing. |
40+
| No console errors | PASS | `expectNoPageFailures` passed in all targeted Palette Tool Playwright tests. |
41+
| Do not run full samples validation | PASS | Full samples smoke was not run. |
42+
43+
## Validation
44+
45+
| Lane | Status | Command | Evidence |
46+
| --- | --- | --- | --- |
47+
| Changed JS syntax | PASS | `node --check toolbox/colors/colors.js` | Colors runtime parses. |
48+
| Changed Playwright syntax | PASS | `node --check tests/playwright/tools/PaletteToolMockRepository.spec.mjs` | Updated Palette tests parse. |
49+
| Static whitespace validation | PASS | `git diff --check` | No whitespace errors; line-ending warnings only. |
50+
| Inline HTML restriction audit | PASS | `rg --pcre2 -n "<style|style=|<script(?![^>]*\\bsrc=)|on(click|change|input|submit)=" toolbox/colors/index.html` | No matches. |
51+
| Targeted Palette Tool Playwright | PASS | `npx playwright test tests/playwright/tools/PaletteToolMockRepository.spec.mjs` | 8/8 passed; validates requested UI/runtime behavior and no console/page failures. |
52+
| Playwright V8 coverage | PASS | Palette Tool Playwright afterAll coverage reporter | `docs_build/dev/reports/playwright_v8_coverage_report.txt` and `coverage_changed_js_guardrail.txt` updated for changed runtime JS. |
53+
| Workspace contract default lane | FAIL (unrelated existing lane blocker) | `npm run test:workspace-v2` | Failed on `RootToolsFutureState.spec.mjs` expecting Admin nav markup for unauthenticated pages. This failure is outside the Colors picker scope and was not introduced by this PR. |
54+
55+
## Skipped Lanes
56+
57+
| Lane | Reason |
58+
| --- | --- |
59+
| Full samples validation | Skipped by request. This PR changes Colors UI/runtime, Theme V2 accordion/swatch styling, and targeted Palette Playwright coverage only; no sample loader/framework was changed. |
60+
61+
## Notes
62+
63+
- Existing SQLite experimental warnings and seed-only audit fallback diagnostics appeared during Playwright runs; they did not fail the targeted Palette lane.
64+
- The default workspace-contract lane was attempted because project instructions mention it for Playwright-impacted work, but it currently fails on header/admin navigation expectations unrelated to this PR. The requested Colors validation items all passed.

docs_build/dev/reports/coverage_changed_js_guardrail.txt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +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 1953/1953; executed functions 165/194
10-
(100%) toolbox/colors/palette-api-client.js - executed lines 19/19; executed functions 4/4
9+
(85%) toolbox/colors/colors.js - executed lines 1938/1938; executed functions 166/195
1110

1211
Guardrail warnings:
1312
(100%) none - no changed runtime JS coverage warnings

docs_build/dev/reports/playwright_v8_coverage_report.txt

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,20 +12,19 @@ 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 1953/1953; executed functions 165/194
21-
(100%) toolbox/colors/palette-api-client.js - executed lines 19/19; executed functions 4/4
20+
(85%) toolbox/colors/colors.js - executed lines 1938/1938; executed functions 166/195
2221

2322
Files with executed line/function counts where available:
2423
(58%) src/engine/api/server-api-client.js - executed lines 159/159; executed functions 11/19
2524
(64%) assets/theme-v2/js/gamefoundry-partials.js - executed lines 525/525; executed functions 28/44
2625
(64%) assets/theme-v2/js/tool-display-mode.js - executed lines 201/201; executed functions 9/14
2726
(78%) toolbox/tool-registry-api-client.js - executed lines 148/148; executed functions 21/27
28-
(85%) toolbox/colors/colors.js - executed lines 1953/1953; executed functions 165/194
27+
(85%) toolbox/colors/colors.js - executed lines 1938/1938; executed functions 166/195
2928
(100%) toolbox/colors/palette-api-client.js - executed lines 19/19; executed functions 4/4
3029

3130
Uncovered or low-coverage changed JS files:
@@ -34,4 +33,3 @@ Uncovered or low-coverage changed JS files:
3433
Changed JS files considered:
3534
(0%) tests/playwright/tools/PaletteToolMockRepository.spec.mjs - changed JS file not collected as browser runtime coverage
3635
(85%) toolbox/colors/colors.js - changed JS file with browser V8 coverage
37-
(100%) toolbox/colors/palette-api-client.js - changed JS file with browser V8 coverage

0 commit comments

Comments
 (0)