From 6651feef4fab63f1181fba57908cb22e2932df3c Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 09:47:40 +0000 Subject: [PATCH 1/6] feat(in-page-nav): convert medications and factsheets, close the series (PR 3 of 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the in-page navigation series. PR 1 (#1740) extracted the template into src/components/in-page-nav/ and converted differentials; PR 2 (#1766) converted the six information routes and deleted the shell-owned pill rail. Three routes were left; this lands two of them and records why the third is not a candidate. Medications (/medications/[slug]) — converted, panel-swap The owner's call: keep the tab swap and drive it from the header's segment track, exactly as differentials/differential-detail-page.tsx does. SectionTabs (a 58-line roving-tabindex tablist) is deleted; activeTab lifts to MedicationRecordPage so the header above the shell can drive it, and the InformationPageBreadcrumbs row goes with it. The tab->section-type grouping moves into the new medication-nav-header.tsx sibling so the segment weights and the rendered panel cannot disagree about what a tab contains. The panel keeps a per-tab id but drops role="tabpanel"/aria-labelledby: the control is now a list of buttons, so claiming the role would name a tab that no longer exists. Factsheets (/factsheets/[slug]) — converted, anchor-scrolling Already mounted InPageNavHeader in its breadcrumb shape; this gives it a real section index. tocFor is deleted rather than ported: it returned display strings with no anchors behind them, painted into an inert
  • "On this page" list, and was wrong in both directions (it named "What is this medicine?" where the page renders "What is ?", and never listed the Sources, More-in-topic or Related sections every sheet renders). The replacement derives sections from what each of the five kinds actually renders, with medLite's headings coming from the record. Differentials presentations — recorded exception, not converted The premise did not survive reading the file. It was carried as "a SectionTabs page that swaps panels"; it swaps nothing — MobileTabs is four <Link>s to other routes with "Compare" hardcoded active, which is the multi-route ModeNav pattern the template already carves out. Its candidate sections are also rendered two or three times per breakpoint in different DOM parents, one nested inside another section's anchor, which PageSection.targetIds does not model. Reasoned exception recorded in docs/search-chrome-behaviour.md. Also: delete the orphaned SecondaryNavigation component (/issues #271) Test-only since PR 2 removed the section kind. Its two stated side-conditions turned out not to exist — nothing outside its own test imports it, and tests/mode-nav-contract.test.ts string-matches page-secondary-navigation.tsx, a different file. Its test fixture was literally the medication tab bar, so it lands with the conversion that retired it. Guards tests/in-page-nav-route-sections.dom.test.tsx grows from 7 routes to 12 (one factsheet case per kind) and gains a panel-swap suite for medications, so both halves of /issues #256's stop rule hold: declared ids are proven against rendered DOM, never a source grep. A new `absent` field asserts the therapy and procedure sheets genuinely do not render More-in-topic rather than skipping it. Both new routes are registered in isHeaderAddonSlotOwnedRoute and the claimant enumeration. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GHmzgER6rv8Va9HbQZw87Q --- docs/codebase-index.md | 4 +- docs/search-chrome-behaviour.md | 75 +++++- .../medication-nav-header.tsx | 150 ++++++++++++ .../medication-record-page.tsx | 172 ++++--------- .../factsheets/factsheet-detail-page.tsx | 126 +++++----- .../factsheets/factsheet-nav-header.tsx | 174 +++++++++++++ src/components/factsheets/factsheets-data.ts | 32 +-- .../in-page-nav/in-page-nav-header.tsx | 9 +- src/components/mode-nav/header-addon-slot.ts | 7 +- src/components/secondary-navigation.tsx | 230 ------------------ tests/factsheets-data.test.ts | 4 +- tests/in-page-nav-route-sections.dom.test.tsx | 131 +++++++++- tests/mode-nav-addon-slot.dom.test.tsx | 20 +- tests/secondary-navigation.dom.test.tsx | 183 -------------- 14 files changed, 672 insertions(+), 645 deletions(-) create mode 100644 src/components/clinical-dashboard/medication-nav-header.tsx create mode 100644 src/components/factsheets/factsheet-nav-header.tsx delete mode 100644 src/components/secondary-navigation.tsx delete mode 100644 tests/secondary-navigation.dom.test.tsx diff --git a/docs/codebase-index.md b/docs/codebase-index.md index 605518a572..0f763f3a89 100644 --- a/docs/codebase-index.md +++ b/docs/codebase-index.md @@ -329,8 +329,8 @@ One shared composer (`master-search-header.tsx`) serves every mode. Placement: - **Result and detail views**: fixed bottom dock on phone (compact variant on submitted searches), sticky top from `sm` up. - **Results routing**: standalone routes own their submitted searches via `?q=…&run=1` (`/services` → `ServicesNavigatorPage`, `/forms` → `FormsSearchResultsPage`, `/differentials` → `DifferentialsHome` results view, `/formulation` → local mechanism results, `/favourites` filters the command library in place). Answer, Documents, and Prescribing submitted searches render inside `ClinicalDashboard` — intentional, since they need retrieval/answer state. Bare `/?mode=<id>` always renders the shared home with that mode preselected; only a submitted deep link (`q` plus `run=1`) resolves to the mode's own search surface (proxy early-redirect still covers favourites/differentials/specifiers for those submitted aliases). - **Intentionally composer-free routes**: `/differentials/presentations/*` and `/differentials/compare` (comparison workflow owns its chrome), `/documents/[id]` viewer (has its own in-document ask composer), `/documents/source/*` (document flow owns mobile chrome). Do not re-flag these in search-consistency audits. -- **Shared in-page navigation**: `src/components/in-page-nav/` is the default template for section navigation on any mode page (`docs/search-chrome-behaviour.md`). `in-page-nav-header.tsx` (`InPageNavHeader`) owns the header row, both sheets and the `PhoneHeaderCollapsePortal` wrapper; `page-section-index.ts` (`PageSection`, `toDocumentSections`, `sectionTargetIds`) is the declaration shape; `use-resolved-page-sections.ts` narrows a declaration to the anchors actually rendered at this breakpoint; `use-in-page-section-nav.ts` composes that with `useDocumentSectionSpy` and `jumpToDocumentSection`; `use-page-section-weights.ts` measures segment weights; `use-in-page-chrome-metrics.ts` publishes `--inpage-anchor-offset`; `in-page-nav-classes.ts` holds the shared anchor (`inPageAnchor`) and actions-sheet row classes. Anchor measurement itself is `src/components/sticky-chrome-metrics.ts` (`useStickyChromeMetrics`), shared with the document viewer's `use-document-chrome-metrics.ts`. Mounted by `differentials/differential-detail-page.tsx`, `services/service-detail-page.tsx`, `forms/form-detail-page.tsx`, `dsm/dsm-differential-considerations-page.tsx`, and — through a `"use client"` sibling module, because those pages are Server Components — `specifiers/specifier-nav-header.tsx`, `formulation/formulation-nav-header.tsx` and `dsm/dsm-diagnosis-nav-header.tsx`. Every declared section is pinned against rendered DOM by `tests/in-page-nav-route-sections.dom.test.tsx`. -- **Shared secondary navigation**: `src/components/secondary-navigation.tsx` (`SecondaryNavigation`, route/action items, roving tablist) and `src/components/page-secondary-navigation.tsx` (`PageSecondaryNavigation`, mode destinations only). Mode destinations come from `src/lib/mode-secondary-navigation.ts` (`modeSecondaryNavigationRegistry`, no "Home" item). `GlobalSearchShell` renders it in normal flow at the top of `#main-content` for its owned namespaced modes; it self-suppresses on clean mode homes, on Therapy Compass, and on every information page — `hasLocalInformationPageNavigation` is now just `isInformationPage`, because each of those routes owns its own in-page navigation. The `section` kind and its "On this page" pill rail were removed once the last six routes moved onto `InPageNavHeader`; `route`/`action` survive as component API with tests but have no production constructor (`/issues #271`). +- **Shared in-page navigation**: `src/components/in-page-nav/` is the default template for section navigation on any mode page (`docs/search-chrome-behaviour.md`). `in-page-nav-header.tsx` (`InPageNavHeader`) owns the header row, both sheets and the `PhoneHeaderCollapsePortal` wrapper; `page-section-index.ts` (`PageSection`, `toDocumentSections`, `sectionTargetIds`) is the declaration shape; `use-resolved-page-sections.ts` narrows a declaration to the anchors actually rendered at this breakpoint; `use-in-page-section-nav.ts` composes that with `useDocumentSectionSpy` and `jumpToDocumentSection`; `use-page-section-weights.ts` measures segment weights; `use-in-page-chrome-metrics.ts` publishes `--inpage-anchor-offset`; `in-page-nav-classes.ts` holds the shared anchor (`inPageAnchor`) and actions-sheet row classes. Anchor measurement itself is `src/components/sticky-chrome-metrics.ts` (`useStickyChromeMetrics`), shared with the document viewer's `use-document-chrome-metrics.ts`. Mounted by `differentials/differential-detail-page.tsx`, `services/service-detail-page.tsx`, `forms/form-detail-page.tsx`, `dsm/dsm-differential-considerations-page.tsx`, and — through a colocated `"use client"` nav-header sibling that owns and exports the route's section table — `specifiers/specifier-nav-header.tsx`, `formulation/formulation-nav-header.tsx`, `dsm/dsm-diagnosis-nav-header.tsx`, `factsheets/factsheet-nav-header.tsx` and `clinical-dashboard/medication-nav-header.tsx`. The sibling is mandatory for the four Server Component pages (neither `onSelectSection` nor a `LucideIcon` crosses the RSC boundary) and the convention for the rest. Two adopters swap panels instead of scrolling — `differential-detail-page.tsx` and the medication record page — so they pass explicit weights, carry no `inPageAnchor`, and use neither `useResolvedPageSections` nor the scroll spy. Every declared section is pinned against rendered DOM by `tests/in-page-nav-route-sections.dom.test.tsx` (anchors for the scrolling routes, swapped-in panels for the tab routes). +- **Shared secondary navigation**: `src/components/page-secondary-navigation.tsx` (`PageSecondaryNavigation`, mode destinations only). Mode destinations come from `src/lib/mode-secondary-navigation.ts` (`modeSecondaryNavigationRegistry`, no "Home" item). `GlobalSearchShell` renders it in normal flow at the top of `#main-content` for its owned namespaced modes; it self-suppresses on clean mode homes, on Therapy Compass, and on every information page — `hasLocalInformationPageNavigation` is now just `isInformationPage`, because each of those routes owns its own in-page navigation. The older shared `SecondaryNavigation` component was deleted here (`/issues #271`): its `section` kind and "On this page" pill rail went when the last six information routes moved onto `InPageNavHeader`, and the surviving `route`/`action` kinds had no production constructor left — `RegistryModeNav` renders `ModeNav`, not `SecondaryNavigation`, so the only remaining caller was its own test file, which went with it. - **Local filter fields** (sidebar "Search chats", document drawer "Find a document"/"Find a source PDF") are scoped filters, not global search; they share the `fieldControlWithIcon`/`fieldIcon` primitives. - **Wiring conventions** for buttons and route navigation (and the gates that enforce them — the dead-button ESLint rule and the orphan-route reachability test) live in `docs/wiring-conventions.md`. diff --git a/docs/search-chrome-behaviour.md b/docs/search-chrome-behaviour.md index 1328a95cc6..150938d00c 100644 --- a/docs/search-chrome-behaviour.md +++ b/docs/search-chrome-behaviour.md @@ -151,11 +151,12 @@ so it was left alone. **Adopted so far:** `/differentials/diagnoses/[slug]`, `/services/[slug]`, `/forms/[slug]`, `/specifiers/[slug]` (record and catalogue reference), `/formulation/[slug]`, -`/dsm/diagnoses/[slug]` and its `/differentials` child, and factsheet detail pages -(`/factsheets/[slug]`). Each is also listed in +`/dsm/diagnoses/[slug]` and its `/differentials` child, `/factsheets/[slug]`, and +`/medications/[slug]`. Each is also listed in `isHeaderAddonSlotOwnedRoute` (`src/components/mode-nav/header-addon-slot.ts`), which is how -the one-header-per-slot rule stays checkable. Still on their own patterns: medications -(`SectionTabs`) and differentials presentations. +the one-header-per-slot rule stays checkable. Two routes remain outside the template on +purpose: `DocumentViewer` (decided above) and the differentials presentations workflow +(decided below). **Visual slots (adapt labels, back href, sections, and actions to the mode):** @@ -179,7 +180,7 @@ the one-header-per-slot rule stays checkable. Still on their own patterns: medic - Do not give the in-page header its own scroll-hide hook; share the universal collapse signal described under “Scroll hide/reveal”. -**The breadcrumb shape (pages with no section index).** The eight record pages behind +**The breadcrumb shape (pages with no section index).** The record pages behind `InformationPageBreadcrumbs` have no sections, so the disclosure would open a sheet listing one item and the track would render one full-width segment. Omit `sections` and `InPageNavHeader` drops both and renders the breadcrumb shape instead — same row grammar, @@ -200,12 +201,74 @@ shape that row: the row; from `sm` it sits inline and costs no extra height (measured on `/factsheets/sertraline`: 131px phone, 75px from `sm`, 65px with no mode). -Adopted by `src/components/factsheets/factsheet-detail-page.tsx`. When a page adopts this, +Used by `medication-nav-header.tsx` while the record is still loading (no record, no +sections) and by `factsheet-nav-header.tsx` for the seven non-`medRich` sheets, which carry +one reading level and so pass no `mode`. When a page adopts this, register its routes in `isHeaderAddonSlotOwnedRoute` (`src/components/mode-nav/header-addon-slot.ts`) and add the component to the expected claimants in `tests/mode-nav-addon-slot.dom.test.tsx`, or that guard fails: the slot holds exactly one page-owned header. +### Panel-swap adopters: the track drives tabs, and the sections carry no anchor + +Two adopters exchange a panel rather than scrolling: `/differentials/diagnoses/[slug]` +(`differential-detail-page.tsx`) and `/medications/[slug]` +(`medication-nav-header.tsx`). Their `PageSection.id` is the tab id, not a DOM anchor id, +and the rules above change in three specific ways: + +- **Pass explicit `weight`s.** `usePageSectionWeights` measures rendered heights and only + the active panel is ever rendered, so measurement would report one full-width segment + beside three empty ones. Both routes derive weights from what each panel holds — section + counts, with a floor so an empty tab stays visible and a cap so one dense tab does not + squeeze the rest to hairlines. +- **No `inPageAnchor`, and no `useInPageSectionNav`.** There is nothing to scroll to, so + there is no scroll margin to set and no scroll spy to run: `activeId` is the active tab + and `onSelectSection` sets it. `useResolvedPageSections` must not be used either — it + would drop the three tabs whose panels are not currently mounted and collapse the track to + one segment. (`differential-detail-page.tsx`'s existing `scroll-mt-24` values are inside + panel bodies and are unrelated; leave them.) +- **Never claim `collapsible`.** The trailing chevron in `DocumentSectionList` means "this + row opens an accordion". Selecting a tab swaps a panel instead. + +The panel is not a `tabpanel` and its control is not a `tab`: the section list is a list of +buttons, so a `role="tab"` / `aria-controls` pair would name a tablist that no longer exists. +Keep a per-tab `id` on the panel — that is the rendered evidence a declared section resolves +to something real, which is what the panel-swap half of +`tests/in-page-nav-route-sections.dom.test.tsx` asserts in place of the anchor check. + +### The differentials presentations workflow keeps its own layout — decided, not pending + +`src/components/differentials/differential-presentation-workflow-page.tsx` is **not** being +converted, and this is a decision rather than a backlog item. Do not re-open it without new +facts against the three reasons below. + +The premise that it was a conversion candidate does not survive reading it. It was carried +forward as "a `SectionTabs` page that swaps panels"; it swaps nothing. Its `MobileTabs` is +four `<Link>`s to **other routes** — the diagnosis detail page, its `?tab=map` and +`?tab=related` views, and the compare route — with "Compare" hardcoded as the active one. +That is mode-level page switching, which the "Not this template" note below already carves +out for Therapy-style `ModeNav`, and the same carve-out covers this row. + +1. **It is a comparison workspace, not a reading spine.** Every candidate section — + `SafetySnapshot`, the comparison table, `ReviewPanels`, `SourceStatusPanel` — is rendered + two or three times at different breakpoints, in different DOM parents: an `xl` sidebar + `<aside>`, an `md`–`lg` grid, and a phone copy nested _inside_ the mobile comparison + section. `PageSection.targetIds` resolves a phone/desktop pair; it does not model three + copies where one is a descendant of another section's anchor. +2. **Its own header row is cross-route navigation plus a phone footer.** The page already + owns a `PhoneFooterLayerPortal` action bar, and its back control is duplicated across two + breakpoint blocks. Mounting the shared header would leave the `MobileTabs` route row in + place beside it — two navigation rows, one in-page and one cross-route, which is the + wrapping-toolbar shape the template exists to remove. +3. **It is a Server Component with no client half.** Adding one is cheap; adding one whose + only job is to declare sections that resolve inconsistently across three breakpoints is + not. + +`isHeaderAddonSlotOwnedRoute` therefore continues to return `false` for +`/differentials/presentations/[slug]`, and `tests/mode-nav-addon-slot.dom.test.tsx` pins +that. If the differentials mode nav is reworked so this page stops owning a route-tab row, +revisit reason 2 then — not before. + **Not this template:** Therapy-style `ModeNav` (multi-route mode tabs via `ModeNavHeaderPortal`) is a different pattern for mode-level page switching. Info-page `PageHeader` / breadcrumb chrome is also not in-page section navigation. Existing diff --git a/src/components/clinical-dashboard/medication-nav-header.tsx b/src/components/clinical-dashboard/medication-nav-header.tsx new file mode 100644 index 0000000000..da64fe1608 --- /dev/null +++ b/src/components/clinical-dashboard/medication-nav-header.tsx @@ -0,0 +1,150 @@ +"use client"; + +import { CalendarDays, ClipboardList, Layers, ShieldAlert } from "lucide-react"; +import type { ReactNode } from "react"; + +import { InPageNavHeader } from "@/components/in-page-nav/in-page-nav-header"; +import type { PageSection } from "@/components/in-page-nav/page-section-index"; +import { appModeHomeHref } from "@/lib/app-modes"; +import type { MedicationRecord, MedicationSection } from "@/lib/medications"; + +export const MEDICATION_TAB_IDS = ["summary", "dosing", "safety", "more"] as const; +export type MedicationTabId = (typeof MEDICATION_TAB_IDS)[number]; + +export function isMedicationTabId(value: string): value is MedicationTabId { + return (MEDICATION_TAB_IDS as readonly string[]).includes(value); +} + +/** + * Which raw section types each tab claims. + * + * This lived inside `MedicationRecordDetail` as a `useMemo` over four inline + * `Set`s. It moves here because it *is* the section index: the header's segment + * weights and row details are derived from the same grouping the panel renders, + * and two copies of it would drift into a track that no longer describes what a + * tab contains. + */ +const tabSectionTypes: Record<MedicationTabId, ReadonlySet<string>> = { + summary: new Set(["summary", "ind", "form"]), + dosing: new Set(["dose"]), + safety: new Set(["risk", "contra", "mon", "safe"]), + more: new Set(["inter", "pearl", "evid", "spec", "comp", "sel", "src"]), +}; + +export function medicationSectionsByTab(record: MedicationRecord): Record<MedicationTabId, MedicationSection[]> { + return { + summary: record.sections.filter((section) => tabSectionTypes.summary.has(section.type)), + dosing: record.sections.filter((section) => tabSectionTypes.dosing.has(section.type)), + safety: record.sections.filter((section) => tabSectionTypes.safety.has(section.type)), + more: record.sections.filter((section) => tabSectionTypes.more.has(section.type)), + }; +} + +/** + * The medication record page's navigable sections. + * + * Ids and labels carried over verbatim from the `SectionTabs` roving-tabindex + * tablist this replaces, so a reader's mental map of the page is unchanged — + * only the control moved into the shared header. + * + * Exported as the base table (the shape `docs/search-chrome-behaviour.md` + * requires a nav-header sibling to own) with `buildMedicationNavSections` adding + * the record-derived weight and detail on top. The ids are tab ids rather than + * DOM anchor ids: these are discrete panels, so there is nothing to scroll to + * and no `inPageAnchor` — the same model `differential-detail-page.tsx` uses. + */ +export const medicationNavSections: readonly PageSection[] = [ + { id: "summary", label: "Summary", icon: ClipboardList }, + { id: "dosing", label: "Dosing", icon: CalendarDays }, + { id: "safety", label: "Safety", icon: ShieldAlert }, + { id: "more", label: "More", icon: Layers }, +]; + +/** + * Raw weights before normalisation, anchored to how many sections each tab + * actually holds so the track reads as proportion rather than as "which of + * four". The floor keeps a tab with nothing in it visible and tappable-looking + * — every tab is always offered, because a track whose length changed between + * medications would read as a rendering bug — and the cap stops one dense tab + * (Safety carries four section types) squeezing the rest to hairlines. + */ +function rawWeight(count: number): number { + return Math.max(1, Math.min(5, count)); +} + +function plural(count: number, singular: string) { + return `${count} ${count === 1 ? singular : `${singular}s`}`; +} + +/** + * Builds the weighted section index for one medication. Pure and deterministic: + * the same record always produces the same weights, so the track never shifts + * between renders of the same page. + * + * Explicit weights rather than measured ones. `usePageSectionWeights` measures + * rendered heights, and only the active panel is ever rendered here — measuring + * would report one full-width segment and three empty ones. + */ +export function buildMedicationNavSections(record: MedicationRecord): PageSection[] { + const byTab = medicationSectionsByTab(record); + const weights = MEDICATION_TAB_IDS.map((id) => rawWeight(byTab[id].length)); + const total = weights.reduce((sum, weight) => sum + weight, 0) || 1; + + return medicationNavSections.map((section, index) => ({ + ...section, + detail: plural(byTab[section.id as MedicationTabId].length, "section"), + weight: (weights[index] ?? 1) / total, + // The trailing chevron in `DocumentSectionList` means "this row opens an + // accordion". Selecting a tab swaps a panel instead, so never claim it. + collapsible: false, + })); +} + +/** + * The medication record page's in-page navigation. + * + * The page is already a Client Component, so this sibling is convention rather + * than necessity — but it is the convention: `docs/search-chrome-behaviour.md` + * pins the section table to a colocated nav-header sibling regardless of the + * page's RSC boundary, so there is one answer to where the table lives and one + * import path for the per-route contract test. + */ +export function MedicationNavHeader({ + title, + record, + activeTab, + onSelectTab, + actions, +}: { + title: string; + /** Absent while the record is loading or unresolvable: no record, no sections. */ + record: MedicationRecord | null; + activeTab: MedicationTabId; + onSelectTab: (id: MedicationTabId) => void; + actions?: ReactNode; +}) { + const back = { href: appModeHomeHref("prescribing"), label: "Medications" }; + const shared = { + back, + title, + actions, + actionsNoun: "medication" as const, + actionsDescription: "Choose how to use this medication.", + testIdPrefix: "medication" as const, + }; + + // No record means no sections to offer, so the header falls back to the + // breadcrumb shape rather than drawing a four-segment track over nothing. + if (!record) return <InPageNavHeader {...shared} />; + + return ( + <InPageNavHeader + {...shared} + sections={buildMedicationNavSections(record)} + activeId={activeTab} + onSelectSection={(id) => { + if (isMedicationTabId(id)) onSelectTab(id); + }} + /> + ); +} diff --git a/src/components/clinical-dashboard/medication-record-page.tsx b/src/components/clinical-dashboard/medication-record-page.tsx index e3bb1d0890..aae1daf087 100644 --- a/src/components/clinical-dashboard/medication-record-page.tsx +++ b/src/components/clinical-dashboard/medication-record-page.tsx @@ -18,10 +18,16 @@ import { Timer, type LucideIcon, } from "lucide-react"; -import { useMemo, useRef, useState, type CSSProperties, type KeyboardEvent as ReactKeyboardEvent } from "react"; +import { useMemo, useState, type CSSProperties } from "react"; import { BadgeCluster } from "@/components/clinical-dashboard/clinical-badge"; import { MedicationConsiderations } from "@/components/clinical-dashboard/medication-considerations"; +import { + MedicationNavHeader, + medicationNavSections, + medicationSectionsByTab, + type MedicationTabId, +} from "@/components/clinical-dashboard/medication-nav-header"; import { PatientProfilePanel } from "@/components/clinical-dashboard/patient-profile-panel"; import { useMedicationDetail } from "@/components/clinical-dashboard/use-medication-catalog"; import { @@ -49,12 +55,7 @@ import { toneSuccess, toneWarning, } from "@/components/ui-primitives"; -import { - InformationPageBreadcrumbs, - InformationPageFooter, - InformationPageShell, -} from "@/components/information-page-shell"; -import { appModeHomeHref } from "@/lib/app-modes"; +import { InformationPageFooter, InformationPageShell } from "@/components/information-page-shell"; const sectionIcons: Record<string, LucideIcon> = { dose: CalendarDays, @@ -172,74 +173,6 @@ function DetailTile({ metric }: { metric: MedicationHeroMetric }) { ); } -const detailTabs = [ - ["summary", "Summary"], - ["dosing", "Dosing"], - ["safety", "Safety"], - ["more", "More"], -] as const; -type MedicationTabId = (typeof detailTabs)[number][0]; - -function SectionTabs({ active, onChange }: { active: MedicationTabId; onChange: (id: MedicationTabId) => void }) { - const tabRefs = useRef(new Map<MedicationTabId, HTMLButtonElement>()); - - function handleKeyDown(event: ReactKeyboardEvent<HTMLElement>) { - const order = detailTabs.map((tab) => tab[0]); - const index = order.indexOf(active); - const next = - event.key === "ArrowRight" - ? order[(index + 1) % order.length] - : event.key === "ArrowLeft" - ? order[(index - 1 + order.length) % order.length] - : event.key === "Home" - ? order[0] - : event.key === "End" - ? order[order.length - 1] - : null; - if (!next) return; - event.preventDefault(); - if (next !== active) onChange(next); - tabRefs.current.get(next)?.focus(); - } - - return ( - <nav - role="tablist" - aria-label="Medication sections" - onKeyDown={handleKeyDown} - className="flex gap-1 border-b border-[color:var(--border)] text-sm font-semibold text-[color:var(--text-muted)]" - > - {detailTabs.map(([id, label]) => { - const isActive = active === id; - return ( - <button - key={id} - ref={(element) => { - if (element) tabRefs.current.set(id, element); - else tabRefs.current.delete(id); - }} - type="button" - role="tab" - id={`medication-tab-${id}`} - aria-selected={isActive} - aria-controls={`medication-panel-${id}`} - tabIndex={isActive ? 0 : -1} - onClick={() => onChange(id)} - className={cn( - "min-h-tap flex-1 whitespace-nowrap border-b-2 px-1 pb-2.5 pt-1.5 text-center text-2xs transition focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[color:var(--focus)] sm:flex-none sm:px-4 sm:text-sm", - isActive - ? "border-[color:var(--clinical-accent)] text-[color:var(--clinical-accent)]" - : "border-transparent hover:text-[color:var(--text-heading)]", - )} - > - {label} - </button> - ); - })} - </nav> - ); -} - function SectionCard({ section }: { section: MedicationSection }) { const Icon = sectionIcons[section.type] || ClipboardList; const toneClass = sectionToneClass[section.type] || defaultSectionTone; @@ -366,29 +299,19 @@ function MedicationAccessPanel({ record }: { record: MedicationRecord }) { function MedicationRecordDetail({ record, governance, + activeTab, }: { record: MedicationRecord; governance?: MedicationGovernance; + /** Owned by `MedicationRecordPage` so the shared header can drive it. */ + activeTab: MedicationTabId; }) { const metrics = useMemo(() => medicationHeroMetrics(record), [record]); const badges = useMemo(() => medicationIdentityBadges(record, governance), [record, governance]); const indication = useMemo(() => medicationIndication(record), [record]); - const [activeTab, setActiveTab] = useState<MedicationTabId>("summary"); - - const sectionsByTab = useMemo(() => { - const summaryTypes = new Set(["summary", "ind", "form"]); - const dosingTypes = new Set(["dose"]); - const safetyTypes = new Set(["risk", "contra", "mon", "safe"]); - const moreTypes = new Set(["inter", "pearl", "evid", "spec", "comp", "sel", "src"]); - return { - summary: record.sections.filter((section) => summaryTypes.has(section.type)), - dosing: record.sections.filter((section) => dosingTypes.has(section.type)), - safety: record.sections.filter((section) => safetyTypes.has(section.type)), - more: record.sections.filter((section) => moreTypes.has(section.type)), - }; - }, [record.sections]); - + const sectionsByTab = useMemo(() => medicationSectionsByTab(record), [record]); const activeSections = sectionsByTab[activeTab]; + const activeTabLabel = medicationNavSections.find((section) => section.id === activeTab)?.label ?? "Medication"; return ( <div className="space-y-3 py-1 sm:py-2" style={medicationAccentStyle(record.accent)}> @@ -439,12 +362,15 @@ function MedicationRecordDetail({ <MedicationConsiderations record={record} /> </section> - <SectionTabs active={activeTab} onChange={setActiveTab} /> - + {/* The panel is no longer a `tabpanel`: the control that swaps it is + the shared header's section list, which is a list of buttons rather + than a tablist, so claiming the role would name a `tab` that no + longer exists. The id stays per-tab — it is the rendered evidence + that a declared section resolves to a real panel, which is what + `tests/in-page-nav-route-sections.dom.test.tsx` asserts. */} <section - role="tabpanel" id={`medication-panel-${activeTab}`} - aria-labelledby={`medication-tab-${activeTab}`} + aria-label={`${activeTabLabel} sections`} className="overflow-hidden rounded-lg border border-[color:var(--border)] border-l-[3px] border-l-[color:var(--med-accent)] bg-[color:var(--surface-raised)] shadow-[var(--shadow-soft)]" > {activeSections.length ? ( @@ -499,37 +425,39 @@ export function MedicationRecordPage({ // flight. A failed request means the authoritative status is unknown, so // don't keep presenting the fixture-derived guess as if it were confirmed. const governance = data?.governance ?? (error ? undefined : fallbackGovernance); + // Owned here rather than in `MedicationRecordDetail` so the shared header — + // which sits above the shell — can drive it. The record can swap underneath + // (SSR fallback → live), and every record offers the same four tabs, so the + // selection survives that swap rather than snapping back to Summary. + const [activeTab, setActiveTab] = useState<MedicationTabId>("summary"); return ( - <InformationPageShell testId={`medication-page-${slug}`} gap={false}> - <InformationPageBreadcrumbs - home={{ - label: "Medications", - // Plain mode home. Carrying the slug as a query made sense when - // `/?mode=prescribing` was the Medication home; that URL is now the shared - // home, so a query here would land the breadcrumb on `/` with the drug - // name prefilled instead of on Medications. - href: appModeHomeHref("prescribing"), - }} - current={record?.name ?? slug} + <> + <MedicationNavHeader + title={record?.name ?? slug} + record={record} + activeTab={activeTab} + onSelectTab={setActiveTab} /> - <div className="mt-3"> - {record ? ( - <MedicationRecordDetail record={record} governance={governance} /> - ) : loading ? ( - <LoadingPanel label="Loading medication reference…" variant="skeleton" lines={6} /> - ) : ( - <div className="rounded-lg border border-[color:var(--danger-border)] bg-[color:var(--danger-bg)] p-4 text-sm text-[color:var(--danger-text)]"> - <div className="flex items-start gap-2"> - <TriangleAlert className="mt-0.5 h-4 w-4 shrink-0" aria-hidden="true" /> - <p>{error ?? "Medication not found."}</p> + <InformationPageShell testId={`medication-page-${slug}`} gap={false}> + <div className="mt-3"> + {record ? ( + <MedicationRecordDetail record={record} governance={governance} activeTab={activeTab} /> + ) : loading ? ( + <LoadingPanel label="Loading medication reference…" variant="skeleton" lines={6} /> + ) : ( + <div className="rounded-lg border border-[color:var(--danger-border)] bg-[color:var(--danger-bg)] p-4 text-sm text-[color:var(--danger-text)]"> + <div className="flex items-start gap-2"> + <TriangleAlert className="mt-0.5 h-4 w-4 shrink-0" aria-hidden="true" /> + <p>{error ?? "Medication not found."}</p> + </div> </div> - </div> - )} - </div> - <InformationPageFooter className="mt-4 pb-1"> - Clinical KB provides evidence summaries, not medical advice. Verify clinical decisions. - </InformationPageFooter> - </InformationPageShell> + )} + </div> + <InformationPageFooter className="mt-4 pb-1"> + Clinical KB provides evidence summaries, not medical advice. Verify clinical decisions. + </InformationPageFooter> + </InformationPageShell> + </> ); } diff --git a/src/components/factsheets/factsheet-detail-page.tsx b/src/components/factsheets/factsheet-detail-page.tsx index f0a73e8f00..317e6077d4 100644 --- a/src/components/factsheets/factsheet-detail-page.tsx +++ b/src/components/factsheets/factsheet-detail-page.tsx @@ -7,7 +7,6 @@ import { Check, ChevronRight, Clock, - Download, HeartHandshake, Printer, Share2, @@ -23,11 +22,11 @@ import { printBlocks, relatedFactsheets, sameTopicFactsheets, - tocFor, type Factsheet, } from "@/components/factsheets/factsheets-data"; import { factsheetGlyph } from "@/components/factsheets/factsheets-icons"; -import { InPageNavHeader } from "@/components/in-page-nav/in-page-nav-header"; +import { FactsheetNavHeader, factsheetBodySectionId } from "@/components/factsheets/factsheet-nav-header"; +import { inPageAnchor } from "@/components/in-page-nav/in-page-nav-classes"; import { InformationPageShell } from "@/components/information-page-shell"; import { cn, toneDanger, toneWarning } from "@/components/ui-primitives"; import { @@ -67,7 +66,6 @@ export function FactsheetDetailPage({ factsheet }: { factsheet: Factsheet }) { const related = relatedFactsheets(factsheet.slug); const moreInTopic = sameTopicFactsheets(factsheet.slug); - const toc = tocFor(factsheet); const blocks = printBlocks(factsheet, readingLevel); useEffect(() => { @@ -112,31 +110,16 @@ export function FactsheetDetailPage({ factsheet }: { factsheet: Factsheet }) { return ( <> <InformationPageShell testId="factsheet-detail-page" width="bleed" className="factsheet-screen"> - {/* The breadcrumb shape of the shared in-page header: no section index, - so no disclosure and no segment track. Reading level is a page-level - view mode rather than an action, so it rides the `mode` slot and - stays out of the row reserved for verbs. */} - <InPageNavHeader - back={{ href: "/factsheets/search", label: "All factsheets" }} - showBackLabel={false} - title={factsheet.title} - primaryAction={{ label: "Download PDF", icon: Download, onClick: downloadPdf }} - mode={ - // Only `medRich` sheets carry both reading levels; the other seven - // must not reserve the band. - factsheet.kind === "medRich" - ? { - label: "Reading level", - value: readingLevel, - options: readingLevelOptions, - onChange: (value) => setReadingLevel(value === "standard" ? "standard" : "easy"), - } - : undefined - } - actionsTitle="This factsheet" - actionsDescription={`${factsheet.title} · updated ${factsheet.reviewedOn}`} - actionsNoun="factsheet" - testIdPrefix="factsheet" + {/* Section index, disclosure and segment track now come from the sibling + that owns the table. Reading level stays a page-level view mode + rather than an action, so it rides the `mode` slot and stays out of + the row reserved for verbs. */} + <FactsheetNavHeader + factsheet={factsheet} + readingLevel={readingLevel} + readingLevelOptions={readingLevelOptions} + onReadingLevelChange={(value) => setReadingLevel(value === "standard" ? "standard" : "easy")} + onDownload={downloadPdf} actions={() => ( // Deliberately does not close the sheet: saving is a state change // you want to see reflected on the control you just pressed. The @@ -206,7 +189,7 @@ export function FactsheetDetailPage({ factsheet }: { factsheet: Factsheet }) { </div> {/* sources */} - <section className="mt-7"> + <section id="factsheet-sources" className={cn(inPageAnchor, "mt-7")}> <Heading>Where this information comes from</Heading> <div className="mt-3 grid gap-2"> {factsheet.sources.map((source) => { @@ -259,7 +242,10 @@ export function FactsheetDetailPage({ factsheet }: { factsheet: Factsheet }) { {/* more in topic */} {moreInTopic.length ? ( - <section className="mt-7 border-t border-[color:var(--border)] pt-6"> + <section + id="factsheet-more-in-topic" + className={cn(inPageAnchor, "mt-7 border-t border-[color:var(--border)] pt-6")} + > <div className="mb-3 flex items-center justify-between gap-3"> <Heading>More in {factsheet.category}</Heading> <Link @@ -305,7 +291,10 @@ export function FactsheetDetailPage({ factsheet }: { factsheet: Factsheet }) { ) : null} {/* related */} - <section className="mt-7 border-t border-[color:var(--border)] pt-6"> + <section + id="factsheet-related" + className={cn(inPageAnchor, "mt-7 border-t border-[color:var(--border)] pt-6")} + > <Heading>Related sheets</Heading> <div className="mt-3 grid gap-2.5 sm:grid-cols-2 lg:grid-cols-3"> {related.map((sheet) => { @@ -362,16 +351,12 @@ export function FactsheetDetailPage({ factsheet }: { factsheet: Factsheet }) { {factsheet.reviewedOn} </p> </div> - <div className="border-t border-[color:var(--border)] p-4"> - <p className="text-2xs font-bold uppercase tracking-label text-[color:var(--text-muted)]">On this page</p> - <ul className="mt-2 grid gap-1.5"> - {toc.map((item) => ( - <li key={item} className="text-sm text-[color:var(--text-muted)]"> - {item} - </li> - ))} - </ul> - </div> + {/* "On this page" used to live here as a plain `<li>` list built by + `tocFor` — display strings with no anchors behind them, so it + read as a table of contents and navigated nowhere. The header's + section disclosure now owns that job at every width, with the + labels derived from what this sheet actually renders, so a second + inert copy would be the same decoration twice. */} <div className="grid gap-2 border-t border-[color:var(--border)] p-4"> <button type="button" @@ -430,7 +415,8 @@ function FactsheetBody({ return ( <div className="flex flex-col gap-6"> <div - className="rounded-2xl border p-5" + id="factsheet-at-a-glance" + className={cn(inPageAnchor, "rounded-2xl border p-5")} style={{ backgroundColor: theme.soft, borderColor: accentBorder(theme.accent) }} > <p className="text-2xs font-bold uppercase tracking-label" style={{ color: theme.accent }}> @@ -445,13 +431,13 @@ function FactsheetBody({ ))} </div> </div> - <section> + <section id="factsheet-what" className={inPageAnchor}> <Heading>What is {factsheet.title.toLowerCase()}?</Heading> <p className="mt-2 max-w-[66ch] text-pretty text-base leading-7 text-[color:var(--text)]"> {readingLevel === "easy" ? factsheet.whatEasy : factsheet.whatStandard} </p> </section> - <section> + <section id="factsheet-howto" className={inPageAnchor}> <Heading>How to take it</Heading> <div className="mt-3 flex flex-col gap-3"> {factsheet.howto.map((step) => ( @@ -469,7 +455,7 @@ function FactsheetBody({ ))} </div> </section> - <section> + <section id="factsheet-side-effects" className={inPageAnchor}> <Heading>Side effects</Heading> <div className="mt-3 grid gap-3.5 sm:grid-cols-2"> <div className="rounded-2xl border border-[color:var(--border)] bg-[color:var(--surface)] p-4"> @@ -510,7 +496,13 @@ function FactsheetBody({ </div> </div> </section> - <div className="flex gap-3.5 rounded-2xl border border-[color:var(--danger-border)] bg-[color:var(--surface)] p-5"> + <div + id="factsheet-urgent" + className={cn( + inPageAnchor, + "flex gap-3.5 rounded-2xl border border-[color:var(--danger-border)] bg-[color:var(--surface)] p-5", + )} + > <span className="grid h-tap w-tap shrink-0 place-items-center rounded-xl bg-[color:var(--danger-solid)] text-[color:var(--danger-solid-contrast)]"> <Zap className="h-5 w-5" aria-hidden="true" /> </span> @@ -527,7 +519,8 @@ function FactsheetBody({ return ( <div className="flex flex-col gap-5"> <div - className="flex gap-3.5 rounded-2xl border p-4" + id="factsheet-timing" + className={cn(inPageAnchor, "flex gap-3.5 rounded-2xl border p-4")} style={{ backgroundColor: theme.soft, borderColor: accentBorder(theme.accent) }} > <Clock className="mt-0.5 h-5 w-5 shrink-0" style={{ color: theme.accent }} aria-hidden="true" /> @@ -536,8 +529,13 @@ function FactsheetBody({ <p className="mt-1 text-sm leading-6 text-[color:var(--text-muted)]">{factsheet.timing}</p> </div> </div> - {factsheet.sections.map((section) => ( - <section key={section.heading} className="border-l-[3px] pl-4" style={{ borderColor: theme.accent }}> + {factsheet.sections.map((section, index) => ( + <section + key={section.heading} + id={factsheetBodySectionId(index)} + className={cn(inPageAnchor, "border-l-[3px] pl-4")} + style={{ borderColor: theme.accent }} + > <h2 className="text-lg-minus font-bold text-[color:var(--text-heading)]">{section.heading}</h2> <p className="mt-1.5 max-w-[64ch] text-pretty text-base-minus leading-7 text-[color:var(--text)]"> {section.body} @@ -549,13 +547,13 @@ function FactsheetBody({ case "condition": return ( <div className="flex flex-col gap-6"> - <section> + <section id="factsheet-plain-terms" className={inPageAnchor}> <Heading>In plain terms</Heading> <p className="mt-2 max-w-[66ch] text-pretty text-base leading-7 text-[color:var(--text)]"> {factsheet.intro} </p> </section> - <section> + <section id="factsheet-signs" className={inPageAnchor}> <Heading>Signs to look for</Heading> <div className="mt-3 grid gap-2.5 sm:grid-cols-2"> {factsheet.signs.map((sign) => ( @@ -574,13 +572,13 @@ function FactsheetBody({ ))} </div> </section> - <section> + <section id="factsheet-why" className={inPageAnchor}> <Heading>Why it happens</Heading> <p className="mt-2 max-w-[66ch] text-pretty text-base leading-7 text-[color:var(--text)]"> {factsheet.why} </p> </section> - <section> + <section id="factsheet-helps" className={inPageAnchor}> <Heading>What helps</Heading> <div className="mt-3 grid gap-3 sm:grid-cols-3"> {factsheet.helps.map((help) => ( @@ -601,7 +599,8 @@ function FactsheetBody({ </div> </section> <div - className="flex gap-3.5 rounded-2xl border p-5" + id="factsheet-support" + className={cn(inPageAnchor, "flex gap-3.5 rounded-2xl border p-5")} style={{ backgroundColor: theme.soft, borderColor: accentBorder(theme.accent) }} > <span @@ -625,13 +624,13 @@ function FactsheetBody({ case "therapy": return ( <div className="flex flex-col gap-6"> - <section> + <section id="factsheet-what-it-is" className={inPageAnchor}> <Heading>What it is</Heading> <p className="mt-2 max-w-[66ch] text-pretty text-base leading-7 text-[color:var(--text)]"> {factsheet.intro} </p> </section> - <section> + <section id="factsheet-how-it-works" className={inPageAnchor}> <Heading>How it works</Heading> <div className="mt-3.5"> {factsheet.steps.map((step, index) => ( @@ -657,7 +656,7 @@ function FactsheetBody({ ))} </div> </section> - <section> + <section id="factsheet-what-to-expect" className={inPageAnchor}> <Heading>What to expect</Heading> <div className="mt-3 grid gap-3 sm:grid-cols-2"> {factsheet.expect.map((item) => ( @@ -676,13 +675,13 @@ function FactsheetBody({ case "procedure": return ( <div className="flex flex-col gap-6"> - <section> + <section id="factsheet-why-it-matters" className={inPageAnchor}> <Heading>Why it matters</Heading> <p className="mt-2 max-w-[66ch] text-pretty text-base leading-7 text-[color:var(--text)]"> {factsheet.why} </p> </section> - <section> + <section id="factsheet-prepare" className={inPageAnchor}> <Heading>How to prepare</Heading> <div className="mt-3 flex flex-col gap-2.5"> {factsheet.prepare.map((item) => ( @@ -701,7 +700,7 @@ function FactsheetBody({ ))} </div> </section> - <section> + <section id="factsheet-timeline" className={inPageAnchor}> <Heading>Step by step</Heading> <div className="mt-3 grid gap-3 sm:grid-cols-3"> {factsheet.timeline.map((step) => ( @@ -717,7 +716,10 @@ function FactsheetBody({ ))} </div> </section> - <div className={cn("flex gap-3.5 rounded-2xl border p-5", toneWarning)}> + <div + id="factsheet-staying-safe" + className={cn(inPageAnchor, "flex gap-3.5 rounded-2xl border p-5", toneWarning)} + > <TriangleAlert className="mt-0.5 h-5 w-5 shrink-0 text-[color:var(--warning)]" aria-hidden="true" /> <div> <p className="text-sm font-bold text-[color:var(--warning)]">Staying safe between tests</p> diff --git a/src/components/factsheets/factsheet-nav-header.tsx b/src/components/factsheets/factsheet-nav-header.tsx new file mode 100644 index 0000000000..40b78cf869 --- /dev/null +++ b/src/components/factsheets/factsheet-nav-header.tsx @@ -0,0 +1,174 @@ +"use client"; + +import { + BookOpen, + Check, + Clock, + Compass, + Download, + HeartHandshake, + LayoutGrid, + Library, + ListChecks, + ListOrdered, + Pill, + ShieldCheck, + Sparkles, + Stethoscope, + TriangleAlert, + Waypoints, + Zap, + type LucideIcon, +} from "lucide-react"; +import { useMemo, type ReactNode } from "react"; + +import type { Factsheet } from "@/components/factsheets/factsheets-data"; +import { InPageNavHeader } from "@/components/in-page-nav/in-page-nav-header"; +import type { PageSection } from "@/components/in-page-nav/page-section-index"; +import { useInPageSectionNav } from "@/components/in-page-nav/use-in-page-section-nav"; +import type { SegmentedControlOption } from "@/components/ui/segmented-control"; + +/** + * Anchor id for the nth `medLite` body section. + * + * `medLite` is the one kind whose headings are content rather than layout — the + * fixture supplies `sections: Array<{ heading, body }>` — so its ids are + * positional. The label still comes from the heading, so the sheet reads as the + * page does. + */ +export function factsheetBodySectionId(index: number): string { + return `factsheet-section-${index}`; +} + +/** Sections every factsheet renders below its kind-specific body. */ +const trailingSections: readonly PageSection[] = [ + { id: "factsheet-sources", label: "Sources", icon: ShieldCheck }, + // Only rendered when the category holds another sheet, so it is declared + // unconditionally and `useResolvedPageSections` drops it when it is not there. + { id: "factsheet-more-in-topic", label: "More in topic", icon: Library }, + { id: "factsheet-related", label: "Related sheets", icon: Waypoints }, +]; + +function bodySections(factsheet: Factsheet): PageSection[] { + switch (factsheet.kind) { + case "medRich": + return [ + { id: "factsheet-at-a-glance", label: "At a glance", icon: LayoutGrid }, + { id: "factsheet-what", label: "What it is", icon: Pill }, + { id: "factsheet-howto", label: "How to take it", icon: ListOrdered }, + { id: "factsheet-side-effects", label: "Side effects", icon: TriangleAlert }, + { id: "factsheet-urgent", label: "Urgent help", icon: Zap }, + ]; + case "medLite": + return [ + { id: "factsheet-timing", label: "How long it takes", icon: Clock }, + ...factsheet.sections.map((section, index) => ({ + id: factsheetBodySectionId(index), + label: section.heading, + icon: BookOpen as LucideIcon, + })), + ]; + case "condition": + return [ + { id: "factsheet-plain-terms", label: "In plain terms", icon: Stethoscope }, + { id: "factsheet-signs", label: "Signs to look for", icon: Check }, + { id: "factsheet-why", label: "Why it happens", icon: Compass }, + { id: "factsheet-helps", label: "What helps", icon: Sparkles }, + { id: "factsheet-support", label: "You're not alone", icon: HeartHandshake }, + ]; + case "therapy": + return [ + { id: "factsheet-what-it-is", label: "What it is", icon: Stethoscope }, + { id: "factsheet-how-it-works", label: "How it works", icon: ListOrdered }, + { id: "factsheet-what-to-expect", label: "What to expect", icon: Sparkles }, + ]; + case "procedure": + return [ + { id: "factsheet-why-it-matters", label: "Why it matters", icon: Compass }, + { id: "factsheet-prepare", label: "How to prepare", icon: ListChecks }, + { id: "factsheet-timeline", label: "Step by step", icon: ListOrdered }, + { id: "factsheet-staying-safe", label: "Staying safe", icon: TriangleAlert }, + ]; + } +} + +/** + * The factsheet detail page's navigable sections, derived from what the page + * actually renders for this sheet's `kind`. + * + * This replaces `tocFor`, a hand-maintained switch that returned display strings + * rather than ids and was wrong in both directions — it named "What is this + * medicine?" where the page renders "What is <title>?", and omitted the Sources, + * More-in-topic and Related sections it renders on every sheet. Nothing jumped + * anywhere, because the strings were painted into a `<li>` list with no anchors + * behind them; the sidebar looked like a table of contents and was decoration. + * + * Derived rather than declared flat, because the five kinds render five + * different bodies and `medLite`'s headings come from the record. That is the + * `docs/search-chrome-behaviour.md` carve-out for data-derived sections — the + * builder still lives in the nav-header sibling, which is what the rule pins. + */ +export function factsheetNavSections(factsheet: Factsheet): PageSection[] { + return [...bodySections(factsheet), ...trailingSections]; +} + +/** + * The factsheet detail page's in-page navigation. + * + * The page is a Client Component, so this sibling is the convention rather than + * an RSC necessity — the same call `medication-nav-header.tsx` makes. It also + * keeps the header's prop surface (a promoted action, a view mode, an actions + * slot and now a section index) out of an 850-line page component. + */ +export function FactsheetNavHeader({ + factsheet, + readingLevel, + readingLevelOptions, + onReadingLevelChange, + onDownload, + actions, +}: { + factsheet: Factsheet; + readingLevel: string; + readingLevelOptions: ReadonlyArray<SegmentedControlOption<string>>; + onReadingLevelChange: (value: string) => void; + onDownload: () => void; + actions: ReactNode | ((close: () => void) => ReactNode); +}) { + // `useResolvedPageSections` treats the declared list as its effect identity, + // so a fresh array each render would re-subscribe the MutationObserver every + // render for nothing. Every other route passes a module-level constant; this + // one cannot, because the list depends on the record. + const declared = useMemo(() => factsheetNavSections(factsheet), [factsheet]); + const { sections, activeId, selectSection } = useInPageSectionNav(declared); + + return ( + <InPageNavHeader + back={{ href: "/factsheets/search", label: "All factsheets" }} + showBackLabel={false} + title={factsheet.title} + sections={sections} + activeId={activeId} + onSelectSection={selectSection} + primaryAction={{ label: "Download PDF", icon: Download, onClick: onDownload }} + mode={ + // Only `medRich` sheets carry both reading levels; the other seven must + // not reserve the band. + factsheet.kind === "medRich" + ? { + label: "Reading level", + value: readingLevel, + options: readingLevelOptions, + onChange: onReadingLevelChange, + } + : undefined + } + sectionSheetTitle={factsheet.title} + actionsTitle="This factsheet" + actionsDescription={`${factsheet.title} · updated ${factsheet.reviewedOn}`} + actionsNoun="factsheet" + testIdPrefix="factsheet" + actions={actions} + /> + ); +} diff --git a/src/components/factsheets/factsheets-data.ts b/src/components/factsheets/factsheets-data.ts index 717a086b02..a78b46fea7 100644 --- a/src/components/factsheets/factsheets-data.ts +++ b/src/components/factsheets/factsheets-data.ts @@ -747,24 +747,14 @@ export function printBlocks(sheet: Factsheet, readingLevel: "easy" | "standard" } } -export function tocFor(sheet: Factsheet): string[] { - switch (sheet.kind) { - case "medRich": - return [ - "At a glance", - "What is this medicine?", - "How to take it", - "Side effects", - "When to get urgent help", - "Sources", - ]; - case "medLite": - return ["How long it takes", ...sheet.sections.map((section) => section.heading), "Sources"]; - case "condition": - return ["In plain terms", "Signs to look for", "Why it happens", "What helps", "You’re not alone", "Sources"]; - case "therapy": - return ["What it is", "How it works", "What to expect", "Sources"]; - case "procedure": - return ["Why it matters", "How to prepare", "Step by step", "Staying safe", "Sources"]; - } -} +/* + * `tocFor` used to live here: a hand-maintained switch over `sheet.kind` + * returning heading *strings*, painted into an inert `<li>` list in the detail + * page's sidebar. It was wrong in both directions — it named "What is this + * medicine?" where the page renders "What is <title>?", and never listed the + * More-in-topic or Related sections the page renders on every sheet — and it + * could not have been right, because nothing tied a string to a rendered + * element. The section index is now `factsheetNavSections` + * (`factsheet-nav-header.tsx`), which returns ids asserted against the rendered + * DOM by `tests/in-page-nav-route-sections.dom.test.tsx`. + */ diff --git a/src/components/in-page-nav/in-page-nav-header.tsx b/src/components/in-page-nav/in-page-nav-header.tsx index 4f2881303d..fbe95102e4 100644 --- a/src/components/in-page-nav/in-page-nav-header.tsx +++ b/src/components/in-page-nav/in-page-nav-header.tsx @@ -110,7 +110,7 @@ export type InPageNavHeaderProps = * header's bottom edge. * * It has two shapes, and the section list decides which. With `sections`, the - * above. Without them — the eight record pages behind `InformationPageBreadcrumbs` + * above. Without them — the record pages behind `InformationPageBreadcrumbs` * have no section index — the disclosure and the track would be a sheet listing * one item and a single full-width segment, so both are dropped and the row * becomes the breadcrumb shape: back, title, an optional `primaryAction`, an @@ -118,9 +118,10 @@ export type InPageNavHeaderProps = * none of the section machinery. * * Extracted from the differentials detail page, which built the template by hand - * first. `DocumentViewer` still carries its own copy — it owns the `<h1>`, uses - * the `edge-glass-header` treatment, and is pinned by visual baselines, so - * converging it is a separate change rather than half of this one. + * first. `DocumentViewer` keeps its own copy — it owns the `<h1>`, uses the + * `edge-glass-header` treatment, and is pinned by visual baselines. That is a + * settled non-adoption, not pending work: see "DocumentViewer keeps its own + * header — decided, not pending" in `docs/search-chrome-behaviour.md`. * * `relative` on the header is load-bearing: the track is absolutely positioned * against it, and a `static` phone header would let the track escape to whichever diff --git a/src/components/mode-nav/header-addon-slot.ts b/src/components/mode-nav/header-addon-slot.ts index 438f30def6..b46d3730b5 100644 --- a/src/components/mode-nav/header-addon-slot.ts +++ b/src/components/mode-nav/header-addon-slot.ts @@ -29,9 +29,12 @@ export function isHeaderAddonSlotOwnedRoute(pathname: string): boolean { // differentials/differential-detail-page.tsx (diagnoses detail only — the // presentations workflow page renders no portal). if (pathname.startsWith("/differentials/diagnoses/")) return true; - // factsheets/factsheet-detail-page.tsx mounts the same shared header in its - // breadcrumb shape (no sections, so no disclosure and no track). + // factsheets/factsheet-nav-header.tsx, mounted by the detail page. if (isSlugDetail(pathname, "/factsheets", ["search"])) return true; + // clinical-dashboard/medication-nav-header.tsx, mounted by + // `MedicationRecordPage`. The header drives the panel swap that + // `SectionTabs` used to own, so the record page now claims the slot too. + if (isSlugDetail(pathname, "/medications")) return true; // The six information routes converted onto the shared `InPageNavHeader`, // which portals through `PhoneHeaderCollapsePortal` exactly as the two above // do. Each is a slug detail page, never the mode home or a diff --git a/src/components/secondary-navigation.tsx b/src/components/secondary-navigation.tsx deleted file mode 100644 index 8560bddc4e..0000000000 --- a/src/components/secondary-navigation.tsx +++ /dev/null @@ -1,230 +0,0 @@ -"use client"; - -import Link from "next/link"; -import { - createContext, - type KeyboardEvent as ReactKeyboardEvent, - type ReactNode, - useContext, - useEffect, - useRef, -} from "react"; -import { createPortal } from "react-dom"; - -import { cn } from "@/components/ui-primitives"; -import { resolveScrollBehavior } from "@/lib/scroll-behavior"; - -const SecondaryNavigationShellHostContext = createContext<HTMLElement | null>(null); - -export function SecondaryNavigationShellHostProvider({ - host, - children, -}: { - host: HTMLElement | null; - children: ReactNode; -}) { - return ( - <SecondaryNavigationShellHostContext.Provider value={host}>{children}</SecondaryNavigationShellHostContext.Provider> - ); -} - -type SecondaryNavigationBaseItem = { - id: string; - elementId?: string; - label: string; - shortLabel?: string; - icon?: ReactNode; -}; - -export type SecondaryNavigationRouteItem = SecondaryNavigationBaseItem & { - kind: "route"; - href: string; - current?: boolean; -}; - -/** - * No live consumer as of the mode-strip removal. The seven modes that built - * action items each registered a single entry that focused an already-visible - * composer, and those were deleted; `therapy-compass` still declares action - * entries but `PageSecondaryNavigation` early-returns on `/therapy-compass*` - * before reading them. - * - * Kept deliberately rather than deleted alongside them: the kind carries the - * `tablist` roving-focus behaviour and is covered directly by - * `tests/secondary-navigation.dom.test.tsx`, so this is component API with - * tests, not orphaned code. Removing it is a clean separate change — do not do - * half of each. Note it is invisible to `check:knip`, which runs without - * `--include exports`. - */ -export type SecondaryNavigationActionItem = SecondaryNavigationBaseItem & { - kind: "action"; - onSelect: () => void; - current?: boolean; - controlsId?: string; - disabled?: boolean; -}; - -export type SecondaryNavigationItem = SecondaryNavigationRouteItem | SecondaryNavigationActionItem; - -export function SecondaryNavigation({ - ariaLabel, - items, - activeId, - sticky = true, - stickyTop = 0, - tablist = false, - placeInShell = false, - className, -}: { - ariaLabel: string; - items: readonly SecondaryNavigationItem[]; - activeId?: string; - sticky?: boolean; - stickyTop?: number | string; - tablist?: boolean; - placeInShell?: boolean; - className?: string; -}) { - const shellHost = useContext(SecondaryNavigationShellHostContext); - const effectiveSticky = sticky && !(placeInShell && shellHost); - const navigationRef = useRef<HTMLElement | null>(null); - const railRef = useRef<HTMLDivElement | null>(null); - const resolvedActiveId = activeId; - const itemRefs = useRef(new Map<string, HTMLElement>()); - - // Keep the active chip in the horizontal rail without calling scrollIntoView — - // block:"nearest" also adjusts the page vertically when the bar is off-screen, - // which yanks readers back to the top as they scroll through long records. - useEffect(() => { - if (!resolvedActiveId) return; - const item = itemRefs.current.get(resolvedActiveId); - const rail = railRef.current; - if (!item || !rail) return; - const railRect = rail.getBoundingClientRect(); - const itemRect = item.getBoundingClientRect(); - const overflowLeft = itemRect.left - railRect.left; - const overflowRight = itemRect.right - railRect.right; - if (overflowLeft >= 0 && overflowRight <= 0) return; - const nextLeft = rail.scrollLeft + (overflowLeft < 0 ? overflowLeft : overflowRight); - if (typeof rail.scrollTo === "function") { - rail.scrollTo({ left: nextLeft, behavior: resolveScrollBehavior() }); - } else { - rail.scrollLeft = nextLeft; - } - }, [placeInShell, resolvedActiveId, shellHost]); - - function handleTabKeyDown(event: ReactKeyboardEvent<HTMLDivElement>) { - if (!tablist) return; - const enabled = items.filter((item) => item.kind === "action" && !item.disabled); - const activeIndex = enabled.findIndex((item) => item.id === resolvedActiveId); - const index = activeIndex >= 0 ? activeIndex : 0; - const next = - event.key === "ArrowRight" - ? enabled[(index + 1 + enabled.length) % enabled.length] - : event.key === "ArrowLeft" - ? enabled[(index - 1 + enabled.length) % enabled.length] - : event.key === "Home" - ? enabled[0] - : event.key === "End" - ? enabled[enabled.length - 1] - : null; - if (!next || next.kind !== "action") return; - event.preventDefault(); - next.onSelect(); - itemRefs.current.get(next.id)?.focus(); - } - - if (!items.length) return null; - - const itemClass = (selected: boolean) => - cn( - "inline-flex min-h-tap shrink-0 items-center justify-center gap-2 rounded-lg border px-3 text-xs font-bold transition motion-reduce:transition-none focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[color:var(--focus)] sm:text-sm", - selected - ? "border-[color:var(--clinical-accent-border)] bg-[color:var(--clinical-accent-soft)] text-[color:var(--clinical-accent)] shadow-[var(--shadow-inset)] forced-colors:outline forced-colors:outline-2 forced-colors:[outline-color:Highlight]" - : "border-transparent text-[color:var(--text-muted)] hover:bg-[color:var(--surface-subtle)] hover:text-[color:var(--text-heading)]", - ); - - const navigation = ( - <nav - ref={navigationRef} - aria-label={ariaLabel} - data-testid="secondary-navigation" - style={effectiveSticky ? { top: stickyTop } : undefined} - className={cn( - "secondary-navigation isolate z-20 w-full border-b border-[color:var(--border)] bg-[color:var(--surface-glass)] text-[color:var(--text)] shadow-[var(--shadow-tight)] backdrop-blur-xl", - effectiveSticky && "sticky top-0 transition-[top] motion-reduce:transition-none", - className, - )} - > - <div - ref={railRef} - role={tablist ? "tablist" : undefined} - aria-label={tablist ? ariaLabel : undefined} - onKeyDown={handleTabKeyDown} - className="polished-scroll mx-auto flex min-h-14 max-w-7xl items-center gap-1 overflow-x-auto overscroll-x-contain px-3 py-1.5 sm:px-5 lg:px-8" - > - {items.map((item) => { - const selected = item.id === resolvedActiveId || Boolean(item.current); - const label = ( - <> - {item.icon} - {item.shortLabel ? ( - <> - <span className="sm:hidden">{item.shortLabel}</span> - <span className="hidden sm:inline">{item.label}</span> - </> - ) : ( - <span>{item.label}</span> - )} - </> - ); - const setRef = (element: HTMLElement | null) => { - if (element) itemRefs.current.set(item.id, element); - else itemRefs.current.delete(item.id); - }; - - if (item.kind === "route") { - return ( - <Link - key={item.id} - ref={setRef as (element: HTMLAnchorElement | null) => void} - href={item.href} - aria-label={item.shortLabel ? item.label : undefined} - aria-current={selected ? "page" : undefined} - className={itemClass(selected)} - > - {label} - </Link> - ); - } - - return ( - <button - key={item.id} - ref={setRef as (element: HTMLButtonElement | null) => void} - type="button" - role={tablist ? "tab" : undefined} - id={ - tablist - ? (item.elementId ?? `${ariaLabel.toLowerCase().replace(/[^a-z0-9]+/g, "-")}-${item.id}`) - : undefined - } - aria-current={!tablist && selected ? "page" : undefined} - aria-label={item.shortLabel ? item.label : undefined} - aria-selected={tablist ? selected : undefined} - aria-controls={item.controlsId} - tabIndex={tablist ? (selected ? 0 : -1) : undefined} - disabled={item.disabled} - onClick={item.onSelect} - className={itemClass(selected)} - > - {label} - </button> - ); - })} - </div> - </nav> - ); - - return placeInShell && shellHost ? createPortal(navigation, shellHost) : navigation; -} diff --git a/tests/factsheets-data.test.ts b/tests/factsheets-data.test.ts index d7c86f8d0b..040421e1a1 100644 --- a/tests/factsheets-data.test.ts +++ b/tests/factsheets-data.test.ts @@ -10,7 +10,6 @@ import { findFactsheet, printBlocks, relatedFactsheets, - tocFor, } from "@/components/factsheets/factsheets-data"; const kinds = new Set(["medRich", "medLite", "condition", "therapy", "procedure"]); @@ -82,13 +81,12 @@ describe("factsheet library", () => { } }); - it("builds a print projection and table of contents for every kind", () => { + it("builds a print projection for every kind", () => { for (const sheet of factsheets) { const blocks = printBlocks(sheet); expect(blocks.length).toBeGreaterThan(0); // The sources block is always the final print block. expect(blocks.at(-1)?.kind).toBe("sources"); - expect(tocFor(sheet)).toContain("Sources"); } }); diff --git a/tests/in-page-nav-route-sections.dom.test.tsx b/tests/in-page-nav-route-sections.dom.test.tsx index 8f18d4fee8..b0808749e6 100644 --- a/tests/in-page-nav-route-sections.dom.test.tsx +++ b/tests/in-page-nav-route-sections.dom.test.tsx @@ -1,7 +1,14 @@ -import { cleanup, render } from "@testing-library/react"; +import { cleanup, render, screen, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; import type { ReactElement } from "react"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { + MEDICATION_TAB_IDS, + medicationNavSections, + medicationSectionsByTab, +} from "@/components/clinical-dashboard/medication-nav-header"; +import { MedicationRecordPage } from "@/components/clinical-dashboard/medication-record-page"; import { DsmDiagnosisPage } from "@/components/dsm/dsm-diagnosis-page"; import { dsmDiagnosisNavSections } from "@/components/dsm/dsm-diagnosis-nav-header"; import { @@ -9,6 +16,9 @@ import { dsmDifferentialNavSections, type DsmDifferentialConsideration, } from "@/components/dsm/dsm-differential-considerations-page"; +import { FactsheetDetailPage } from "@/components/factsheets/factsheet-detail-page"; +import { factsheetNavSections } from "@/components/factsheets/factsheet-nav-header"; +import { factsheets, type Factsheet } from "@/components/factsheets/factsheets-data"; import { FormDetailPage, formNavSections } from "@/components/forms/form-detail-page"; import { FormulationMechanismPage } from "@/components/formulation/formulation-mechanism-page"; import { formulationNavSections } from "@/components/formulation/formulation-nav-header"; @@ -20,6 +30,7 @@ import { SpecifierRecordPage } from "@/components/specifiers/specifier-record-pa import { SpecifierReferencePage } from "@/components/specifiers/specifier-reference-page"; import { dsmDiagnoses } from "@/lib/dsm"; import { formRecords } from "@/lib/forms"; +import { loadMedicationSnapshot } from "@/lib/medication-snapshot"; import { formulationMechanisms } from "@/lib/formulation"; import { serviceRecords } from "@/lib/services"; import { specifierCatalogItems, curatedEnrichmentFor } from "@/lib/specifiers-content"; @@ -38,6 +49,16 @@ vi.mock("@/components/account-data-provider", () => ({ }), })); +// The medication page refreshes its record through a live owner-aware fetch and +// mounts two data-owning sidebar panels. None of that is section navigation, so +// the page runs on its SSR fallback record here — exactly the content-first +// state `tests/medication-record-page.dom.test.tsx` pins. +vi.mock("@/components/clinical-dashboard/use-medication-catalog", () => ({ + useMedicationDetail: () => ({ data: null, loading: false, error: null }), +})); +vi.mock("@/components/clinical-dashboard/patient-profile-panel", () => ({ PatientProfilePanel: () => null })); +vi.mock("@/components/clinical-dashboard/medication-considerations", () => ({ MedicationConsiderations: () => null })); + afterEach(cleanup); /** @@ -65,6 +86,17 @@ type RouteCase = { * being skipped. */ conditional?: readonly string[]; + /** + * Declared sections this fixture legitimately does not render, because no + * fixture of its kind carries the data (the therapy and procedure factsheets + * are each alone in their category, so neither has a "More in topic" list). + * + * These are asserted *absent* rather than skipped: an unrendered anchor is + * the case `useResolvedPageSections` must drop, so proving it is missing is + * as load-bearing as proving the others are present. Do not use this to + * silence a section that should render. + */ + absent?: readonly string[]; }; const specifierRecord = specifierRecords[0]; @@ -74,6 +106,29 @@ const dsmDiagnosisWithKeyFeatures = dsmDiagnoses.find( ); const dsmDiagnosisWithDifferentials = dsmDiagnoses.find((diagnosis) => diagnosis.differentials.length > 0); +/** + * One factsheet per `kind`. The five kinds render five different bodies, so a + * single fixture would leave four section sets unguarded — which is how `tocFor` + * (the hand-maintained switch this index replaces) drifted out of step with the + * page in the first place. + */ +function factsheetOfKind(kind: Factsheet["kind"]): Factsheet { + const sheet = factsheets.find((candidate) => candidate.kind === kind); + if (!sheet) throw new Error(`Expected a ${kind} factsheet fixture`); + return sheet; +} + +function factsheetRoute(kind: Factsheet["kind"], absent?: readonly string[]): RouteCase { + const factsheet = factsheetOfKind(kind); + return { + name: `/factsheets/[slug] (${kind})`, + sections: factsheetNavSections(factsheet), + render: () => <FactsheetDetailPage factsheet={factsheet} />, + conditional: ["factsheet-more-in-topic"], + absent, + }; +} + function buildConsiderations(values: string[]): DsmDifferentialConsideration[] { return values.map((value, index) => ({ id: `${index}-consideration`, @@ -133,6 +188,13 @@ const routes: RouteCase[] = [ /> ), }, + factsheetRoute("medRich"), + factsheetRoute("medLite"), + factsheetRoute("condition"), + // `cbt` is the only Therapies sheet and `lithium-monitoring` the only Tests & + // procedures sheet, so neither has a topic sibling to list. + factsheetRoute("therapy", ["factsheet-more-in-topic"]), + factsheetRoute("procedure", ["factsheet-more-in-topic"]), ]; describe("in-page navigation section contracts", () => { @@ -142,11 +204,18 @@ describe("in-page navigation section contracts", () => { const { container } = render(route.render()); for (const section of route.sections) { + const found = sectionTargetIds(section).some((id) => container.querySelector(`#${CSS.escape(id)}`)); + if (route.absent?.includes(section.id)) { + // The other side of the same contract: this fixture carries no data + // for the section, so nothing may render its anchor and + // `useResolvedPageSections` drops the entry. + expect(found, `${route.name}: "${section.id}" renders an anchor it was declared not to have`).toBe(false); + continue; + } // A section may declare several breakpoint copies; jsdom applies no // Tailwind, so both are in the DOM here and any one of them proves the // anchor exists. Which copy is *displayed* is resolved at runtime by // `useResolvedPageSections`, and covered by the Playwright pair check. - const found = sectionTargetIds(section).some((id) => container.querySelector(`#${CSS.escape(id)}`)); expect(found, `${route.name}: no element renders an anchor for "${section.id}"`).toBe(true); } }, @@ -160,6 +229,7 @@ describe("in-page navigation section contracts", () => { const { container } = render(route.render()); for (const section of route.sections) { + if (route.absent?.includes(section.id)) continue; const anchor = sectionTargetIds(section) .map((id) => container.querySelector(`#${CSS.escape(id)}`)) .find((element): element is Element => element !== null); @@ -193,8 +263,59 @@ describe("in-page navigation section contracts", () => { }); it("covers every route that mounts the shared header", () => { - // A seventh component converted without a case here would leave its - // declared sections unguarded, which is the whole failure mode. - expect(routes).toHaveLength(7); + // A component converted without a case here would leave its declared + // sections unguarded, which is the whole failure mode. Seven anchor-scrolling + // routes plus one factsheet case per `kind`; the medication page swaps + // panels rather than scrolling and is guarded by the suite below. + expect(routes).toHaveLength(12); + }); +}); + +/** + * The panel-swap half of the same contract. + * + * Medications declare tab ids rather than DOM anchors — selecting one exchanges + * the rendered panel instead of scrolling to it, so there is nothing to carry + * `inPageAnchor` and the assertions above do not apply. What still must hold is + * `/issues #256`'s stop rule: every declared section resolves to something the + * route actually renders, proven against the DOM rather than by reading the + * table back to itself. Here that is the panel each tab id swaps in. + */ +describe("in-page navigation panel-swap contracts", () => { + // Every tab must have content, or "the panel swapped" would be indistinguishable + // from the empty state on a sparse record. + const medication = loadMedicationSnapshot().find((record) => { + const byTab = medicationSectionsByTab(record); + return MEDICATION_TAB_IDS.every((id) => byTab[id].length > 0); + }); + + it("has a medication fixture that fills all four tabs", () => { + expect(medication, "no snapshot medication renders every declared tab").toBeDefined(); + }); + + it("/medications/[slug] swaps in a panel for every declared section", async () => { + const user = userEvent.setup(); + render(<MedicationRecordPage slug={medication!.slug} fallbackRecord={medication!} />); + + for (const section of medicationNavSections) { + await user.click(screen.getByTestId("medication-section-trigger")); + const sheet = screen.getByTestId("medication-section-sheet"); + await user.click(within(sheet).getByRole("button", { name: new RegExp(`^${section.label}`) })); + + const panel = document.querySelector(`#medication-panel-${CSS.escape(section.id)}`); + expect(panel, `no panel renders for the declared "${section.id}" section`).not.toBeNull(); + // The panel is the live one, not a stale sibling left in the DOM. + expect(document.querySelectorAll('[id^="medication-panel-"]')).toHaveLength(1); + } + }); + + it("offers no dead segment: every declared tab holds at least one section", () => { + // The track always draws four segments, so a tab that could never hold + // content would be a permanently empty destination. This pins the grouping + // predicate against the real corpus rather than against itself. + const byTab = medicationSectionsByTab(medication!); + for (const id of MEDICATION_TAB_IDS) { + expect(byTab[id].length, `the "${id}" tab renders no sections`).toBeGreaterThan(0); + } }); }); diff --git a/tests/mode-nav-addon-slot.dom.test.tsx b/tests/mode-nav-addon-slot.dom.test.tsx index 5776e2a804..bfa6e109c0 100644 --- a/tests/mode-nav-addon-slot.dom.test.tsx +++ b/tests/mode-nav-addon-slot.dom.test.tsx @@ -47,6 +47,10 @@ describe("header addon slot ownership", () => { expect(isHeaderAddonSlotOwnedRoute("/formulation/rumination")).toBe(true); expect(isHeaderAddonSlotOwnedRoute("/dsm/diagnoses/major-depressive-disorder")).toBe(true); expect(isHeaderAddonSlotOwnedRoute("/dsm/diagnoses/major-depressive-disorder/differentials")).toBe(true); + // Factsheet and medication detail, converted onto the shared header. + expect(isHeaderAddonSlotOwnedRoute("/factsheets/sertraline")).toBe(true); + expect(isHeaderAddonSlotOwnedRoute("/medications/sertraline")).toBe(true); + expect(isHeaderAddonSlotOwnedRoute("/medications")).toBe(false); // The presentations workflow page renders no portal, and the shell index is // not a document detail route. @@ -82,6 +86,8 @@ describe("header addon slot ownership", () => { "/formulation/rumination", "/dsm/diagnoses/major-depressive-disorder", "/dsm/diagnoses/major-depressive-disorder/differentials", + "/factsheets/sertraline", + "/medications/sertraline", ]) { expect(isHeaderAddonSlotOwnedRoute(pathname)).toBe(true); expect(hasLocalInformationPageNavigation(pathname)).toBe(true); @@ -155,16 +161,20 @@ describe("header addon slot ownership", () => { }; walk(join(process.cwd(), "src/components")); - // The four `*-nav-header.tsx` modules are the client halves of Server - // Component pages: sections carry `LucideIcon` values and the header needs - // hooks, neither of which crosses the RSC boundary, so the route's claim is - // registered in a sibling module rather than in the page itself. + // The `*-nav-header.tsx` modules are the section-table siblings + // `docs/search-chrome-behaviour.md` pins every new conversion to. For the + // four Server Component pages they are also a necessity — sections carry + // `LucideIcon` values and the header needs hooks, neither of which crosses + // the RSC boundary — while `factsheet-` and `medication-nav-header.tsx` + // adopt the same shape from Client Component pages by convention. Either + // way the route's claim is registered in the sibling, not the page. expect(claimants.sort()).toEqual([ "src/components/DocumentViewer.tsx", + "src/components/clinical-dashboard/medication-nav-header.tsx", "src/components/differentials/differential-detail-page.tsx", "src/components/dsm/dsm-diagnosis-nav-header.tsx", "src/components/dsm/dsm-differential-considerations-page.tsx", - "src/components/factsheets/factsheet-detail-page.tsx", + "src/components/factsheets/factsheet-nav-header.tsx", "src/components/forms/form-detail-page.tsx", "src/components/formulation/formulation-nav-header.tsx", "src/components/services/service-detail-page.tsx", diff --git a/tests/secondary-navigation.dom.test.tsx b/tests/secondary-navigation.dom.test.tsx deleted file mode 100644 index 5b659cadef..0000000000 --- a/tests/secondary-navigation.dom.test.tsx +++ /dev/null @@ -1,183 +0,0 @@ -import { render, screen, waitFor } from "@testing-library/react"; -import userEvent from "@testing-library/user-event"; -import { useState } from "react"; -import { afterEach, describe, expect, it, vi } from "vitest"; - -import { - SecondaryNavigation, - SecondaryNavigationShellHostProvider, - type SecondaryNavigationItem, -} from "@/components/secondary-navigation"; - -afterEach(() => { - window.history.replaceState(null, "", "/"); - document.documentElement.removeAttribute("data-motion"); - Reflect.deleteProperty(HTMLElement.prototype, "scrollTo"); -}); - -function ControlledTabs() { - const [active, setActive] = useState("summary"); - const items: SecondaryNavigationItem[] = ["summary", "dosing", "safety", "more"].map((id) => ({ - kind: "action", - id, - label: id[0].toUpperCase() + id.slice(1), - controlsId: `panel-${id}`, - onSelect: () => setActive(id), - })); - return <SecondaryNavigation ariaLabel="Medication sections" items={items} activeId={active} tablist />; -} - -function ShellPlacedNavigation() { - const [host, setHost] = useState<HTMLDivElement | null>(null); - return ( - <SecondaryNavigationShellHostProvider host={host}> - <div data-testid="shell-navigation-host" ref={setHost} /> - <div data-testid="page-content"> - <SecondaryNavigation - ariaLabel="Page sections" - placeInShell - items={[{ kind: "route", id: "overview", label: "Overview", href: "/overview" }]} - /> - </div> - </SecondaryNavigationShellHostProvider> - ); -} - -describe("SecondaryNavigation", () => { - it("exposes route and action current-page semantics without a Home item", () => { - render( - <SecondaryNavigation - ariaLabel="Differentials mode" - activeId="diagnoses" - items={[ - { kind: "route", id: "search", label: "Search", href: "/differentials" }, - { kind: "route", id: "diagnoses", label: "Diagnoses", href: "/differentials/diagnoses" }, - { kind: "action", id: "compare", label: "Compare", onSelect: vi.fn() }, - ]} - />, - ); - - expect(screen.getByRole("link", { name: "Diagnoses" })).toHaveAttribute("aria-current", "page"); - expect(screen.getByRole("link", { name: "Search" })).not.toHaveAttribute("aria-current"); - expect(screen.queryByText("Home")).toBeNull(); - }); - - it("places page-owned navigation in the shell host without a second sticky layer", async () => { - render(<ShellPlacedNavigation />); - - const navigation = screen.getByRole("navigation", { name: "Page sections" }); - const host = screen.getByTestId("shell-navigation-host"); - await waitFor(() => expect(host).toContainElement(navigation)); - expect(screen.getByTestId("page-content")).not.toContainElement(navigation); - expect(navigation).not.toHaveClass("sticky"); - }); - - it("keeps active-chip sync on the horizontal rail without scrolling the page", async () => { - const scrollTo = vi.fn(); - Object.defineProperty(HTMLElement.prototype, "scrollTo", { - configurable: true, - writable: true, - value: scrollTo, - }); - vi.spyOn(HTMLElement.prototype, "getBoundingClientRect").mockImplementation(function getRect(this: HTMLElement) { - if (this.classList.contains("polished-scroll")) { - return { x: 0, y: 0, top: 0, left: 0, right: 200, bottom: 56, width: 200, height: 56, toJSON() {} }; - } - if (this.textContent === "Safety") { - return { x: 260, y: 8, top: 8, left: 260, right: 340, bottom: 48, width: 80, height: 40, toJSON() {} }; - } - return { x: 0, y: 0, top: 0, left: 0, right: 80, bottom: 40, width: 80, height: 40, toJSON() {} }; - }); - - const { rerender } = render( - <SecondaryNavigation - ariaLabel="On this page" - sticky={false} - activeId="one" - items={[ - { kind: "action", id: "one", label: "Overview", onSelect: vi.fn() }, - { kind: "action", id: "two", label: "Safety", onSelect: vi.fn() }, - ]} - />, - ); - vi.mocked(Element.prototype.scrollIntoView).mockClear(); - scrollTo.mockClear(); - - rerender( - <SecondaryNavigation - ariaLabel="On this page" - sticky={false} - activeId="two" - items={[ - { kind: "action", id: "one", label: "Overview", onSelect: vi.fn() }, - { kind: "action", id: "two", label: "Safety", onSelect: vi.fn() }, - ]} - />, - ); - - await waitFor(() => expect(scrollTo).toHaveBeenCalledWith(expect.objectContaining({ left: expect.any(Number) }))); - expect(Element.prototype.scrollIntoView).not.toHaveBeenCalled(); - }); - - it("implements roving tab focus with Arrow, Home, and End keys", async () => { - const user = userEvent.setup(); - render(<ControlledTabs />); - - const summary = screen.getByRole("tab", { name: "Summary" }); - const dosing = screen.getByRole("tab", { name: "Dosing" }); - const more = screen.getByRole("tab", { name: "More" }); - expect(summary).toHaveAttribute("aria-selected", "true"); - expect(summary).toHaveAttribute("aria-controls", "panel-summary"); - - summary.focus(); - await user.keyboard("{ArrowRight}"); - expect(dosing).toHaveFocus(); - expect(dosing).toHaveAttribute("aria-selected", "true"); - - await user.keyboard("{End}"); - expect(more).toHaveFocus(); - expect(more).toHaveAttribute("aria-selected", "true"); - - await user.keyboard("{Home}"); - expect(summary).toHaveFocus(); - expect(summary).toHaveAttribute("aria-selected", "true"); - }); - - it("reveals the active item horizontally without overriding motion preferences", async () => { - document.documentElement.setAttribute("data-motion", "reduced"); - const scrollTo = vi.fn(); - Object.defineProperty(HTMLElement.prototype, "scrollTo", { - configurable: true, - writable: true, - value: scrollTo, - }); - vi.spyOn(HTMLElement.prototype, "getBoundingClientRect").mockImplementation(function getRect(this: HTMLElement) { - if (this.classList.contains("polished-scroll")) { - return { x: 0, y: 0, top: 0, left: 0, right: 120, bottom: 56, width: 120, height: 56, toJSON() {} }; - } - if (this.textContent === "Last") { - return { x: 180, y: 8, top: 8, left: 180, right: 260, bottom: 48, width: 80, height: 40, toJSON() {} }; - } - return { x: 0, y: 0, top: 0, left: 0, right: 80, bottom: 40, width: 80, height: 40, toJSON() {} }; - }); - - render( - <SecondaryNavigation - ariaLabel="Mode" - activeId="last" - items={[ - { kind: "action", id: "first", label: "First", onSelect: vi.fn() }, - { kind: "action", id: "last", label: "Last", onSelect: vi.fn() }, - ]} - />, - ); - - await waitFor(() => - expect(scrollTo).toHaveBeenCalledWith({ - left: expect.any(Number), - behavior: "auto", - }), - ); - expect(Element.prototype.scrollIntoView).not.toHaveBeenCalled(); - }); -}); From 84f2182abaf1ad10dec04ca7edc676f87e6ae48b Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Sun, 9 Aug 2026 09:51:08 +0000 Subject: [PATCH 2/6] docs(ledger): record the in-page-nav PR 3 review Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GHmzgER6rv8Va9HbQZw87Q --- docs/branch-review-ledger.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 3f5fdc0ceb..19cbac8494 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -842,3 +842,4 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie | 2026-08-09 | pull/1771 | 466ec4216272c31c5f754db213dbdc529583b167 | PR 1771 runtime floor enforcement | P2: Cloud and Desktop setup paths remain major-only; do not merge until range-aware | static review; check:runtime PASS; check:codex-cloud PASS; ledger PASS; outstanding issues PASS; focused Vitest blocked by active Playwright lease | | 2026-08-09 | claude/document-viewer-phase-3-bj5k5v | 156db63f1b60f09791e426b043ea90d427b789ab | post-#1772 test simplification: replace the viewer perf source-text grep with behavioural coverage; de-literalise rail window and keyboard label assertions | PR #1777 opened. Self-review of #1772's own tests against an excessive-strictness challenge. Finding: the client-performance-boundaries grep for resolveLiveCanvasWindow / resolveRenderAheadPages / liveCanvasLimit / requestIdleCallback was not merely brittle, it was INEFFECTIVE - replacing the budget call with a hardcoded 3 leaves every identifier in the file, so it stayed green while the viewer retained three full-zoom canvases (measured both ways). Replaced by a DOM case that binds the budget (VIEWER_MAX_ZOOM at dpr 3 gives ~16.8M backing px against the 24M budget, window collapses to 1) and fails on exactly that substitution. Also exported RAIL_IMAGE_WINDOW so the rail test derives its counts (verified by tuning 6->8: all 7 still pass), and relaxed the keyboard aria-label assertions from exact prose to the key names. Pre-existing greps for disableAutoFetch / canvas.width = 0 / pageToCleanup left alone deliberately - two are now redundant but they are another author's guard. | verify:pr-local (1 pre-existing root-only failure: pr-handoff-stop #291; 5872 passed), build OK 80s + client bundle secret check, eval:rag:offline 36 golden cases / 574 tests, lint + typecheck clean. Sabotage-verified in both directions. Browser gates unrunnable here (#279) - unchanged by this diff. | | 2026-08-09 | claude/disabled-button-accessibility-piclvr | 722abdb780c715c0a89df268ed48f6c741ffd569 | disabled-placeholder buttons -> aria-disabled + inert handler (25 sites, 13 components); controlDisabled/therapy recipe aria-disabled styling; require-button-wiring redundantDisabledPair gate; wiring-conventions contract rewrite (settles #291) | authored — PR #1778 opened | lint (uncached, exit 0); typecheck; test 5878 passed/1 pre-existing root-env failure in pr-handoff-stop; build; check:rag:fixtures 36 golden cases; prettier --check clean; verify:ui not run (no browser in container) | +| 2026-08-09 | claude/in-page-nav-pr-3-i6gi8n | 6651feef4fab63f1181fba57908cb22e2932df3c | in-page-nav PR 3: convert /medications/[slug] (panel-swap) and /factsheets/[slug] (anchors) onto InPageNavHeader; record the differentials-presentations exception; delete orphaned SecondaryNavigation (#271) | converted 2 of 3 routes, 3rd recorded as a reasoned lasting exception; tocFor and SecondaryNavigation deleted; route-sections contract 7 -> 12 routes plus a panel-swap suite | verify:pr-local (1 pre-existing root-permission failure in pr-handoff-stop.test.ts, all else green); test 5932 passed; in-page-nav-route-sections 29 passed; verify:phone-chrome 3/4 stages (focused-browser blocked by #255 Chromium 1194 vs 1234); build + bundle-budget + rag:fixtures green; verify:ui not run (#255, delegated to CI) | From fc624f31791062925f62fcf1cd9a031ef70ef9f2 Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Sun, 9 Aug 2026 11:27:40 +0000 Subject: [PATCH 3/6] feat(in-page-nav): two-rail medications header, joined action group, phone reading level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Design follow-up on the PR-3 conversions, from review of the shipped phone headers. Medications: the two-rail header `InPageNavHeader` gains an optional `rail={{ label }}` that swaps the thin weighted track for `InPageSectionRail` — every section named in a visible row with its icon, label and a count badge, active one underlined. Medications is the only adopter and the prop exists so it stays that way by choice rather than drift: factsheets' eight anchored sections would overflow the row this is meant to simplify. The rail changes the row above it. From `sm`, where the whole rail fits, the title stops being a disclosure — the chevron would open a list of the same destinations — and renders as plain text. Below `sm` the rail scrolls and the disclosure returns as its overflow, which is the two-rail shape. The rail is deliberately not a `role="tablist"`: the same sections are reachable from the sheet on a phone, so a roving-tabindex group would put half the destinations behind arrow keys and half behind Tab. `PageSection.count` is a new field, separate from `detail`. The sheet row has room for "3 sections" and a badge does not, and parsing the digits back out of the prose would break the first time a route worded its detail differently. Patient details move into the header Both body cards — `PatientProfilePanel` and `MedicationConsiderations` — are removed from the page body and now open from a patients control in rail one. They are a per-patient overlay on a reference record rather than part of the record, and inline they pushed every section below a permanently-empty prompt. The feature is relocated, not retired. Factsheets: the phone band is gone `mode` now renders inside the actions sheet below `sm` instead of claiming a full-width band under the row — it was the only second phone row on any converted page. Both copies are always in the DOM with CSS choosing one per breakpoint, so there is no state to keep in step. `mode` consequently requires `actions`: with no sheet to move into, a phone would have no way to reach it. One joined control group `primaryAction` and `actions` now render inside a single bordered, clipped group with a hairline between them, replacing a bordered promoted action sitting beside a borderless ellipsis. `primaryActionIconOnly` drops the label at every width for a glyph that carries its own meaning. Two things found while building, both real The rail first used `focus-ring-tab`, which sets `border-radius` on all four corners and rendered the 2px active underline as a detached pill; grouped buttons had the same utility and painted a second corner against the group's clipped edge. Both now carry explicit focus-visible outlines and no radius. The rail also briefly used `min-h-11` with a comment rationalising it — that is the exact substitution AGENTS.md forbids because it reintroduces a known `ui-smoke` sub-pixel flake. It is `min-h-12` like every other production tap target. Guards Rail contents and counts, rail-driven panel swap, the joined group's membership, the sheet-hosted reading level driving body copy, and the patient panels being absent from the body but present in the sheet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GHmzgER6rv8Va9HbQZw87Q --- docs/codebase-index.md | 2 +- docs/search-chrome-behaviour.md | 51 ++++- .../medication-nav-header.tsx | 37 +++- .../medication-record-page.tsx | 29 ++- .../in-page-nav/in-page-nav-header.tsx | 180 +++++++++++++----- .../in-page-nav/in-page-section-rail.tsx | 120 ++++++++++++ .../in-page-nav/page-section-index.ts | 9 + tests/factsheet-detail-header.dom.test.tsx | 29 +++ tests/in-page-nav-route-sections.dom.test.tsx | 25 +++ tests/medication-record-page.dom.test.tsx | 33 +++- 10 files changed, 435 insertions(+), 80 deletions(-) create mode 100644 src/components/in-page-nav/in-page-section-rail.tsx diff --git a/docs/codebase-index.md b/docs/codebase-index.md index 0f763f3a89..06a6bec462 100644 --- a/docs/codebase-index.md +++ b/docs/codebase-index.md @@ -329,7 +329,7 @@ One shared composer (`master-search-header.tsx`) serves every mode. Placement: - **Result and detail views**: fixed bottom dock on phone (compact variant on submitted searches), sticky top from `sm` up. - **Results routing**: standalone routes own their submitted searches via `?q=…&run=1` (`/services` → `ServicesNavigatorPage`, `/forms` → `FormsSearchResultsPage`, `/differentials` → `DifferentialsHome` results view, `/formulation` → local mechanism results, `/favourites` filters the command library in place). Answer, Documents, and Prescribing submitted searches render inside `ClinicalDashboard` — intentional, since they need retrieval/answer state. Bare `/?mode=<id>` always renders the shared home with that mode preselected; only a submitted deep link (`q` plus `run=1`) resolves to the mode's own search surface (proxy early-redirect still covers favourites/differentials/specifiers for those submitted aliases). - **Intentionally composer-free routes**: `/differentials/presentations/*` and `/differentials/compare` (comparison workflow owns its chrome), `/documents/[id]` viewer (has its own in-document ask composer), `/documents/source/*` (document flow owns mobile chrome). Do not re-flag these in search-consistency audits. -- **Shared in-page navigation**: `src/components/in-page-nav/` is the default template for section navigation on any mode page (`docs/search-chrome-behaviour.md`). `in-page-nav-header.tsx` (`InPageNavHeader`) owns the header row, both sheets and the `PhoneHeaderCollapsePortal` wrapper; `page-section-index.ts` (`PageSection`, `toDocumentSections`, `sectionTargetIds`) is the declaration shape; `use-resolved-page-sections.ts` narrows a declaration to the anchors actually rendered at this breakpoint; `use-in-page-section-nav.ts` composes that with `useDocumentSectionSpy` and `jumpToDocumentSection`; `use-page-section-weights.ts` measures segment weights; `use-in-page-chrome-metrics.ts` publishes `--inpage-anchor-offset`; `in-page-nav-classes.ts` holds the shared anchor (`inPageAnchor`) and actions-sheet row classes. Anchor measurement itself is `src/components/sticky-chrome-metrics.ts` (`useStickyChromeMetrics`), shared with the document viewer's `use-document-chrome-metrics.ts`. Mounted by `differentials/differential-detail-page.tsx`, `services/service-detail-page.tsx`, `forms/form-detail-page.tsx`, `dsm/dsm-differential-considerations-page.tsx`, and — through a colocated `"use client"` nav-header sibling that owns and exports the route's section table — `specifiers/specifier-nav-header.tsx`, `formulation/formulation-nav-header.tsx`, `dsm/dsm-diagnosis-nav-header.tsx`, `factsheets/factsheet-nav-header.tsx` and `clinical-dashboard/medication-nav-header.tsx`. The sibling is mandatory for the four Server Component pages (neither `onSelectSection` nor a `LucideIcon` crosses the RSC boundary) and the convention for the rest. Two adopters swap panels instead of scrolling — `differential-detail-page.tsx` and the medication record page — so they pass explicit weights, carry no `inPageAnchor`, and use neither `useResolvedPageSections` nor the scroll spy. Every declared section is pinned against rendered DOM by `tests/in-page-nav-route-sections.dom.test.tsx` (anchors for the scrolling routes, swapped-in panels for the tab routes). +- **Shared in-page navigation**: `src/components/in-page-nav/` is the default template for section navigation on any mode page (`docs/search-chrome-behaviour.md`). `in-page-nav-header.tsx` (`InPageNavHeader`) owns the header row, both sheets and the `PhoneHeaderCollapsePortal` wrapper; `page-section-index.ts` (`PageSection`, `toDocumentSections`, `sectionTargetIds`) is the declaration shape; `use-resolved-page-sections.ts` narrows a declaration to the anchors actually rendered at this breakpoint; `use-in-page-section-nav.ts` composes that with `useDocumentSectionSpy` and `jumpToDocumentSection`; `use-page-section-weights.ts` measures segment weights; `use-in-page-chrome-metrics.ts` publishes `--inpage-anchor-offset`; `in-page-nav-classes.ts` holds the shared anchor (`inPageAnchor`) and actions-sheet row classes; `in-page-section-rail.tsx` (`InPageSectionRail`) is the optional visible second rail, opted into with `rail={{ label }}` by panel-swap routes with few sections (medications only) in place of the weighted track. Anchor measurement itself is `src/components/sticky-chrome-metrics.ts` (`useStickyChromeMetrics`), shared with the document viewer's `use-document-chrome-metrics.ts`. Mounted by `differentials/differential-detail-page.tsx`, `services/service-detail-page.tsx`, `forms/form-detail-page.tsx`, `dsm/dsm-differential-considerations-page.tsx`, and — through a colocated `"use client"` nav-header sibling that owns and exports the route's section table — `specifiers/specifier-nav-header.tsx`, `formulation/formulation-nav-header.tsx`, `dsm/dsm-diagnosis-nav-header.tsx`, `factsheets/factsheet-nav-header.tsx` and `clinical-dashboard/medication-nav-header.tsx`. The sibling is mandatory for the four Server Component pages (neither `onSelectSection` nor a `LucideIcon` crosses the RSC boundary) and the convention for the rest. Two adopters swap panels instead of scrolling — `differential-detail-page.tsx` and the medication record page — so they pass explicit weights, carry no `inPageAnchor`, and use neither `useResolvedPageSections` nor the scroll spy. Every declared section is pinned against rendered DOM by `tests/in-page-nav-route-sections.dom.test.tsx` (anchors for the scrolling routes, swapped-in panels for the tab routes). - **Shared secondary navigation**: `src/components/page-secondary-navigation.tsx` (`PageSecondaryNavigation`, mode destinations only). Mode destinations come from `src/lib/mode-secondary-navigation.ts` (`modeSecondaryNavigationRegistry`, no "Home" item). `GlobalSearchShell` renders it in normal flow at the top of `#main-content` for its owned namespaced modes; it self-suppresses on clean mode homes, on Therapy Compass, and on every information page — `hasLocalInformationPageNavigation` is now just `isInformationPage`, because each of those routes owns its own in-page navigation. The older shared `SecondaryNavigation` component was deleted here (`/issues #271`): its `section` kind and "On this page" pill rail went when the last six information routes moved onto `InPageNavHeader`, and the surviving `route`/`action` kinds had no production constructor left — `RegistryModeNav` renders `ModeNav`, not `SecondaryNavigation`, so the only remaining caller was its own test file, which went with it. - **Local filter fields** (sidebar "Search chats", document drawer "Find a document"/"Find a source PDF") are scoped filters, not global search; they share the `fieldControlWithIcon`/`fieldIcon` primitives. - **Wiring conventions** for buttons and route navigation (and the gates that enforce them — the dead-button ESLint rule and the orphan-route reachability test) live in `docs/wiring-conventions.md`. diff --git a/docs/search-chrome-behaviour.md b/docs/search-chrome-behaviour.md index 150938d00c..43ba61f38f 100644 --- a/docs/search-chrome-behaviour.md +++ b/docs/search-chrome-behaviour.md @@ -191,15 +191,26 @@ shape that row: - `showBackLabel={false}` keeps the arrow alone at every width when the row also carries an action or a mode, so the title owns the space. `back.label` is still the accessible name and becomes the desktop tooltip. -- `primaryAction` promotes exactly **one** page action, as `Button variant="secondary"` — - not the filled `--command` slab, because a control pinned to every scroll position should - not be the page's heaviest. Its label is `sr-only` below `sm` so the accessible name does - not change with the breakpoint. A second promoted control is what turns the row back into - the wrapping toolbar this shape replaced; everything else belongs in `actions`. +- `primaryAction` promotes exactly **one** page action. It is **not** the filled `--command` + slab, because a control pinned to every scroll position should not be the page's heaviest. + Its label is `sr-only` below `sm` so the accessible name does not change with the + breakpoint; `primaryActionIconOnly` drops the label at every width for a glyph that carries + its own meaning (the patients control on medications). A second promoted control is what + turns the row back into the wrapping toolbar this shape replaced; everything else belongs + in `actions`. +- **`primaryAction` and `actions` render as one joined group**, not two free-standing + controls: a single border and radius around both, with a hairline between them. A bordered + promoted action beside a borderless ellipsis reads as two unrelated things competing at the + end of the row — which is what `/factsheets/[slug]` shipped before this. A lone `actions` + trigger still gets the group's border, so the two cases look like the same control. - `mode` is a page-level **view** mode — how the page renders, not where you are in it — and - uses the shared `SegmentedControl`. Below `sm` it wraps to its own full-width band under - the row; from `sm` it sits inline and costs no extra height (measured on - `/factsheets/sertraline`: 131px phone, 75px from `sm`, 65px with no mode). + uses the shared `SegmentedControl`. From `sm` it sits inline in the row and costs no extra + height. Below `sm` it renders **inside the actions sheet** under its own label, because a + mode you set once and then read past does not earn permanent pinned chrome on the smallest + screen; the full-width band it used to claim was the only second phone row on any converted + page. Both copies are always in the DOM with CSS choosing one per breakpoint, so there is no + state to keep in step. `mode` therefore **requires `actions`** — with no sheet to move into, + a phone would have no way to reach it. Used by `medication-nav-header.tsx` while the record is still loading (no record, no sections) and by `factsheet-nav-header.tsx` for the seven non-`medRich` sheets, which carry @@ -236,6 +247,30 @@ Keep a per-tab `id` on the panel — that is the rendered evidence a declared se to something real, which is what the panel-swap half of `tests/in-page-nav-route-sections.dom.test.tsx` asserts in place of the anchor check. +**The two-rail variant (`rail`).** A panel-swap route with few enough sections to name in a +row may pass `rail={{ label }}` and get `InPageSectionRail` in place of the weighted track: +icon, label and a `count` badge per section, active one underlined. `/medications/[slug]` is +the only adopter, and the prop exists so it stays the only one by choice rather than by +drift — `/factsheets/[slug]`'s eight anchored sections would overflow the row this is meant +to simplify, and a scrolling route already has a spy moving the active state continuously. + +The rail changes three things about the row above it: + +- **From `sm` the title stops being a disclosure.** Every section is already named in the + rail, so the chevron would open a list of the same destinations. The title renders as plain + text and the section sheet is unreachable. Below `sm` the rail scrolls, so the disclosure + returns as its overflow — that is the "two rails" shape. +- **The rail is not a `role="tablist"`.** The same sections are reachable from the sheet on a + phone, so a roving-tabindex group would put half the destinations behind arrow keys and half + behind Tab. Ordinary buttons are reachable both ways. +- **`count` is a separate field from `detail`.** The sheet row has space for "3 sections" and + the badge does not; parsing digits back out of the prose would break the first time a route + worded its detail differently. + +Rail items are `min-h-12` like every other production tap target. Two rails are tall on a +phone and `min-h-11` would buy back 4px per rail — do not take it. That is the substitution +`AGENTS.md` calls out, and it reintroduces a known `ui-smoke` sub-pixel flake. + ### The differentials presentations workflow keeps its own layout — decided, not pending `src/components/differentials/differential-presentation-workflow-page.tsx` is **not** being diff --git a/src/components/clinical-dashboard/medication-nav-header.tsx b/src/components/clinical-dashboard/medication-nav-header.tsx index da64fe1608..1d20d83ac4 100644 --- a/src/components/clinical-dashboard/medication-nav-header.tsx +++ b/src/components/clinical-dashboard/medication-nav-header.tsx @@ -1,6 +1,6 @@ "use client"; -import { CalendarDays, ClipboardList, Layers, ShieldAlert } from "lucide-react"; +import { CalendarDays, ClipboardList, Layers, ShieldAlert, UserRound } from "lucide-react"; import type { ReactNode } from "react"; import { InPageNavHeader } from "@/components/in-page-nav/in-page-nav-header"; @@ -90,14 +90,18 @@ export function buildMedicationNavSections(record: MedicationRecord): PageSectio const weights = MEDICATION_TAB_IDS.map((id) => rawWeight(byTab[id].length)); const total = weights.reduce((sum, weight) => sum + weight, 0) || 1; - return medicationNavSections.map((section, index) => ({ - ...section, - detail: plural(byTab[section.id as MedicationTabId].length, "section"), - weight: (weights[index] ?? 1) / total, - // The trailing chevron in `DocumentSectionList` means "this row opens an - // accordion". Selecting a tab swaps a panel instead, so never claim it. - collapsible: false, - })); + return medicationNavSections.map((section, index) => { + const count = byTab[section.id as MedicationTabId].length; + return { + ...section, + count, + detail: plural(count, "section"), + weight: (weights[index] ?? 1) / total, + // The trailing chevron in `DocumentSectionList` means "this row opens an + // accordion". Selecting a tab swaps a panel instead, so never claim it. + collapsible: false, + }; + }); } /** @@ -114,6 +118,7 @@ export function MedicationNavHeader({ record, activeTab, onSelectTab, + onOpenPatientDetails, actions, }: { title: string; @@ -121,6 +126,12 @@ export function MedicationNavHeader({ record: MedicationRecord | null; activeTab: MedicationTabId; onSelectTab: (id: MedicationTabId) => void; + /** + * Opens the patient-details sheet. This is where the two cards that used to + * sit in the page body now live — the entry point moved into the header, the + * feature did not go anywhere. + */ + onOpenPatientDetails: () => void; actions?: ReactNode; }) { const back = { href: appModeHomeHref("prescribing"), label: "Medications" }; @@ -131,10 +142,15 @@ export function MedicationNavHeader({ actionsNoun: "medication" as const, actionsDescription: "Choose how to use this medication.", testIdPrefix: "medication" as const, + // Icon-only at every width: a person glyph carries "patient" on its own, and + // a labelled control here would push the group wide enough to squeeze the + // record title on a phone. + primaryAction: { label: "Patient details", icon: UserRound, onClick: onOpenPatientDetails }, + primaryActionIconOnly: true, }; // No record means no sections to offer, so the header falls back to the - // breadcrumb shape rather than drawing a four-segment track over nothing. + // breadcrumb shape rather than drawing a four-segment rail over nothing. if (!record) return <InPageNavHeader {...shared} />; return ( @@ -145,6 +161,7 @@ export function MedicationNavHeader({ onSelectSection={(id) => { if (isMedicationTabId(id)) onSelectTab(id); }} + rail={{ label: "Medication sections" }} /> ); } diff --git a/src/components/clinical-dashboard/medication-record-page.tsx b/src/components/clinical-dashboard/medication-record-page.tsx index aae1daf087..697986920c 100644 --- a/src/components/clinical-dashboard/medication-record-page.tsx +++ b/src/components/clinical-dashboard/medication-record-page.tsx @@ -18,7 +18,7 @@ import { Timer, type LucideIcon, } from "lucide-react"; -import { useMemo, useState, type CSSProperties } from "react"; +import { useMemo, useRef, useState, type CSSProperties } from "react"; import { BadgeCluster } from "@/components/clinical-dashboard/clinical-badge"; import { MedicationConsiderations } from "@/components/clinical-dashboard/medication-considerations"; @@ -56,6 +56,7 @@ import { toneWarning, } from "@/components/ui-primitives"; import { InformationPageFooter, InformationPageShell } from "@/components/information-page-shell"; +import { Sheet } from "@/components/ui/sheet"; const sectionIcons: Record<string, LucideIcon> = { dose: CalendarDays, @@ -357,10 +358,11 @@ function MedicationRecordDetail({ ))} </section> - <section className="space-y-2.5"> - <PatientProfilePanel defaultOpen={false} /> - <MedicationConsiderations record={record} /> - </section> + {/* The patient-profile and considerations cards used to sit here, + between the hero stats and the sections. They moved behind the + header's patients control: they are a per-patient overlay on a + reference record, not part of the record, and inline they pushed + every section below a permanently-empty prompt. */} {/* The panel is no longer a `tabpanel`: the control that swaps it is the shared header's section list, which is a list of buttons rather @@ -430,6 +432,8 @@ export function MedicationRecordPage({ // (SSR fallback → live), and every record offers the same four tabs, so the // selection survives that swap rather than snapping back to Summary. const [activeTab, setActiveTab] = useState<MedicationTabId>("summary"); + const [patientOpen, setPatientOpen] = useState(false); + const patientTriggerRef = useRef<HTMLElement | null>(null); return ( <> @@ -438,7 +442,22 @@ export function MedicationRecordPage({ record={record} activeTab={activeTab} onSelectTab={setActiveTab} + onOpenPatientDetails={() => setPatientOpen(true)} /> + <Sheet + open={patientOpen} + onClose={() => setPatientOpen(false)} + title="Patient details" + description="Optional. Used only in this browser to tailor dosing, safety and contraindication notes." + closeLabel="Close patient details" + returnFocusRef={patientTriggerRef} + testId="medication-patient-sheet" + > + <div className="grid gap-3"> + <PatientProfilePanel defaultOpen /> + {record ? <MedicationConsiderations record={record} /> : null} + </div> + </Sheet> <InformationPageShell testId={`medication-page-${slug}`} gap={false}> <div className="mt-3"> {record ? ( diff --git a/src/components/in-page-nav/in-page-nav-header.tsx b/src/components/in-page-nav/in-page-nav-header.tsx index fbe95102e4..b71548cc97 100644 --- a/src/components/in-page-nav/in-page-nav-header.tsx +++ b/src/components/in-page-nav/in-page-nav-header.tsx @@ -7,10 +7,10 @@ import { useRef, useState, type ReactNode } from "react"; import { PhoneHeaderCollapsePortal } from "@/components/clinical-dashboard/phone-header-collapse-portal"; import { DocumentSectionList, DocumentSectionTrack } from "@/components/document-viewer/section-nav"; +import { InPageSectionRail } from "@/components/in-page-nav/in-page-section-rail"; import { toDocumentSections, type PageSection } from "@/components/in-page-nav/page-section-index"; import { useInPageChromeMetrics } from "@/components/in-page-nav/use-in-page-chrome-metrics"; import { usePageSectionWeights } from "@/components/in-page-nav/use-page-section-weights"; -import { Button } from "@/components/ui/button"; import { SegmentedControl, type SegmentedControlOption } from "@/components/ui/segmented-control"; import { Sheet } from "@/components/ui/sheet"; import { cn, pageContainer } from "@/components/ui-primitives"; @@ -40,10 +40,23 @@ type InPageNavHeaderSharedProps = { * belongs in `actions`. */ primaryAction?: { label: string; icon: LucideIcon; onClick: () => void }; + /** + * `true` drops the promoted action's text label at every width, leaving the + * icon. Use when the icon carries the meaning on its own (a person glyph for + * patient details) and the label would only widen the group. + */ + primaryActionIconOnly?: boolean; /** * A page-level view mode — how the page renders, not where you are in it. - * Below `sm` it wraps to its own full-width band under the row; from `sm` it - * sits inline and costs no extra height at all. + * + * From `sm` it sits inline in the row and costs no extra height. Below `sm` it + * moves **into the actions sheet** rather than claiming a full-width band + * under the row: a view mode is set once and then read past, so it does not + * earn permanent pinned chrome on the smallest screen. That band was the only + * thing on any converted page that took a second phone row. + * + * It therefore requires `actions` — without a sheet to move into there would + * be no way to reach it on a phone. */ mode?: { /** Group label, e.g. "Reading level". */ @@ -100,6 +113,17 @@ export type InPageNavHeaderProps = sections: readonly PageSection[]; activeId?: string | null; onSelectSection: (id: string) => void; + /** + * Render the sections as a visible second rail instead of the weighted + * track (`docs/search-chrome-behaviour.md`, "Two-rail adopters"). + * + * Only for a route whose sections are discrete panels and few enough to + * name in a row. The rail then owns "where am I", so from `sm` — where + * every section fits — the title stops being a disclosure and the sheet is + * not rendered at all. Below `sm` the rail scrolls and the disclosure + * returns as its overflow. + */ + rail?: { label: string }; }); /** @@ -137,6 +161,7 @@ export function InPageNavHeader(props: InPageNavHeaderProps) { title, titleAs = "span", primaryAction, + primaryActionIconOnly = false, mode, sectionSheetTitle, actions, @@ -152,6 +177,7 @@ export function InPageNavHeader(props: InPageNavHeaderProps) { const sections = props.sections ?? []; const activeId = props.activeId ?? null; const onSelectSection = props.sections ? props.onSelectSection : undefined; + const rail = props.sections ? props.rail : undefined; // Both sheets record the route they were opened on rather than a bare // boolean, so navigating closes them without an effect that resets state. // This is load-bearing, not tidiness: most page actions are `<Link>`s, and @@ -212,6 +238,16 @@ export function InPageNavHeader(props: InPageNavHeaderProps) { <ArrowLeft className="h-5 w-5 shrink-0" aria-hidden /> {showBackLabel ? <span className="hidden sm:inline">{back.label}</span> : null} </Link> + {rail ? ( + // With a rail, every section is already named in the row below, so + // from `sm` — where the whole rail fits — the disclosure would open + // a list of the same destinations. The title goes back to being a + // title. `sm:hidden` on the button rather than a second render of + // the whole header keeps one DOM node per concern. + <TitleTag className="hidden min-w-0 flex-1 truncate text-sm font-semibold text-[color:var(--text-heading)] sm:block sm:text-base"> + {title} + </TitleTag> + ) : null} {documentSections.length > 0 ? ( // The title is the section-list disclosure. Line two names where // you are, which the track can place but never label. @@ -222,7 +258,10 @@ export function InPageNavHeader(props: InPageNavHeaderProps) { aria-expanded={sectionSheetOpen} aria-haspopup="dialog" data-testid={`${testIdPrefix}-section-trigger`} - className="focus-ring-tab flex min-h-tap min-w-0 flex-1 items-center gap-1.5 rounded-lg px-1 text-left transition hover:bg-[color:var(--surface-subtle)]" + className={cn( + "focus-ring-tab flex min-h-tap min-w-0 flex-1 items-center gap-1.5 rounded-lg px-1 text-left transition hover:bg-[color:var(--surface-subtle)]", + rail && "sm:hidden", + )} > <span className="min-w-0 flex-1"> <TitleTag className="block truncate text-sm font-semibold leading-tight text-[color:var(--text-heading)] sm:text-base"> @@ -248,54 +287,65 @@ export function InPageNavHeader(props: InPageNavHeaderProps) { {title} </TitleTag> )} - {primaryAction ? ( - // Bordered rather than the filled `--command` slab: a header that - // is pinned to every scroll position should not carry the page's - // heaviest control. The label is `sr-only` below `sm` so the - // accessible name never changes with the breakpoint. - // `sm:order-2` pairs with the mode control's `sm:order-1` so the - // phone DOM order (actions before mode) stays the keyboard order - // while desktop still paints mode between the title and the verbs. - <Button - variant="secondary" - size="sm" - icon={primaryAction.icon} - onClick={primaryAction.onClick} - testId={`${testIdPrefix}-primary-action`} - className="ml-auto shrink-0 max-sm:w-tap max-sm:gap-0 max-sm:px-0 sm:order-2" - > - <span className="max-sm:sr-only">{primaryAction.label}</span> - </Button> - ) : null} - {actions ? ( - <button - type="button" - ref={actionsTriggerRef} - onClick={() => setActionsOpen(true)} - aria-label={`Open ${actionsNoun} actions`} - aria-haspopup="dialog" - aria-expanded={actionsOpen} - title={`${actionsNoun.charAt(0).toUpperCase()}${actionsNoun.slice(1)} actions`} - data-testid={`${testIdPrefix}-actions-trigger`} - className={cn( - "focus-ring-tab grid h-tap w-tap shrink-0 place-items-center rounded-xl text-[color:var(--text-muted)] transition hover:bg-[color:var(--surface-subtle)] hover:text-[color:var(--text-heading)] sm:order-3", - // Beside a promoted action the bordered face would be its - // visual twin and the row would read as two equal buttons. - // Alone, it is the row's only control and keeps its own face. - primaryAction - ? "bg-transparent" - : "ml-auto border border-[color:var(--border-lux)] bg-[color:var(--surface-raised)] shadow-[var(--shadow-inset)] hover:border-[color:var(--border-strong)]", - )} + {primaryAction || actions ? ( + // One joined group, not two free-standing controls. A bordered + // promoted action beside a borderless ellipsis reads as two + // unrelated things competing at the end of the row; a single + // border with a hairline between the members reads as one control + // with two actions. `sm:order-2` keeps the phone DOM order + // (verbs before mode) as the keyboard order while desktop still + // paints mode between the title and the verbs. + <span + data-testid={`${testIdPrefix}-action-group`} + className="ml-auto inline-flex shrink-0 items-stretch overflow-hidden rounded-xl border border-[color:var(--border-lux)] bg-[color:var(--surface-raised)] shadow-[var(--shadow-inset)] sm:order-2" > - <Ellipsis className="h-5 w-5" strokeWidth={2.25} aria-hidden /> - </button> + {primaryAction ? ( + // Not the filled `--command` slab: a control pinned to every + // scroll position should not be the page's heaviest. The label + // is `sr-only` below `sm` so the accessible name never changes + // with the breakpoint. + <button + type="button" + onClick={primaryAction.onClick} + title={primaryAction.label} + data-testid={`${testIdPrefix}-primary-action`} + className={cn( + // Explicit focus styles rather than `focus-ring-tab`: that + // utility sets a `border-radius`, and a rounded child + // inside a rounded, clipped group paints a second corner + // against the group's own edge. + "flex min-h-tap items-center justify-center gap-2 px-3 text-sm font-bold text-[color:var(--text-heading)] transition hover:bg-[color:var(--surface-subtle)] focus-visible:outline focus-visible:outline-2 focus-visible:-outline-offset-2 focus-visible:outline-[color:var(--focus)]", + primaryActionIconOnly ? "w-tap px-0" : "max-sm:w-tap max-sm:gap-0 max-sm:px-0", + )} + > + <primaryAction.icon className="h-5 w-5 shrink-0 text-[color:var(--text-muted)]" aria-hidden /> + <span className={primaryActionIconOnly ? "sr-only" : "max-sm:sr-only"}>{primaryAction.label}</span> + </button> + ) : null} + {actions ? ( + <button + type="button" + ref={actionsTriggerRef} + onClick={() => setActionsOpen(true)} + aria-label={`Open ${actionsNoun} actions`} + aria-haspopup="dialog" + aria-expanded={actionsOpen} + title={`${actionsNoun.charAt(0).toUpperCase()}${actionsNoun.slice(1)} actions`} + data-testid={`${testIdPrefix}-actions-trigger`} + className={cn( + "grid h-tap w-tap shrink-0 place-items-center text-[color:var(--text-muted)] transition hover:bg-[color:var(--surface-subtle)] hover:text-[color:var(--text-heading)] focus-visible:outline focus-visible:outline-2 focus-visible:-outline-offset-2 focus-visible:outline-[color:var(--focus)]", + primaryAction && "border-l border-[color:var(--border)]", + )} + > + <Ellipsis className="h-5 w-5" strokeWidth={2.25} aria-hidden /> + </button> + ) : null} + </span> ) : null} {mode ? ( - // DOM order places this after the verbs so phone keyboard order - // matches the painted rows (mode is the lower full-width band). - // From `sm`, `order-1` pulls it back beside the title; the - // primitive's own `w-full` plus `sm:w-auto` is what drops the - // extra phone band without costing desktop height. + // Inline from `sm` only. Below that it renders inside the actions + // sheet instead — see the `mode` prop docs. `order-1` pulls it + // back beside the title on desktop. <SegmentedControl label={mode.label} value={mode.value} @@ -309,11 +359,19 @@ export function InPageNavHeader(props: InPageNavHeaderProps) { // the segments to their labels, and the phone band gets its even // split from the child override instead of from the floor. layout="fit" - className="max-sm:[&>button]:flex-1 sm:order-1 sm:w-auto sm:shrink-0 sm:flex-nowrap" + className="hidden sm:order-1 sm:flex sm:w-auto sm:shrink-0 sm:flex-nowrap" /> ) : null} </div> - {documentSections.length > 0 ? ( + {rail ? ( + <InPageSectionRail + sections={sections} + activeId={activeSection?.id ?? null} + onSelect={(id) => onSelectSection?.(id)} + label={rail.label} + testIdPrefix={testIdPrefix} + /> + ) : documentSections.length > 0 ? ( <DocumentSectionTrack sections={documentSections} activeId={activeSection?.id ?? null} /> ) : null} </header> @@ -356,6 +414,26 @@ export function InPageNavHeader(props: InPageNavHeaderProps) { returnFocusRef={actionsTriggerRef} testId={`${testIdPrefix}-actions-sheet`} > + {mode ? ( + // The phone home for the view mode. `sm:hidden` rather than a + // conditional render so there is exactly one `SegmentedControl` per + // breakpoint and no state to keep in step — the inline copy above is + // `hidden` below `sm`, this one from `sm`. Full-width here because a + // sheet row has the space the header row does not. + <div className="mb-4 sm:hidden" data-testid={`${testIdPrefix}-sheet-mode`}> + <p className="mb-2 text-3xs font-black uppercase tracking-kicker text-[color:var(--text-muted)]"> + {mode.label} + </p> + <SegmentedControl + label={mode.label} + value={mode.value} + options={mode.options} + onChange={mode.onChange} + layout="equal" + className="w-full [&>button]:flex-1" + /> + </div> + ) : null} {typeof actions === "function" ? actions(() => setActionsOpen(false)) : actions} </Sheet> ) : null} diff --git a/src/components/in-page-nav/in-page-section-rail.tsx b/src/components/in-page-nav/in-page-section-rail.tsx new file mode 100644 index 0000000000..efcbf38714 --- /dev/null +++ b/src/components/in-page-nav/in-page-section-rail.tsx @@ -0,0 +1,120 @@ +"use client"; + +import { useEffect, useRef } from "react"; + +import type { PageSection } from "@/components/in-page-nav/page-section-index"; +import { cn } from "@/components/ui-primitives"; + +/** + * The second rail of the two-rail header: every section as a visible destination + * rather than a list behind a chevron. + * + * Only for routes whose sections are **discrete panels** (medications today). + * A scrolling route already has a scroll spy moving the active state + * continuously, and a rail of eight anchored sections would overflow the row it + * is meant to simplify — those keep the weighted track. + * + * Not a `role="tablist"`. The panel it swaps is a plain region, and the same + * sections are reachable from the sheet on a phone, so a roving-tabindex group + * here would put half the destinations behind arrow keys and half behind Tab. + * A row of ordinary buttons is reachable both ways. + */ +export function InPageSectionRail({ + sections, + activeId, + onSelect, + label, + testIdPrefix, +}: { + sections: readonly PageSection[]; + activeId: string | null; + onSelect: (id: string) => void; + /** Accessible name for the rail, e.g. "Medication sections". */ + label: string; + testIdPrefix: string; +}) { + const scrollerRef = useRef<HTMLDivElement | null>(null); + const activeRef = useRef<HTMLButtonElement | null>(null); + + // Below `sm` the rail scrolls, so a section chosen from the sheet can be off + // screen when the sheet closes. Bring it back into view — inline only, so the + // page itself never scrolls as a side effect of the rail catching up. + useEffect(() => { + const scroller = scrollerRef.current; + const active = activeRef.current; + if (!scroller || !active) return; + if (scroller.scrollWidth <= scroller.clientWidth) return; + active.scrollIntoView({ block: "nearest", inline: "center", behavior: "auto" }); + }, [activeId]); + + if (sections.length === 0) return null; + + return ( + <div className="relative"> + <nav + ref={scrollerRef} + aria-label={label} + data-testid={`${testIdPrefix}-section-rail`} + className="flex items-stretch gap-0.5 overflow-x-auto scrollbar-none border-t border-[color:var(--border)] [scrollbar-width:none] [&::-webkit-scrollbar]:hidden" + > + {sections.map((section) => { + const selected = section.id === activeId; + const Icon = section.icon; + + return ( + <button + key={section.id} + ref={selected ? activeRef : undefined} + type="button" + onClick={() => onSelect(section.id)} + aria-current={selected ? "true" : undefined} + className={cn( + // `min-h-12`, the production tap floor. Not `min-h-11`: the two + // rails together are tall on a phone and 44px would buy back + // 4px, but that is the exact substitution AGENTS.md forbids + // because it reintroduces a known `ui-smoke` sub-pixel flake. + // + // `rounded-t-md` and an explicit focus outline rather than + // `focus-ring-tab`: that utility sets `border-radius` on all four + // corners, which rounds the 2px active underline into a detached + // pill instead of an underline. + "group flex min-h-12 shrink-0 items-center gap-1.5 whitespace-nowrap rounded-t-md border-b-2 px-3 text-sm font-bold transition focus-visible:outline focus-visible:outline-2 focus-visible:-outline-offset-2 focus-visible:outline-[color:var(--focus)]", + selected + ? "border-[color:var(--clinical-accent)] text-[color:var(--clinical-accent)]" + : "border-transparent text-[color:var(--text-muted)] hover:text-[color:var(--text-heading)]", + )} + > + <Icon + aria-hidden + className={cn( + "h-4 w-4 shrink-0", + selected ? "text-[color:var(--clinical-accent)]" : "text-[color:var(--decoration-soft)]", + )} + /> + {section.label} + {typeof section.count === "number" ? ( + <span + aria-hidden + className={cn( + "nums ml-0.5 grid h-[1.125rem] min-w-[1.125rem] shrink-0 place-items-center rounded-full px-1 text-3xs font-black", + selected + ? "bg-[color:var(--clinical-accent-soft)] text-[color:var(--clinical-accent)]" + : "bg-[color:var(--surface-inset)] text-[color:var(--text-muted)]", + )} + > + {section.count} + </span> + ) : null} + </button> + ); + })} + </nav> + {/* Only paints where the rail actually overflows: a fade over a rail that + fits would read as a cut-off row that is not cut off. */} + <span + aria-hidden + className="pointer-events-none absolute inset-y-0 right-0 w-10 bg-gradient-to-r from-transparent to-[color:var(--surface)] sm:hidden" + /> + </div> + ); +} diff --git a/src/components/in-page-nav/page-section-index.ts b/src/components/in-page-nav/page-section-index.ts index ea722350d2..60f7de18be 100644 --- a/src/components/in-page-nav/page-section-index.ts +++ b/src/components/in-page-nav/page-section-index.ts @@ -37,6 +37,15 @@ export type PageSection = { /** Renders as an exclusive-accordion `<details>`; drives the trailing chevron. */ collapsible?: boolean; pending?: boolean; + /** + * Magnitude as a bare number, for the badge on `InPageSectionRail`. + * + * Deliberately separate from `detail`: the sheet row has room for "3 sections" + * and the rail badge does not, and parsing the digits back out of the prose + * would break the moment a route words its detail differently. Only the rail + * reads this, so a route without one simply renders no badge. + */ + count?: number; }; /** diff --git a/tests/factsheet-detail-header.dom.test.tsx b/tests/factsheet-detail-header.dom.test.tsx index fd7724cf75..297c71e7c3 100644 --- a/tests/factsheet-detail-header.dom.test.tsx +++ b/tests/factsheet-detail-header.dom.test.tsx @@ -77,6 +77,35 @@ describe("factsheet detail header", () => { expect(screen.queryByText(factsheet.whatEasy)).toBeNull(); }); + it("puts the reading level in the actions sheet as well as the row", async () => { + // The phone home for the view mode. Both copies exist in the DOM and CSS + // picks one per breakpoint, so the assertion is that the sheet copy is + // present and drives the same state — not that only one is rendered. + const user = userEvent.setup(); + renderFactsheet("sertraline"); + + // Only the inline copy exists before the sheet is opened. + expect(screen.getAllByRole("radiogroup", { name: "Reading level" })).toHaveLength(1); + + await user.click(screen.getByRole("button", { name: "Open factsheet actions" })); + const sheetMode = screen.getByTestId("factsheet-sheet-mode"); + expect(sheetMode).toBeInTheDocument(); + + await user.click(within(sheetMode).getByRole("radio", { name: "Standard" })); + const factsheet = findFactsheet("sertraline"); + if (factsheet?.kind !== "medRich") throw new Error("Expected sertraline to be the medRich fixture"); + expect(screen.getAllByText(factsheet.whatStandard).length).toBeGreaterThan(0); + }); + + it("pairs the download and the ellipsis into one control group", () => { + // The fix for two competing bordered shapes at the end of the row: one + // group, both members inside it. + renderFactsheet("sertraline"); + const group = screen.getByTestId("factsheet-action-group"); + expect(within(group).getByRole("button", { name: "Download PDF" })).toBeInTheDocument(); + expect(within(group).getByRole("button", { name: "Open factsheet actions" })).toBeInTheDocument(); + }); + it("reserves no reading-level control on a factsheet that has one level", () => { // Seven of the eight sheets are not `medRich`; none of them should carry // the band or an empty gap where it would be. diff --git a/tests/in-page-nav-route-sections.dom.test.tsx b/tests/in-page-nav-route-sections.dom.test.tsx index b0808749e6..281e7fa066 100644 --- a/tests/in-page-nav-route-sections.dom.test.tsx +++ b/tests/in-page-nav-route-sections.dom.test.tsx @@ -309,6 +309,31 @@ describe("in-page navigation panel-swap contracts", () => { } }); + it("names every declared section in the visible rail, with its count", () => { + // The rail is the desktop navigation, so a section missing from it is + // unreachable above `sm` even though the sheet still lists it. + render(<MedicationRecordPage slug={medication!.slug} fallbackRecord={medication!} />); + const rail = screen.getByTestId("medication-section-rail"); + const byTab = medicationSectionsByTab(medication!); + + for (const section of medicationNavSections) { + const entry = within(rail).getByRole("button", { name: new RegExp(`^${section.label}`) }); + expect(entry, `"${section.id}" is missing from the rail`).toBeInTheDocument(); + expect(entry).toHaveTextContent(String(byTab[section.id as keyof typeof byTab].length)); + } + }); + + it("swaps the panel from the rail, not only from the sheet", async () => { + const user = userEvent.setup(); + render(<MedicationRecordPage slug={medication!.slug} fallbackRecord={medication!} />); + const rail = screen.getByTestId("medication-section-rail"); + + await user.click(within(rail).getByRole("button", { name: /^Safety/ })); + + expect(document.querySelector("#medication-panel-safety")).not.toBeNull(); + expect(within(rail).getByRole("button", { name: /^Safety/ })).toHaveAttribute("aria-current", "true"); + }); + it("offers no dead segment: every declared tab holds at least one section", () => { // The track always draws four segments, so a tab that could never hold // content would be a permanently empty destination. This pins the grouping diff --git a/tests/medication-record-page.dom.test.tsx b/tests/medication-record-page.dom.test.tsx index 1e19bf91db..aecf41b330 100644 --- a/tests/medication-record-page.dom.test.tsx +++ b/tests/medication-record-page.dom.test.tsx @@ -1,4 +1,5 @@ -import { render, screen } from "@testing-library/react"; +import { render, screen, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; import { describe, expect, it, vi } from "vitest"; import { MedicationRecordPage } from "@/components/clinical-dashboard/medication-record-page"; @@ -7,10 +8,15 @@ import type { MedicationRecord } from "@/lib/medications"; // Controllable data-hook mock so each test drives one content-first state. const { useMedicationDetail } = vi.hoisted(() => ({ useMedicationDetail: vi.fn() })); vi.mock("@/components/clinical-dashboard/use-medication-catalog", () => ({ useMedicationDetail })); -// The two heavy sidebar panels carry their own data concerns; stub them so these -// tests isolate the page's content-first + governance-reconciliation logic. -vi.mock("@/components/clinical-dashboard/patient-profile-panel", () => ({ PatientProfilePanel: () => null })); -vi.mock("@/components/clinical-dashboard/medication-considerations", () => ({ MedicationConsiderations: () => null })); +// The two patient panels carry their own data concerns; stub them to a findable +// marker so these tests can assert *where* they render without pulling in the +// profile store. +vi.mock("@/components/clinical-dashboard/patient-profile-panel", () => ({ + PatientProfilePanel: () => <p>patient-profile-panel</p>, +})); +vi.mock("@/components/clinical-dashboard/medication-considerations", () => ({ + MedicationConsiderations: () => <p>medication-considerations</p>, +})); function mockDetail(state: { data: unknown; loading: boolean; error: string | null }) { useMedicationDetail.mockReturnValue(state); @@ -78,6 +84,23 @@ describe("MedicationRecordPage content-first states", () => { expect(screen.queryByText("Reviewed")).not.toBeInTheDocument(); }); + it("moves the patient panels out of the body and behind the header control", async () => { + // Both cards left the page body; the feature did not leave with them. The + // entry point is the header's patients control, and the panels render inside + // the sheet it opens. + const user = userEvent.setup(); + mockDetail({ data: { record: liveDrug, governance: null }, loading: false, error: null }); + render(<MedicationRecordPage slug="test-med" fallbackRecord={fallbackDrug} />); + + expect(screen.queryByText("patient-profile-panel")).not.toBeInTheDocument(); + expect(screen.queryByText("medication-considerations")).not.toBeInTheDocument(); + + await user.click(screen.getByRole("button", { name: "Patient details" })); + const sheet = screen.getByTestId("medication-patient-sheet"); + expect(within(sheet).getByText("patient-profile-panel")).toBeInTheDocument(); + expect(within(sheet).getByText("medication-considerations")).toBeInTheDocument(); + }); + it("keeps the SSR fallback governance badge while the fetch is still in flight", () => { mockDetail({ data: null, loading: true, error: null }); render( From 060ef20f7e211a9cb9c445a7f5672ff20bfb57ab Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Sun, 9 Aug 2026 11:39:45 +0000 Subject: [PATCH 4/6] chore(design-system): regenerate the adoption manifest `in-page-nav-header.tsx` no longer imports `Button` (the promoted action is now a plain button inside the joined control group) and `medication-record-page.tsx` gained `Sheet` for the relocated patient panels. The manifest is generated, so this is the recorded consequence of both, not a hand edit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GHmzgER6rv8Va9HbQZw87Q --- docs/design-system/COMPONENTS.md | 4 ++-- docs/design-system/adoption-manifest.json | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/design-system/COMPONENTS.md b/docs/design-system/COMPONENTS.md index 601576f07b..099fbc7083 100644 --- a/docs/design-system/COMPONENTS.md +++ b/docs/design-system/COMPONENTS.md @@ -958,7 +958,7 @@ This generated snapshot is a local source-derived inventory. It does not assert | `AnswerFooter` | answer | yes | yes | no | yes | no | 0 | | `AsyncButton` | controls | yes | yes | inherited-global-root | yes | no | 4 | | `Breadcrumb` | layout | yes | yes | inherited-global-root | yes | no | 1 | -| `Button` | controls | yes | yes | inherited-global-root | yes | no | 3 | +| `Button` | controls | yes | yes | inherited-global-root | yes | no | 2 | | `Checkbox` | controls | yes | yes | no | yes | no | 0 | | `Chip` | controls | yes | yes | inherited-global-root | yes | no | 3 | | `Citation` | source | yes | yes | no | yes | no | 0 | @@ -992,7 +992,7 @@ This generated snapshot is a local source-derived inventory. It does not assert | `SearchField` | controls | yes | yes | no | yes | no | 0 | | `SegmentedControl` | controls | yes | yes | inherited-global-root | yes | no | 3 | | `Select` | controls | yes | yes | inherited-global-root | yes | no | 2 | -| `Sheet` | layout | yes | yes | inherited-global-root | yes | no | 23 | +| `Sheet` | layout | yes | yes | inherited-global-root | yes | no | 24 | | `Skeleton` | feedback | yes | yes | inherited-global-root | yes | no | 6 | | `SourceDesignationBadge` | source | yes | yes | inherited-global-root | yes | no | 1 | | `SourceProvenance` | source | yes | yes | inherited-global-root | yes | no | 1 | diff --git a/docs/design-system/adoption-manifest.json b/docs/design-system/adoption-manifest.json index 6cf4478078..f2487bde14 100644 --- a/docs/design-system/adoption-manifest.json +++ b/docs/design-system/adoption-manifest.json @@ -272,13 +272,11 @@ "directImportFiles": [ "src/components/AccessibleTable.tsx", "src/components/clinical-dashboard/signed-image.tsx", - "src/components/in-page-nav/in-page-nav-header.tsx", "src/components/ui/confirm-dialog.tsx" ], "productImportFiles": [ "src/components/AccessibleTable.tsx", - "src/components/clinical-dashboard/signed-image.tsx", - "src/components/in-page-nav/in-page-nav-header.tsx" + "src/components/clinical-dashboard/signed-image.tsx" ], "designSync": { "listedInSourceMap": true, @@ -1428,6 +1426,7 @@ "src/components/clinical-dashboard/document-search-results.tsx", "src/components/clinical-dashboard/image-lightbox.tsx", "src/components/clinical-dashboard/master-search-header.tsx", + "src/components/clinical-dashboard/medication-record-page.tsx", "src/components/clinical-dashboard/mode-action-popup.tsx", "src/components/clinical-dashboard/result-filter-control.tsx", "src/components/clinical-dashboard/settings-dialog.tsx", @@ -1454,6 +1453,7 @@ "src/components/clinical-dashboard/document-search-results.tsx", "src/components/clinical-dashboard/image-lightbox.tsx", "src/components/clinical-dashboard/master-search-header.tsx", + "src/components/clinical-dashboard/medication-record-page.tsx", "src/components/clinical-dashboard/mode-action-popup.tsx", "src/components/clinical-dashboard/result-filter-control.tsx", "src/components/clinical-dashboard/settings-dialog.tsx", From 2a28b579c0bca1c91601c544038eb829714d1578 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sun, 9 Aug 2026 21:07:54 +0800 Subject: [PATCH 5/6] Align prescribing smoke test with back-link accessible name (#1787) --- tests/ui-smoke.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ui-smoke.spec.ts b/tests/ui-smoke.spec.ts index 2297d80857..9e48aab3d7 100644 --- a/tests/ui-smoke.spec.ts +++ b/tests/ui-smoke.spec.ts @@ -3406,7 +3406,7 @@ test.describe("Clinical KB UI smoke coverage", () => { await acamprosateResult.click(); await expect(page).toHaveURL(/\/medications\/acamprosate$/, { timeout: 30_000 }); await expectSingleMedicationPage(page); - await expect(page.getByRole("link", { name: "Medications", exact: true }).first()).toBeVisible(); + await expect(page.getByRole("link", { name: "Back to medications" }).first()).toBeVisible(); expect(parentNodeErrors).toEqual([]); }); From 5910a195de340af14282998cfda57221775074c1 Mon Sep 17 00:00:00 2001 From: Cursor Agent <cursoragent@cursor.com> Date: Sun, 9 Aug 2026 13:30:25 +0000 Subject: [PATCH 6/6] fix(in-page-nav): unblock PR 1781 Production UI failures Complete the prescribing smoke back-link assertion (aria-label form on phone too), floor the icon-only phone back control with min-w-tap, and scope form-detail-header through visibleByTestId for #093 duplicates. Add offline contracts so a half-patched smoke or bare testid cannot regress the same red required CI. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> --- .../in-page-nav/in-page-nav-header.tsx | 8 ++- tests/in-page-nav-header.dom.test.tsx | 9 ++++ tests/in-page-nav-playwright-contract.test.ts | 50 +++++++++++++++++++ tests/playwright-settlement-contract.test.ts | 11 ++++ tests/ui-forms-section-nav.spec.ts | 9 +++- tests/ui-smoke.spec.ts | 8 ++- 6 files changed, 90 insertions(+), 5 deletions(-) create mode 100644 tests/in-page-nav-playwright-contract.test.ts diff --git a/src/components/in-page-nav/in-page-nav-header.tsx b/src/components/in-page-nav/in-page-nav-header.tsx index b71548cc97..4a4ed30e39 100644 --- a/src/components/in-page-nav/in-page-nav-header.tsx +++ b/src/components/in-page-nav/in-page-nav-header.tsx @@ -231,8 +231,12 @@ export function InPageNavHeader(props: InPageNavHeaderProps) { aria-label={`Back to ${back.label.toLowerCase()}`} title={showBackLabel ? undefined : back.label} className={cn( - "inline-flex min-h-tap shrink-0 items-center gap-1.5 rounded-full pl-1.5 text-sm font-semibold text-[color:var(--text-muted)] transition hover:bg-[color:var(--surface-subtle)] hover:text-[color:var(--text-heading)]", - showBackLabel ? "pr-3" : "pr-1.5", + // `min-w-tap` is load-bearing on phones: the visible label is + // `hidden sm:inline`, so without a width floor the control shrinks + // to the icon + horizontal padding (~40px) and fails the production + // tap-target contract that Production UI asserts on medications. + "inline-flex min-h-tap min-w-tap shrink-0 items-center justify-center gap-1.5 rounded-full text-sm font-semibold text-[color:var(--text-muted)] transition hover:bg-[color:var(--surface-subtle)] hover:text-[color:var(--text-heading)]", + showBackLabel ? "pl-1.5 pr-3 max-sm:px-1.5" : "px-1.5", )} > <ArrowLeft className="h-5 w-5 shrink-0" aria-hidden /> diff --git a/tests/in-page-nav-header.dom.test.tsx b/tests/in-page-nav-header.dom.test.tsx index 3d0763106e..8d90319120 100644 --- a/tests/in-page-nav-header.dom.test.tsx +++ b/tests/in-page-nav-header.dom.test.tsx @@ -136,6 +136,15 @@ describe("InPageNavHeader", () => { expect(screen.getByTestId("service-detail-header")).toBeInTheDocument(); }); + it("keeps the phone-icon back control at the production tap-target floor", () => { + // The visible label is `hidden sm:inline`. Without min-w-tap the hit target + // collapses to ~40×48 on a phone and Production UI fails expectMinTouchTarget. + renderHeader(); + const back = screen.getByRole("link", { name: "Back to services" }); + expect(back.className).toMatch(/\bmin-h-tap\b/); + expect(back.className).toMatch(/\bmin-w-tap\b/); + }); + it("leaves the page's single h1 to the record body", () => { // Information pages keep their large title in the body, so the header title // must not be a second heading. diff --git a/tests/in-page-nav-playwright-contract.test.ts b/tests/in-page-nav-playwright-contract.test.ts new file mode 100644 index 0000000000..4ff3b65973 --- /dev/null +++ b/tests/in-page-nav-playwright-contract.test.ts @@ -0,0 +1,50 @@ +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; + +/** + * Guards the Production UI failures that blocked PR #1781: + * + * 1. InPageNavHeader's back control is always named `Back to ${label}` via + * aria-label. Asserting the bare mode label (`Medications`, exact) passes on + * desktop (visible text) and fails on phone (`hidden sm:inline`). An autofix + * that only patches the desktop prescribing smoke left shard 2 red. + * 2. `form-detail-header` (and every `*-detail-header` from InPageNavHeader) is a + * #093 settlement surface: bare getByTestId trips strict mode under + * Production UI load when a hidden twin remains. + */ + +const UI_SMOKE = "tests/ui-smoke.spec.ts"; +const UI_FORMS_SECTION_NAV = "tests/ui-forms-section-nav.spec.ts"; +const IN_PAGE_NAV_HEADER = "src/components/in-page-nav/in-page-nav-header.tsx"; + +describe("in-page-nav Playwright contract", () => { + it("prescribing smoke asserts the aria-label back name on both desktop and phone", () => { + const source = readFileSync(UI_SMOKE, "utf8"); + const prescribingBlock = source.slice( + source.indexOf('test("prescribing workflow uses in-app medication routes'), + source.indexOf('test("tablet document chrome keeps one new-chat action'), + ); + + expect(prescribingBlock).toContain('name: "Back to medications"'); + expect(prescribingBlock).not.toMatch(/getByRole\(\s*["']link["']\s*,\s*\{\s*name:\s*["']Medications["']/); + // Both the desktop and phone prescribing tests must use the aria-label form. + expect(prescribingBlock.match(/Back to medications/g)?.length ?? 0).toBeGreaterThanOrEqual(2); + }); + + it("forms section-nav scopes form-detail-header through visibleByTestId", () => { + const source = readFileSync(UI_FORMS_SECTION_NAV, "utf8"); + expect(source).toMatch(/import\s*\{[^}]*\bvisibleByTestId\b[^}]*\}\s*from\s*["']\.\/playwright-settlement["']/); + expect(source).toContain('visibleByTestId(page, "form-detail-header")'); + expect(source).not.toMatch(/getByTestId\(\s*["']form-detail-header["']\s*\)/); + }); + + it("InPageNavHeader keeps the Back to ${label} aria-label contract", () => { + const source = readFileSync(IN_PAGE_NAV_HEADER, "utf8"); + expect(source).toContain("Back to ${back.label.toLowerCase()}"); + }); + + it("InPageNavHeader floors the back control with min-w-tap for phone icon-only width", () => { + const source = readFileSync(IN_PAGE_NAV_HEADER, "utf8"); + expect(source).toMatch(/min-h-tap\s+min-w-tap/); + }); +}); diff --git a/tests/playwright-settlement-contract.test.ts b/tests/playwright-settlement-contract.test.ts index 8c3fe68009..02089cdf0c 100644 --- a/tests/playwright-settlement-contract.test.ts +++ b/tests/playwright-settlement-contract.test.ts @@ -21,6 +21,8 @@ const PAGE_ROOT_TEST_IDS = [ "search-query-ribbon", ] as const; +const FORMS_SECTION_NAV = "tests/ui-forms-section-nav.spec.ts"; + describe("playwright settlement contract (#093)", () => { it("ui-route-coverage scopes page-root testids through visibleByTestId", () => { const source = readFileSync(ROUTE_COVERAGE, "utf8"); @@ -33,4 +35,13 @@ describe("playwright settlement contract (#093)", () => { ); } }); + + it("ui-forms-section-nav scopes form-detail-header through visibleByTestId", () => { + // Production UI shard 1 on PR #1781: bare form-detail-header resolved to 2 + // under full-suite load (in-flow + phone-portaled / streaming twin). + const source = readFileSync(FORMS_SECTION_NAV, "utf8"); + expect(source).toMatch(/import\s*\{[^}]*\bvisibleByTestId\b[^}]*\}\s*from\s*["']\.\/playwright-settlement["']/); + expect(source).toContain('visibleByTestId(page, "form-detail-header")'); + expect(source).not.toMatch(/getByTestId\(\s*["']form-detail-header["']\s*\)/); + }); }); diff --git a/tests/ui-forms-section-nav.spec.ts b/tests/ui-forms-section-nav.spec.ts index ed8d723960..f50d0affd3 100644 --- a/tests/ui-forms-section-nav.spec.ts +++ b/tests/ui-forms-section-nav.spec.ts @@ -1,5 +1,7 @@ import { expect, test } from "playwright/test"; +import { visibleByTestId } from "./playwright-settlement"; + /** * `/issues` #256, proven where it actually failed: in the browser. * @@ -65,7 +67,12 @@ test.describe("Forms section navigation", () => { ] as const) { await page.setViewportSize({ width, height: 900 }); await page.goto(FORM_ROUTE, { waitUntil: "domcontentloaded" }); - await expect(page.getByTestId("form-detail-header")).toBeVisible({ timeout: 20_000 }); + // Ledger #093: under Production UI load Next can leave a hidden twin of the + // in-page header (in-flow under the reserve pad + portaled phone copy, or a + // streaming clone). Bare getByTestId then fails Playwright strict mode even + // when only one owner is visible — the same defect that already gates + // ui-route-coverage through visibleByTestId. + await expect(visibleByTestId(page, "form-detail-header")).toBeVisible({ timeout: 20_000 }); const visible = await page.evaluate(() => [ diff --git a/tests/ui-smoke.spec.ts b/tests/ui-smoke.spec.ts index 9e48aab3d7..60b3148766 100644 --- a/tests/ui-smoke.spec.ts +++ b/tests/ui-smoke.spec.ts @@ -3406,7 +3406,7 @@ test.describe("Clinical KB UI smoke coverage", () => { await acamprosateResult.click(); await expect(page).toHaveURL(/\/medications\/acamprosate$/, { timeout: 30_000 }); await expectSingleMedicationPage(page); - await expect(page.getByRole("link", { name: "Back to medications" }).first()).toBeVisible(); + await expect(page.getByRole("link", { name: "Back to medications" }).filter({ visible: true })).toBeVisible(); expect(parentNodeErrors).toEqual([]); }); @@ -3438,7 +3438,11 @@ test.describe("Clinical KB UI smoke coverage", () => { await acamprosateCard.click(); await expect(page).toHaveURL(/\/medications\/acamprosate$/, { timeout: 30_000 }); - const backLink = page.getByRole("link", { name: "Medications", exact: true }); + // InPageNavHeader always names the control `Back to ${label}` via aria-label; + // the visible "Medications" text is `hidden sm:inline` and absent on phone. + // Scope to the visible owner — phone portals the header into the collapse + // addon, and #093 streaming can leave a hidden twin under full-suite load. + const backLink = page.getByRole("link", { name: "Back to medications" }).filter({ visible: true }); await expect(backLink).toBeVisible(); await expectMinTouchTarget(backLink); await backLink.click();