From 2e150e97cf401c2d392fef8c9896dec4efce9c74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 4 Oct 2026 15:08:42 +0200 Subject: [PATCH] fix(nav): stop the sidebar flashing scrollbars while it expands The rail animates its width from w-16 to w-64, and the labels laid out against the growing width: every row overran the nav and multi-word labels wrapped, so the nav's overflow-y-auto painted a vertical and a horizontal scrollbar for the length of the transition. Lay the expanded column out at its final width from the first frame and let the rail clip it. The final layout is unchanged; the column fills the expanded rail exactly. A unit test pins the structure and an e2e spec samples the nav on every frame of a real expand and collapse. --- e2e/sidebar-expand-no-scrollbar.spec.ts | 89 +++++++ .../layout/__tests__/sidebar-nav.test.tsx | 61 +++++ src/components/layout/sidebar-nav.tsx | 222 ++++++++++-------- 3 files changed, 275 insertions(+), 97 deletions(-) create mode 100644 e2e/sidebar-expand-no-scrollbar.spec.ts diff --git a/e2e/sidebar-expand-no-scrollbar.spec.ts b/e2e/sidebar-expand-no-scrollbar.spec.ts new file mode 100644 index 000000000..c8bfe58df --- /dev/null +++ b/e2e/sidebar-expand-no-scrollbar.spec.ts @@ -0,0 +1,89 @@ +import { expect, test } from "./setup/test"; + +import { STORAGE_STATE_PATH } from "./setup/global-setup"; + +/** + * Expanding the sidebar must never flash a scrollbar on the nav list. + * + * The rail animates `width` from `w-16` to `w-64`. When the labels laid out + * against the growing width, every row overran the 48 px nav and the + * multi-word labels wrapped, so the nav's `overflow-y-auto` painted a + * vertical and a horizontal scrollbar until the transition ended. The fix + * lays the column out at its final width from the first frame and lets the + * rail clip it (see `sidebar-column` in `src/components/layout/sidebar-nav.tsx`). + * + * This samples the nav on every animation frame of a real expand and a real + * collapse, so it fails on the overflow itself rather than on a class name. + * Desktop only: the sidebar is hidden below `md`. + */ +test.describe("sidebar width transition", () => { + test.use({ storageState: STORAGE_STATE_PATH }); + + test.beforeEach(({}, testInfo) => { + test.skip( + testInfo.project.name === "chromium-mobile", + "global sidebar is desktop-only (md+)", + ); + }); + + test("the nav list never overflows while the rail expands or collapses", async ({ + page, + }) => { + await page.goto("/", { waitUntil: "domcontentloaded" }); + const toggle = page.locator('[data-slot="sidebar-collapse-toggle"]'); + await expect(toggle).toBeVisible(); + const nav = page.locator("aside nav").first(); + await expect(nav).toBeVisible(); + + async function sampleToggle() { + const heightBefore = await nav.evaluate((el) => el.scrollHeight); + await page.evaluate(() => { + const el = document.querySelector("aside nav") as HTMLElement; + const samples: { sw: number; cw: number; sh: number }[] = []; + (window as unknown as { __navSamples: typeof samples }).__navSamples = + samples; + const t0 = performance.now(); + const tick = () => { + samples.push({ + sw: el.scrollWidth, + cw: el.clientWidth, + sh: el.scrollHeight, + }); + if (performance.now() - t0 < 600) requestAnimationFrame(tick); + }; + requestAnimationFrame(tick); + }); + await toggle.click(); + await page.waitForTimeout(700); + const samples = await page.evaluate( + () => + ( + window as unknown as { + __navSamples: { sw: number; cw: number; sh: number }[]; + } + ).__navSamples, + ); + const heightAfter = await nav.evaluate((el) => el.scrollHeight); + // Expanded rows are taller than rail rows, so the list may legitimately + // be either rest height while the width moves, never more. + return { samples, restHeight: Math.max(heightBefore, heightAfter) }; + } + + // Two round trips, so the run covers both directions whichever state + // the stored preference started in. + for (let i = 0; i < 4; i++) { + const { samples, restHeight } = await sampleToggle(); + // A 200 ms transition yields several frames; too few means the + // sampler never ran and the assertions below would prove nothing. + expect(samples.length).toBeGreaterThan(5); + for (const s of samples) { + // No horizontal overflow: no row is wider than the nav. + expect(s.sw).toBeLessThanOrEqual(s.cw + 1); + } + // No vertical growth: no label wraps mid-transition, so the list is + // never taller than at rest and cannot summon a scrollbar. + const tallest = Math.max(...samples.map((s) => s.sh)); + expect(tallest).toBeLessThanOrEqual(restHeight + 1); + } + }); +}); diff --git a/src/components/layout/__tests__/sidebar-nav.test.tsx b/src/components/layout/__tests__/sidebar-nav.test.tsx index a0dd34873..cbc5566a8 100644 --- a/src/components/layout/__tests__/sidebar-nav.test.tsx +++ b/src/components/layout/__tests__/sidebar-nav.test.tsx @@ -57,6 +57,7 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { I18nProvider } from "@/lib/i18n/context"; import { TooltipProvider } from "@/components/ui/tooltip"; import { + SIDEBAR_EXPANDED_CONTENT_WIDTH, SidebarCollapseToggle, SidebarNav, readSidebarCollapsedPref, @@ -498,3 +499,63 @@ describe(" collapse preference persistence", () => { expect(html.indexOf('href="/admin"')).toBeGreaterThan(toggle); }); }); + +/** + * The rail animates `width` between `w-16` and `w-64`. Expanding used to lay + * the labels out against the growing width: multi-word labels wrapped, the + * rows overran the nav, and its `overflow-y-auto` painted a vertical and a + * horizontal scrollbar for the length of the transition. The content now + * lays out at its final width from the first frame and the rail clips it, so + * nothing inside can overflow a scroll container while the width moves. + */ +describe(" width transition never overflows the nav", () => { + const KEY = "healthlog-sidebar-collapsed"; + + afterEach(() => { + vi.unstubAllGlobals(); + mockMountedRef.value = false; + }); + + function asideClass(html: string): string { + return /]*class="([^"]*)"/.exec(html)?.[1] ?? ""; + } + function columnClass(html: string): string { + return ( + /data-slot="sidebar-column"[^>]*class="([^"]*)"|class="([^"]*)"[^>]*data-slot="sidebar-column"/ + .exec(html) + ?.slice(1) + .find(Boolean) ?? "" + ); + } + + it("clips the content to the animating rail", () => { + const aside = asideClass(render()); + expect(aside).toContain("transition-[width]"); + expect(aside.split(" ")).toContain("overflow-hidden"); + }); + + it("lays the expanded column out at the expanded rail's content width", () => { + const column = columnClass(render()); + expect(column.split(" ")).toContain(SIDEBAR_EXPANDED_CONTENT_WIDTH); + // `w-64` minus the rail's 1 px `border-r`: a wider floor would push the + // right padding under the clip, a narrower one would let the labels + // re-flow while the width grows. + expect(SIDEBAR_EXPANDED_CONTENT_WIDTH).toBe("min-w-[calc(16rem-1px)]"); + expect(asideClass(render()).split(" ")).toContain("border-r"); + }); + + it("drops the floor on the collapsed rail, so the icons follow the narrowing width", () => { + const store = new Map([[KEY, "true"]]); + vi.stubGlobal("window", globalThis); + vi.stubGlobal("localStorage", { + getItem: (k: string) => store.get(k) ?? null, + setItem: (k: string, v: string) => void store.set(k, v), + }); + mockMountedRef.value = true; + const html = render(); + expect(asideClass(html).split(" ")).toContain("w-16"); + expect(columnClass(html).split(" ")).not.toContain( + SIDEBAR_EXPANDED_CONTENT_WIDTH, + ); + }); +}); diff --git a/src/components/layout/sidebar-nav.tsx b/src/components/layout/sidebar-nav.tsx index b0c6baae4..1e8fe6969 100644 --- a/src/components/layout/sidebar-nav.tsx +++ b/src/components/layout/sidebar-nav.tsx @@ -54,6 +54,14 @@ import { const SIDEBAR_COLLAPSED_STORAGE_KEY = "healthlog-sidebar-collapsed"; +/** + * The expanded sidebar's content width: `w-64` minus the rail's 1 px + * `border-r`, so the column fills the expanded rail exactly. Applied as a + * floor while expanded, it keeps the content at its final layout during the + * width transition (see the `sidebar-column` comment in the render). + */ +export const SIDEBAR_EXPANDED_CONTENT_WIDTH = "min-w-[calc(16rem-1px)]"; + /** * The stored collapse choice: `true` / `false`, or `null` when this browser * has none (or storage is unavailable), which lets the viewport default decide. @@ -554,119 +562,139 @@ export function SidebarNav() { );