diff --git a/e2e/settings-admin-one-piece.spec.ts b/e2e/settings-admin-one-piece.spec.ts new file mode 100644 index 000000000..99148668e --- /dev/null +++ b/e2e/settings-admin-one-piece.spec.ts @@ -0,0 +1,329 @@ +import { expect, test } from "./setup/test"; + +import { STORAGE_STATE_PATH } from "./setup/global-setup"; +import { settleBeforeMeasure } from "./utils/settle"; + +/** + * Settings + Admin read as one surface — measured, not eyeballed. + * + * Three claims, each with a number behind it: + * + * 1. **No jump on a route switch.** Every settings and admin route puts its + * heading at the SAME distance from the top of the scroll container, on + * both viewports. This is the U3 complaint stated as a measurement: the + * shells hand-rolled the heading, one page used `text-xs sm:text-sm` for + * the explainer and another `text-sm`, so the first card started at a + * different y depending on where you came from. + * 2. **One card rhythm.** Inside a settings/admin card, the header's bottom + * and the first body block's top sit exactly one gap apart — the card's + * own gap, `gap-4` under `md` and `gap-6` above it. Three call-site + * techniques used to produce three values here. + * 3. **One reading edge.** Every card body starts at the card's inner edge. + * The `pl-7` gutter put some bodies 28 px in and some at the edge, on the + * same page, one card apart. + * + * Runs in EN and DE, at desktop and phone widths. German runs 15-25% longer, + * and an explainer that wraps in German where English fits is the same defect + * the one-sentence rule exists to prevent — so at desktop width a wrap is a + * failure in either language. On a phone it is reported, not asserted: a + * 70-character sentence needs about 380 px of column at `text-sm`, so a + * 393 px viewport wraps most of them, and the alternative is the truncation + * this work removed. + */ + +test.use({ storageState: STORAGE_STATE_PATH }); + +const SETTINGS_ROUTES = [ + "/settings", + "/settings/account", + "/settings/security", + "/settings/notifications", + "/settings/integrations", + "/settings/ai", + "/settings/coach", + "/settings/modules", + "/settings/layout", + "/settings/thresholds", + "/settings/sources", + "/settings/environment", + "/settings/export", + "/settings/gesundheitsakte", + "/settings/privacy", + "/settings/api", + "/settings/mcp", + "/settings/advanced", + "/settings/about", +] as const; + +const ADMIN_ROUTES = [ + "/admin", + "/admin/system-status", + "/admin/general", + "/admin/services", + "/admin/users", + "/admin/invites", + "/admin/integrations", + "/admin/coach", + "/admin/reminders", + "/admin/backups", + "/admin/encryption", + "/admin/app-logs", + "/admin/api-tokens", + "/admin/login-overview", + "/admin/danger-zone", +] as const; + +const ROUTES = [...SETTINGS_ROUTES, ...ADMIN_ROUTES]; + +/** + * The card's own gap. `SettingsCard` inherits the `` contract — + * `gap-4` under `md`, `gap-6` from `md` up — so the expected value is a + * function of the viewport, not a constant. Asserting a constant here would + * have hidden exactly the defect this run found: the `as="section"` branch + * was writing a flat `gap-4` and sat one step tighter than its neighbour on + * every desktop width. + */ +const cardGapFor = (width: number) => (width >= 768 ? 24 : 16); +/** Sub-pixel and font-metric slack. */ +const TOLERANCE = 1.5; + +type HeaderGeometry = { + route: string; + /** `

` top, relative to the scroll container's content top. */ + headingTop: number; + /** Height of the whole title + explainer block. */ + headerHeight: number; + /** Rendered line count of the explainer (1 = it stayed on one line). */ + explainerLines: number; + explainerText: string; +}; + +async function measureHeader( + page: import("@playwright/test").Page, + route: string, +): Promise { + return page.evaluate( + ({ route }) => { + const main = document.querySelector("main"); + // On a desktop width the visible heading is the real `

`; on a phone + // the shells paint it above the chip strip as + // `role="heading" aria-level="1"` and the `

` in the grid row is + // display:none. Measure whichever one has a box — measuring the hidden + // copy reports a constant zero and calls it stability. + const heading = Array.from( + document.querySelectorAll( + 'main h1, main [role="heading"][aria-level="1"]', + ), + ).find((el) => el.getBoundingClientRect().height > 0); + if (!main || !heading) return null; + const block = heading.parentElement!; + const explainer = block.querySelector("p"); + const mainTop = main.getBoundingClientRect().top; + const headBox = heading.getBoundingClientRect(); + const explainerBox = explainer?.getBoundingClientRect(); + const style = explainer ? getComputedStyle(explainer) : null; + const lineHeight = style ? parseFloat(style.lineHeight) : 0; + return { + route, + headingTop: headBox.top - mainTop, + // Heading top to explainer bottom. The parent block's own box is not + // a reliable proxy on a phone, where the heading is rendered outside + // the grid row it belongs to on desktop. + headerHeight: (explainerBox?.bottom ?? headBox.bottom) - headBox.top, + explainerLines: + lineHeight > 0 && explainerBox + ? Math.round(explainerBox.height / lineHeight) + : 0, + explainerText: explainer?.textContent?.trim() ?? "", + }; + }, + { route }, + ); +} + +/** + * Header-to-body gap and body left edge, per card on the page. + * Returns one entry per card that has both a header and a following sibling. + */ +async function measureCards(page: import("@playwright/test").Page) { + return page.evaluate(() => { + const out: { gap: number; bodyInset: number }[] = []; + for (const card of document.querySelectorAll('[data-slot="card"]')) { + const header = card.querySelector(":scope > header"); + if (!header) continue; + const next = header.nextElementSibling as HTMLElement | null; + if (!next) continue; + if (next.getBoundingClientRect().height === 0) continue; + const cardBox = card.getBoundingClientRect(); + const cardStyle = getComputedStyle(card); + const cardInnerLeft = cardBox.left + parseFloat(cardStyle.paddingLeft); + out.push({ + gap: + next.getBoundingClientRect().top - + header.getBoundingClientRect().bottom, + bodyInset: next.getBoundingClientRect().left - cardInnerLeft, + }); + } + return out; + }); +} + +for (const locale of ["en", "de"] as const) { + test.describe(`one piece — ${locale}`, () => { + test(`page headers land at one height across every route (${locale})`, async ({ + page, + }) => { + test.setTimeout(420_000); + await page.context().addCookies([ + { + name: "healthlog-locale", + value: locale, + url: process.env.E2E_BASE_URL ?? "http://localhost:3000", + }, + ]); + + const measured: HeaderGeometry[] = []; + const skipped: string[] = []; + for (const route of ROUTES) { + const response = await page.goto(route, { + waitUntil: "domcontentloaded", + }); + if (response && response.status() >= 400) continue; + // A hub route may client-redirect to its first section; wait for the + // heading and skip a route that legitimately has none rather than + // failing the sweep on it. + const heading = page + .locator( + 'main h1:visible, main [role="heading"][aria-level="1"]:visible', + ) + .first(); + if (!(await heading.isVisible().catch(() => false))) { + await heading + .waitFor({ state: "visible", timeout: 8_000 }) + .catch(() => {}); + } + if (!(await heading.isVisible().catch(() => false))) { + skipped.push(route); + continue; + } + await settleBeforeMeasure(page, heading); + const geometry = await measureHeader(page, route); + if (geometry) measured.push(geometry); + } + + console.log( + `[${locale}] routes without an

: ${skipped.join(", ") || "none"}`, + ); + // The sweep has to actually look at something. + expect(measured.length).toBeGreaterThan(25); + + const tops = measured.map((m) => m.headingTop); + const heights = measured.map((m) => m.headerHeight); + const spreadTop = Math.max(...tops) - Math.min(...tops); + const spreadHeight = Math.max(...heights) - Math.min(...heights); + + // Attached so a failure reports WHICH route moved, not just that one did. + console.log( + `[${locale}] heading top: ${Math.min(...tops).toFixed(1)}…${Math.max(...tops).toFixed(1)} (spread ${spreadTop.toFixed(1)}px)\n` + + `[${locale}] header height: ${Math.min(...heights).toFixed(1)}…${Math.max(...heights).toFixed(1)} (spread ${spreadHeight.toFixed(1)}px)\n` + + measured + .map( + (m) => + ` ${m.route.padEnd(32)} top ${m.headingTop.toFixed(1)} h ${m.headerHeight.toFixed(1)} lines ${m.explainerLines}`, + ) + .join("\n"), + ); + + // A page explainer that wraps is a text problem, and it is the one the + // ≤120-character rule exists to prevent. Reported per locale. + const wrapped = measured.filter((m) => m.explainerLines > 1); + console.log( + `[${locale}] explainers wrapping past one line: ${wrapped.length}` + + wrapped.map((m) => `\n ${m.route}: ${m.explainerText}`).join(""), + ); + // The no-jump claim, on every viewport: the title starts at the same y + // whichever route you came from. + expect( + spreadTop, + "heading top drifts between routes", + ).toBeLessThanOrEqual(TOLERANCE); + + // The header's HEIGHT can only be constant where every explainer fits on + // one line, and on a 393 px phone most one-sentence explainers do not — + // a 70-character sentence at `text-sm` needs about 380 px of column. The + // honest rule there is that the block grows by whole lines of text, not + // that it never grows; forcing one height on a phone means truncating, + // which is the defect this work removed. So the height is pinned at the + // width where one line is achievable, and on a phone the wrap count is + // reported rather than asserted. + const isPhone = (page.viewportSize()?.width ?? 1280) < 768; + if (!isPhone) { + expect( + spreadHeight, + "header block height drifts between routes", + ).toBeLessThanOrEqual(TOLERANCE); + expect( + wrapped.map((m) => `${m.route}: ${m.explainerText}`), + "a page explainer wraps to a second line at desktop width", + ).toEqual([]); + } else { + // Every header is one or two lines of the same type scale — never a + // third, which would mean a subtitle nobody trimmed. + expect( + measured.filter((m) => m.explainerLines > 2).map((m) => m.route), + "a page explainer runs to three lines on a phone", + ).toEqual([]); + } + }); + + test(`cards share one header gap and one reading edge (${locale})`, async ({ + page, + }) => { + test.setTimeout(420_000); + await page.context().addCookies([ + { + name: "healthlog-locale", + value: locale, + url: process.env.E2E_BASE_URL ?? "http://localhost:3000", + }, + ]); + + const gaps: number[] = []; + const insets: number[] = []; + const expectedGap = cardGapFor(page.viewportSize()?.width ?? 1280); + for (const route of ROUTES) { + const response = await page.goto(route, { + waitUntil: "domcontentloaded", + }); + if (response && response.status() >= 400) continue; + const heading = page + .locator( + 'main h1:visible, main [role="heading"][aria-level="1"]:visible', + ) + .first(); + if (!(await heading.isVisible().catch(() => false))) { + await heading + .waitFor({ state: "visible", timeout: 8_000 }) + .catch(() => {}); + } + await settleBeforeMeasure(page, page.locator("main").first()); + for (const card of await measureCards(page)) { + gaps.push(card.gap); + insets.push(card.bodyInset); + } + } + + expect(gaps.length).toBeGreaterThan(30); + + const offGap = gaps.filter((g) => Math.abs(g - expectedGap) > TOLERANCE); + const offEdge = insets.filter((i) => Math.abs(i) > TOLERANCE); + console.log( + `[${locale}] cards measured: ${gaps.length}; header→body ≠ ${expectedGap}px: ${offGap.length} (${[...new Set(offGap.map((g) => g.toFixed(1)))].join(", ")})\n` + + `[${locale}] bodies off the card edge: ${offEdge.length} (${[...new Set(offEdge.map((i) => i.toFixed(1)))].join(", ")})`, + ); + + expect(offGap, "header→body gap is not the card's own gap").toEqual([]); + expect(offEdge, "card body does not start at the card edge").toEqual([]); + }); + }); +} diff --git a/messages/de.json b/messages/de.json index e14b9250b..6426b798d 100644 --- a/messages/de.json +++ b/messages/de.json @@ -6562,7 +6562,7 @@ }, "coach": { "title": "Coach", - "subtitle": "Assistenz-Oberflächen, Server-Schlüssel sowie die Feedback-Qualitäts-Tabellen für Coach und Auswertungen an einem Ort." + "subtitle": "Assistenz-Oberflächen, Server-Schlüssel und Feedback-Qualität an einem Ort." }, "reminders": { "title": "Erinnerungen", diff --git a/messages/en.json b/messages/en.json index 28896381e..5d17836b8 100644 --- a/messages/en.json +++ b/messages/en.json @@ -6562,7 +6562,7 @@ }, "coach": { "title": "Coach", - "subtitle": "Assistant surfaces, server key, and the Coach and Insights feedback-quality tables in one place." + "subtitle": "Assistant surfaces, the server key, and feedback quality in one place." }, "reminders": { "title": "Reminders", diff --git a/messages/es.json b/messages/es.json index 60b51791a..e3e1cbdc7 100644 --- a/messages/es.json +++ b/messages/es.json @@ -6562,7 +6562,7 @@ }, "coach": { "title": "Coach", - "subtitle": "Superficies del asistente, clave del servidor y las tablas de calidad de comentarios del Coach y de los análisis, en un solo lugar." + "subtitle": "Superficies del asistente, clave del servidor y calidad del feedback." }, "reminders": { "title": "Recordatorios", diff --git a/messages/fr.json b/messages/fr.json index fa0456381..f2d33a192 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -6562,7 +6562,7 @@ }, "coach": { "title": "Coach", - "subtitle": "Surfaces d'assistance, clé du serveur et les tableaux de qualité des retours du Coach et des analyses, au même endroit." + "subtitle": "Surfaces d'assistance, clé du serveur et qualité des retours." }, "reminders": { "title": "Rappels", diff --git a/messages/it.json b/messages/it.json index 3cd36daab..c9975b1a8 100644 --- a/messages/it.json +++ b/messages/it.json @@ -6562,7 +6562,7 @@ }, "coach": { "title": "Coach", - "subtitle": "Superfici dell'assistente, chiave del server e le tabelle di qualità del feedback di Coach e analisi, in un unico posto." + "subtitle": "Superfici dell'assistente, chiave del server e qualità del feedback." }, "reminders": { "title": "Promemoria", diff --git a/messages/pl.json b/messages/pl.json index 0bc30002d..545a11b35 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -6562,7 +6562,7 @@ }, "coach": { "title": "Coach", - "subtitle": "Powierzchnie asystenta, klucz serwera oraz tabele jakości opinii Coacha i analiz w jednym miejscu." + "subtitle": "Powierzchnie asystenta, klucz serwera i jakość opinii." }, "reminders": { "title": "Przypomnienia", diff --git a/scripts/measure-description-lengths.mjs b/scripts/measure-description-lengths.mjs index 0beb623ed..3f8383b85 100644 --- a/scripts/measure-description-lengths.mjs +++ b/scripts/measure-description-lengths.mjs @@ -12,7 +12,9 @@ */ import { readFileSync } from "node:fs"; -const locale = process.argv[2]?.startsWith("--") ? "de" : (process.argv[2] ?? "de"); +const locale = process.argv[2]?.startsWith("--") + ? "de" + : (process.argv[2] ?? "de"); const topIdx = process.argv.indexOf("--top"); const top = topIdx === -1 ? 40 : Number(process.argv[topIdx + 1]); @@ -44,7 +46,8 @@ for (const ns of ["settings", "admin"]) { } const lengths = entries.map(([, v]) => v.length).sort((a, b) => a - b); -const at = (q) => lengths[Math.min(lengths.length - 1, Math.floor(lengths.length * q))]; +const at = (q) => + lengths[Math.min(lengths.length - 1, Math.floor(lengths.length * q))]; const multi = entries.filter(([, v]) => sentences(v) >= 2); console.log(`locale ${locale} — ${entries.length} description/hint strings`); @@ -54,11 +57,15 @@ console.log( console.log( `> 120 chars: ${lengths.filter((n) => n > 120).length}, > 200: ${lengths.filter((n) => n > 200).length}`, ); -console.log(`>= 2 sentences: ${multi.length}, >= 3: ${entries.filter(([, v]) => sentences(v) >= 3).length}`); +console.log( + `>= 2 sentences: ${multi.length}, >= 3: ${entries.filter(([, v]) => sentences(v) >= 3).length}`, +); console.log(`\nLONGEST ${top}:`); for (const [key, value] of [...entries] .sort((a, b) => b[1].length - a[1].length) .slice(0, top)) { - console.log(`${String(value.length).padStart(4)}ch ${sentences(value)}S ${key}`); + console.log( + `${String(value.length).padStart(4)}ch ${sentences(value)}S ${key}`, + ); console.log(` ${value.slice(0, 130)}${value.length > 130 ? "…" : ""}`); } diff --git a/scripts/split-description.mjs b/scripts/split-description.mjs index 660f22492..199f5d2ec 100644 --- a/scripts/split-description.mjs +++ b/scripts/split-description.mjs @@ -64,7 +64,9 @@ for (const locale of LOCALES) { const head = parts[0].trim(); const tail = parts.slice(1).join(" ").trim(); if (dry) { - console.log(`${locale} ${key}\n head(${head.length}): ${head}\n tail: ${tail}`); + console.log( + `${locale} ${key}\n head(${head.length}): ${head}\n tail: ${tail}`, + ); continue; } set(bundle, key, head); diff --git a/src/components/settings/__tests__/card-description-length.test.ts b/src/components/settings/__tests__/card-description-length.test.ts index 6e700d490..66e04c492 100644 --- a/src/components/settings/__tests__/card-description-length.test.ts +++ b/src/components/settings/__tests__/card-description-length.test.ts @@ -110,7 +110,9 @@ describe("card header descriptions — one sentence, ≤120 characters", () => { const value = resolve(bundle, key); if (value === undefined) continue; // the i18n coverage guard owns this if (sentenceCount(value) > 1) { - offenders.push(`${key} (${sentenceCount(value)} sentences) — ${files[0]}`); + offenders.push( + `${key} (${sentenceCount(value)} sentences) — ${files[0]}`, + ); } } expect(offenders).toEqual([]); @@ -134,8 +136,10 @@ describe("card header descriptions — one sentence, ≤120 characters", () => { readFileSync(join(ROOT, "messages", "en.json"), "utf8"), ); for (const [key, reason] of Object.entries(LENGTH_EXCEPTIONS)) { - expect(keys.has(key), `${key} is allowlisted but no longer a description`) - .toBe(true); + expect( + keys.has(key), + `${key} is allowlisted but no longer a description`, + ).toBe(true); expect( resolve(bundle, key)!.length, `${key} is allowlisted but now fits the guideline`, diff --git a/src/components/settings/settings-card.tsx b/src/components/settings/settings-card.tsx index 151cc6e93..0819a6419 100644 --- a/src/components/settings/settings-card.tsx +++ b/src/components/settings/settings-card.tsx @@ -50,7 +50,7 @@ export function SettingsCard({ // Mirror the ui Card shell so a semantic landmark card paints // identically to the default -backed one. className={cn( - "bg-card text-card-foreground flex flex-col gap-4 rounded-xl border p-4 shadow-sm md:p-6", + "bg-card text-card-foreground flex flex-col gap-4 rounded-xl border p-4 shadow-sm md:gap-6 md:p-6", className, )} {...props}