From a4bb2adcb204fc2a04df86b3a02addded0f362ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 30 Jul 2026 14:35:08 +0200 Subject: [PATCH] fix(e2e): select the measured wrapper by data-slot, not by birth order The navigation-geometry spec failed on a merge commit whose tree was byte-identical to the branch head that had passed minutes earlier, so nothing had regressed. The spec resolved the element it measures with `main.firstElementChild`, independently in each of its two `page.evaluate` calls. Any node mounting ahead of the wrapper between the two reads made it measure two different elements, which is why the failure was 64 against 131.92 rather than a two-pixel drift. A timing problem gives a small difference or a null; this was a substitution. The shell's content wrapper now carries a stable `data-slot`, both reads select it by that, and the wait gates on the element rather than on scroll state alone. The same positional pattern in the overscroll spec is fixed with it, so the class is gone from the measurement paths rather than from one spec. Re-running the job made it green, which is a workaround. This is the fix. --- e2e/settings-overscroll.spec.ts | 14 +++++- ...341-navigation-and-insights-mobile.spec.ts | 50 +++++++++++++------ src/components/layout/auth-shell.tsx | 5 +- 3 files changed, 51 insertions(+), 18 deletions(-) diff --git a/e2e/settings-overscroll.spec.ts b/e2e/settings-overscroll.spec.ts index dc7ab9ec3..ecc4d8434 100644 --- a/e2e/settings-overscroll.spec.ts +++ b/e2e/settings-overscroll.spec.ts @@ -68,12 +68,22 @@ test.describe("settings + admin vertical over-scroll guard", () => { // long when the page is ready and too short on a loaded runner, and // it never says what it is waiting for. Gate on the element the // measurement reads instead. - await settleBeforeMeasure(page, page.locator("#main-content")); + const wrapperLocator = page.locator( + '#main-content [data-slot="main-content-wrapper"]', + ); + await settleBeforeMeasure(page, wrapperLocator); const dims = await page.evaluate(() => { const main = document.getElementById("main-content"); if (!main) return null; - const wrapper = main.firstElementChild as HTMLElement | null; + // Selected by the stable `data-slot` (auth-shell.tsx), not by + // birth order: `firstElementChild` would resolve to whatever + // lazily-mounted sibling (portal, toast, hydrating region) landed + // first, silently measuring the wrong subtree instead of the + // real content wrapper. + const wrapper = main.querySelector( + '[data-slot="main-content-wrapper"]', + ); if (!wrapper) return null; // Lowest content edge: max CONTENT-box bottom over the wrapper's // visible, non-fixed descendants, in the scroll container's diff --git a/e2e/v1341-navigation-and-insights-mobile.spec.ts b/e2e/v1341-navigation-and-insights-mobile.spec.ts index 2c446c677..f01782f7d 100644 --- a/e2e/v1341-navigation-and-insights-mobile.spec.ts +++ b/e2e/v1341-navigation-and-insights-mobile.spec.ts @@ -21,25 +21,38 @@ test.describe("v1.34.1 navigation geometry", () => { const heading = page.getByRole("heading", { level: 1 }); const sidebar = page.locator("aside").first(); + // The wrapper carries a stable `data-slot` (auth-shell.tsx) so both + // reads below measure the same DOM node instead of resolving + // `firstElementChild` twice — a lazily-mounted sibling (portal, + // toast, hydrating region) landing ahead of it between the two + // reads used to substitute a different element into the second + // measurement, producing an unrelated number rather than a + // drifted one. Resolving the handle once removes the second + // resolution step entirely. + const wrapper = page.locator('[data-slot="main-content-wrapper"]'); await settleBeforeMeasure(page, heading); await expect(sidebar).toBeVisible(); + await expect(wrapper).toBeVisible(); + const wrapperHandle = await wrapper.elementHandle(); + if (!wrapperHandle) { + throw new Error("main content wrapper not found"); + } - const before = await page.evaluate(() => { + const before = await page.evaluate((wrapperEl) => { const main = document.getElementById("main-content"); const heading = main?.querySelector("h1"); const aside = main?.querySelector("aside"); - const wrapper = main?.firstElementChild; - if (!main || !heading || !aside || !wrapper) return null; + if (!main || !heading || !aside || !wrapperEl) return null; return { headingTop: heading.getBoundingClientRect().top, asideTop: aside.getBoundingClientRect().top, - wrapperLeft: wrapper.getBoundingClientRect().left, + wrapperLeft: wrapperEl.getBoundingClientRect().left, mainWidth: main.clientWidth, documentScrollable: document.documentElement.scrollHeight > document.documentElement.clientHeight + 1, }; - }); + }, wrapperHandle); expect(before).not.toBeNull(); if (!before) return; @@ -51,26 +64,33 @@ test.describe("v1.34.1 navigation geometry", () => { await page.locator("#main-content").evaluate((main) => { main.scrollTop = Math.min(320, main.scrollHeight - main.clientHeight); }); - await page.waitForFunction(() => { + // Gate on the wrapper being present and laid out (not just on + // scroll state) — the measurement below reads it, so the wait + // must cover everything that measurement depends on. + await page.waitForFunction((wrapperEl) => { const main = document.getElementById("main-content"); + if (!main) return true; + const wrapperReady = + !!wrapperEl && + wrapperEl.isConnected && + wrapperEl.getBoundingClientRect().width > 0; return ( - !main || - main.scrollTop > 0 || - main.scrollHeight <= main.clientHeight + wrapperReady && + (main.scrollTop > 0 || main.scrollHeight <= main.clientHeight) ); - }); + }, wrapperHandle); - const after = await page.evaluate(() => { + const after = await page.evaluate((wrapperEl) => { const main = document.getElementById("main-content"); const aside = main?.querySelector("aside"); - const wrapper = main?.firstElementChild; - if (!main || !aside || !wrapper) return null; + if (!main || !aside || !wrapperEl) return null; return { asideTop: aside.getBoundingClientRect().top, - wrapperLeft: wrapper.getBoundingClientRect().left, + wrapperLeft: wrapperEl.getBoundingClientRect().left, mainWidth: main.clientWidth, }; - }); + }, wrapperHandle); + await wrapperHandle.dispose(); expect(after).not.toBeNull(); if (!after) return; diff --git a/src/components/layout/auth-shell.tsx b/src/components/layout/auth-shell.tsx index ab3b440ee..5b1d8ecc9 100644 --- a/src/components/layout/auth-shell.tsx +++ b/src/components/layout/auth-shell.tsx @@ -335,7 +335,10 @@ export function AuthShell({ the desktop bottom-6 anchor), so the last line of content can always scroll out from under the floating button. */} -
+
{children}