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.
2026-11-08 12:32:43 +00:00 · 2026-10-04 15:08:42 +02:00
1 parent 4cad43e4d5
commit 2e150e97cf
@@ -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);
+    }
+  });
+});
@@ -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("<SidebarNav> 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("<SidebarNav> width transition never overflows the nav", () => {
+  const KEY = "healthlog-sidebar-collapsed";
+
+  afterEach(() => {
+    vi.unstubAllGlobals();
+    mockMountedRef.value = false;
+  });
+
+  function asideClass(html: string): string {
+    return /<aside[^>]*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,
+    );
+  });
+});
@@ -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() {
      <aside
        aria-label={t("nav.sidebar")}
        className={cn(
-          "bg-sidebar border-sidebar-border relative hidden h-full flex-shrink-0 border-r transition-[width] duration-200 motion-reduce:transition-none md:flex md:flex-col",
+          "bg-sidebar border-sidebar-border relative hidden h-full flex-shrink-0 overflow-hidden border-r transition-[width] duration-200 motion-reduce:transition-none md:flex md:flex-col",
          collapsed ? "w-16" : "w-64",
        )}
      >
-        {/* Logo band. Height + bottom border come from `SHELL_HEADER_BAND`,
+        {/* The rail animates its width; the content does not. Expanded, this
+            column lays out at the final width from the first frame
+            (`SIDEBAR_EXPANDED_CONTENT_WIDTH`) and the rail's
+            `overflow-hidden` reveals it as the width grows. Laying the
+            labels out against the growing width instead wrapped every
+            multi-word label, pushed the list past the nav's height and its
+            rows past its width, and the nav's `overflow-y-auto` painted both
+            scrollbars for the length of the transition. Collapsing needs no
+            floor: the icon rows already fit the narrowing rail. */}
+        <div
+          data-slot="sidebar-column"
+          className={cn(
+            "flex h-full w-full flex-col",
+            !collapsed && SIDEBAR_EXPANDED_CONTENT_WIDTH,
+          )}
+        >
+          {/* Logo band. Height + bottom border come from `SHELL_HEADER_BAND`,
            which the top bar reads too, so the two borders draw one
            continuous line. The band owns the 4rem; the link inside
            stretches with `h-full` and never restates a height. */}
-        <div
-          data-slot="sidebar-header"
-          className={cn(
-            "border-sidebar-border",
-            SHELL_HEADER_BAND,
-            collapsed ? "px-3" : "px-6",
-          )}
-        >
-          <Link
-            href="/"
+          <div
+            data-slot="sidebar-header"
            className={cn(
-              "flex h-full items-center",
-              collapsed ? "justify-center px-0" : "gap-2",
+              "border-sidebar-border",
+              SHELL_HEADER_BAND,
+              collapsed ? "px-3" : "px-6",
            )}
          >
-            <Logo className="text-primary shrink-0" size={24} />
-            {!collapsed && (
-              <span className="text-lg font-bold tracking-tight">
-                HealthLog
-              </span>
-            )}
-          </Link>
-        </div>
-
-        <nav
-          aria-label={t("nav.mainNavigation")}
-          className={cn("flex-1 overflow-y-auto", collapsed ? "p-1.5" : "p-3")}
-        >
-          <div className="space-y-1">
-            {visibleNavItems.map((item) => {
-              const isActive = isNavDestinationActive(
-                item.href,
-                pathname,
-                visibleNavItems,
-              );
-              const label = t(item.tKey);
-
-              if (collapsed) {
-                return (
-                  <Tooltip key={item.href}>
-                    <TooltipTrigger asChild>
-                      <Link
-                        href={item.href}
-                        aria-current={isActive ? "page" : undefined}
-                        data-tour-id={item.tourId}
-                        {...(item.href === "/medications" ? medsIntent : {})}
-                        className={cn(
-                          "flex items-center justify-center rounded-lg p-2.5 transition-colors",
-                          isActive
-                            ? "bg-primary/10 text-primary"
-                            : "text-foreground hover:bg-accent",
-                        )}
-                      >
-                        <item.icon className="h-4 w-4" />
-                      </Link>
-                    </TooltipTrigger>
-                    <TooltipContent side="right" sideOffset={8}>
-                      {label}
-                    </TooltipContent>
-                  </Tooltip>
-                );
-              }
-
-              return (
-                <Link
-                  key={item.href}
-                  href={item.href}
-                  aria-current={isActive ? "page" : undefined}
-                  data-tour-id={item.tourId}
-                  {...(item.href === "/medications" ? medsIntent : {})}
-                  className={cn(
-                    "flex items-center gap-3 rounded-lg px-3 py-2.5 text-sm font-medium transition-colors",
-                    isActive
-                      ? "bg-primary/10 text-primary"
-                      : "text-foreground hover:bg-accent",
-                  )}
-                >
-                  <item.icon className="h-4 w-4" />
-                  {label}
-                </Link>
-              );
-            })}
+            <Link
+              href="/"
+              className={cn(
+                "flex h-full items-center",
+                collapsed ? "justify-center px-0" : "gap-2",
+              )}
+            >
+              <Logo className="text-primary shrink-0" size={24} />
+              {!collapsed && (
+                <span className="text-lg font-bold tracking-tight">
+                  HealthLog
+                </span>
+              )}
+            </Link>
          </div>
-        </nav>

-        {/* Bottom utility links — the shared utility tail (minus
+          <nav
+            aria-label={t("nav.mainNavigation")}
+            className={cn(
+              "flex-1 overflow-y-auto",
+              collapsed ? "p-1.5" : "p-3",
+            )}
+          >
+            <div className="space-y-1">
+              {visibleNavItems.map((item) => {
+                const isActive = isNavDestinationActive(
+                  item.href,
+                  pathname,
+                  visibleNavItems,
+                );
+                const label = t(item.tKey);
+
+                if (collapsed) {
+                  return (
+                    <Tooltip key={item.href}>
+                      <TooltipTrigger asChild>
+                        <Link
+                          href={item.href}
+                          aria-current={isActive ? "page" : undefined}
+                          data-tour-id={item.tourId}
+                          {...(item.href === "/medications" ? medsIntent : {})}
+                          className={cn(
+                            "flex items-center justify-center rounded-lg p-2.5 transition-colors",
+                            isActive
+                              ? "bg-primary/10 text-primary"
+                              : "text-foreground hover:bg-accent",
+                          )}
+                        >
+                          <item.icon className="h-4 w-4" />
+                        </Link>
+                      </TooltipTrigger>
+                      <TooltipContent side="right" sideOffset={8}>
+                        {label}
+                      </TooltipContent>
+                    </Tooltip>
+                  );
+                }
+
+                return (
+                  <Link
+                    key={item.href}
+                    href={item.href}
+                    aria-current={isActive ? "page" : undefined}
+                    data-tour-id={item.tourId}
+                    {...(item.href === "/medications" ? medsIntent : {})}
+                    className={cn(
+                      "flex items-center gap-3 rounded-lg px-3 py-2.5 text-sm font-medium transition-colors",
+                      isActive
+                        ? "bg-primary/10 text-primary"
+                        : "text-foreground hover:bg-accent",
+                    )}
+                  >
+                    <item.icon className="h-4 w-4" />
+                    {label}
+                  </Link>
+                );
+              })}
+            </div>
+          </nav>
+
+          {/* Bottom utility links — the shared utility tail (minus
            Notifications, which lives in the avatar menu) with the
            role-gated Admin entry inserted before Settings. The collapse
            control heads the group, so it sits directly above Admin for an
            administrator and directly above Settings for everyone else. */}
-        <div className={cn("space-y-1 pb-1", collapsed ? "px-1.5" : "px-3")}>
-          <SidebarCollapseToggle
-            collapsed={collapsed}
-            onToggle={toggleCollapsed}
-          />
-          {footerUtilityItems
-            .filter((item) => !isSettingsUtilityDestination(item))
-            .map((item) => renderUtilityLink(item))}
-          {renderAdminLink()}
-          {footerUtilityItems
-            .filter((item) => isSettingsUtilityDestination(item))
-            .map((item) => renderUtilityLink(item))}
-        </div>
+          <div className={cn("space-y-1 pb-1", collapsed ? "px-1.5" : "px-3")}>
+            <SidebarCollapseToggle
+              collapsed={collapsed}
+              onToggle={toggleCollapsed}
+            />
+            {footerUtilityItems
+              .filter((item) => !isSettingsUtilityDestination(item))
+              .map((item) => renderUtilityLink(item))}
+            {renderAdminLink()}
+            {footerUtilityItems
+              .filter((item) => isSettingsUtilityDestination(item))
+              .map((item) => renderUtilityLink(item))}
+          </div>

-        <SidebarUserSection collapsed={collapsed} />
+          <SidebarUserSection collapsed={collapsed} />
+        </div>
      </aside>
    </TooltipProvider>
  );