mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 04:50:59 +00:00
fix(admin): suppress painted scrollbar on mobile section strip
Production probe at /admin/api-tokens on a Pixel-5 viewport pinned the
scrollbar that kept showing up after two earlier fixes: it lives in
the AdminShell mobile section strip (13 entries, scrollWidth ~1700 vs
clientWidth ~360), not in the api-tokens table. The strip sits right
above the api-tokens card, which is why every previous attempt
targeted the table while the bar stayed put.
Adds a `no-scrollbar` utility in globals.css (scrollbar-width: none +
::-webkit-scrollbar { display: none }) and applies it to the strip in
both AdminShell and SettingsShell. Swipe and keyboard-arrow scrolling
keep working; the painted bar simply doesn't draw.
New AdminShell unit suite locks in the class so a future refactor
can't silently regress it; settings-shell test gets the matching
assertion. The api-tokens e2e spec gains a regression check that the
strip declares `no-scrollbar` and overflow-x-auto together.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
15c852f48e
commit
3e1607471b
6 files changed
+197
-4
No files matched your search
@@ -74,4 +74,48 @@ test.describe("/admin/api-tokens mobile no-overflow", () => {
|
||||
`scrollWidth=${dims.scrollWidth}, innerWidth=${dims.innerWidth}`,
|
||||
).toBeLessThanOrEqual(dims.innerWidth + 1);
|
||||
});
|
||||
|
||||
// v1.4.18 phase A2 — production probe at Pixel-5 viewport pinned the
|
||||
// visible "scrollbar" Marc reported for the third time as the
|
||||
// `<AdminShell>` mobile section strip, NOT the api-tokens table. The
|
||||
// strip carries 13 admin sections so its scrollWidth (~1700 CSS px)
|
||||
// permanently exceeds clientWidth (~360 CSS px). Adding the
|
||||
// `no-scrollbar` utility class (`globals.css`) suppresses the
|
||||
// painted bar while preserving swipe + keyboard-arrow scrolling.
|
||||
// This regression guard fails if a future refactor drops the class.
|
||||
test("Pixel-5 viewport: admin section strip suppresses its painted scrollbar", async ({
|
||||
page,
|
||||
}) => {
|
||||
await page.route("**/api/admin/tokens", (route) =>
|
||||
route.fulfill({
|
||||
status: 200,
|
||||
contentType: "application/json",
|
||||
body: JSON.stringify({ data: [], error: null }),
|
||||
}),
|
||||
);
|
||||
|
||||
await page.goto("/admin/api-tokens", { waitUntil: "domcontentloaded" });
|
||||
await page.waitForLoadState("networkidle");
|
||||
|
||||
// Find the mobile strip (`<nav aria-label>` matches both English
|
||||
// "Admin sections" and German "Admin-Bereiche").
|
||||
const strip = page.locator("nav.no-scrollbar.overflow-x-auto").first();
|
||||
await expect(strip).toBeAttached();
|
||||
|
||||
// The strip itself must declare `no-scrollbar` so the painted
|
||||
// horizontal scrollbar — which production showed at Pixel-5 — is
|
||||
// suppressed.
|
||||
const cls = await strip.getAttribute("class");
|
||||
expect(cls ?? "").toContain("no-scrollbar");
|
||||
expect(cls ?? "").toContain("overflow-x-auto");
|
||||
|
||||
// And the strip must still carry useful overflow internally
|
||||
// (clientWidth < scrollWidth), proving scroll behaviour isn't
|
||||
// disabled — only the bar is invisible.
|
||||
const stripDims = await strip.evaluate((el) => ({
|
||||
scrollWidth: (el as HTMLElement).scrollWidth,
|
||||
clientWidth: (el as HTMLElement).clientWidth,
|
||||
}));
|
||||
expect(stripDims.scrollWidth).toBeGreaterThan(stripDims.clientWidth);
|
||||
});
|
||||
});
|
||||
@@ -199,6 +199,29 @@
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* `.no-scrollbar` — keeps a container scrollable but hides the visible
|
||||
* bar. Used by the mobile section strips in `<AdminShell>` and
|
||||
* `<SettingsShell>` so 13 admin / 10 settings entries can swipe
|
||||
* horizontally without painting an always-on scrollbar at the top of
|
||||
* every admin/settings page (a recurring Marc complaint — most
|
||||
* recently mistaken for a `/admin/api-tokens` table scrollbar in
|
||||
* v1.4.16/17 because the strip sits right above the api-tokens card).
|
||||
*
|
||||
* Standard `scrollbar-width: none` (Firefox) + `::-webkit-scrollbar`
|
||||
* (Chromium/WebKit) covers every browser HealthLog targets. Touch
|
||||
* scrolling, keyboard arrow scrolling, and `Tab` focus traversal all
|
||||
* keep working — this is a purely cosmetic suppression, not an
|
||||
* `overflow: hidden`.
|
||||
*/
|
||||
.no-scrollbar {
|
||||
scrollbar-width: none;
|
||||
-ms-overflow-style: none;
|
||||
}
|
||||
.no-scrollbar::-webkit-scrollbar {
|
||||
display: none;
|
||||
}
|
||||
|
||||
/* ── Insight card animations ─────────────────────── */
|
||||
@keyframes insight-fade-in {
|
||||
from {
|
||||
|
||||
@@ -0,0 +1,100 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { renderToStaticMarkup } from "react-dom/server";
|
||||
|
||||
/**
|
||||
* `<AdminShell>` mirrors `<SettingsShell>`: a sticky desktop sidebar
|
||||
* plus a horizontal section strip on mobile. With 13 sections the strip
|
||||
* is always wider than a 393 CSS-px (Pixel-5) viewport, so v1.4.18
|
||||
* phase A2 added the `no-scrollbar` class to the strip — Chromium /
|
||||
* WebKit otherwise paint an always-on horizontal scrollbar at the top
|
||||
* of every admin page, which Marc kept mis-attributing to the
|
||||
* `/admin/api-tokens` table card right below it. Three previous fix
|
||||
* attempts (v1.4.15 column-hide, v1.4.16 mobile-card-list, then this
|
||||
* one) all targeted the table while the bar lived in the shell.
|
||||
*
|
||||
* This suite locks in the shell-level fix so a future refactor can't
|
||||
* silently regress it.
|
||||
*/
|
||||
|
||||
const mockPathnameRef = { value: "/admin/system-status" };
|
||||
vi.mock("next/navigation", () => ({
|
||||
usePathname: () => mockPathnameRef.value,
|
||||
}));
|
||||
|
||||
import { I18nProvider } from "@/lib/i18n/context";
|
||||
import {
|
||||
ADMIN_SECTIONS,
|
||||
AdminShell,
|
||||
} from "../admin-shell";
|
||||
|
||||
function renderShell(props: {
|
||||
active?: (typeof ADMIN_SECTIONS)[number]["slug"];
|
||||
pathname?: string;
|
||||
}) {
|
||||
mockPathnameRef.value = props.pathname ?? "/admin/system-status";
|
||||
return renderToStaticMarkup(
|
||||
<I18nProvider initialLocale="en">
|
||||
<AdminShell active={props.active}>
|
||||
<div>section body</div>
|
||||
</AdminShell>
|
||||
</I18nProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
describe("<AdminShell>", () => {
|
||||
it("renders every section link in both the mobile strip and the desktop sidebar", () => {
|
||||
const html = renderShell({ active: "system-status" });
|
||||
for (const section of ADMIN_SECTIONS) {
|
||||
const matches = html.match(
|
||||
new RegExp(`href="/admin/${section.slug}"`, "g"),
|
||||
);
|
||||
// Two renders — mobile strip + desktop sidebar.
|
||||
expect(matches?.length ?? 0).toBe(2);
|
||||
}
|
||||
});
|
||||
|
||||
it("mobile section strip uses `no-scrollbar` so the swipe area doesn't paint a horizontal scrollbar", () => {
|
||||
// The 13-section strip is ~1700 CSS px wide. Without
|
||||
// `no-scrollbar` (defined in `globals.css`) Chromium/WebKit paint
|
||||
// an always-on horizontal scrollbar at the top of every admin
|
||||
// page. Production probe at https://healthlog.bombeck.io/admin/
|
||||
// api-tokens at Pixel-5 viewport pins this as the *only* element
|
||||
// with `overflow-x:auto AND scrollWidth > clientWidth`, so the
|
||||
// class is the load-bearing fix.
|
||||
const html = renderShell({ active: "api-tokens" });
|
||||
const nav = html.match(/<nav\b[^>]*md:hidden[^>]*>/);
|
||||
expect(nav).not.toBeNull();
|
||||
expect(nav![0]).toMatch(/\bno-scrollbar\b/);
|
||||
// Scroll behaviour itself is preserved — the strip still uses
|
||||
// `overflow-x-auto` so swipe / keyboard-arrow scrolling continues
|
||||
// to work, only the painted bar is suppressed.
|
||||
expect(nav![0]).toMatch(/\boverflow-x-auto\b/);
|
||||
});
|
||||
|
||||
it('marks the active section with aria-current="page" in both layouts', () => {
|
||||
const html = renderShell({ active: "api-tokens" });
|
||||
const occurrences = html.match(/aria-current="page"/g);
|
||||
// Two layouts (mobile strip + desktop sidebar) → two matches.
|
||||
expect(occurrences?.length ?? 0).toBe(2);
|
||||
// Active link is on the api-tokens href in both renders.
|
||||
const activeLinkRegex =
|
||||
/<a\b[^>]*\baria-current="page"[^>]*\bhref="\/admin\/api-tokens"|<a\b[^>]*\bhref="\/admin\/api-tokens"[^>]*\baria-current="page"/g;
|
||||
expect(html.match(activeLinkRegex)?.length ?? 0).toBe(2);
|
||||
});
|
||||
|
||||
it("derives the active slug from the current pathname when `active` is omitted", () => {
|
||||
const html = renderShell({ pathname: "/admin/api-tokens" });
|
||||
const activeLinkRegex =
|
||||
/<a\b[^>]*\baria-current="page"[^>]*\bhref="\/admin\/api-tokens"|<a\b[^>]*\bhref="\/admin\/api-tokens"[^>]*\baria-current="page"/g;
|
||||
expect(html.match(activeLinkRegex)?.length ?? 0).toBe(2);
|
||||
});
|
||||
|
||||
it("does NOT mark any section active on `/admin` overview", () => {
|
||||
const html = renderShell({ pathname: "/admin" });
|
||||
// Overview-link only — no section gets aria-current.
|
||||
const sectionActive = html.match(
|
||||
/aria-current="page"[^>]*\bhref="\/admin\/[^"]+"/g,
|
||||
);
|
||||
expect(sectionActive?.length ?? 0).toBe(0);
|
||||
});
|
||||
});
|
||||
@@ -148,10 +148,16 @@ export function AdminShell({ active, children }: AdminShellProps) {
|
||||
|
||||
return (
|
||||
<div className="mx-auto w-full max-w-screen-xl px-4 py-6 md:px-6 md:py-8">
|
||||
{/* Mobile section strip — horizontal scroll, hidden on md+. */}
|
||||
{/* Mobile section strip — horizontal scroll, hidden on md+.
|
||||
`no-scrollbar` (defined in `globals.css`) suppresses the
|
||||
painted scrollbar at the top of every admin page; the
|
||||
horizontal swipe + keyboard arrow scrolling still work, the
|
||||
bar just doesn't draw. Without this the 13-section strip
|
||||
would render an always-on scrollbar that Marc kept
|
||||
mis-attributing to the api-tokens table below it. */}
|
||||
<nav
|
||||
aria-label={t("admin.shell.sectionsNav")}
|
||||
className="-mx-4 mb-4 overflow-x-auto px-4 md:hidden"
|
||||
className="no-scrollbar -mx-4 mb-4 overflow-x-auto px-4 md:hidden"
|
||||
>
|
||||
<ul className="flex min-w-max gap-2">
|
||||
{ADMIN_SECTIONS.map((section) => {
|
||||
|
||||
@@ -166,4 +166,18 @@ describe("<SettingsShell>", () => {
|
||||
const html = renderShell({ active: "account", locale: "en" });
|
||||
expect(html).not.toContain("settings.sections.");
|
||||
});
|
||||
|
||||
it("mobile section strip uses `no-scrollbar` so the swipe area doesn't paint a horizontal scrollbar", () => {
|
||||
// The 10-section strip is wider than 393 CSS px, so without
|
||||
// `no-scrollbar` Chromium/WebKit paints an always-on horizontal
|
||||
// scrollbar at the top of every settings page. The class is
|
||||
// defined in `globals.css` and combines `scrollbar-width: none`
|
||||
// (Firefox) with `::-webkit-scrollbar { display: none }` (everyone
|
||||
// else) — scroll behaviour is preserved.
|
||||
const html = renderShell({ active: "account" });
|
||||
const nav = html.match(/<nav\b[^>]*md:hidden[^>]*>/);
|
||||
expect(nav).not.toBeNull();
|
||||
expect(nav![0]).toMatch(/\bno-scrollbar\b/);
|
||||
expect(nav![0]).toMatch(/\boverflow-x-auto\b/);
|
||||
});
|
||||
});
|
||||
@@ -131,10 +131,16 @@ export function SettingsShell({ active, children }: SettingsShellProps) {
|
||||
|
||||
return (
|
||||
<div className="mx-auto w-full max-w-screen-xl px-4 py-6 md:px-6 md:py-8">
|
||||
{/* Mobile section strip — horizontal scroll, hidden on md+. */}
|
||||
{/* Mobile section strip — horizontal scroll, hidden on md+.
|
||||
`no-scrollbar` (defined in `globals.css`) suppresses the
|
||||
painted scrollbar; the horizontal swipe + keyboard arrow
|
||||
scrolling still work. Without it the 10-section strip
|
||||
renders an always-on scrollbar at the top of every settings
|
||||
page, which makes the page feel like every section card has
|
||||
an overflow problem. */}
|
||||
<nav
|
||||
aria-label={t("settings.shell.sectionsNav")}
|
||||
className="-mx-4 mb-4 overflow-x-auto px-4 md:hidden"
|
||||
className="no-scrollbar -mx-4 mb-4 overflow-x-auto px-4 md:hidden"
|
||||
>
|
||||
<ul className="flex min-w-max gap-2">
|
||||
{SETTINGS_SECTIONS.map((section) => {
|
||||
|
||||
Reference in new issue
Block a user