Skip to content

fix(navigation): wire the forms section anchors, drop the dead presentation set#1697

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
BigSimmo merged 4 commits into from
Aug 7, 2026
Merged

fix(navigation): wire the forms section anchors, drop the dead presentation set #1697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1693230
fix(navigation): wire the forms section anchors, drop the dead presen…
claude Aug 7, 2026
8a548b2
docs(ledger): record the #256 section-nav fix review (PR #1697)
claude Aug 7, 2026
2b99db1
merge(main): sync PR #1697 with origin/main
cursoragent Aug 7, 2026
4c19050
Merge branch 'main' into claude/issues-256-section-nav-clean
BigSimmo Aug 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/branch-review-ledger.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -703,4 +703,5 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie
| 2026-08-07 | claude/handover-review-nlhuln | 4ff613c10fbf734b1e740a31611296c17c791ec7 | mode nav remaining modes: vestigial strip removal (PR #1679) | Removed the single-button action strip from answer/documents/services/forms/favourites/prescribing/tools; deleted the registry index-0 fallback (TS2493-forced) and the dead documents clause; stripped modeItems/onSearch/modeAriaLabel/stickyTop from PageSecondaryNavigation, keeping the empty-registry return below the information-section branch; kept the action kind with a no-live-consumer note. Completes the 13-mode navigation rollout. | lint exit 0; typecheck clean; focused 5 files 97 tests; test 518/519 files (pr-handoff-stop re-confirmed pre-existing on this base via stashed re-run); ui-mode-nav-density + ui-accessibility 71 passed (landmark scan green); branch-order guard mutation-checked (hoisting it fails 2 tests); format committed; verify:pr-local blocked at check:installed-lock-parity (playwright 1.62.0 vs 1.62.1) |
| 2026-08-05 | HEAD | 2a8881afad230880166de60a533e17588d9920ec | seven-report repo-wide audit | P2 confirmed: live drift and live-only migration; mobile CLS; ACL runner; OOXML declared-size robustness; assertion false positives. Numerous report claims stale, narrowed, or unsafe. | verify:cheap (5088 pass, 3 skip, 1 timeout); focused retry 21/21 pass; build pass; bundle budget pass; offline RAG 574/574; assertions 98%; live drift 34; browser CLS 0.228/0.218 |
| 2026-08-07 | codex/consolidated-ledger-updates (PR #1683) | 413e679bb92cb19717d6d8301764df44694eb73e | review-and-fix PR #1683 | synced origin/main (behind-but-clean DIRTY cleared); restored main ledger order + sole seven-report row; Bugbot none; no P0/P1; merge-tree clean | verify:pr-local docs scope PASS (format:changed Prettier; check:branch-review-ledger 648; docs links 1650; outstanding-issues 258); merge-tree clean |
| 2026-08-07 | claude/issues-256-section-nav-clean | 169323053db5c572d183d59c113ecd0c76e7aca5 | issues #256: forms section anchors + differentials presentation set (PR #1697) | Wired all six formSections anchors in form-detail-page.tsx (four direct ids, two breakpoint pairs via existing mobile wrappers and single-child desktop wrappers, no component signature change); deleted differentialPresentationSections and declared /differentials/presentations/ locally-owned instead, since three of its six sections declared a -mobile targetId ReviewPanels can never satisfy and the page owns MobileTabs below xl plus the xl review sidebar. Added a registered browser spec because source-text and jsdom guards both structurally cannot see breakpoint-variant resolution. | lint exit 0; typecheck clean; test 519/520 files (pr-handoff-stop confirmed pre-existing via stashed re-run); check:gate-manifest and check:ci-scope pass with the new spec in both playwright allowlists; ui-forms-section-nav + ui-accessibility 18 passed incl real-record nav with 6 links and exactly one variant per pair visible at 390px and 1280px; binding guard mutation-checked red on one removed id; browser spec observed failing when nav genuinely absent; format clean. Environment: npm ci blocked (main lockfile needs Node >=24.15, container has 24.13), tailwind-merge@3.6.0 materialised from tarball only |
| 2026-08-07 | cursor/inpage-nav-default-235a | c6d72e406c03e205bac86d23e84813c41332c205 | docs: default in-page nav DocumentViewer chrome + PhoneHeaderCollapsePortal | docs-only; verify:pr-local passed (low-risk docs scope) | verify:pr-local --files AGENTS.md,docs/search-chrome-behaviour.md |
4 changes: 2 additions & 2 deletions playwright.config.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,15 +23,15 @@ const chromiumExecutablePath = process.env.PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH;
// `tests/playwright-project-isolation.test.ts` asserts every such file on disk is
// matched here.
const productionSpecPattern =
/.*(?:answer-progress-ui-smoke|ui-(smoke|stress|accessibility|tools|overlap|universal-search|specifiers|formulation|chrome-scroll|therapy-nav-scroll|mode-nav-density|phone-scroll(?:-[a-z0-9-]+)?|pwa|route-coverage|style-contract|visual-artifacts|hydration))\.spec\.ts/;
/.*(?:answer-progress-ui-smoke|ui-(smoke|stress|accessibility|tools|overlap|universal-search|specifiers|formulation|forms-section-nav|chrome-scroll|therapy-nav-scroll|mode-nav-density|phone-scroll(?:-[a-z0-9-]+)?|pwa|route-coverage|style-contract|visual-artifacts|hydration))\.spec\.ts/;
const mockupSpecPattern =
/.*ui-(document-top-navigation-mockup|therapy-navigation-mockup|tools|tools-collapse|tools-task-directory)\.spec\.ts/;
const mockupTag = /@mockup/;

export default defineConfig({
testDir: "./tests",
testMatch:
/.*(?:answer-progress-ui-smoke|ui-(smoke|stress|accessibility|document-top-navigation-mockup|therapy-navigation-mockup|tools|tools-collapse|tools-task-directory|overlap|universal-search|specifiers|formulation|chrome-scroll|therapy-nav-scroll|mode-nav-density|phone-scroll(?:-[a-z0-9-]+)?|pwa|route-coverage|style-contract|visual-artifacts|hydration))\.spec\.ts/,
/.*(?:answer-progress-ui-smoke|ui-(smoke|stress|accessibility|document-top-navigation-mockup|therapy-navigation-mockup|tools|tools-collapse|tools-task-directory|overlap|universal-search|specifiers|formulation|forms-section-nav|chrome-scroll|therapy-nav-scroll|mode-nav-density|phone-scroll(?:-[a-z0-9-]+)?|pwa|route-coverage|style-contract|visual-artifacts|hydration))\.spec\.ts/,
timeout: 60_000,
retries: 0,
// Fail the run if a stray `test.only` is committed: otherwise it silently
Expand Down
35 changes: 27 additions & 8 deletions src/components/forms/form-detail-page.tsx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -696,7 +696,10 @@ export function FormDetailPage({ form }: { form: FormRecord }) {

<div className="grid gap-4 lg:grid-cols-[minmax(0,1fr)_22rem] xl:grid-cols-[minmax(0,1fr)_24rem]">
<div className="min-w-0 space-y-4">
<section className="rounded-lg border border-[color:var(--border-lux)] bg-[color:var(--surface-lux)] p-3 shadow-[var(--shadow-inset)] sm:p-5">
<section
id="form-overview"
className="rounded-lg border border-[color:var(--border-lux)] bg-[color:var(--surface-lux)] p-3 shadow-[var(--shadow-inset)] sm:p-5"
>
<div className="grid grid-cols-[3.75rem_minmax(0,1fr)_2.75rem] gap-x-3 gap-y-2.5 sm:grid-cols-[6rem_minmax(0,1fr)_auto] sm:gap-x-4 sm:gap-y-3 xl:grid-cols-[auto_minmax(0,1fr)_auto] xl:items-start">
<FormCodeBadge code={code} variant="hero" />
<div className="min-w-0">
Expand DownExpand Up@@ -817,7 +820,7 @@ export function FormDetailPage({ form }: { form: FormRecord }) {
/>
</div>

<section aria-label="Priority facts" className="space-y-2.5 sm:space-y-3">
<section id="form-priority-facts" aria-label="Priority facts" className="space-y-2.5 sm:space-y-3">
<h2 className="text-base-minus font-semibold leading-5 text-[color:var(--text-heading)] sm:text-base">
Priority facts
</h2>
Expand All@@ -828,7 +831,10 @@ export function FormDetailPage({ form }: { form: FormRecord }) {
</div>
</section>

<section className="rounded-lg border border-[color:var(--warning-border)] bg-[color:var(--warning-soft)]/30 p-4 shadow-[var(--shadow-inset)]">
<section
id="form-legal-boundary"
className="rounded-lg border border-[color:var(--warning-border)] bg-[color:var(--warning-soft)]/30 p-4 shadow-[var(--shadow-inset)]"
>
<div className="grid gap-3 sm:grid-cols-[2.5rem_minmax(0,1fr)]">
<span className="grid h-10 w-10 place-items-center rounded-lg bg-[color:var(--warning-soft)] text-[color:var(--warning)] shadow-[var(--shadow-inset)]">
<ShieldCheck className="h-5 w-5" aria-hidden />
Expand All@@ -850,7 +856,7 @@ export function FormDetailPage({ form }: { form: FormRecord }) {
</div>
</section>

<section aria-label="Form information" className="grid gap-2">
<section id="form-information" aria-label="Form information" className="grid gap-2">
{detailRows.map((row) => {
const label = row.label.toLowerCase();
const Icon = label.includes("only")
Expand All@@ -868,7 +874,12 @@ export function FormDetailPage({ form }: { form: FormRecord }) {
})}
</section>

<div className="grid gap-3 lg:hidden">
{/* The `-mobile`/`-desktop` id pairs below are the section anchors
`formSections` declares. Only one of each pair is ever visible:
`AvailableInformationPageNavigation` resolves a section to its
first VISIBLE target, and `lg:hidden` / `hidden lg:block` make
exactly one side `display:none` per breakpoint. */}
<div id="form-source-verification-mobile" className="grid gap-3 lg:hidden">
<SourceSnapshotCard form={form} />
<ActionPanel
sourceHref={form.source?.url ?? null}
Expand All@@ -877,14 +888,22 @@ export function FormDetailPage({ form }: { form: FormRecord }) {
/>
</div>

<div className="lg:hidden">
<div id="form-decision-context-mobile" className="lg:hidden">
<PathwayContextCard form={form} code={code} criteria={criteria} testId="form-decision-context-mobile" />
</div>
</div>

<aside className="polished-scroll hidden min-w-0 space-y-3 lg:sticky lg:top-[5.75rem] lg:block lg:max-h-[calc(100dvh-7rem)] lg:self-start lg:overflow-y-auto lg:pr-1">
<PathwayContextCard form={form} code={code} criteria={criteria} testId="form-decision-context-desktop" />
<SourceSnapshotCard form={form} />
<div id="form-decision-context-desktop">
<PathwayContextCard form={form} code={code} criteria={criteria} testId="form-decision-context-desktop" />
</div>
{/* Anchors the source band. The two RailCards below ("Source status",
"Verification") are its siblings in this scrollable rail, so
landing here brings them into view without a wrapper spanning
all three. */}
<div id="form-source-verification-desktop">
<SourceSnapshotCard form={form} />
</div>

<RailCard icon={FileText} title="Source status">
<div className="rounded-lg border border-[color:var(--border)] bg-[color:var(--surface)] p-3">
Expand Down
58 changes: 7 additions & 51 deletions src/components/page-secondary-navigation.tsx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -63,56 +63,6 @@ const formulationSections: readonly InformationPageSectionDefinition[] = [
{ id: "evidence", label: "Evidence / source", targetIds: ["formulation-evidence"] },
];

const differentialPresentationSections: readonly InformationPageSectionDefinition[] = [
{ id: "overview", label: "Overview", targetIds: ["differential-presentation-overview"] },
{
id: "comparison",
label: "Comparison",
targetIds: ["differential-presentation-comparison-mobile", "differential-presentation-comparison-desktop"],
fragmentId: "differential-presentation-comparison",
},
{
id: "safety",
label: "Safety",
targetIds: [
"differential-presentation-safety-mobile",
"differential-presentation-safety-tablet",
"differential-presentation-safety-desktop",
],
fragmentId: "differential-presentation-safety",
},
{
id: "urgency",
label: "Highest urgency",
targetIds: [
"differential-presentation-urgency-mobile",
"differential-presentation-urgency-tablet",
"differential-presentation-urgency-desktop",
],
fragmentId: "differential-presentation-urgency",
},
{
id: "handoff",
label: "Handoff",
targetIds: [
"differential-presentation-handoff-mobile",
"differential-presentation-handoff-tablet",
"differential-presentation-handoff-desktop",
],
fragmentId: "differential-presentation-handoff",
},
{
id: "sources",
label: "Source status",
targetIds: [
"differential-presentation-sources-mobile",
"differential-presentation-sources-tablet",
"differential-presentation-sources-desktop",
],
fragmentId: "differential-presentation-sources",
},
];

const dsmDiagnosisSections: readonly InformationPageSectionDefinition[] = [
{ id: "criteria", label: "Criteria", targetIds: ["criteria"] },
{ id: "features", label: "Key features", targetIds: ["key-features"] },
Expand DownExpand Up@@ -155,7 +105,6 @@ export function informationPageSectionDefinitions(pathname: string): readonly In
!["/formulation/builder", "/formulation/compare", "/formulation/map"].includes(pathname)
)
return formulationSections;
if (pathname.startsWith("/differentials/presentations/")) return differentialPresentationSections;
if (pathname.endsWith("/differentials") && pathname.startsWith("/dsm/diagnoses/")) return dsmDifferentialSections;
if (pathname.startsWith("/dsm/diagnoses/")) return dsmDiagnosisSections;
if (pathname.startsWith("/documents/") && pathname !== "/documents/search") return documentSections;
Expand All@@ -168,6 +117,13 @@ export function hasLocalInformationPageNavigation(pathname: string): boolean {
return (
pathname.startsWith("/medications/") ||
pathname.startsWith("/differentials/diagnoses/") ||
// The presentation workflow owns navigation at every width: `MobileTabs`
// (`xl:hidden`) below xl, and an always-visible "Differential review
// sidebar" aside at xl that shows every panel at once. It used to declare a
// `differentialPresentationSections` set instead, whose six targetIds no
// component rendered — so the route was claimed and nothing was drawn
// (/issues #256). Declaring local ownership is what was true all along.
pathname.startsWith("/differentials/presentations/") ||
(pathname.startsWith("/factsheets/") && pathname !== "/factsheets/search") ||
pathname.startsWith("/therapy-compass/") ||
// DocumentViewer owns DocumentViewerAnchors (PDF/Evidence/Text/Summary/Images).
Expand Down
52 changes: 37 additions & 15 deletions tests/page-secondary-navigation.dom.test.tsx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -33,10 +33,6 @@ describe("PageSecondaryNavigation", () => {
"/formulation/avoidance",
["Overview", "What matters now", "Fit", "5 Ps", "Treatment leverage", "Evidence / source"],
],
[
"/differentials/presentations/acute-confusion-encephalopathy",
["Overview", "Comparison", "Safety", "Highest urgency", "Handoff", "Source status"],
],
[
"/dsm/diagnoses/major-depressive-disorder",
["Criteria", "Key features", "Specifiers", "Documentation", "Record summary"],
Expand DownExpand Up@@ -65,15 +61,32 @@ describe("PageSecondaryNavigation", () => {
});

it("uses stable semantic fragments for breakpoint-specific section targets", () => {
// A fragmentId is the stable href when a section's target differs across
// breakpoints; SecondaryNavigation still scrolls via the targetId, so the
// fragment never needs to exist as an element.
const formDecisionContext = informationPageSectionDefinitions("/forms/form-1").find(
(section) => section.id === "decision-context",
);
const differentialSafety = informationPageSectionDefinitions(
"/differentials/presentations/acute-confusion-encephalopathy",
).find((section) => section.id === "safety");
const formSourceVerification = informationPageSectionDefinitions("/forms/form-1").find(
(section) => section.id === "verification",
);

expect(formDecisionContext?.fragmentId).toBe("form-decision-context");
expect(differentialSafety?.fragmentId).toBe("differential-presentation-safety");
expect(formSourceVerification?.fragmentId).toBe("form-source-verification");
});

it("claims no section set for the differentials presentation workflow", () => {
// It declared six sections whose targetIds no component rendered, so
// AvailableInformationPageNavigation filtered them all out and returned
// null — the route claimed and nothing drawn (/issues #256). The set is
// deleted rather than wired: the page owns navigation at every width
// already (MobileTabs below xl, the "Differential review sidebar" aside at
// xl showing every panel at once), and three of the six declared a -mobile
// variant that no render could ever satisfy.
expect(informationPageSectionDefinitions("/differentials/presentations/acute-confusion-encephalopathy")).toEqual(
[],
);
expect(hasLocalInformationPageNavigation("/differentials/presentations/acute-confusion-encephalopathy")).toBe(true);
});

it("binds service section targets to IDs rendered by service-detail-page", () => {
Expand All@@ -85,6 +98,18 @@ describe("PageSecondaryNavigation", () => {
}
});

it("binds form section targets to IDs rendered by form-detail-page", () => {
// The gap /issues #256 was about: this page declared six sections and
// rendered zero `id=` attributes, so its nav filtered to nothing and drew
// nothing at all. Every literal below is at a call site in this file, so a
// vacuous pass is not possible — a target moved behind a variable would
// fail here and needs a rendered-DOM assertion instead.
const formPage = readFileSync(join(process.cwd(), "src/components/forms/form-detail-page.tsx"), "utf8");
for (const targetId of informationPageSectionDefinitions("/forms/form-1").flatMap((section) => section.targetIds)) {
expect(formPage).toContain(`id="${targetId}"`);
}
});

it("binds specifier section targets to IDs rendered by specifier record pages", () => {
const recordPage = readFileSync(join(process.cwd(), "src/components/specifiers/specifier-record-page.tsx"), "utf8");
const referencePage = readFileSync(
Expand DownExpand Up@@ -165,13 +190,10 @@ describe("PageSecondaryNavigation", () => {
// /documents/<id> record.
//
// The anchors are planted here rather than taken from the real page on
// purpose: this asserts branch ORDER, not that /forms/form-1 currently
// draws a nav. It does not — form-detail-page.tsx carries
// "form-decision-context-mobile" as a `testId`, not an element id, and the
// other five declared targetIds are rendered nowhere, so
// AvailableInformationPageNavigation filters everything out and returns
// null. That is /issues #256 (declared section sets whose targets nothing
// renders), live for forms, pre-existing and out of scope here.
// purpose: this asserts branch ORDER only. That form-detail-page.tsx now
// renders these ids for real is the separate binding guard above — jsdom
// applies no Tailwind, so `lg:hidden` produces no `display:none` here and
// this could not tell a -mobile target from its -desktop twin anyway.
render(
<div>
<PageSecondaryNavigation modeId="forms" pathname="/forms/form-1" hasSubmittedSearch />
Expand Down
Loading
Loading