feat(search-band): give every mode the documents phone filter - #1706
Conversation
Six search modes plus the tools launcher shipped a `w-full` native `<select>` as their phone filter — formulation and specifiers passed two, in a two-column grid. That control cost the results band a whole second row, could not report how many filters were active, and, because the iOS anti-zoom rule in globals.css pins every native select to 16px below `sm`, rendered its value at the same size as the query heading above it. All seven now pass the badged trigger documents already used, so the one-line phone band is universal rather than a documents/therapy-compass exception. Verified in a real browser: the differentials band is geometrically identical to the documents band at 390px (89px), 414px and 430px (60px). The idiom is now shared rather than copied. `result-filter-control.tsx` holds `ResultFilterTrigger`, lifted verbatim from `DocumentFilterTrigger` so the control is the same component everywhere, and `ResultFilterSheet`, which renders one `role="radiogroup"` per dimension — real radio semantics, because these dimensions are one-of-N and a bank of `aria-pressed` toggles asserts they are not. Documents keeps its own panel: multi-select facet groups with counts, a find-a-filter field and collapse-by-default are not expressible as radios. Desktop is untouched. The ribbon renders `filterControls` from `sm` up and `mobileControls` below it, never both, so every mode keeps the chip row or tab strip it already had on a wide screen. `MobileResultFilterControl` is deleted; no caller remains. `mobileControlsPlacement` still defaults to `row` — nothing relies on that fallback now, and it stays so a mode that forgets the prop degrades to a second row rather than to an unreadable line. Closes outstanding-issues #247. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013mcqh46SDdiR5KxScihkFQ
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
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: 15 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 (6)
📝 WalkthroughWalkthroughThe PR replaces native phone result-filter controls across seven modes with shared ChangesMobile result-filter migration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ModePage
participant ResultFilterTrigger
participant ResultFilterSheet
participant FilterState
ModePage->>ResultFilterTrigger: Render active-filter count and panel state
ResultFilterTrigger->>ResultFilterSheet: Open or close filter panel
ResultFilterSheet->>FilterState: Apply, clear, or reset filter
FilterState->>ModePage: Update results and visible count
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013mcqh46SDdiR5KxScihkFQ
There was a problem hiding this comment.
Pull request overview
This PR standardizes the mobile (“phone”) filter UI across multiple search modes by replacing full-width native <select> controls with a shared badged trigger + sheet pattern, aligning those modes with the existing Documents-style one-line results band.
Changes:
- Introduces shared
ResultFilterTriggerandResultFilterSheetcomponents for mobile single-choice filtering. - Updates multiple mode pages (differentials, prescribing, services, factsheets, formulation, specifiers, tools launcher) to use the shared trigger + sheet and opt into
mobileControlsPlacement="inline". - Updates Playwright and DOM/unit tests plus docs to reflect the new mobile filter idiom and remove
MobileResultFilterControl.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/ui-tools.spec.ts | Updates launcher/services/differentials mobile filter journeys to use trigger + radio-sheet interactions. |
| tests/ui-stress.spec.ts | Updates mobile medication filter selector test id to the new trigger. |
| tests/ui-specifiers.spec.ts | Updates specifiers phone filter assertions to use trigger + multi-group radio sheet and badge counts. |
| tests/ui-smoke.spec.ts | Updates factsheets phone filter smoke path to use trigger + sheet and asserts hidden state above sm. |
| tests/ui-formulation.spec.ts | Updates formulation phone filter path to open the sheet and validate default radio selection. |
| tests/ui-accessibility.spec.ts | Updates accessibility test to validate phone filter trigger focus/Enter and radio semantics in the sheet. |
| tests/search-results-header-band.dom.test.tsx | Replaces MobileResultFilterControl usage with ResultFilterTrigger and adds focused unit coverage for trigger + sheet behavior. |
| src/components/specifiers/specifiers-home-page.tsx | Replaces two phone selects with one trigger opening a two-group ResultFilterSheet (family + diagnosis). |
| src/components/services/services-navigator-page.tsx | Replaces phone quick-filter select with trigger + sheet; closes sheet on query-rewriting selections. |
| src/components/formulation/formulation-home-page.tsx | Replaces two phone selects with trigger + sheet (pattern preset + domain), keeping domain as the counted “narrowing” filter. |
| src/components/factsheets/factsheets-search-page.tsx | Replaces phone category select with trigger + sheet; closes sheet on navigation. |
| src/components/clinical-dashboard/search-results-header-band.tsx | Removes MobileResultFilterControl and updates docs/comments around mobileControlsPlacement. |
| src/components/clinical-dashboard/result-filter-control.tsx | Adds new shared mobile filter trigger and single-choice filter sheet (radio-group per dimension). |
| src/components/clinical-dashboard/medication-prescribing-workspace.tsx | Replaces phone medication filter select with trigger + sheet and adds count hints. |
| src/components/clinical-dashboard/differentials-home.tsx | Replaces phone differentials filter select with trigger + sheet, preserving desktop chip controls. |
| src/components/applications-launcher-page.tsx | Replaces phone tool category select with trigger + sheet for the tools launcher. |
| docs/search-results-bar-decisions.md | Updates the design decision record to document the new universal trigger + sheet approach. |
| docs/search-chrome-behaviour.md | Updates the search chrome contract to codify “trigger + sheet, never a select” for phone filters. |
| docs/outstanding-issues.md | Marks #247 as resolved and updates related ledger notes/next-id. |
| docs/design-system/COMPONENTS.md | Updates generated component inventory counts (Sheet usage). |
| docs/design-system/adoption-manifest.json | Adds the new result-filter-control.tsx to the design-system adoption manifest. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
… tabIndex keyboard navigation - Fix contradictory JSDoc: Sheet unmounts when open=false; remove "stays mounted while closed" which contradicted the true behaviour. - Extract FilterRadioGroup sub-component with useRef + roving tabIndex (single tab stop on selected option) and onKeyDown handler for ArrowLeft/Right/Up/Down + Home/End keyboard navigation, matching the SegmentedControl pattern. Dead-end options retain tabIndex=-1 so arrow keys skip them while Tab still reaches them for explanation. Co-authored-by: BigSimmo <87357024+BigSimmo@users.noreply.github.com>
Head branch was pushed to by a user without write access
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/branch-review-ledger.md`:
- Line 714: Update the 2026-08-07 entry in the branch-review ledger to include
the decisive success-summary output for the typecheck, lint, and build gates,
rather than listing only their command names. Preserve the existing verification
results and format the added summaries consistently with the other recorded gate
evidence.
In `@docs/outstanding-issues.md`:
- Line 161: Update the execution-queue entry for item `#253` to remove the
obsolete hand-merge `#1606` instruction. Replace it with the remaining human
supersession/closure decision for the deleted MobileResultFilterControl and
require keyboard-parity verification.
In `@docs/search-chrome-behaviour.md`:
- Around line 132-150: Update the shared migration count from six to seven in
docs/search-chrome-behaviour.md lines 132-150 and in
src/components/clinical-dashboard/search-results-header-band.tsx lines 175-188,
including the converted-select count in the header-band implementation. No other
changes are needed.
In `@src/components/clinical-dashboard/result-filter-control.tsx`:
- Line 134: Replace the arbitrary padding, viewport breakpoint, height, and
minimum-width values in the result filter control with named Tailwind 4 design
tokens. Define the corresponding tokens in globals.css using the project’s
established `@theme` conventions, then update the classes around the result filter
control and its responsive/icon elements to reference those tokens instead of
bracketed values or hardcoded breakpoints.
- Around line 253-255: Update the tabIndex expression in the option rendering to
assign tabIndex={0} to every dead-end option, while preserving the existing
roving tab stop for selectable options based on option.value === tabStopValue
and keeping other selectable options at -1.
In `@src/components/services/services-navigator-page.tsx`:
- Around line 740-745: Update the activeQuickFilter onClearAll handler in the
services navigator page to clear both localQuery and the URL’s q parameter, so
reloads do not restore the filter. Add or reuse a shared clear-handler path that
updates the route, and use that same handler for the desktop Clear action.
In `@tests/search-results-header-band.dom.test.tsx`:
- Around line 593-627: Update
tests/search-results-header-band.dom.test.tsx:593-627 to assert the selected
disabled placeholder remains checked and focusable, then verify focus moves to
Crisis and checked states update after keyboard interaction. Add Chromium
coverage in tests/ui-accessibility.spec.ts:483-499 for Arrow keys, Home, End,
focus movement, and checked-state changes in ResultFilterSheet. Replace the
stale native-radio statement in docs/outstanding-issues.md:295 with the current
bespoke roving-tabIndex keyboard contract, then run the focused Chromium gate
via npm run ensure and its printed URL.
In `@tests/ui-tools.spec.ts`:
- Around line 417-425: Update the filter interaction flow around categoryTrigger
so it does not click the underlying trigger while the sheet is open: select “All
tools” from the currently open Category radiogroup, then assert the trigger’s
accessible name is “No filters active.” Remove the premature
categoryTrigger.click() used to reopen or close the sheet.
🪄 Autofix
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 886a3f33-c1e1-448a-926e-4982b513385a
📒 Files selected for processing (22)
docs/branch-review-ledger.mddocs/design-system/COMPONENTS.mddocs/design-system/adoption-manifest.jsondocs/outstanding-issues.mddocs/search-chrome-behaviour.mddocs/search-results-bar-decisions.mdsrc/components/applications-launcher-page.tsxsrc/components/clinical-dashboard/differentials-home.tsxsrc/components/clinical-dashboard/medication-prescribing-workspace.tsxsrc/components/clinical-dashboard/result-filter-control.tsxsrc/components/clinical-dashboard/search-results-header-band.tsxsrc/components/factsheets/factsheets-search-page.tsxsrc/components/formulation/formulation-home-page.tsxsrc/components/services/services-navigator-page.tsxsrc/components/specifiers/specifiers-home-page.tsxtests/search-results-header-band.dom.test.tsxtests/ui-accessibility.spec.tstests/ui-formulation.spec.tstests/ui-smoke.spec.tstests/ui-specifiers.spec.tstests/ui-stress.spec.tstests/ui-tools.spec.ts
|
The branch was updated while autofix was in progress. Please try again. |
…d-end reachability The roving-tabIndex implementation landed in 17d7ba6 with no test coverage, so the contract it added could regress silently. Adds DOM coverage that fails without it: one tab stop per group, Arrow/Home/End moving focus and selection, wrap staying inside a dimension, the checked placeholder as the tab stop, and the fallback when `value` matches no option. Verified by mutation — removing the roving tabIndex and the key handler fails exactly these three. Adds the same assertions as Chromium coverage in ui-accessibility, because jsdom cannot vouch for focus behaviour under the sheet's real focus trap. Dead ends now sit on the arrow path but are never selected by it. Previously they were excluded from both the arrow cycle and the tab order, so a keyboard-only reader could not reach the option or hear its "Not selectable from here" note — while the comment claimed they stayed Tab-reachable, which `tabIndex={-1}` denies. Moving focus without committing selection is the ARIA guidance for a disabled radio and is the only arrangement that keeps the single tab stop the role requires; giving dead ends `tabIndex={0}` would restore reachability by adding a tab stop per option to a control whose point is having one. No call site produces a dead end today, so this path is defensive and is asserted rather than assumed. Also corrects two records against the code: `#253` claimed the sheet inherits native radio semantics, when it is a bespoke roving handler over buttons, and its execution-queue row still asked for a `#1606` hand-merge that the deletion of MobileResultFilterControl made impossible. Disambiguates the six-versus-seven count — six band modes shipped a select, plus the tools launcher, which carries no band. Removes `activeResultFilterCount`, which no caller ever used. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013mcqh46SDdiR5KxScihkFQ
…buu0io' into claude/search-bar-mobile-layout-buu0io
|
Dispositions for the 8 CodeRabbit findings, all against Fixed
Declined
Not actioned: the ledger row asking for decisive gate output. Also worth stating plainly for anyone reading CI: the two Generated by Claude Code |
Summary
w-fullnative<select>as their phone filter — formulation and specifiers passed two, in a two-column grid. All seven now pass the same compact badged trigger documents already used, withmobileControlsPlacement="inline", so the one-line phone results band is universal rather than a documents/therapy-compass exception. Converted: differentials, services, factsheets, prescribing, the tools launcher, formulation, specifiers.src/components/clinical-dashboard/result-filter-control.tsxholdsResultFilterTrigger— lifted verbatim fromDocumentFilterTrigger, so the control is literally the same component everywhere — andResultFilterSheet, a single-choice sheet rendering onerole="radiogroup"per dimension. Real radio semantics, because these dimensions are genuinely one-of-N and a bank ofaria-pressedtoggles asserts they are not. Documents keeps its own panel: multi-select facet groups with counts, a find-a-filter field and collapse-by-default are not expressible as radios.filterControlsfromsmup andmobileControlsbelow it, never both, so every mode keeps the chip row or tab strip it already had on a wide screen. No desktop control changed.MobileResultFilterControlis deleted — no caller remains. Its cost was concrete: a whole second band row, no way to report how many filters were active without spending label width on it, and (because the iOS anti-zoom rule inglobals.csspins every native select to 16px belowsm) a value rendered at the same size as the query heading above it.mobileControlsPlacementstill defaults torow. Nothing relies on that fallback now that every caller passesinline; it stays so a new mode that forgets the prop degrades to a second row rather than to an unreadable 58px line at 320px. The#247"Stop: do not flip the default" is honoured.docs/search-chrome-behaviour.md(list renumbered 5–9), the phone-control section ofdocs/search-results-bar-decisions.md, and the design-system adoption manifest regenerated for the new component. Closes outstanding-issues#247;#253updated because the component open PR Ship soft Category filter (replace phone select blue highlight) #1606 rewrites no longer exists.Verification
npm run verify:pr-localRan to completion except for the last two steps, which were run directly afterwards because the gate aborts on a pre-existing failure (below). Decisive lines:
The single failure is
tests/pr-handoff-stop.test.ts > emits handoff context only when the marker file exists. It is pre-existing and not caused by this diff — baselined by stashing the whole working tree and re-running against the unmodified checkout, where it fails identically (1 failed | 10 passed). It is a session-hook marker artefact in this container, and the diff touches nothing under.claude/.The two steps after
npm run test, run directly:npm run check:bundle-budget:client chunks: 298 files, 1494.5 KiB gzipagainstbaseline 1406.4 KiB gzip; within tolerance(+6.3%, tolerance 10%).npm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changedUI verification not run as the full
verify:uisweep: the container's Playwright browser build (chromium-1194) does not match the lockfile pin (chromium_headless_shell-1234), which is the known gap tracked as#255. Every affected owner and journey was proved instead, against a real production build, withPLAYWRIGHT_CHROMIUM_EXECUTABLE_PATHpointed at the installed Chromium:Browser measurement of the change itself, at the phone widths that matter — the differentials band is now geometrically identical to the documents band:
npm run verify:releasebefore release or handoff confidence claims — not run; no release claim is made here.npm run eval:retrieval:quality,eval:rag,eval:quality: not applicable and not run. This diff touches no retrieval, ranking, selection, chunking, scoring, or answer-generation surface — it is presentation-layer filter chrome. See the RAG impact line below.npm run check:production-readiness: not run — no clinical workflow, privacy, environment, Supabase, source-governance or deployment behaviour changed.RAG impact: no retrieval behaviour change — this diff only replaces the phone filter control in the results ribbon (a native
<select>swapped for a trigger plus sheet) and touches no file undersrc/lib/rag/**, no clinical-search, retrieval-selection, ranking-config, result-sort or answer-ranking surface, no retrieval RPC, and neither the eval harness nor the golden fixture. The filter values each mode reads and writes are unchanged; only the widget that sets them differs.Risk and rollout
mobileControlsfromsmup. The realistic failure mode is a filter dimension whose values are wired wrongly at one call site, which is what the per-mode journeys above exercise. Two behaviours deliberately preserved from the selects they replace: services' and formulation's selected-but-disabled placeholder options (they name the state the reader is already in and remain unselectable), and factsheets'/services'/formulation's navigation-not-filtering semantics, where choosing an option runs a new search and closes the sheet rather than narrowing the current list.git revertthe feature commit. It is self-contained — the new component file is added, not extracted from a shared dependency, so reverting restoresMobileResultFilterControland all seven call sites together with no partial state.Notes
Two things worth a reviewer's attention, neither blocking:
The
max-[413px]:flex-wrapthreshold is now provably stale. The band's first line wraps below 414px, justified in its own comment as "one line provably cannot hold count + query + sort + filter even with the query fully truncated". Both halves of that premise are gone: Sort becamesm-and-up in fix(search-band): drop the sort segments from the phone results bar #1689, and the phone select is gone here. Measured in a real browser with the wrap class stripped at runtime,scrollWidth - clientWidthis 0 at 320, 360, 375, 390 and 402px. Deliberately left unchanged in this PR, because deleting it also changes documents — the reference layout this PR was asked to match — and it is shared geometry with a 320–540px clip sweep asserting against it. Captured as its own outstanding-issues row.Open PR Ship soft Category filter (replace phone select blue highlight) #1606 is superseded, not conflicted. It replaces
MobileResultFilterControl's native<select>with a portal menu to fix the iOS system-blue selected-value highlight. That function no longer exists after this PR, so the defect is gone by removal and Ship soft Category filter (replace phone select blue highlight) #1606 cannot be rebased onto it.#253has been updated to say so; closing Ship soft Category filter (replace phone select blue highlight) #1606 is a human call and that row stays open until someone makes it.Summary by CodeRabbit
New Features
Documentation
Tests