Merge pull request #683 from MBombeck/fix/header-border-seam

Give the shell header band one owner

e2e/chrome-header-seam.spec.ts

@@ -0,0 +1,135 @@
+import { expect, test } from "./setup/test";
+
+import { STORAGE_STATE_PATH } from "./setup/global-setup";
+
+/**
+ * The bottom border of the top bar and the bottom border of the sidebar logo
+ * band form one horizontal line across the top of the app. The two elements
+ * sit in different columns of the shell flex row, so nothing in the layout
+ * makes them agree; both read `SHELL_HEADER_BAND`
+ * (`src/components/layout/shell-metrics.ts`) for the height AND the border,
+ * which is what keeps the line continuous.
+ *
+ * The structural half of that contract is asserted in
+ * `src/components/layout/__tests__/chrome-header-seam.test.tsx`. This spec
+ * asserts the painted result: same border bottom edge, same border width, at
+ * four widths spanning the sidebar's visible range, in both the expanded
+ * sidebar and the collapsed icon rail.
+ *
+ * The waits below settle on the two bands being VISIBLE and on the viewport
+ * having actually resized. Nothing here waits on a height or an offset, so a
+ * regression cannot hide behind the setup.
+ */
+
+// 768 px is the `md` breakpoint where the sidebar first appears; 1024 px is
+// `lg`, where an unset collapse preference flips the rail to icons. Both
+// boundaries and one width either side of them.
+const WIDTHS = [768, 900, 1024, 1280] as const;
+
+const SIDEBAR_COLLAPSED_KEY = "healthlog-sidebar-collapsed";
+
+type BandGeometry = {
+  bottom: number;
+  height: number;
+  borderBottomWidth: number;
+};
+
+test.describe("app-chrome header seam", () => {
+  test.use({ storageState: STORAGE_STATE_PATH });
+
+  test.beforeEach(({}, testInfo) => {
+    test.skip(
+      testInfo.project.name === "chromium-mobile",
+      "the global sidebar is desktop-only (md+), so below md there is no seam",
+    );
+  });
+
+  for (const collapsed of [false, true] as const) {
+    test(`top bar and sidebar borders share one line (${
+      collapsed ? "collapsed rail" : "expanded sidebar"
+    })`, async ({ page }) => {
+      // Pin the rail state instead of letting the viewport default decide, so
+      // each run covers both shapes at every width.
+      await page.addInitScript(
+        ([key, value]) => {
+          window.localStorage.setItem(key, value);
+        },
+        [SIDEBAR_COLLAPSED_KEY, String(collapsed)] as const,
+      );
+
+      await page.goto("/", { waitUntil: "domcontentloaded" });
+
+      const topBar = page.locator('[data-slot="top-bar"]');
+      const sidebarHeader = page.locator('[data-slot="sidebar-header"]');
+
+      for (const width of WIDTHS) {
+        await page.setViewportSize({ width, height: 900 });
+
+        // Settle on paint, not on geometry: both bands present and visible,
+        // the logo link inside the sidebar band rendered, and the viewport
+        // actually at the requested width.
+        await expect(topBar).toBeVisible();
+        await expect(sidebarHeader).toBeVisible();
+        await expect(sidebarHeader.locator("a[href='/']")).toBeVisible();
+        await expect
+          .poll(() => page.evaluate(() => window.innerWidth))
+          .toBe(width);
+
+        const seam = await page.evaluate(() => {
+          const read = (slot: string) => {
+            const el = document.querySelector(`[data-slot="${slot}"]`);
+            if (!el) return null;
+            const rect = el.getBoundingClientRect();
+            const style = getComputedStyle(el);
+            return {
+              bottom: rect.bottom,
+              height: rect.height,
+              borderBottomWidth: parseFloat(style.borderBottomWidth),
+            };
+          };
+          return {
+            topBar: read("top-bar"),
+            sidebar: read("sidebar-header"),
+          } as { topBar: BandGeometry | null; sidebar: BandGeometry | null };
+        });
+
+        const at = `at ${width}px (${collapsed ? "collapsed" : "expanded"})`;
+        expect(seam.topBar, `top bar missing ${at}`).not.toBeNull();
+        expect(seam.sidebar, `sidebar band missing ${at}`).not.toBeNull();
+        const top = seam.topBar!;
+        const side = seam.sidebar!;
+
+        // Two borders that do not exist also "line up". Prove each band
+        // actually paints a line, and paints it at the same weight, before
+        // comparing where the lines sit.
+        expect(
+          top.borderBottomWidth,
+          `top bar has no bottom border ${at}`,
+        ).toBeGreaterThan(0);
+        expect(
+          side.borderBottomWidth,
+          `sidebar band has no bottom border ${at}`,
+        ).toBeGreaterThan(0);
+        expect(
+          side.borderBottomWidth,
+          `the two borders are drawn at different weights ${at}`,
+        ).toBeCloseTo(top.borderBottomWidth, 1);
+
+        // The bands are collapsed neither to zero nor to a sliver.
+        expect(top.height, `top bar has no height ${at}`).toBeGreaterThan(32);
+        expect(side.height, `sidebar band has no height ${at}`).toBeGreaterThan(
+          32,
+        );
+
+        // The seam itself: the bottom border sits on the band's border-box
+        // bottom edge, so equal bottoms means one continuous line.
+        expect(
+          Math.abs(side.bottom - top.bottom),
+          `header seam steps by ${(side.bottom - top.bottom).toFixed(
+            2,
+          )}px ${at}`,
+        ).toBeLessThan(1);
+      }
+    });
+  }
+});

src/components/layout/__tests__/chrome-header-seam.test.tsx

@@ -0,0 +1,123 @@
+/**
+ * The top bar and the sidebar logo band draw one continuous line across the
+ * top of the app. They are siblings in different columns of the shell flex
+ * row, so nothing in the layout makes them agree: the only thing that does is
+ * that both read `SHELL_HEADER_BAND` for their height AND their bottom
+ * border.
+ *
+ * This suite asserts the structural property, which is stronger than "the two
+ * borders happen to land on the same y today": each band carries every token
+ * of the shared constant, and neither band declares a height of its own. A
+ * second height class on either element replaces the shared one through
+ * `twMerge`, so both halves of the assertion fire on the same mutation.
+ *
+ * The painted result is measured in `e2e/chrome-header-seam.spec.ts`.
+ */
+import { describe, expect, it, vi } from "vitest";
+import { renderToStaticMarkup } from "react-dom/server";
+
+vi.mock("next/navigation", () => ({
+  usePathname: () => "/",
+  useRouter: () => ({ push: vi.fn(), replace: vi.fn() }),
+}));
+
+vi.mock("@/hooks/use-auth", () => ({
+  useAuth: () => ({
+    user: {
+      id: "u1",
+      username: "testuser",
+      email: "user@example.com",
+      role: "USER" as const,
+      avatarUrl: null,
+      modules: {} as Record<string, boolean>,
+    },
+    isAuthenticated: true,
+    isLoading: false,
+    refetch: vi.fn(),
+  }),
+  useLogout: () => ({ mutate: vi.fn(), isPending: false }),
+}));
+
+vi.mock("@/components/providers", () => ({
+  useTheme: () => ({
+    theme: "system" as const,
+    resolvedTheme: "dark" as const,
+    setTheme: vi.fn(),
+  }),
+}));
+
+import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
+import { I18nProvider } from "@/lib/i18n/context";
+import { SHELL_HEADER_BAND } from "../shell-metrics";
+import { SidebarNav } from "../sidebar-nav";
+import { TopBar } from "../top-bar";
+
+function render(node: React.ReactElement): string {
+  return renderToStaticMarkup(
+    <QueryClientProvider client={new QueryClient()}>
+      <I18nProvider initialLocale="en">{node}</I18nProvider>
+    </QueryClientProvider>,
+  );
+}
+
+/**
+ * Pull the class list off the element carrying `data-slot="<slot>"`. The
+ * pattern spans the whole opening tag, so it holds whichever side of
+ * `data-slot` React emits the class attribute on.
+ */
+function classTokens(html: string, slot: string): string[] {
+  const tag = new RegExp(`<[a-z]+[^>]*\\bdata-slot="${slot}"[^>]*>`, "i").exec(
+    html,
+  );
+  expect(
+    tag,
+    `no element with data-slot="${slot}" in the rendered markup`,
+  ).not.toBeNull();
+  const cls = /\bclass="([^"]*)"/.exec(tag![0]);
+  expect(
+    cls,
+    `the data-slot="${slot}" element carries no class attribute`,
+  ).not.toBeNull();
+  return cls![1].split(/\s+/).filter(Boolean);
+}
+
+const isHeightToken = (t: string) =>
+  /^(?:(?:sm|md|lg|xl|2xl):)?(?:min-|max-)?h-/.test(t);
+
+const BAND_TOKENS = SHELL_HEADER_BAND.split(/\s+/).filter(Boolean);
+const BAND_HEIGHT_TOKENS = BAND_TOKENS.filter(isHeightToken);
+
+const BANDS: ReadonlyArray<[string, string, () => string]> = [
+  ["top bar", "top-bar", () => render(<TopBar />)],
+  ["sidebar logo band", "sidebar-header", () => render(<SidebarNav />)],
+];
+
+describe("app-chrome header seam", () => {
+  it("SHELL_HEADER_BAND declares both a height and a bottom border", () => {
+    // The whole point of the constant is that the border ships INSIDE the
+    // 4rem box. A constant that carried only the height would let the two
+    // call sites put the line on different elements again.
+    expect(BAND_HEIGHT_TOKENS.length).toBeGreaterThan(0);
+    expect(BAND_TOKENS).toContain("border-b");
+  });
+
+  for (const [label, slot, renderBand] of BANDS) {
+    it(`${label} takes its height and border from SHELL_HEADER_BAND`, () => {
+      const tokens = classTokens(renderBand(), slot);
+      for (const token of BAND_TOKENS) {
+        expect(
+          tokens,
+          `the ${label} dropped "${token}" from the shared band`,
+        ).toContain(token);
+      }
+    });
+
+    it(`${label} declares no height of its own`, () => {
+      const tokens = classTokens(renderBand(), slot);
+      expect(
+        tokens.filter(isHeightToken).sort(),
+        `the ${label} carries a height class that is not the shared band's`,
+      ).toEqual([...BAND_HEIGHT_TOKENS].sort());
+    });
+  }
+});

src/components/layout/shell-metrics.ts

@@ -27,3 +27,31 @@
  * scope for the shell-only over-scroll fix.)
  */
 export const SUB_SHELL_GRID_FLOOR = "md:min-h-[calc(100dvh-10.5rem)]";
+
+/**
+ * The app-chrome header band: one 4rem strip whose bottom border draws the
+ * single horizontal line across the top of the app.
+ *
+ * Two elements paint that line. The top bar (`top-bar.tsx`) draws it over the
+ * content column, the sidebar logo band (`sidebar-nav.tsx`) draws it over the
+ * rail, and the two live in different columns of the shell flex row, so
+ * nothing in the layout makes them agree. Each used to declare `h-16` on its
+ * own, and the two lines still landed a pixel apart: every box in the app is
+ * `border-box`, so `h-16 border-b` on ONE element puts the line inside the
+ * 4rem (63 to 64 px) while `h-16` on a child of a `border-b` wrapper puts it
+ * after the 4rem (64 to 65 px). Same number, different line.
+ *
+ * The height and the border therefore travel together, from here. The element
+ * that carries this class IS the band; neither call site restates a height,
+ * and anything nested inside stretches with `h-full`.
+ *
+ * Border COLOUR stays per-surface (`border-border` on the top bar,
+ * `border-sidebar-border` on the rail): the two chrome surfaces are tinted
+ * apart by design and only the geometry has to match.
+ *
+ * Guards: `chrome-header-seam.test.tsx` asserts both elements render this
+ * exact class and declare no height of their own;
+ * `e2e/chrome-header-seam.spec.ts` measures the two painted borders at four
+ * widths in both the expanded and the collapsed rail.
+ */
+export const SHELL_HEADER_BAND = "h-16 border-b";

src/components/layout/sidebar-nav.tsx

@@ -22,6 +22,7 @@ import {
   visibleNavDestinations,
   visibleUtilityDestinations,
 } from "@/components/layout/nav-model";
+import { SHELL_HEADER_BAND } from "@/components/layout/shell-metrics";
 import { cn } from "@/lib/utils";
 import { Logo } from "@/components/ui/logo";
 import { useAuth, useLogout } from "@/hooks/use-auth";
@@ -435,17 +436,22 @@ export function SidebarNav() {
           collapsed ? "w-16" : "w-64",
         )}
       >
-        {/* Logo */}
+        {/* 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 border-b",
+            "border-sidebar-border",
+            SHELL_HEADER_BAND,
             collapsed ? "px-3" : "px-6",
           )}
         >
           <Link
             href="/"
             className={cn(
-              "flex h-16 items-center",
+              "flex h-full items-center",
               collapsed ? "justify-center px-0" : "gap-2",
             )}
           >

src/components/layout/top-bar.tsx

@@ -11,6 +11,8 @@ import {
   Shield,
   Sun,
 } from "lucide-react";
+import { SHELL_HEADER_BAND } from "@/components/layout/shell-metrics";
+import { cn } from "@/lib/utils";
 import { Logo } from "@/components/ui/logo";
 import { Avatar, AvatarFallback, AvatarImage } from "@/components/ui/avatar";
 import { Skeleton } from "@/components/ui/skeleton";
@@ -56,12 +58,21 @@ export function TopBar() {
 
   return (
     <header
-      className="bg-card/80 border-border sticky top-0 z-40 flex h-16 items-center justify-between border-b px-4 backdrop-blur-md md:px-6"
+      data-slot="top-bar"
+      // Height + bottom border come from `SHELL_HEADER_BAND`, which the
+      // sidebar logo band reads too, so the two borders draw one
+      // continuous line. Never restate a height here.
+      className={cn(
+        "bg-card/80 border-border sticky top-0 z-40 flex items-center justify-between px-4 backdrop-blur-md md:px-6",
+        SHELL_HEADER_BAND,
+      )}
       // iOS PWA on notched iPhones overlays the status bar onto the
       // sticky header unless we reserve the safe-area inset. The
       // inline style adds `safe-area-inset-top` as padding-top on
       // devices that report one and is a no-op on every other
-      // platform; the inner h-16 content area stays nominally 64 px.
+      // platform. The band stays 4rem tall either way (border-box), so
+      // on a device that reports an inset the content area inside it
+      // gives up those pixels.
       style={{ paddingTop: "env(safe-area-inset-top, 0px)" }}
     >
       {/* Mobile logo */}