fix: 🐛 pcb layers tab

This commit is contained in:
Istvan Matejcsok 2026-06-16 09:33:32 +02:00
commit 58fb1c5db7
2 changed files with 74 additions and 6 deletions

View file

@ -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 so any rebuild can flip these specs. They were red before this
consolidation (the "31 passed" gates) and stay red after it; greening consolidation (the "31 passed" gates) and stay red after it; greening
them is the async feature's exit criterion. 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.

View file

@ -67,8 +67,10 @@ async function selectTab(page: Page, label: string): Promise<void> {
await page.waitForTimeout(400); 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). // (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<Record<string, number | null>> { async function rowLabelTops(page: Page, labels: string[]): Promise<Record<string, number | null>> {
return page.evaluate((wanted: string[]) => { return page.evaluate((wanted: string[]) => {
const out: Record<string, number | null> = {}; const out: Record<string, number | null> = {};
@ -83,6 +85,39 @@ async function rowLabelTops(page: Page, labels: string[]): Promise<Record<string
}, labels); }, labels);
} }
// Whether each row label is genuinely PAINTED (not merely present in the DOM).
// A bare getBoundingClientRect is not enough: when the layers scrolled window
// collapses on a tab round-trip the rows keep their layout box but are fully
// removed by an ancestor's clip-path. clip-path also affects hit-testing, so we
// sample points down each row's box and treat it as visible only if the row (or
// its own content) is actually returned by elementsFromPoint somewhere. This is
// what makes the "pages came back blank" regression fail the assertion below.
async function rowsVisible(page: Page, labels: string[]): Promise<Record<string, boolean>> {
return page.evaluate((wanted: string[]) => {
const out: Record<string, boolean> = {};
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.describe('Appearance panel (Layers/Objects/Nets)', () => {
test.beforeEach(async ({ page }) => { test.beforeEach(async ({ page }) => {
await page.goto('/kicad/pcbnew.html'); await page.goto('/kicad/pcbnew.html');
@ -107,11 +142,13 @@ test.describe('Appearance panel (Layers/Objects/Nets)', () => {
await selectTab(page, 'Layers'); await selectTab(page, 'Layers');
await page.screenshot({ path: 'test-results/appearance-03-layers-again.png' }); await page.screenshot({ path: 'test-results/appearance-03-layers-again.png' });
// Layer rows must survive the tab round-trip (regression: // Layer rows must survive the tab round-trip (regression pcbjam#8:
// pages came back blank after switching away and back) // pages came back blank after switching away and back — the rows stayed
const tops = await rowLabelTops(page, ['F.Cu', 'B.Cu']); // in the DOM but their scrolled window collapsed and clip-pathed them
expect(tops['F.Cu'], 'F.Cu row visible after tab round-trip').not.toBeNull(); // all away, so this must assert real paint, not just DOM presence).
expect(tops['B.Cu'], 'B.Cu row visible after tab round-trip').not.toBeNull(); 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 }) => { test('layer list scrolls with the wheel and clips at the pane', async ({ page }) => {