Skip to content
Merged
37 changes: 37 additions & 0 deletions docs/design-system/DECISIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,43 @@ acceptance, product restyling, or v2 root activation.

---

## C7 · `mode-home-template.tsx` and `search-results-header-band.tsx` stay outside the `PageHeader` vocabulary (18 August 2026, `#222`)

**Chose.** Neither file converges onto `PageHeader`. Both are recorded here as permanently
declined conversions rather than left as an open question for the next reader to re-derive.

- `mode-home-template.tsx`'s `ModeHomeHero` is a centred, `text-center`/`items-center`/
`justify-center` display hero on the fluid `text-hero` token, and it is the slot the
in-flow phone composer sits directly beneath (`desktopComposerSlotId` /
`DesktopComposerPortalSlot` render immediately after it). `PageHeader` is a left-aligned
title stack. Converting the hero would be a visual redesign of all thirteen mode homes
(`src/lib/app-modes.ts`) and risks colliding with the "one composer per page" contract in
`docs/search-chrome-behaviour.md` — out of proportion to a header-vocabulary convergence.
- `search-results-header-band.tsx` is a results spine carrying live status
(`role="status"`), result counts, and applied-filter chips — not a page-title stack.
`tests/search-results-header-band.dom.test.tsx` pins its current shape and stays
unconverted.

**Already converged, separately from this decision.** `ModeHomeStatusNotice` (also in
`mode-home-template.tsx`) delegates to the DS `EmptyState` (PR #1842, ledger `#221`) — that
was always a different conversion from the `PageHeader` question this row asks, and it is
done. `DsmPageHeader`, `InformationPageHeader`, and `InformationPageBreadcrumbs` converged
onto `PageHeader`/`Breadcrumb` in the same wave (PR-J, Builder A). This decision closes only
the two files that wave explicitly declined.

**Rejected.** Converging either file to force header-surface uniformity. Both declines have a
structural reason tied to a different contract (composer placement; results-spine semantics),
not inertia — forcing the conversion would fix a vocabulary inconsistency by breaking a
different, more load-bearing one.

**Why.** `#222` asked for exactly this: a decision on whether either file is in scope at all,
recorded so a later session does not re-derive the same two options. `PageHeader`
vocabulary now permanently excludes these two call sites; a future redesign of mode-home
hero layout or the results spine is a separate, explicitly-scoped change, not a header
convergence.

---

## Q&A record — clinical inputs (31 July 2026)

| # | Question | Answer | Design consequence |
Expand Down
36 changes: 36 additions & 0 deletions docs/design-system/GATES.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ theme-list parity, and remote design-project publication remain separate concern
| `dark:` colour override | `check:design-system-contract` — `darkColorOverrides`, baseline 0 | **implemented-blocking** (7 Aug 2026) — all 3 instances retired; see §4 |
| Truncate a page title, dialog title, drug name or review warning | — | **manual** |
| Restate a token value in prose | — | **manual** (eleven divergences came from this; TOKENS.md is the only inventory) |
| Add a named `--breakpoint-*` token for a device-band edge | — | **manual** (decided 18 Aug 2026, `#336`) — stay raw `min-[…]`/`max-[…]`; see §3b |

## 3a · Rules added by the perfection pass (31 July, second half)

Expand All @@ -142,6 +143,41 @@ theme-list parity, and remote design-project publication remain separate concern
| Per-component contracts (COMPONENTS §9) bind at review; defects map to closing PRs | Review checklist; contract tests land with each named PR | manual until each PR |
| `ui-primitives.tsx` changes require the focused DOM tests | `tests/ui-primitives.dom.test.tsx` + `icon-button` | implemented (run discipline manual) |

## 3b · Responsive breakpoint windows stay raw — decided 18 August 2026 (`#336`)

**Chose (a): no `--breakpoint-*` tokens.** The repo defines zero `--breakpoint-*` entries and
nine call sites hand-write the arbitrary form — `min-[414px]:max-[429px]` at
`clinical-dashboard/result-filter-control.tsx`, `max-[359px]` at three sites
(`search-heading-mockups.tsx` ×3, `differentials/diagnosis-map-panel.tsx`), and `max-[389px]`
at three sites (`factsheets/factsheets-search-page.tsx`,
`clinical-dashboard/search-results-header-band.tsx`,
`factsheets/factsheets-compact-view-mockups.tsx`). These stay exactly as raw Tailwind
arbitrary values. Two of the nine sit in `*-mockups.tsx` files and were already out of gate
scope; the decision covers all nine uniformly so the answer does not depend on which file a
future site happens to land in.

**Rejected (b): name them.** Tailwind 4's `--breakpoint-<name>` generates both a `<name>:`
(min-width) and a `max-<name>:` variant **for every utility in the build**, not just the call
sites that use it. Three named breakpoints for what is currently nine single-purpose
consumers would add two global variant families per name to justify one `sr-only` toggle
each. `max-[429px]` is also inclusive while a generated `max-<bp>:` variant is exclusive, so
naming the 414–429 window needs _two_ boundary tokens (414px and 430px) to reproduce today's
behaviour exactly — an easy off-by-one for a later editor to reintroduce.

**Why.** These are per-device band edges with a measured, one-off justification at their own
call site (fitting a specific label at a specific chip/card width), not a reusable scale the
way `--spacing-icon-*` or the type-scale steps are — reusability is the property that makes a
named token cheaper than the value it replaces, and none of these nine sites share a reason
with any other. Recording the choice here, rather than migrating quietly, is what `#336`
asked for: the next session that wants to add a tenth `max-[…]:` site can use the same raw
form without re-deriving whether a token was supposed to exist.

**Stop.** Do not add a `--breakpoint-*` `@theme` entry for a single-consumer device-band
edge; use the raw `min-[…]`/`max-[…]` form and, if the reason is not obvious from the
surrounding markup, say why in a comment at the call site. Revisit only if a fourth or later
consumer genuinely needs the _same_ window (not just a similarly-sized one) — that is the
point where the scale argument for (b) would start being true.

## 4 · Recorded verification evidence

Recorded at export (31 July 2026), **not re-run for this document set**:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"version": 2,
"id": "1ead9bb9-5e9d-4b54-b530-87586d594f0a",
"createdOn": "2026-08-18",
"action": "cancel",
"payload": {
"requestId": "779bc0e9-c28f-40ab-abda-cfdcf93ea752",
"reason": "Superseded by d25147c8-3da0-4062-9ce0-356e59a16c63 (this PR): #321 has a newer PARTIAL update reflecting the filmstrip fix actually shipped in this PR."
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"version": 2,
"id": "4302ff5b-582a-4204-94bf-d554c953900f",
"createdOn": "2026-08-18",
"action": "done",
"payload": {
"id": "#222",
"outcome": "DECIDED AND RECORDED 18 August 2026 as DECISIONS.md C7. mode-home-template.tsx and search-results-header-band.tsx are permanently declared outside the PageHeader vocabulary: the mode-home hero is a centred display hero the in-flow composer sits beneath (redesign risk + one-composer-per-page collision), and the results-header band is a status/count/filter spine, not a title stack, pinned by tests/search-results-header-band.dom.test.tsx. Noted separately in the same decision: ModeHomeStatusNotice already converged onto the DS EmptyState via PR #1842 (#221) — a different, already-closed conversion from the PageHeader question this row asked. Docs-only change.",
"baseRowFingerprint": "ba6621ae785bf636071d361b64b416ca35d8e6bc893eb6bb1ccef662dae46465"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"version": 2,
"id": "48e960d3-36e0-4be0-af16-7959d62b1907",
"createdOn": "2026-08-18",
"action": "cancel",
"payload": {
"requestId": "1505ba13-1b2a-47c9-b6cc-03b060f7a878",
"reason": "Superseded by 993a1c72-96bd-4e48-b1a4-3ea3b603acac (this PR): #336's decision is now made and recorded in GATES.md/DECISIONS.md, replacing the earlier 'stays open' flag."
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"version": 2,
"id": "8650e564-7835-47b3-a7e2-476f369b9eea",
"createdOn": "2026-08-18",
"action": "cancel",
"payload": {
"requestId": "a39af37c-b4dd-4e4b-84dc-395d86937615",
"reason": "Superseded by 4302ff5b-582a-4204-94bf-d554c953900f (this PR): #222's decision is now made and recorded in DECISIONS.md C7, replacing the earlier 'stays open' flag."
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"version": 2,
"id": "993a1c72-96bd-4e48-b1a4-3ea3b603acac",
"createdOn": "2026-08-18",
"action": "done",
"payload": {
"id": "#336",
"outcome": "DECIDED AND RECORDED 18 August 2026. Chose (a): responsive breakpoint windows stay raw min-[…]/max-[…] everywhere; no --breakpoint-* @theme tokens added. Decision and full rationale recorded in docs/design-system/GATES.md §3 (prohibition-table row) and new §3b (18 Aug 2026), covering all nine current call sites (result-filter-control.tsx, search-heading-mockups.tsx x3, diagnosis-map-panel.tsx, factsheets-search-page.tsx, search-results-header-band.tsx, factsheets-compact-view-mockups.tsx). Docs-only change; no migration performed, per the row's own stop rule.",
"baseRowFingerprint": "757b5240cff62bd5071d12e4f2ada7e7c3faebb8018843b45a8675913b7442d9"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"version": 2,
"id": "d25147c8-3da0-4062-9ce0-356e59a16c63",
"createdOn": "2026-08-18",
"action": "update",
"payload": {
"id": "#321",
"detail": "PARTIAL 18 August 2026. Of the four follow-up groups: (1) the filmstrip 'Page unknown' control is FIXED — document-image-filmstrip.tsx converted its data-driven disabled state from native disabled to aria-disabled=true + ignoreUnavailableActivation + an sr-only reason, per docs/wiring-conventions.md's stated-reason pattern (settles this one control from #291's follow-up list); tests/document-image-filmstrip.dom.test.tsx gained a focused case (aria-disabled, not natively disabled, accessible description, click is a no-op), vitest run: 3 passed. The other three groups are unchanged and still not single-PR-sized: the six differential comparison page controls remain coupled to its own planned rewrite and pinned density test; DocumentViewer's persistent-access-reason/transient-loading split is a classification design decision, not yet made; the pin-limit control remains a capacity-state judgement call. Stays open for those three.",
"baseRowFingerprint": "b1b6db157f9d95ca18ab31ba8ccc709dc28cf603838d599b19f1b694f88b4ac5"
}
}
57 changes: 32 additions & 25 deletions src/components/document-viewer/document-image-filmstrip.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"use client";

import { cn, textMuted } from "@/components/ui-primitives";
import { cn, ignoreUnavailableActivation, textMuted } from "@/components/ui-primitives";
import type { ImageRow } from "@/components/document-viewer/types";

/**
Expand Down Expand Up @@ -35,31 +35,38 @@ export function DocumentImageFilmstrip({
`Figure ${index + 1}`;
const shortLabel = labelSeed.length > 28 ? `${labelSeed.slice(0, 27)}…` : labelSeed;

const pageUnavailableId = page === null ? `filmstrip-page-unknown-${image.id}` : undefined;

return (
<button
key={image.id}
type="button"
disabled={page === null}
aria-current={isActive ? "page" : undefined}
aria-label={page === null ? `${labelSeed} — page unknown` : `Show PDF page ${page} for ${labelSeed}`}
title={page === null ? "Page unknown" : `Go to page ${page}`}
onClick={() => {
if (page !== null) onSelectPage(page);
}}
className={cn(
"inline-flex min-h-tap shrink-0 items-center gap-1.5 rounded-md border px-2.5 text-xs font-semibold transition",
"focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[color:var(--focus)]",
"disabled:cursor-not-allowed disabled:opacity-45",
isActive
? "border-[color:var(--clinical-accent)]/40 bg-[color:var(--clinical-accent-soft)] text-[color:var(--clinical-accent)]"
: "border-[color:var(--border)] bg-[color:var(--surface)] text-[color:var(--text-muted)] hover:bg-[color:var(--surface-subtle)] hover:text-[color:var(--text)]",
)}
>
<span className={cn("nums shrink-0", isActive ? undefined : textMuted)}>
{page === null ? "—" : `p.${page}`}
</span>
<span className="max-w-[9rem] truncate">{shortLabel}</span>
</button>
<div key={image.id} className="contents">
<button
type="button"
aria-disabled={page === null ? "true" : undefined}
onClick={page === null ? ignoreUnavailableActivation : () => onSelectPage(page)}
aria-current={isActive ? "page" : undefined}
aria-label={page === null ? `${labelSeed} — page unknown` : `Show PDF page ${page} for ${labelSeed}`}
aria-describedby={pageUnavailableId}
title={page === null ? "Page unknown — this figure has no recorded page number" : `Go to page ${page}`}
className={cn(
"inline-flex min-h-tap shrink-0 items-center gap-1.5 rounded-md border px-2.5 text-xs font-semibold transition",
"focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[color:var(--focus)]",
"aria-disabled:cursor-not-allowed aria-disabled:opacity-45 hover:not-aria-disabled:bg-[color:var(--surface-subtle)] hover:not-aria-disabled:text-[color:var(--text)]",
isActive
? "border-[color:var(--clinical-accent)]/40 bg-[color:var(--clinical-accent-soft)] text-[color:var(--clinical-accent)]"
: "border-[color:var(--border)] bg-[color:var(--surface)] text-[color:var(--text-muted)]",
)}
>
<span className={cn("nums shrink-0", isActive ? undefined : textMuted)}>
{page === null ? "—" : `p.${page}`}
</span>
<span className="max-w-[9rem] truncate">{shortLabel}</span>
</button>
{pageUnavailableId ? (
<span id={pageUnavailableId} className="sr-only">
This figure has no recorded page number, so it cannot be jumped to.
</span>
) : null}
</div>
);
})}
</div>
Expand Down
19 changes: 19 additions & 0 deletions tests/document-image-filmstrip.dom.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,25 @@ describe("document image filmstrip page sync", () => {
expect(onSelectPage).toHaveBeenCalledExactlyOnceWith(2);
});

it("keeps a page-unknown chip reachable and inert, never the native disabled attribute", () => {
const onSelectPage = vi.fn();
const images = [image({ id: "img-3", page_number: null, tableLabel: "Undated appendix table" })];

render(<DocumentImageFilmstrip images={images} activePage={1} onSelectPage={onSelectPage} />);

const chip = screen.getByRole("button", { name: /Undated appendix table — page unknown/i });

// `aria-disabled`, never `disabled`: the native attribute removes the tab
// stop, so the reason a keyboard/screen-reader user needs would never be
// reached. See docs/wiring-conventions.md.
expect(chip).toHaveAttribute("aria-disabled", "true");
expect(chip).not.toBeDisabled();
expect(chip).toHaveAccessibleDescription("This figure has no recorded page number, so it cannot be jumped to.");

fireEvent.click(chip);
expect(onSelectPage).not.toHaveBeenCalled();
});

it("lets DocumentImage page badges jump the PDF without requiring a second signed-URL fetch", () => {
const onSelectPage = vi.fn();
render(
Expand Down
Loading