From 58fb1c5db792726bab458b6cd4a60a43e8f8e44d Mon Sep 17 00:00:00 2001 From: Istvan Matejcsok <119620946+matejcsok-ee@users.noreply.github.com> Date: Tue, 16 Jun 2026 09:33:32 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20=F0=9F=90=9B=20pcb=20layers=20tab?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/features/wx-dom-port/visual-notes.md | 31 ++++++++++++++ tests/kicad/appearance.spec.ts | 49 ++++++++++++++++++++--- 2 files changed, 74 insertions(+), 6 deletions(-) diff --git a/docs/features/wx-dom-port/visual-notes.md b/docs/features/wx-dom-port/visual-notes.md index 078d4e8..0c29dc9 100644 --- a/docs/features/wx-dom-port/visual-notes.md +++ b/docs/features/wx-dom-port/visual-notes.md @@ -235,3 +235,34 @@ trees fault identically; binary timing decides whether the race is lost, so any rebuild can flip these specs. They were red before this consolidation (the "31 passed" gates) and stay red after it; greening them is the async feature's exit criterion. + +## Layers tab goes blank after a tab round-trip (2026-06-15, pcbjam#8) + +Switching the pcbnew appearance notebook Layers → Objects/Nets → Layers left +the Layers page blank: every layer row was present in the DOM but fully +clip-pathed away, and the layers `wxScrolledWindow` viewport had collapsed to +zero height (`154x0`). + +Root cause (two layers): + 30. KiCad's `APPEARANCE_CONTROLS::OnNotebookPageChanged` calls + `m_panelLayers->Fit()` on every page change, which shrinks the page to + its content min size and collapses the proportion-1 scrolled child's + viewport to height 0. `wxBookCtrlBase::DoSetSelection()` fires + `wxEVT_NOTEBOOK_PAGE_CHANGED` LAST, so this happens after the page was + sized/shown. + 31. `wxWindowWasm::DoSetSize()` only emits `wxEVT_SIZE` (which drives + auto-Layout) when the size actually changes. Resizing the page back to + the page area is therefore a no-op that never re-runs the sizer, so the + collapsed scrolled child sticks and `UpdateDomGeometry` clip-paths every + row out of the empty ancestor viewport. + +Fix: `wxNotebook::OnDomEvent` now calls `WasmRelayoutSelectedPage()` after +`SetSelection()` returns (all PAGE_CHANGED handlers done) — it re-asserts the +page to the full page area, forces `page->Layout()`, and re-projects the DOM +geometry. wxwidgets-layer only; KiCad untouched. + +Test hardening: `appearance.spec.ts`'s `rowLabelTops()` previously returned +`getBoundingClientRect().top`, which is `0` (not null) for a clip-pathed row, +so the round-trip assertion passed even when the panel was blank. It now +hit-tests the row centre via `elementsFromPoint` (clip-path affects +hit-testing), so a clipped-away row reads as null and fails the assertion. diff --git a/tests/kicad/appearance.spec.ts b/tests/kicad/appearance.spec.ts index 4ad82ff..4af186b 100644 --- a/tests/kicad/appearance.spec.ts +++ b/tests/kicad/appearance.spec.ts @@ -67,8 +67,10 @@ async function selectTab(page: Page, label: string): Promise { await page.waitForTimeout(400); } -// DOM port only: viewport rects of row labels inside the appearance pane +// DOM port only: viewport tops of row labels inside the appearance pane // (spans are real elements there; the canvas port draws them as pixels). +// Returns the layout-box top regardless of clipping — used to track row +// MOVEMENT under scrolling, where rows legitimately clip at the pane edges. async function rowLabelTops(page: Page, labels: string[]): Promise> { return page.evaluate((wanted: string[]) => { const out: Record = {}; @@ -83,6 +85,39 @@ async function rowLabelTops(page: Page, labels: string[]): Promise> { + return page.evaluate((wanted: string[]) => { + const out: Record = {}; + for (const w of wanted) out[w] = false; + if (!window.wxDomControls) return out; + const visible = (el: HTMLElement): boolean => { + const cs = getComputedStyle(el); + if (cs.display === 'none' || cs.visibility === 'hidden') return false; + const r = el.getBoundingClientRect(); + if (r.width < 1 || r.height < 1) return false; + const cx = r.left + r.width / 2; + const ys = [r.top + 1, r.top + r.height / 2, r.bottom - 1]; + return ys.some(y => { + const hits = document.elementsFromPoint(cx, y); + return hits.some(h => h === el || el.contains(h)); + }); + }; + for (const [, el] of window.wxDomControls) { + const txt = el.textContent || ''; + if (el.tagName === 'SPAN' && wanted.includes(txt) && visible(el)) + out[txt] = true; + } + return out; + }, labels); +} + test.describe('Appearance panel (Layers/Objects/Nets)', () => { test.beforeEach(async ({ page }) => { await page.goto('/kicad/pcbnew.html'); @@ -107,11 +142,13 @@ test.describe('Appearance panel (Layers/Objects/Nets)', () => { await selectTab(page, 'Layers'); await page.screenshot({ path: 'test-results/appearance-03-layers-again.png' }); - // Layer rows must survive the tab round-trip (regression: - // pages came back blank after switching away and back) - const tops = await rowLabelTops(page, ['F.Cu', 'B.Cu']); - expect(tops['F.Cu'], 'F.Cu row visible after tab round-trip').not.toBeNull(); - expect(tops['B.Cu'], 'B.Cu row visible after tab round-trip').not.toBeNull(); + // Layer rows must survive the tab round-trip (regression pcbjam#8: + // pages came back blank after switching away and back — the rows stayed + // in the DOM but their scrolled window collapsed and clip-pathed them + // all away, so this must assert real paint, not just DOM presence). + const vis = await rowsVisible(page, ['F.Cu', 'B.Cu']); + expect(vis['F.Cu'], 'F.Cu row visible after tab round-trip').toBe(true); + expect(vis['B.Cu'], 'B.Cu row visible after tab round-trip').toBe(true); }); test('layer list scrolls with the wheel and clips at the pane', async ({ page }) => {