Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line numberDiff line numberDiff line change
Expand Up@@ -700,3 +700,4 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie
| 2026-08-07 | cursor/viewer-phase0-gesture-a11y-1db8 (PR #1660) | 097dfd245f798f8105eeb6c1cf4fc077f969496f | prlanded | MERGED; squash tip empty vs branch tip 810cfc9b4a1c476a0dcc995bffb00d7329a85686; Phase 0 gesture INP, preview a11y, image decode | content tree empty vs squash; no provider-backed checks run |
| 2026-08-07 | claude/pr-handoff-loop-prevention-54y5zr (PR #1670) | dfe2946110e0ff93bd4acc571ae79c26b79a7a85 | PR #1670 heavy review-and-fix | synced origin/main (behind-but-clean DIRTY cleared); fixed CodeRabbit checks-cell to name exact #1649 gates + incomplete verify:pr-local/ui + no provider checks; Bugbot none; no P0/P1; #258/#ledger delta accurate; merge-tree clean; threads cleared | verify:cheap 519 files/5493 passed; verify:pr-local docs scope (format+docs+ledger+outstanding-issues); check:branch-review-ledger; check:outstanding-issues; no provider gates |
| 2026-08-07 | claude/handover-review-nlhuln | 978623337c12dc1721fe5236eadbf9a5ad929f03 | mode nav remaining modes: factsheets adoption (PR #1674) | Adopted the shared ModeNav for factsheets (Topics + Search); replaced the action-only entry, added the activeId branch, q/category/run carry, BookOpenText icon; three pinned adopted-mode lists updated together; record-route protection pinned at render now the item-count protection has expired | lint clean; typecheck clean; test 518/519 files (pr-handoff-stop failure confirmed pre-existing via stashed re-run); focused 5 files 95 tests; ui-mode-nav-density 55 passed incl 7 new factsheets rows; two mutation checks confirmed red; format committed; verify:pr-local blocked at check:installed-lock-parity (playwright 1.62.0 vs 1.62.1) |
| 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) |
Comment thread
BigSimmo marked this conversation as resolved.
1 change: 0 additions & 1 deletion src/components/clinical-dashboard/global-search-shell.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -1011,7 +1011,6 @@ function GlobalStandaloneSearchShellBody({
pathname={pathname}
hasSubmittedSearch={hasSubmittedModeSearch}
searchParamString={searchParamString}
onSearch={() => inputRef.current?.focus({ preventScroll: true })}
sticky={false}
/>
) : null}
Expand Down
69 changes: 14 additions & 55 deletions src/components/page-secondary-navigation.tsx
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,15 @@
"use client";

import { useEffect, useMemo, useState } from "react";
import { useEffect, useState } from "react";

import { isDocumentViewerOwnedRoute } from "@/components/clinical-dashboard/mobile-composer-reserve";
import { RegistryModeNav } from "@/components/mode-nav/registry-mode-nav";
import {
SecondaryNavigation,
type SecondaryNavigationItem,
type SecondaryNavigationSectionItem,
} from "@/components/secondary-navigation";
import { appModeDefinition, type AppModeId } from "@/lib/app-modes";
import { SecondaryNavigation, type SecondaryNavigationSectionItem } from "@/components/secondary-navigation";
import { type AppModeId } from "@/lib/app-modes";
import { isInformationPage } from "@/lib/information-pages";
import {
activeModeSecondaryNavigationId,
isModeSecondaryNavigationRoute,
modeSecondaryNavigationEntries,
modeSecondaryNavigationHref,
modeUsesHeaderModeNav,
} from "@/lib/mode-secondary-navigation";

Expand DownExpand Up@@ -250,66 +244,42 @@ export function PageSecondaryNavigation({
modeId,
pathname,
hasSubmittedSearch,
onSearch,
/**
* Bridged query string from GlobalStandaloneSearchShellBody. Must not call
* useSearchParams here — that reintroduces a nested Suspense boundary under
* the standalone shell body (search-chrome invariant 17).
*/
searchParamString = "",
sticky = true,
stickyTop,
}: {
modeId: AppModeId;
pathname: string;
hasSubmittedSearch: boolean;
onSearch: () => void;
searchParamString?: string;
sticky?: boolean;
stickyTop?: number | string;
}) {
const informationDefinitions = informationPageSectionDefinitions(pathname);
const locallyOwnedInformationNavigation = hasLocalInformationPageNavigation(pathname);
const activeId = activeModeSecondaryNavigationId(modeId, pathname);
const modeLabel = appModeDefinition(modeId).label;
const modeAriaLabel = modeLabel.toLowerCase().endsWith("mode") ? modeLabel : `${modeLabel} mode`;
const modeItems = useMemo<SecondaryNavigationItem[]>(
() =>
modeSecondaryNavigationEntries(modeId).map((entry) =>
entry.href
? {
kind: "route" as const,
id: entry.id,
label: entry.label,
shortLabel: entry.shortLabel,
href: modeSecondaryNavigationHref({
modeId,
itemId: entry.id,
href: entry.href,
currentSearchParams: new URLSearchParams(searchParamString),
}),
current: entry.id === activeId,
}
: {
kind: "action" as const,
id: entry.id,
label: entry.label,
shortLabel: entry.shortLabel,
onSelect: onSearch,
current: entry.id === activeId,
},
),
[activeId, modeId, onSearch, searchParamString],
);

// Therapy Compass owns both its workflow bindings and its dynamic detail
// sections inside TcProvider; rendering the shell registry as well would
// duplicate the bar and discard its URL/state-aware action bindings.
if (pathname === "/therapy-compass" || pathname.startsWith("/therapy-compass/")) return null;
if (locallyOwnedInformationNavigation) return null;
// ORDER IS LOAD-BEARING: this must stay above the mode branch. `services`,
// `forms`, `documents` and `prescribing` register no destinations at all yet
// still own real "On this page" section navs. Hoisting the mode guard below
// up to here would silently delete navigation from every `/services/*`,
// `/forms/*`, `/medications/*` and `/documents/<id>` record.
if (informationDefinitions.length) {
return <AvailableInformationPageNavigation definitions={informationDefinitions} sticky={sticky} />;
}
// A mode with no registered destinations gets no bar and no landmark. The
// seven that used to register a lone `action` entry each rendered one
// <button> that focused a composer already on screen; it was deleted rather
// than ported, so there is nothing left to draw for them.
if (!modeUsesHeaderModeNav(modeId)) return null;
if (!isModeSecondaryNavigationRoute({ modeId, pathname, hasSubmittedSearch })) return null;
// An adopted mode's bar portals into the header's single addon slot, which
// holds ONE page-owned header. What keeps it to one is the
Expand All@@ -320,16 +290,5 @@ export function PageSecondaryNavigation({
// `isHeaderAddonSlotOwnedRoute` names the claimants and
// `tests/mode-nav-addon-slot.dom.test.tsx` fails if a future one falls
// outside that cover, which is when this needs its own guard.
if (modeUsesHeaderModeNav(modeId)) {
return <RegistryModeNav modeId={modeId} activeId={activeId} searchParamString={searchParamString} />;
}
return (
<SecondaryNavigation
ariaLabel={modeAriaLabel}
items={modeItems}
activeId={activeId ?? undefined}
sticky={sticky}
stickyTop={stickyTop}
/>
);
return <RegistryModeNav modeId={modeId} activeId={activeId} searchParamString={searchParamString} />;
}
14 changes: 14 additions & 0 deletions src/components/secondary-navigation.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -56,6 +56,20 @@ export type SecondaryNavigationSectionItem = SecondaryNavigationBaseItem & {
fragmentId?: string;
};

/**
* 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;
Expand Down
39 changes: 26 additions & 13 deletions src/lib/mode-secondary-navigation.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -21,11 +21,18 @@ export type ModeSecondaryNavigationEntry = {
* secondary bar must never repeat a generic Home destination.
*/
export const modeSecondaryNavigationRegistry = {
answer: [{ id: "ask", label: "Ask", action: "search" }],
documents: [{ id: "search", label: "Search", action: "search" }],
services: [{ id: "search", label: "Search", action: "search" }],
forms: [{ id: "search", label: "Search", action: "search" }],
favourites: [{ id: "search", label: "Search", action: "search" }],
// Empty is a real answer, not a gap. These seven modes each registered one
// `action: "search"` entry, which rendered a lone <button> inside its own
// <nav> landmark whose only effect was focusing a composer already visible on
// the same screen — a landmark and a tab stop spent on a no-op. Every one of
// them is genuinely single-surface (records, or one page), so there was no
// destination to adopt onto the shared bar and nothing to replace the button
// with. Deleted rather than ported.
answer: [],
documents: [],
services: [],
forms: [],
favourites: [],
differentials: [
{ id: "search", label: "Search", href: appModeHomeHref("differentials", { focus: true }) },
{ id: "diagnoses", label: "Diagnoses", href: "/differentials/diagnoses" },
Expand All@@ -47,8 +54,8 @@ export const modeSecondaryNavigationRegistry = {
{ id: "compare", label: "Compare", href: "/formulation/compare" },
{ id: "map", label: "Map", href: "/formulation/map" },
],
prescribing: [{ id: "search", label: "Search", action: "search" }],
tools: [{ id: "search", label: "Search", action: "search" }],
prescribing: [],
tools: [],
// Inert: `PageSecondaryNavigation` early-returns on `/therapy-compass*`, and
// the mode's live destination list is `useTherapyNavItems` in
// `src/components/therapy-compass/nav.tsx`, which feeds the shared `ModeNav`.
Expand DownExpand Up@@ -157,11 +164,17 @@ export function activeModeSecondaryNavigationId(modeId: AppModeId, pathname: str
if (pathname === "/factsheets/search" || pathname.startsWith("/factsheets/search?")) return "search";
if (pathname === "/factsheets" || pathname.startsWith("/factsheets?")) return "topics";
// `/factsheets/<slug>` is a record. It cannot reach `ModeNav` today —
// `hasLocalInformationPageNavigation` returns null for it first — but the
// array-index-0 fallback below would mark Topics current if it ever did.
// `hasLocalInformationPageNavigation` returns null for it first — but
// without this branch it would inherit the mode's first entry.
return null;
}
return modeSecondaryNavigationRegistry[modeId][0]?.id ?? null;
// Every mode with destinations has a branch above; the rest register none, so
// nothing can be current. This used to be
// `modeSecondaryNavigationRegistry[modeId][0]?.id ?? null`, which existed only
// to keep a lone action button lit. With real multi-tab modes it would mark
// the first slot current on every unmatched path — the exact bug this
// function's doc comment warns callers about.
return null;
}

export function isModeSecondaryNavigationRoute(params: {
Expand All@@ -170,11 +183,11 @@ export function isModeSecondaryNavigationRoute(params: {
hasSubmittedSearch: boolean;
}): boolean {
const { modeId, pathname, hasSubmittedSearch } = params;
// Load-bearing for all five adopted modes: it is the only thing that puts the
// bar on a submitted-search mode home, e.g. `/differentials?q=…&run=1`, whose
// clause below lists only the workflow routes. Not leftover gating.
if (hasSubmittedSearch) return true;

// /documents/search is the documents mode home (composer already visible); do
// not add a lone Search focus control until a query has been submitted.
if (modeId === "documents") return false;
if (modeId === "differentials") {
return pathname === "/differentials/diagnoses" || pathname === "/differentials/presentations";
}
Expand Down
25 changes: 15 additions & 10 deletions tests/mode-nav-addon-slot.dom.test.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,7 +7,11 @@ import { afterEach, describe, expect, it, vi } from "vitest";
import { isHeaderAddonSlotOwnedRoute } from "@/components/mode-nav/header-addon-slot";
import { hasLocalInformationPageNavigation, PageSecondaryNavigation } from "@/components/page-secondary-navigation";
import { phoneHeaderCollapseAddonSlotId } from "@/lib/mode-home-composer";
import { MODE_NAV_ADOPTED_MODES, modeUsesHeaderModeNav } from "@/lib/mode-secondary-navigation";
import {
MODE_NAV_ADOPTED_MODES,
modeSecondaryNavigationEntries,
modeUsesHeaderModeNav,
} from "@/lib/mode-secondary-navigation";

vi.mock("next/navigation", () => ({
usePathname: () => "/",
Expand DownExpand Up@@ -76,7 +80,6 @@ describe("header addon slot ownership", () => {
modeId="differentials"
pathname="/differentials/diagnoses/delirium"
hasSubmittedSearch
onSearch={vi.fn()}
/>,
);
await waitFor(() => expect(view.occupants()).toHaveLength(0));
Expand All@@ -88,7 +91,7 @@ describe("header addon slot ownership", () => {
expect(isHeaderAddonSlotOwnedRoute("/dsm/compare")).toBe(false);

const view = renderIntoHeaderWithAddonSlot(
<PageSecondaryNavigation modeId="dsm" pathname="/dsm/compare" hasSubmittedSearch={false} onSearch={vi.fn()} />,
<PageSecondaryNavigation modeId="dsm" pathname="/dsm/compare" hasSubmittedSearch={false} />,
);
await waitFor(() => expect(view.occupants()).toHaveLength(1));
view.cleanupSlot();
Expand DownExpand Up@@ -123,17 +126,19 @@ describe("header addon slot ownership", () => {
]);
});

it("keeps single-destination modes off the bar entirely", () => {
// `documents` owns the slot on every detail route and has one registered
// destination. Adopting it would be a deletion decision about that lone
// entry, not a port — deliberately out of this rollout's scope.
it("keeps the modes that register no destinations off the bar entirely", () => {
// These four each carried a single `action` entry that focused a composer
// already on screen. It was deleted rather than adopted: a one-button <nav>
// is a landmark and a tab stop spent on a no-op, and ModeNav renders
// nothing below two items, so adopting would have removed the control and
// put nothing back (PR #1645). They now have neither the header bar nor the
// in-flow strip.
//
// `factsheets` left this list when it gained a real second destination:
// `/factsheets` (browse) and `/factsheets/search` are separate components,
// so it is a port rather than a deletion. The others still have one surface
// each, and ModeNav renders nothing below two items — adopting them would
// remove the control they have and put nothing back (PR #1645).
// so it was a port rather than a deletion.
for (const modeId of ["documents", "answer", "prescribing", "tools"] as const) {
expect(modeSecondaryNavigationEntries(modeId)).toEqual([]);
expect([...MODE_NAV_ADOPTED_MODES]).not.toContain(modeId);
expect(modeUsesHeaderModeNav(modeId)).toBe(false);
}
Expand Down
Loading
Loading