Skip to content

Commit f57513c

Browse files
committed
Fix Palette fullscreen accordion shared height behavior - PR_26156_174-palette-fullscreen-accordion-flex-fix
1 parent d854721 commit f57513c

7 files changed

Lines changed: 189 additions & 40 deletions

File tree

assets/theme-v2/css/accordion.css

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,3 +140,43 @@ details.vertical-accordion[open] summary:active:after {
140140
color: var(--muted);
141141
line-height: 1.5
142142
}
143+
144+
body.tool-focus-mode .accordion-fill-stack {
145+
display: flex;
146+
flex: 1 1 auto;
147+
flex-direction: column;
148+
gap: var(--space-18);
149+
min-height: var(--space-0)
150+
}
151+
152+
body.tool-focus-mode .accordion-fill-stack>.accordion-fill-panel {
153+
flex: 0 0 auto;
154+
min-height: var(--space-0)
155+
}
156+
157+
body.tool-focus-mode .accordion-fill-stack>.accordion-fill-panel[open] {
158+
flex: 1 1 var(--space-0);
159+
min-height: var(--space-0);
160+
position: relative
161+
}
162+
163+
body.tool-focus-mode .accordion-fill-panel[open]>summary {
164+
min-block-size: var(--space-52);
165+
min-height: var(--space-0)
166+
}
167+
168+
body.tool-focus-mode .accordion-fill-panel[open]>.accordion-scroll-body {
169+
position: absolute;
170+
inset: var(--space-52) var(--space-0) var(--space-0);
171+
display: flex;
172+
flex-direction: column;
173+
gap: var(--space-18);
174+
min-height: var(--space-0);
175+
overflow: hidden
176+
}
177+
178+
body.tool-focus-mode .accordion-scroll-body>.scroll-region {
179+
flex: 1 1 var(--space-0);
180+
min-height: var(--space-0);
181+
overflow: auto
182+
}

assets/theme-v2/css/layout.css

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,14 @@ body.tool-focus-mode .tool-center-panel {
232232
overflow: auto
233233
}
234234

235+
body.tool-focus-mode .tool-center-panel--scroll-contained {
236+
display: flex;
237+
flex-direction: column;
238+
height: 100%;
239+
min-height: var(--space-0);
240+
overflow: hidden
241+
}
242+
235243
body.tool-focus-mode .tool-column:first-of-type {
236244
grid-column: 1;
237245
grid-row: 1 / span 2
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
# PR_26156_174 Palette Fullscreen Accordion Flex Fix Report
2+
3+
## Result
4+
PASS
5+
6+
## Summary
7+
- Fixed the Palette Tool fullscreen accordion layout regression with reusable Theme V2 layout classes.
8+
- Active Project Palette and Source Palette Browser now share available fullscreen vertical space evenly when both accordions are expanded.
9+
- Collapsing either accordion lets the remaining expanded accordion take the available space.
10+
- Scrolling is contained to the swatch grid wrappers; accordion headers, filter rows, and sort/size controls remain visible.
11+
12+
## Theme V2 Gap
13+
Theme V2 did not have a reusable pattern for a fullscreen stacked accordion group where open accordions split remaining height and inner content scrolls independently from the accordion header and controls.
14+
15+
Reusable classes were added to fill that gap:
16+
- `tool-center-panel--scroll-contained`
17+
- `accordion-fill-stack`
18+
- `accordion-fill-panel`
19+
- `accordion-scroll-body`
20+
- `scroll-region`
21+
22+
## Implementation Notes
23+
- The Palette Tool center panel uses `tool-center-panel--scroll-contained` in focus mode to keep the center column height bounded by the fullscreen grid row.
24+
- The fullscreen accordion stack uses `accordion-fill-stack` and `accordion-fill-panel` so expanded panels share available height while collapsed panels keep only their header height.
25+
- Open accordion bodies use `accordion-scroll-body` and the swatch wrappers use `scroll-region` so only the palette grids scroll.
26+
- Existing Source Palette Browser loading, search, sort, size, Pin All, pin behavior, and empty-state behavior were preserved.
27+
- Existing Active Project Palette selected swatch, remove/unpin, sort, size, and tag behavior were preserved.
28+
29+
## Validation
30+
- `node --check tests/playwright/tools/PaletteToolMockRepository.spec.mjs`
31+
- PASS
32+
- `rg "style=|<style|onclick|onchange|oninput|onsubmit" toolbox/colors/index.html assets/theme-v2/css/accordion.css assets/theme-v2/css/layout.css tests/playwright/tools/PaletteToolMockRepository.spec.mjs`
33+
- PASS, no matches
34+
- `npx playwright test tests/playwright/tools/PaletteToolMockRepository.spec.mjs --project=playwright --workers=1 --reporter=list`
35+
- PASS, 3 tests
36+
- `npm run test:playwright:static`
37+
- PASS
38+
39+
## Manual Verification Coverage
40+
- Both expanded accordions share fullscreen vertical space 50/50.
41+
- Collapsing Source Palette Browser expands Active Project Palette into the remaining space.
42+
- Collapsing Active Project Palette expands Source Palette Browser into the remaining space.
43+
- Swatch wrappers scroll independently while headers and control rows stay visible.
44+
- Source Palette Browser source data, Pin All, pin/unpin, sort, size, and search behavior remain covered by the targeted Palette Tool lane.
45+
- Active Project Palette selected swatch, remove/unpin, sort, size, and tag behavior remain covered by the targeted Palette Tool lane.
46+
47+
## Skipped
48+
- Full samples smoke was not run, per BUILD instruction.

docs_build/dev/reports/playwright_v8_coverage_report.txt

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@ Exercised tool entry points detected:
1818

1919
Changed runtime JS files covered:
2020
(87%) toolbox/colors/palette-workspace-repository.js - executed lines 1118/1118; executed functions 110/127
21-
(95%) toolbox/colors/colors.js - executed lines 745/745; executed functions 74/78
22-
(100%) toolbox/colors/palette-source-mock-db.js - executed lines 927/927; executed functions 6/6
2321

2422
Files with executed line/function counts where available:
2523
(65%) toolbox/project-workspace/project-workspace-mock-repository.js - executed lines 402/402; executed functions 20/31
@@ -36,5 +34,3 @@ Uncovered or low-coverage changed JS files:
3634
Changed JS files considered:
3735
(0%) tests/playwright/tools/PaletteToolMockRepository.spec.mjs - changed JS file not collected as browser runtime coverage
3836
(87%) toolbox/colors/palette-workspace-repository.js - changed JS file with browser V8 coverage
39-
(95%) toolbox/colors/colors.js - changed JS file with browser V8 coverage
40-
(100%) toolbox/colors/palette-source-mock-db.js - changed JS file with browser V8 coverage

docs_build/dev/reports/testing_lane_execution_report.md

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,51 +1,41 @@
1-
# PR_26156_173 Testing Lane Execution Report
1+
# PR_26156_174 Testing Lane Execution Report
22

33
## Result
44
PASS
55

66
## Commands Run
7-
- `node --check toolbox/colors/palette-source-mock-db.js`
8-
- PASS
9-
- `node --check toolbox/colors/palette-workspace-repository.js`
10-
- PASS
11-
- `node --check toolbox/colors/colors.js`
12-
- PASS
137
- `node --check tests/playwright/tools/PaletteToolMockRepository.spec.mjs`
148
- PASS
15-
- `rg "DEFAULT_SOURCE_PALETTES" toolbox/colors tests/playwright/tools/PaletteToolMockRepository.spec.mjs`
9+
- `rg "style=|<style|onclick|onchange|oninput|onsubmit" toolbox/colors/index.html assets/theme-v2/css/accordion.css assets/theme-v2/css/layout.css tests/playwright/tools/PaletteToolMockRepository.spec.mjs`
1610
- PASS, no matches
17-
- `node --input-type=module -e "import('./toolbox/colors/palette-workspace-repository.js').then(({createProjectWorkspacePaletteRepository})=>{ const repo=createProjectWorkspacePaletteRepository(); console.log(repo.sourcePaletteOptions().map((option)=>option.label + ' (' + option.swatchCount + ')').join('|')); })"`
18-
- PASS, source options include `8-color set`, `16-color set`, `24-color set`, `32-color set`, `48-color set`, `64-color set`, `96-color set`, `120-color set`, `150-color set`, `JavaScript`, and `W3C`
1911
- `npx playwright test tests/playwright/tools/PaletteToolMockRepository.spec.mjs --project=playwright --workers=1 --reporter=list`
2012
- PASS, 3 tests
2113
- `npm run test:playwright:static`
2214
- PASS
15+
- `git diff --check`
16+
- PASS
2317

2418
## Required Lanes
25-
- Targeted Palette Tool runtime/UI lane: PASS.
19+
- Targeted Palette Tool fullscreen UI lane: PASS.
2620
- Changed-file/static validation: PASS.
2721

2822
## Playwright Coverage
29-
- DB-backed source palette dropdown lists required mock DB palettes: PASS.
30-
- Source palette records render visible swatches: PASS.
31-
- Empty source table shows `No source palette`: PASS.
32-
- Invalid source records with no dropdown options show visible diagnostic: PASS.
33-
- Pin from Source Palette Browser adds to active Palette Colors: PASS.
34-
- Repeat pinning does not duplicate an existing active palette color: PASS.
35-
- Clicking the active Palette Colors pin indicator removes that color only from the active user palette: PASS.
36-
- Palette Colors count updates after add/remove: PASS.
37-
- Selected state clears when selected color is removed: PASS.
38-
- Source palette records remain available after active color removal: PASS.
39-
- Color Harmony Schemes does not render the selected/base swatch: PASS.
40-
- Harmony Add Selected and Add All continue from the remaining generated swatches: PASS.
23+
- Fullscreen center panel overflow is hidden so page-level center scrolling is disabled: PASS.
24+
- Active Project Palette and Source Palette Browser split fullscreen vertical space evenly when both are expanded: PASS.
25+
- Collapsing Source Palette Browser expands Active Project Palette into the remaining available space: PASS.
26+
- Collapsing Active Project Palette expands Source Palette Browser into the remaining available space: PASS.
27+
- Active Project Palette swatch wrapper scrolls while its summary and control row stay fixed: PASS.
28+
- Source Palette Browser swatch wrapper scrolls while its summary and filter/control rows stay fixed: PASS.
29+
- Source Palette Browser DB-backed source loading, Pin All, pin, sort, size, and search behavior remain covered: PASS.
30+
- Active Project Palette selected swatch, remove/unpin, sort, size, and tag behavior remain covered: PASS.
4131

4232
## Impacted Lane
4333
- Palette Tool runtime/UI lane.
44-
- Theme V2 static validation for the reusable swatch pin interaction target.
34+
- Theme V2 static validation for reusable fullscreen accordion layout classes.
4535

4636
## Skipped Lanes
4737
- Full samples smoke was skipped by BUILD instruction.
48-
- Broader tool lanes were skipped because changes are confined to Palette Tool runtime/mock DB behavior, one reusable Theme V2 swatch pin interaction, and the targeted Palette Tool spec.
38+
- Broader tool lanes were skipped because changes are confined to Palette Tool fullscreen layout, reusable Theme V2 layout CSS, and the targeted Palette Tool spec.
4939

5040
## Not Run
5141
- Full samples smoke was not run, per BUILD instruction.

tests/playwright/tools/PaletteToolMockRepository.spec.mjs

Lines changed: 70 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,41 @@ async function expectControlGroupsShareLine(page, sortSelector, sizeSelector) {
150150
expect(Math.abs(sortCenter - sizeCenter)).toBeLessThan(4);
151151
}
152152

153+
async function expectSwatchRegionScrollsOnly(page, scrollSelector, controlsSelector, summarySelector) {
154+
const scrollRegion = page.locator(scrollSelector);
155+
const controls = page.locator(controlsSelector);
156+
const summary = page.locator(summarySelector);
157+
const metrics = await scrollRegion.evaluate((node) => {
158+
const style = getComputedStyle(node);
159+
return {
160+
clientHeight: node.clientHeight,
161+
overflowY: style.overflowY,
162+
scrollHeight: node.scrollHeight
163+
};
164+
});
165+
expect(metrics.overflowY).toMatch(/auto|scroll/);
166+
expect(metrics.scrollHeight).toBeGreaterThan(metrics.clientHeight);
167+
const controlsBefore = await controls.boundingBox();
168+
const summaryBefore = await summary.boundingBox();
169+
await scrollRegion.evaluate((node) => {
170+
node.scrollTop = node.scrollHeight;
171+
});
172+
const controlsAfter = await controls.boundingBox();
173+
const summaryAfter = await summary.boundingBox();
174+
expect(Math.abs((controlsBefore?.y || 0) - (controlsAfter?.y || 0))).toBeLessThan(2);
175+
expect(Math.abs((summaryBefore?.y || 0) - (summaryAfter?.y || 0))).toBeLessThan(2);
176+
}
177+
178+
async function expectFullscreenAccordionSplit(page) {
179+
const projectBox = await page.locator("[data-palette-project-accordion]").boundingBox();
180+
const sourceBox = await page.locator("[data-palette-source-accordion]").boundingBox();
181+
expect(projectBox?.height || 0).toBeGreaterThan(180);
182+
expect(sourceBox?.height || 0).toBeGreaterThan(180);
183+
expect(Math.abs((projectBox?.height || 0) - (sourceBox?.height || 0))).toBeLessThan(12);
184+
expect(sourceBox?.y || 0).toBeGreaterThan((projectBox?.y || 0) + (projectBox?.height || 0) - 4);
185+
return { projectBox, sourceBox };
186+
}
187+
153188
test("Palette repository owns active project swatches without mutating invalid payloads", async () => {
154189
const emptyRepository = createProjectWorkspacePaletteRepository({ tables: { palette_source_swatches: [] } });
155190
expect(emptyRepository.sourcePaletteOptions()).toEqual([]);
@@ -475,14 +510,46 @@ test("Palette Tool adds, updates, pins, validates, and shows project-owned detai
475510
await page.locator("[data-palette-harmony-add-all]").click();
476511
await expect(page.locator("[data-palette-log]")).toContainText("Harmony add complete");
477512

513+
await page.locator("[data-palette-source-select]").selectOption("javascript");
514+
await expect(page.locator("[data-palette-source-index]")).toHaveCount(140);
515+
await page.locator("[data-palette-source-pin-all]").click();
516+
await expect(page.locator("[data-palette-log]")).toContainText("Pin All complete");
517+
478518
await page.getByLabel("Tool Display Mode").click();
479519
await expect(page.locator("body")).toHaveClass(/tool-focus-mode/);
480-
const projectBox = await page.locator("[data-palette-project-accordion]").boundingBox();
481-
const sourceBox = await page.locator("[data-palette-source-accordion]").boundingBox();
520+
await expect(page.locator(".tool-center-panel")).toHaveCSS("overflow-y", "hidden");
521+
const { projectBox, sourceBox } = await expectFullscreenAccordionSplit(page);
482522
expect(projectBox?.width || 0).toBeGreaterThan(300);
483523
expect(sourceBox?.width || 0).toBeGreaterThan(300);
484-
expect(sourceBox?.y || 0).toBeGreaterThan((projectBox?.y || 0) + (projectBox?.height || 0) - 4);
485524
expect(Math.abs((projectBox?.width || 0) - (sourceBox?.width || 0))).toBeLessThan(80);
525+
await expectSwatchRegionScrollsOnly(
526+
page,
527+
"[data-palette-user-scroll]",
528+
"[aria-label='Active project palette controls']",
529+
"[data-palette-project-accordion] > summary"
530+
);
531+
await expectSwatchRegionScrollsOnly(
532+
page,
533+
"[data-palette-source-scroll]",
534+
"[aria-label='Source palette controls']",
535+
"[data-palette-source-accordion] > summary"
536+
);
537+
538+
await page.locator("[data-palette-source-accordion] > summary").click();
539+
await expect(page.locator("[data-palette-source-accordion]")).not.toHaveAttribute("open", "");
540+
const projectOnlyBox = await page.locator("[data-palette-project-accordion]").boundingBox();
541+
const collapsedSourceBox = await page.locator("[data-palette-source-accordion]").boundingBox();
542+
expect(projectOnlyBox?.height || 0).toBeGreaterThan((projectBox?.height || 0) * 1.45);
543+
expect(collapsedSourceBox?.height || 0).toBeLessThan((sourceBox?.height || 0) * 0.45);
544+
545+
await page.locator("[data-palette-source-accordion] > summary").click();
546+
await expect(page.locator("[data-palette-source-accordion]")).toHaveAttribute("open", "");
547+
await page.locator("[data-palette-project-accordion] > summary").click();
548+
await expect(page.locator("[data-palette-project-accordion]")).not.toHaveAttribute("open", "");
549+
const collapsedProjectBox = await page.locator("[data-palette-project-accordion]").boundingBox();
550+
const sourceOnlyBox = await page.locator("[data-palette-source-accordion]").boundingBox();
551+
expect(sourceOnlyBox?.height || 0).toBeGreaterThan((sourceBox?.height || 0) * 1.45);
552+
expect(collapsedProjectBox?.height || 0).toBeLessThan((projectBox?.height || 0) * 0.45);
486553

487554
expectNoPageFailures(failures);
488555
} finally {

toolbox/colors/index.html

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ <h2>Palette</h2>
9191
</div>
9292
</aside>
9393
<div data-tool-display-mode data-asset-root="assets/theme-v2/images" data-tool-slug="colors" data-tool-icon-src="assets/theme-v2/images/badges/colors.png" data-tool-character-src="assets/theme-v2/images/characters/palette-manager.png"></div>
94-
<section class="tool-center-panel">
94+
<section class="tool-center-panel tool-center-panel--scroll-contained">
9595
<h2>Palette Colors</h2>
9696
<div class="grid cols-3" aria-label="Palette summary">
9797
<article class="mini-stat"><strong data-palette-count>0</strong>Palette Colors</article>
@@ -108,24 +108,24 @@ <h3 id="palette-tool-project-overlay-title">Active Project Required</h3>
108108
<a class="btn" href="toolbox/project-workspace/index.html">Open Project Workspace</a>
109109
</div>
110110
</article>
111-
<div class="content-stack" data-palette-fullscreen-panels>
112-
<details class="vertical-accordion" open data-palette-project-accordion>
111+
<div class="content-stack accordion-fill-stack" data-palette-fullscreen-panels>
112+
<details class="vertical-accordion accordion-fill-panel" open data-palette-project-accordion>
113113
<summary>Active Project Palette</summary>
114-
<div class="accordion-body content-stack">
114+
<div class="accordion-body content-stack accordion-scroll-body">
115115
<div class="grid cols-2" aria-label="Active project palette controls">
116116
<div class="action-group" aria-label="Active project palette sort" data-palette-user-sort></div>
117117
<div class="action-group action-group--end" aria-label="Active project palette size" data-palette-user-size></div>
118118
</div>
119-
<div class="palette-swatch-list-wrapper" data-palette-user-scroll>
119+
<div class="palette-swatch-list-wrapper scroll-region" data-palette-user-scroll>
120120
<div class="palette-swatch-list" role="group" aria-label="Active project palette colors" data-palette-user-list>
121121
<p class="status">No project palette colors yet.</p>
122122
</div>
123123
</div>
124124
</div>
125125
</details>
126-
<details class="vertical-accordion" open data-palette-source-accordion>
126+
<details class="vertical-accordion accordion-fill-panel" open data-palette-source-accordion>
127127
<summary>Source Palette Browser</summary>
128-
<div class="accordion-body content-stack">
128+
<div class="accordion-body content-stack accordion-scroll-body">
129129
<div class="content-stack content-stack--compact" aria-label="Source palette controls">
130130
<div class="content-cluster" aria-label="Source palette filters">
131131
<label for="paletteSourceSelect">Source</label>
@@ -139,7 +139,7 @@ <h3 id="palette-tool-project-overlay-title">Active Project Required</h3>
139139
<div class="action-group action-group--end" aria-label="Source swatches size" data-palette-source-size></div>
140140
</div>
141141
</div>
142-
<div class="palette-swatch-list-wrapper" data-palette-source-scroll>
142+
<div class="palette-swatch-list-wrapper scroll-region" data-palette-source-scroll>
143143
<div class="palette-swatch-list" role="group" aria-label="Source palette colors" data-palette-source-list>
144144
<p class="status">No source colors available.</p>
145145
</div>

0 commit comments

Comments
 (0)