mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 13:01:00 +00:00
fix(ui): four hand-rolled page titles go through PageHeader
The insights sub-page shell (text-xl sm:text-2xl semibold), the medication detail, the check-in screen and its result (semibold) each read a step lighter than every other page title. They now render through PageHeader, which gains a focusable heading with a visible ring for screens that move focus to their title, and an accessory slot beside the H1 that stays outside its accessible name. A structural test lists where a different H1 is the point (heroes, sign-in, error and prose pages, the first-run steps) and refuses any other.
This commit is contained in:
1 parent
9d44d1c47f
commit
6885377f35
7 files changed
+261
-111
No files matched your search
@@ -0,0 +1,85 @@
|
||||
/**
|
||||
* A module page's title is a `PageHeader` (UI standards §1, §5): the H1 on
|
||||
* one tier (`text-2xl font-bold tracking-tight`), its explainer muted under
|
||||
* it, its actions beside it.
|
||||
*
|
||||
* Four in-app headings were hand-rolled beside it — the insights sub-page
|
||||
* shell (`text-xl sm:text-2xl font-semibold`), the medication detail, the
|
||||
* check-in and its result (`font-semibold`) — and each read a step lighter
|
||||
* than every other page title. They go through `PageHeader` now; this guard
|
||||
* keeps a new `<h1>` literal from appearing outside the places where a
|
||||
* different heading is the point.
|
||||
*
|
||||
* Matched: an `<h1` tag in JSX under `src/components` and `src/app`. A
|
||||
* mention in a comment counts too, which is why the files that only talk
|
||||
* about an H1 are listed with that reason.
|
||||
*/
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
|
||||
import { walkSourceFiles } from "./helpers/source-files";
|
||||
|
||||
const SRC = join(__dirname, "..");
|
||||
|
||||
const ALLOWED: Record<string, string> = {
|
||||
"components/ui/page-header.tsx": "the primitive",
|
||||
// Hero identity surfaces, not page titles.
|
||||
"components/insights/hero-strip.tsx": "insights hero greeting",
|
||||
"components/insights/coach-panel/coach-hero.tsx": "Coach hero",
|
||||
"components/insights/coach-panel/coach-conversation.tsx":
|
||||
"screen-reader title while the hero is folded away",
|
||||
// The first-run flow: full-screen steps with their own focus management.
|
||||
"components/onboarding/step-heading.tsx": "first-run step heading",
|
||||
// Outside the app shell: sign-in, register, claim, invite, the share-link
|
||||
// gate, the key-mismatch gate.
|
||||
"app/auth/login/page.tsx": "sign-in card",
|
||||
"app/auth/register/page.tsx": "register card",
|
||||
"components/auth/claim-profile.tsx": "claim card",
|
||||
"app/invite/[token]/page.tsx": "invalid invite screen",
|
||||
"components/clinician/share-unlock-gate.tsx": "share-link unlock gate",
|
||||
"app/key-mismatch-page.tsx": "key-mismatch gate",
|
||||
// Error and not-found screens.
|
||||
"app/not-found.tsx": "not-found screen",
|
||||
"app/c/[token]/not-found.tsx": "share-link not-found screen",
|
||||
"app/c/error.tsx": "share-link error screen",
|
||||
"app/global-error.tsx": "renders without the stylesheet",
|
||||
"components/error-details.tsx": "error boundary",
|
||||
// Prose pages (the prose H1 tier, §5).
|
||||
"app/about/page.tsx": "prose page",
|
||||
"app/privacy/page.tsx": "prose page",
|
||||
// Mention an H1 in a comment only.
|
||||
"app/insights/ecg/page.tsx": "comment",
|
||||
"components/admin/reminders-section.tsx": "comment",
|
||||
"components/charts/health-chart.tsx": "comment",
|
||||
"components/insights/coach-panel/coach-top-bar-trail.tsx": "comment",
|
||||
"components/insights/ecg-section.tsx": "comment",
|
||||
"components/measurement-reminders/vorsorge-section.tsx": "comment",
|
||||
"components/settings/settings-shell.tsx": "comment",
|
||||
};
|
||||
|
||||
describe("page titles go through PageHeader", () => {
|
||||
const files = walkSourceFiles(SRC, { floor: 2500 }).filter(
|
||||
(p) =>
|
||||
p.endsWith(".tsx") &&
|
||||
(p.startsWith("components/") || p.startsWith("app/")) &&
|
||||
!p.includes("__tests__/"),
|
||||
);
|
||||
const withH1 = files.filter((p) =>
|
||||
readFileSync(join(SRC, p), "utf8").includes("<h1"),
|
||||
);
|
||||
|
||||
it("finds the H1 sites at all", () => {
|
||||
expect(withH1.length).toBeGreaterThan(10);
|
||||
});
|
||||
|
||||
it("has no hand-rolled H1 outside the allowlist", () => {
|
||||
expect(withH1.filter((p) => !(p in ALLOWED))).toEqual([]);
|
||||
});
|
||||
|
||||
it("keeps every allowlist entry live", () => {
|
||||
const set = new Set(withH1);
|
||||
for (const key of Object.keys(ALLOWED))
|
||||
expect(set.has(key), key).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -15,6 +15,7 @@ import { useScrollResetOnRoute } from "@/hooks/use-scroll-reset-on-route";
|
||||
import { useTranslations } from "@/lib/i18n/context";
|
||||
import { SUB_PAGE_SLUGS } from "@/lib/insights/sub-page-metric";
|
||||
import { cn } from "@/lib/utils";
|
||||
import { PageHeader } from "@/components/ui/page-header";
|
||||
|
||||
const INSIGHT_METRIC_RETURN_PATHS: Readonly<Record<string, true>> =
|
||||
Object.fromEntries(
|
||||
@@ -333,95 +334,101 @@ export function SubPageShell({
|
||||
with and without a nudge now align. `items-start` keeps the
|
||||
Coach icon at the top of the row when the title wraps to two
|
||||
lines. */}
|
||||
<div className="flex min-h-10 items-start justify-between gap-2">
|
||||
<div className="flex min-w-0 flex-wrap items-center gap-2">
|
||||
<h1
|
||||
ref={headingRef}
|
||||
id="insights-subpage-title"
|
||||
tabIndex={-1}
|
||||
className={cn(
|
||||
"text-xl font-semibold sm:text-2xl",
|
||||
// a11y: sighted-keyboard users (e.g. "Skip to content")
|
||||
// need a visible focus indicator on the programmatic
|
||||
// `headingRef.focus()` call below. Match the focus ring
|
||||
// vocabulary used on insights pills + Coach affordances.
|
||||
"focus-visible:ring-ring/50 rounded-sm focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:outline-none",
|
||||
)}
|
||||
>
|
||||
{title}
|
||||
</h1>
|
||||
{/* v1.8.6 — the diversity nudge rides the heading as a
|
||||
`Lightbulb` glyph (the node self-gates to nothing when the
|
||||
spread is healthy), replacing the old inline block. */}
|
||||
{diversityNudge}
|
||||
{badge ? (
|
||||
<Badge variant="outline" className="border text-xs">
|
||||
{badge}
|
||||
</Badge>
|
||||
) : null}
|
||||
</div>
|
||||
{/* Header action cluster, pinned top-right at heading height by
|
||||
the row's `justify-between` so it aligns with the title
|
||||
regardless of badge / glyph presence or a wrapped title.
|
||||
{/* The heading row is the canonical PageHeader: the H1 on the
|
||||
page-title tier, a programmatic focus target after a client
|
||||
navigation, with the diversity nudge and the badge beside it
|
||||
(outside the heading's name) and the action cluster pinned
|
||||
top-right. The row keeps a constant `min-h-10` so the baseline
|
||||
holds whether or not the 44 px nudge glyph is present. */}
|
||||
<PageHeader
|
||||
title={title}
|
||||
titleId="insights-subpage-title"
|
||||
headingRef={headingRef}
|
||||
focusable
|
||||
className="[&_[data-slot=page-header-row]]:min-h-10"
|
||||
titleAccessory={
|
||||
diversityNudge || badge ? (
|
||||
<>
|
||||
{/* v1.8.6 — the diversity nudge rides the heading as a
|
||||
`Lightbulb` glyph (the node self-gates to nothing when
|
||||
the spread is healthy). */}
|
||||
{diversityNudge}
|
||||
{badge ? (
|
||||
<Badge variant="outline" className="border text-xs">
|
||||
{badge}
|
||||
</Badge>
|
||||
) : null}
|
||||
</>
|
||||
) : undefined
|
||||
}
|
||||
actions={
|
||||
<>
|
||||
{/* Header action cluster, pinned top-right at heading height by
|
||||
the row's `justify-between` so it aligns with the title
|
||||
regardless of badge / glyph presence or a wrapped title.
|
||||
|
||||
The target-adjust gear sits to the LEFT of the Coach icon.
|
||||
It self-gates on a registered editable target (a metric with
|
||||
no numeric band registers nothing, so the gear stays hidden),
|
||||
and the Coach icon self-gates on the Coach flag + per-user
|
||||
opt-out — so the cluster collapses cleanly to one, both, or
|
||||
neither control.
|
||||
The target-adjust gear sits to the LEFT of the Coach icon.
|
||||
It self-gates on a registered editable target (a metric with
|
||||
no numeric band registers nothing, so the gear stays hidden),
|
||||
and the Coach icon self-gates on the Coach flag + per-user
|
||||
opt-out — so the cluster collapses cleanly to one, both, or
|
||||
neither control.
|
||||
|
||||
`gap-3` (12 px) keeps the siblings' extended hit areas
|
||||
(`before:-inset-1.5`, 6 px per edge) from overlapping —
|
||||
with a tighter gap the later sibling's invisible halo sat
|
||||
on top of its neighbour's clickable edge. */}
|
||||
<div className="flex shrink-0 items-center gap-3">
|
||||
{headerAction}
|
||||
{captureHref ? (
|
||||
<SubPageAddButton label={captureLabel} href={captureHref} />
|
||||
) : null}
|
||||
{/* v1.16.8 — "show all readings" rides the header cluster
|
||||
as an icon button, LEFT of the target-adjust gear (it
|
||||
used to be a full-width outline button at the page foot).
|
||||
Same 40 px box + extended hit area as its siblings; the
|
||||
label travels via aria-label + title. */}
|
||||
{showAllValuesType ? (
|
||||
<Button
|
||||
asChild
|
||||
variant="ghost"
|
||||
size="icon"
|
||||
data-slot="metric-show-all-values"
|
||||
className={cn(
|
||||
// Match the cluster's 40 px icon box + extended hit
|
||||
// area (see `<TargetAdjustButton>` for the WCAG
|
||||
// 2.5.5 note).
|
||||
"text-muted-foreground hover:text-foreground relative size-10",
|
||||
"before:absolute before:-inset-1.5 before:content-['']",
|
||||
)}
|
||||
>
|
||||
<Link
|
||||
href={`/insights/values/${showAllValuesType}${
|
||||
pathname ? `?from=${encodeURIComponent(pathname)}` : ""
|
||||
}`}
|
||||
aria-label={t("insights.subPage.showAllValues")}
|
||||
title={t("insights.subPage.showAllValues")}
|
||||
>
|
||||
<ListOrdered className="size-4" aria-hidden="true" />
|
||||
</Link>
|
||||
</Button>
|
||||
) : null}
|
||||
<TargetAdjustButton />
|
||||
{/* v1.18.6 — the per-metric Coach launch icon is
|
||||
gone. With the Coach FAB anchored bottom-right on every
|
||||
authenticated page, a second Coach affordance in each
|
||||
metric header was redundant. The `coachLaunch` prop is
|
||||
still accepted (every metric page passes it) but no longer
|
||||
paints a control — the FAB is the single Coach entry. */}
|
||||
{/* v1.16.8 — the v1.16.4 customise cog is gone: the sticky
|
||||
tab strip above already carries the same control on every
|
||||
insights surface, so the header copy was a duplicate. */}
|
||||
</div>
|
||||
</div>
|
||||
`gap-3` (12 px) keeps the siblings' extended hit areas
|
||||
(`before:-inset-1.5`, 6 px per edge) from overlapping —
|
||||
with a tighter gap the later sibling's invisible halo sat
|
||||
on top of its neighbour's clickable edge. */}
|
||||
<div className="flex shrink-0 items-center gap-3">
|
||||
{headerAction}
|
||||
{captureHref ? (
|
||||
<SubPageAddButton label={captureLabel} href={captureHref} />
|
||||
) : null}
|
||||
{/* v1.16.8 — "show all readings" rides the header cluster
|
||||
as an icon button, LEFT of the target-adjust gear (it
|
||||
used to be a full-width outline button at the page foot).
|
||||
Same 40 px box + extended hit area as its siblings; the
|
||||
label travels via aria-label + title. */}
|
||||
{showAllValuesType ? (
|
||||
<Button
|
||||
asChild
|
||||
variant="ghost"
|
||||
size="icon"
|
||||
data-slot="metric-show-all-values"
|
||||
className={cn(
|
||||
// Match the cluster's 40 px icon box + extended hit
|
||||
// area (see `<TargetAdjustButton>` for the WCAG
|
||||
// 2.5.5 note).
|
||||
"text-muted-foreground hover:text-foreground relative size-10",
|
||||
"before:absolute before:-inset-1.5 before:content-['']",
|
||||
)}
|
||||
>
|
||||
<Link
|
||||
href={`/insights/values/${showAllValuesType}${
|
||||
pathname
|
||||
? `?from=${encodeURIComponent(pathname)}`
|
||||
: ""
|
||||
}`}
|
||||
aria-label={t("insights.subPage.showAllValues")}
|
||||
title={t("insights.subPage.showAllValues")}
|
||||
>
|
||||
<ListOrdered className="size-4" aria-hidden="true" />
|
||||
</Link>
|
||||
</Button>
|
||||
) : null}
|
||||
<TargetAdjustButton />
|
||||
{/* v1.18.6 — the per-metric Coach launch icon is
|
||||
gone. With the Coach FAB anchored bottom-right on every
|
||||
authenticated page, a second Coach affordance in each
|
||||
metric header was redundant. The `coachLaunch` prop is
|
||||
still accepted (every metric page passes it) but no longer
|
||||
paints a control — the FAB is the single Coach entry. */}
|
||||
{/* v1.16.8 — the v1.16.4 customise cog is gone: the sticky
|
||||
tab strip above already carries the same control on every
|
||||
insights surface, so the header copy was a duplicate. */}
|
||||
</div>
|
||||
</>
|
||||
}
|
||||
/>
|
||||
{explainerMetric ? (
|
||||
// v1.8.4 — surface the metric's static definition inline,
|
||||
// directly beneath the heading.
|
||||
|
||||
@@ -27,6 +27,7 @@ import {
|
||||
useDateFormatPreference,
|
||||
} from "@/lib/i18n/context";
|
||||
import { formatDate as formatCalendarDate } from "@/lib/date-format";
|
||||
import { PageHeader } from "@/components/ui/page-header";
|
||||
|
||||
export interface MedicationDetailSummaryProps {
|
||||
name: string;
|
||||
@@ -109,8 +110,7 @@ export function MedicationDetailSummary({
|
||||
|
||||
return (
|
||||
<div className="space-y-1.5" data-slot="medication-detail-summary">
|
||||
<h1 className="text-2xl font-semibold tracking-tight">{name}</h1>
|
||||
<p className="text-muted-foreground text-sm">{dose}</p>
|
||||
<PageHeader title={name} description={dose} />
|
||||
<div
|
||||
className="text-muted-foreground flex items-center gap-2 text-xs"
|
||||
data-slot="medication-detail-status-row"
|
||||
|
||||
@@ -26,6 +26,7 @@ import { INSTRUMENTS, needsFollowUp } from "@/lib/mental-health/instruments";
|
||||
|
||||
import { CrisisCard } from "./crisis-card";
|
||||
import type { CreateResponse } from "./types";
|
||||
import { PageHeader } from "@/components/ui/page-header";
|
||||
|
||||
export function AssessmentResult({
|
||||
result,
|
||||
@@ -67,18 +68,12 @@ export function AssessmentResult({
|
||||
{t("mentalHealth.backToList")}
|
||||
</Button>
|
||||
|
||||
<header className="flex flex-col gap-1">
|
||||
<h1
|
||||
ref={headingRef}
|
||||
tabIndex={-1}
|
||||
className="scroll-mt-4 text-2xl font-semibold outline-none"
|
||||
>
|
||||
{t(`mentalHealth.instrument.${key}`)}
|
||||
</h1>
|
||||
<p className="text-muted-foreground text-sm">
|
||||
{t(`mentalHealth.instrumentDescription.${key}`)}
|
||||
</p>
|
||||
</header>
|
||||
<PageHeader
|
||||
title={t(`mentalHealth.instrument.${key}`)}
|
||||
description={t(`mentalHealth.instrumentDescription.${key}`)}
|
||||
headingRef={headingRef}
|
||||
focusable
|
||||
/>
|
||||
|
||||
<p className="text-muted-foreground text-xs" role="status">
|
||||
{t("mentalHealth.saved")}
|
||||
|
||||
@@ -45,6 +45,7 @@ import {
|
||||
UNANSWERED,
|
||||
} from "./check-in-nav";
|
||||
import type { InstrumentId } from "./types";
|
||||
import { PageHeader } from "@/components/ui/page-header";
|
||||
|
||||
const FUNCTIONAL_SCALE = [0, 1, 2, 3] as const;
|
||||
|
||||
@@ -142,13 +143,10 @@ export function CheckInWizard({
|
||||
{/* In-test header — instrument title + ONE standardized explanation line.
|
||||
No disclaimer here (§2); the landing carries it. */}
|
||||
<header className="flex flex-col gap-1">
|
||||
{/* Wizard hero H1: intentionally semibold, same hero exception as onboarding (UI-STANDARDS §5). Do not sweep to font-bold. */}
|
||||
<h1 className="text-2xl font-semibold">
|
||||
{t(`mentalHealth.instrument.${key}`)}
|
||||
</h1>
|
||||
<p className="text-muted-foreground text-sm">
|
||||
{t(`mentalHealth.instrumentDescription.${key}`)}
|
||||
</p>
|
||||
<PageHeader
|
||||
title={t(`mentalHealth.instrument.${key}`)}
|
||||
description={t(`mentalHealth.instrumentDescription.${key}`)}
|
||||
/>
|
||||
{/* Honest translation note: the instrument is validated in its source
|
||||
language; the surrounding chrome stays localized. */}
|
||||
{!hasValidatedItems(instrument, locale) && (
|
||||
|
||||
@@ -0,0 +1,31 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { createRef } from "react";
|
||||
import { renderToStaticMarkup } from "react-dom/server";
|
||||
|
||||
import { PageHeader } from "../page-header";
|
||||
|
||||
describe("<PageHeader> heading options", () => {
|
||||
it("is a plain h1 by default, with no tab stop", () => {
|
||||
const html = renderToStaticMarkup(<PageHeader title="Pulse" />);
|
||||
expect(html).toMatch(
|
||||
/<h1[^>]*class="[^"]*text-2xl font-bold tracking-tight/,
|
||||
);
|
||||
expect(html).not.toContain("tabindex");
|
||||
});
|
||||
|
||||
it("becomes a programmatic focus target with a visible ring", () => {
|
||||
const ref = createRef<HTMLHeadingElement>();
|
||||
const html = renderToStaticMarkup(
|
||||
<PageHeader title="Pulse" titleId="t" headingRef={ref} focusable />,
|
||||
);
|
||||
expect(html).toMatch(/<h1[^>]*tabindex="-1"/);
|
||||
expect(html).toContain("focus-visible:ring-2");
|
||||
});
|
||||
|
||||
it("keeps the accessory beside the heading, outside its name", () => {
|
||||
const html = renderToStaticMarkup(
|
||||
<PageHeader title="Pulse" titleAccessory={<span>Beta</span>} />,
|
||||
);
|
||||
expect(html).toMatch(/<h1[^>]*>Pulse<\/h1><span>Beta<\/span>/);
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,4 @@
|
||||
import type { ReactNode } from "react";
|
||||
import type { ReactNode, Ref } from "react";
|
||||
|
||||
import { BackLink } from "@/components/ui/back-link";
|
||||
import { cn } from "@/lib/utils";
|
||||
@@ -25,6 +25,14 @@ import { cn } from "@/lib/utils";
|
||||
* row — and only one of the two may be a real `<h1>`. Without this the two
|
||||
* shells hand-rolled the whole block to get the second copy, which is how
|
||||
* they drifted apart from every module page in the first place.
|
||||
*
|
||||
* `focusable` makes the H1 a programmatic focus target (`tabIndex={-1}`
|
||||
* with a visible ring) and `headingRef` hands the caller the element: a
|
||||
* sub-page or a result screen moves focus to its title after a client
|
||||
* navigation. `titleAccessory` sits beside the H1 and
|
||||
* wraps with it (a nudge glyph, a badge) without becoming part of the
|
||||
* heading's accessible name. Both exist so the insights sub-page shell and
|
||||
* the check-in screens stop hand-rolling their own heading.
|
||||
*/
|
||||
/**
|
||||
* A long one-word German title (the illness page was once
|
||||
@@ -44,6 +52,9 @@ export function PageHeader({
|
||||
topSlot,
|
||||
actions,
|
||||
stackActionsOnPhone = false,
|
||||
headingRef,
|
||||
focusable = false,
|
||||
titleAccessory,
|
||||
className,
|
||||
}: {
|
||||
title: ReactNode;
|
||||
@@ -61,6 +72,12 @@ export function PageHeader({
|
||||
* the title and leave this off.
|
||||
*/
|
||||
stackActionsOnPhone?: boolean;
|
||||
/** The H1 element, for a caller that moves focus to it. */
|
||||
headingRef?: Ref<HTMLHeadingElement>;
|
||||
/** Makes the H1 a programmatic focus target with a visible ring. */
|
||||
focusable?: boolean;
|
||||
/** Beside the H1, outside its accessible name (a nudge glyph, a badge). */
|
||||
titleAccessory?: ReactNode;
|
||||
className?: string;
|
||||
}) {
|
||||
const heading =
|
||||
@@ -69,10 +86,27 @@ export function PageHeader({
|
||||
{title}
|
||||
</div>
|
||||
) : (
|
||||
<h1 id={titleId} className={HEADING_CLASS}>
|
||||
<h1
|
||||
ref={headingRef}
|
||||
id={titleId}
|
||||
tabIndex={focusable ? -1 : undefined}
|
||||
className={cn(
|
||||
HEADING_CLASS,
|
||||
focusable &&
|
||||
"focus-visible:ring-ring/50 rounded-sm focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:outline-none",
|
||||
)}
|
||||
>
|
||||
{title}
|
||||
</h1>
|
||||
);
|
||||
const titleBlock = titleAccessory ? (
|
||||
<div className="flex min-w-0 flex-wrap items-center gap-2">
|
||||
{heading}
|
||||
{titleAccessory}
|
||||
</div>
|
||||
) : (
|
||||
heading
|
||||
);
|
||||
|
||||
// Title, description and actions share one grid. On a phone the
|
||||
// description spans the full row under the title + actions: a header with
|
||||
@@ -91,7 +125,7 @@ export function PageHeader({
|
||||
actions && stackActionsOnPhone && "max-sm:grid-cols-1",
|
||||
)}
|
||||
>
|
||||
{heading}
|
||||
{titleBlock}
|
||||
{actions ? (
|
||||
<div
|
||||
className={cn(
|
||||
|
||||
Reference in new issue
Block a user