feat(nav): pin a shared ModeNav inside the universal header, wired to Therapy search - #1390
Conversation
The three design-direction mockups were committed by housekeeping without their sitemap entries or a formatting pass, so `sitemap:check` and `format:check` both fail in CI against them. - add the three `/mockups/therapy-navigation-*` routes to docs/site-map.md via `npm run sitemap:update` - give the decorative lucide `<X>` in the compare tray an `aria-hidden`, which `local/require-lucide-icon-aria` requires even inside design scratch - run Prettier over the four mockup sources Design scratch only: these routes 404 in production and are exempt from the button-wiring and route-reachability gates. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Introduces a reusable mode navigation bar and wires it to Therapy's search
route as a first beachhead. Replaces, for that route only, a strip of seven
pill buttons that flattened three kinds of destination into one visual rank,
carried no state, and needed ~700px in a 390px viewport — leaving four
destinations off-screen with no scroll affordance.
Anchoring, without touching the header
--------------------------------------
`ModeNavHeaderPortal` claims the existing `#phone-header-collapse-addon-slot`
at every width. That slot was never breakpoint-gated; only
`PhoneHeaderCollapsePortal` was, via its own `(max-width: 639px)` query, which
is why the old strip never travelled with the header above phones. Occupying
the slot inherits the header's `grid-template-rows: 1fr -> 0fr` collapse, so
the bar hides and reveals with it on phone, tablet and desktop with no second
scroll listener that could drift out of step. No header or footer file is
modified.
Density
-------
Slot capacity steps with CONTAINER width in `rem`, never px and never runtime
measurement: 3 slots from 16rem, 4 from 26rem, 5 from 34rem, and a single
current-page control below that. The unit is the mechanism — raising the OS or
browser text size grows the root font, so a 320px phone at 200% text resolves
to 10rem and collapses exactly when its labels would stop fitting (WCAG 1.4.4,
1.4.10). Which destinations survive each band comes from the item count alone
(`planModeNavBands`), so the bar's contents can never change under the user
mid-gesture. Overflow only ever fills from the tail, so slots that stay never
move; if the active page has folded into More, that slot takes its label, icon
and rule.
Therapy
-------
Four destinations: Search, Compare (basket fill `n/4`), Recommend, Pathways.
Home duplicated the mode pill directly above it. Brief intervention and Patient
sheet act on a selected therapy and already exist as availability-guarded
buttons on the record page; in the strip they were duplicated *and* unsafe,
silently opening the first catalogue record that had one when nothing was
selected. Every non-search Therapy route keeps the original strip, so this is
reversible by one line of workspace.tsx.
Tests
-----
Five specs asserted the old strip on the search route. `ui-therapy-nav-scroll`
and the `ui-phone-scroll` surface entry move to `/therapy-compass/compare`,
where that strip still ships, preserving their coverage; `ui-route-coverage`
swaps a centred-nav assertion for a no-inline-overflow one; `ui-accessibility`
moves from `getByRole("button")` to `getByRole("link")`, since the bar now
routes with real hrefs. `ui-chrome-scroll` gains the Therapy search surface,
giving tablet and desktop proof for this mode that did not exist before.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>…design-350a0e # Conflicts: # src/app/mockups/mockups-layout-client.tsx # tests/ui-route-coverage.spec.ts
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:35 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:dad09bec42
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
BigSimmo
commented
Jul 29, 2026
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
`ModeNav` has two triggers for one Sheet, and CSS decides which one is displayed: below a 16rem container only the collapsed control renders, at 16rem and up only the More slot does (`@container mode-nav` in globals.css). Both stay in the DOM either way. `returnFocusRef` was pinned to the More slot, so opening the sheet from the collapsed control handed the Sheet a `display: none` element to restore focus to. The browser declines to focus it, the 50ms retry declines again, and the keyboard or screen-reader user is dropped on <body> — losing their place in the very state this bar was designed around, since a 320px phone collapses to the single control at 200% text zoom (the WCAG 1.4.4 / 1.4.10 path the CSS comment describes). Capture the clicked button instead: `event.currentTarget` is by definition the displayed one, so the restore target is always focusable. This also removes the two dedicated refs. Test pins both bands. It deliberately does not pre-focus the opener — jsdom leaves focus on <body> through a synthetic click, so the only thing that can move it is the explicit restore, which makes the wait a real wait rather than a poll satisfied by the pre-existing focus. jsdom will happily focus a hidden element and cannot model the browser's focusability rule, so the assertion pins which opener the Sheet was told to return to. Verified red against the previous implementation: × returns focus to the collapsed control below the bar band AssertionError: expected <button …>…(1)</button> to be <button …>…(4)</button>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FvU8z73P6TXUXoYBqN5K1P
BigSimmo
commented
Jul 29, 2026
Review — one defect found and fixed, in |
Uh oh!
There was an error while loading. Please reload this page.
Summary
ModeNav— an underline bar of icon + label slots that pins inside the universal header's collapse row, so it hides and reveals with the header at phone, tablet and desktop. Wired to Therapy only, on/therapy-compass/searchonly, as a first beachhead.#phone-header-collapse-addon-slot(master-search-header.tsx:2153) is already rendered ungated insidedata-testid="universal-header-collapse"; the phone-only limit lives inPhoneHeaderCollapsePortal's ownmatchMedia("(max-width: 639px)"), not in the header.ModeNavHeaderPortalis a sibling portal with that gate dropped, so it claims the same slot at every width and inherits the header'sgrid-template-rows: 1fr → 0frcollapse for free — one scroll source, no second listener, no possibility of desync.rem, never px, never measured at runtime: ≥34rem → 5 slots · 26–34rem → 4 · 16–26rem → 3 (last becomesMore) · <16rem → current page + chevron.remis the mechanism — raising the OS/browser text size grows the root font, so a phone collapses exactly when its labels would stop fitting rather than clipping them (WCAG 1.4.4 Resize Text, 1.4.10 Reflow at 320px). Labels are never abbreviated: a slot shows its real word or moves intoMore.Search · Compare (n/4) · Recommend · Pathways. Order is load-bearing — at three slots the survivors are the library door and the only stateful destination.workspace.tsxchanges one line.<button onClick>→<Link href>, so middle-click, deep links, back and prefetch start working on these destinations.Files
New:
src/components/mode-nav/{mode-nav,mode-nav-portal,mode-nav-bands}.{tsx,ts},tests/mode-nav-contract.test.ts.Modified:
globals.css(container + density bands +--shadow-lift),therapy-compass/nav.tsx(addsTherapyModeNavalongside the untouchedTherapyCompassNav),therapy-compass/workspace.tsx(one line), and five Playwright specs that pinned the old strip on the search route.Fold planning lives in
mode-nav-bands.tsas a pure module specifically so it is unit-tested as real logic rather than asserted by grepping source text.Verification
Run against the merged tree (this branch merged
origin/mainat74229001a, which itself reworked phone header reveal — every earlier green run was re-run from scratch after that merge).npm run verify:cheap—Test Files 427 passed (427)/Tests 4395 passed | 3 skipped (4398), exit 0npx tsc --noEmit— cleanmode-nav-contract,therapy-compass-responsive-contract,header-scroll-hide-contract,therapy-compass-mode-wiring,ui-overlay-css-contract,verify-phone-chrome) — 6 passed, 76 tests passednpx prettier --checkon all changed files — cleanrun-playwright.mjs --project=chromium --grep-invert "@quarantine|@mockup" -g "Therapy|mode nav"—11 passed (1.1m), exit 0. Decisive lines:ok 2 tests\ui-chrome-scroll.spec.ts:208 › tablet: top bar hides on scroll down and returns mid-page on shell mode nav (4.8s)ok 4 tests\ui-chrome-scroll.spec.ts:208 › desktop: top bar hides on scroll down and returns mid-page on shell mode nav (5.7s)ok 9 tests\ui-phone-scroll.spec.ts:1561 › phone Therapy mode navigation uses the universal collapse owner (3.8s)ok 11 tests\ui-therapy-nav-scroll.spec.ts:61 › phone Therapy section nav hides and returns with the universal header (3.6s)— the old strip, still green on/comparenpm run verify:pr-local— Verification not run: the repo run coordinator holds full-Vitest/build leases exclusively across ~40 worktrees;verify:cheapcovers its unit surface and the conditional build/RAG-fixture legs are untouched by this diff (nosrc/lib/rag/**, no manifest, no build config).npm run verify:release— not a release handoff.eval:retrieval:quality/eval:rag/check:production-readiness— not applicable, and provider-backed. No retrieval, ranking, selection, chunking, scoring, answer-generation, source-rendering, ingestion, privacy or production-env surface is touched.New coverage this adds
ui-chrome-scroll.spec.tsgains/therapy-compass/search?q=CBT&run=1— tablet 834×1112 and desktop 1440×900 proof for Therapy specifically, which did not exist before this PR.ui-phone-scroll.spec.tsgains themode-navsurface.mode-nav-contract.test.ts— 14 tests: band planning, the rem-not-px density contract, header anchoring, item contract.Spec updates (all five visit the route gaining the bar)
ui-therapy-nav-scroll.spec.ts/search/compare, where the old strip still shipsui-phone-scroll.spec.ts/search/compare; newmode-navsurface addedui-route-coverage.spec.tsui-accessibility.spec.tsgetByRole("button", { name: "Search" })getByRole("link", …)scoped to the nav — genuine role change from the<button>→<Link>fixui-chrome-scroll.spec.tstherapy-compass-responsive-contract.test.tsis deliberately unchanged: the old strip and its CSS both survive.Risk and rollout
use-hide-on-scrollare untouched — the mechanism is a portal claiming an already-ungated slot. The one accepted cost is that during the beachhead Therapy has two nav styles: navigating search → compare moves the nav from inside the header to the sticky strip below it. That is the direct consequence of "search page only" and disappears as the rollout continues.workspace.tsx({isHome ? null : <TherapyCompassNav />}) and the old strip returns everywhere, with the new component left inert. Full revert is the branch revert.Notes
ModeNavis built as a shared component, not a Therapy one — Therapy just supplies the first item list. Extending it to another mode is an item array plus one gate line.Morereuses the existingSheet(src/components/ui/sheet.tsx) withportal, so it mounts ondocument.bodyand is never inside the collapsing grid; focus trap, Escape, backdrop dismiss, swipe-to-dismiss and focus restore all come with it.min-h-12(48px), clear ofexpectMinTouchTarget's 44px default and the sub-pixel rounding flake thatmin-h-11caused.use-hide-on-scroll.ts:222-229and:470-474carry stale comments claiming wide chrome "sticks and translates"; that path was deleted. Header-adjacent, so not touched here.🤖 Generated with Claude Code