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() { );