Skip to content

feat(search): rebuild the results band as a single-line spine - #1280

Merged
BigSimmo merged 18 commits into
mainfrom
claude/top-search-design-mockups-w53znc
Jul 28, 2026
Merged

feat(search): rebuild the results band as a single-line spine#1280
BigSimmo merged 18 commits into
mainfrom
claude/top-search-design-mockups-w53znc

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Summary

  • Rebuild the shared SearchResultsHeaderBand around a single-line spine: the QUERY / RESULTS FOR eyebrow is dropped and query, divider and count share one row. Phone height goes ~148px → 120px; desktop min-height 4.5rem → 3.75rem.
  • Replace the green count pill with neutral text and a tabular-nums figure, so success colour keeps meaning a state that was actually achieved. The role="status" / aria-live announcement is unchanged.
  • Turn sort into a segmented aria-pressed group instead of a two-option <select> — a menu you had to open to learn nothing.
  • Render active scopes as accent chips at the head of the utility group, with a desktop-only spacer keeping them beside the query while controls stay at the trailing edge.
  • Make the utility group a swipe rail below lg rather than a second tinted band. Children keep natural width; the right-edge fade applies only while it actually overflows, measured by a ResizeObserver.
  • Record the band's rules in docs/search-chrome-behaviour.md.

Root cause worth calling out. The reported clipping (Current searchCurrent searcl) and the controls out-shouting the query came from the same place: the intentionally unlayered rule in globals.css that pins every input/select/textarea to 16px below 640px so iOS Safari does not zoom on focus. It deliberately beats Tailwind's text-* utilities, so text-xs never applied to those selects — 16px extra-bold values rendered inside boxes sized for 12px text. That rule is correct and is left alone. Hierarchy is restored around it: the query steps to 18px, select values step down to font-semibold, and truncate gives them a real ellipsis. Size is not available on those controls, so weight and colour carry the hierarchy. The new doc section states this so the next change does not fight it with !important.

Verified by hand in the running app at /services?q=CMHT&run=1 — the exact scenario from the reported screenshots — at 1440px and 390px.

Verification

  • npm run verify:pr-local

Verification not run: verify:cheap and verify:ui were used as the gates for this change; verify:pr-local should run before this leaves draft.

  • npm run verify:cheap — green (392 test files, 3537 tests passed, 1 skipped; plus lint, typecheck, runtime, Actions-pin, CI-scope and sitemap checks)
  • npm run verify:ui when UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changed

Run with PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH=/opt/pw-browsers/chromium (the escape hatch playwright.config.ts already provides for sandboxed containers that block browser downloads): 320 passed, 3 failed. None of the 3 is attributable to this branch, established rather than assumed:

  • ui-pwa.spec.ts:143 (Chromium PWA installability) and ui-smoke.spec.ts:3498 (document viewer PDF ordering) fail identically on untouched origin/main in this container — pre-existing and environmental (this image ships Chromium 1194; CI uses 1228).

  • ui-phone-scroll.spec.ts:481 (calculators phone dock) passes in isolation on this branch (2.9s) and failed only at position Simplify operational tooling and runbooks #56 of the full suite (12.3s) — a suite-order timing flake. /calculators never renders SearchResultsHeaderBand, so this diff cannot reach it.

  • All three pass in CI's Production UI job on this branch.

  • npm run verify:release before release or handoff confidence claims

Retrieval/ranking evals are not applicable: no retrieval, ranking, selection, chunking, or scoring behaviour is touched. src/lib/result-sort.ts is consumed through readResultSort and is not modified.

Risk and rollout

  • Risk: low–moderate. Presentation-only, but SearchResultsHeaderBand is shared chrome rendered by 12 call sites (services, documents, differentials, forms, DSM, factsheets, formulation, specifiers, medication, favourites, applications launcher, therapy compass), so a layout regression would surface on every mode at once. No data, API, auth, or ranking behaviour changes.
  • Rollback: revert the two commits; no migration, config, or state to unwind.
  • Provider or production effects: None.

Clinical Governance Preflight

  • Source-backed claims still require linked source verification before clinical use — unchanged; no source or citation rendering touched
  • No patient-identifiable document workflow was introduced or expanded without explicit governance approval
  • Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy) — no Supabase surface touched
  • Service-role keys and private document access remain server-only — no server code touched
  • Demo/synthetic content remains clearly separated from real clinical sources
  • Source metadata, review status, and outdated/unknown-source behavior remain conservative — unchanged
  • Deployment classification/TGA SaMD impact was checked when clinical decision-support behavior changed — no decision-support behaviour changed; this is result-header presentation

Notes

  • Changing sort from a <select> to a segmented group broke two Playwright specs that reach the control by its accessible name rather than by component or test id, so component/test-id greps did not surface them. Both are fixed in c307c98 and verified passing: ui-smoke.spec.tsdocument search mode lists matching documents and scope actions @critical, and ui-tools.spec.tsresult sorting persists in the URL and restores through browser history. The latter also read the persisted order via toHaveValue after goBack/goForward; a segmented control has no value, so those became aria-pressed assertions while the URL and result-order assertions stayed untouched.
  • Two existing assertions in tests/ui-tools.spec.ts counted two <label> elements in the phone control pair. Sort is no longer a <label>, so those measure the pair's direct children and additionally assert the 44px tap-target floor. The no-internal-scroll assertion is kept: overflow belongs to the rail that owns the pair, not the pair.
  • ResultSortControl loses its compact prop (it existed to hide a "Sort" label the segmented control no longer has). It is not imported outside this module. MobileResultFilterControl's public API is unchanged — it is used by 10 call sites and only gains truncate plus the weight step-down.
  • Three added DOM tests cover the segmented sort's pressed state, the neutral count, and the absence of the eyebrow.

The shared results band spent its space on labels rather than answers, and
below `sm` one of its two controls was cut mid-word ("Current search" ->
"Current searcl").

Root cause of the clipping and of the controls out-shouting the query: the
unlayered iOS anti-zoom rule in globals.css pins every input/select/textarea
to 16px below 640px so Safari does not zoom on focus. It deliberately beats
Tailwind's text-* utilities, so `text-xs` never applied to those selects --
16px extra-bold values rendered in boxes sized for 12px text. That rule is
correct and stays; hierarchy is restored around it instead.

- Drop the QUERY / RESULTS FOR eyebrow; query, divider and count share one
  row. Phone height ~148px -> 120px, desktop min-height 4.5rem -> 3.75rem.
- Count becomes neutral text with a tabular-nums figure instead of a success
  pill, so green keeps meaning a state that was achieved. The role="status"
  live announcement is unchanged.
- Sort becomes a segmented aria-pressed group rather than a two-option
  select, removing a menu you had to open to learn nothing.
- Active scopes render as accent chips at the head of the utility group, with
  a desktop-only spacer keeping them near the query while controls stay at
  the trailing edge.
- The utility group is a swipe rail below `lg` instead of a second tinted
  band; children keep natural width and the right-edge fade applies only
  while it actually overflows, measured by a ResizeObserver.
- Query steps to 18px and select values to semibold with `truncate`, so the
  16px-floored controls read quieter and ellipse instead of clipping.

Records the band's rules, including the 16px floor, in
docs/search-chrome-behaviour.md so the next change does not fight it with
!important.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY
@supabase

supabase Bot commented Jul 27, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@BigSimmo, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 58 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 23f03a22-a77a-4ffc-af2b-d014610df16b

📥 Commits

Reviewing files that changed from the base of the PR and between 686ea39 and baff33f.

📒 Files selected for processing (6)
  • docs/branch-review-ledger.md
  • docs/search-chrome-behaviour.md
  • src/components/clinical-dashboard/search-results-header-band.tsx
  • tests/search-results-header-band.dom.test.tsx
  • tests/ui-smoke.spec.ts
  • tests/ui-tools.spec.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/top-search-design-mockups-w53znc

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

CI triage

CI failed on this PR. Automated classification of the 2 failed job(s):

  • Production UIneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.
  • PR requiredneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.

Compared with main CI run #5955 (success).

Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger.

claude added 4 commits July 27, 2026 14:36
Two production UI specs still drove result sort as a native select, which
broke when the control became a segmented aria-pressed group:

- ui-smoke "document search mode lists matching documents and scope
  actions" called selectOption on the group element.
- ui-tools "result sorting persists in the URL and restores through
  browser history" located `select[aria-label="Sort results"]` and read
  the persisted order via toHaveValue after goBack/goForward.

Both now click the labelled option button and assert aria-pressed, which
is where a segmented control carries its state. The URL and result-order
assertions are unchanged, so the specs still prove sort persists through
history rather than only that the control moved.

These two call sites were missed because they reference the control by
its accessible name rather than by component or test id.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY
An accent-soft chip that swaps to the neutral --border-strong on hover
reads as losing its active state. Deepen the chip's own accent instead,
matching how the other active-state surfaces in the band behave.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY

Copy link
Copy Markdown
Owner Author

Production UI on 93a9f90 — classified as a fail/pass race, not a regression

Failure: tests/ui-smoke.spec.ts:2327answer search URL opens chat without the answer home copy.

expect(locator).toBeVisible() failed
Locator: getByLabel('Loading answer')
Error: strict mode violation: getByLabel('Loading answer') resolved to 2 elements:
  1) <div role="status" class="space-y-4" aria-label="Loading answer">…
  2) <div role="status" class="space-y-4" aria-label="Loading answer">…
  - unexpected value "hidden"

Two identical answer-loading skeletons in the DOM, the second hidden — the duplicate hidden-clone signature described in docs/search-chrome-behaviour.md invariant 17 (a useSearchParams() Suspense ancestor leaving a persistent hidden S: clone). It is a rendering-duplication race in the Answer view, not an assertion this branch changed.

Why it is not attributable to this branch

  • The full diff vs main is 5 files: search-results-header-band.tsx, docs/search-chrome-behaviour.md, and 3 test files. Nothing touches Answer mode, src/lib/ui-copy.ts (which owns the Loading answer label), or the answer skeleton.
  • SearchResultsHeaderBand is not rendered in Answer mode.
  • The one edit this branch makes to ui-smoke.spec.ts is at ~line 3259 (document-search sort control), not line 2340.
  • The previous head 80f185f — identical product code — passed Production UI green. 93a9f90 adds only a merge of main (b3c5e18).

Reproduction, per the docs/testing.md flake policy (three runs on the same SHA):

PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH=/opt/pw-browsers/chromium \
  npm run test:e2e -- tests/ui-smoke.spec.ts --project=chromium \
  --grep "answer search URL opens chat without the answer home copy" --repeat-each=3

3 passed (13.4s) — 3/3 green on 93a9f90. It does not reproduce, so it is a fail/pass race rather than a repeatable regression, and it does not qualify for tests/flake-ledger.json (entries require a candidate that reproduces, plus a @quarantine title).

Not done, needs a maintainer: the triage bot gates re-runs on classification, which is now done, but CI re-runs are confirmation-required in this repo (AGENTS.mdpr-ci-fix). I have not re-run the job. A re-run of Production UI is the expected next step; if it goes red again on the same test, that would change the classification and I will treat it as a regression.


Generated by Claude Code

@BigSimmo
BigSimmo marked this pull request as ready for review July 27, 2026 21:34
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@BigSimmo
BigSimmo enabled auto-merge (squash) July 27, 2026 21:34
@BigSimmo

Copy link
Copy Markdown
Owner Author

@cursoragent resolve failing CI testing

@cursor

cursor Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

CI on #1280 is resolved.

Cause: Production UI failed on answer search URL opens chat without the answer home copy when getByLabel("Loading answer") matched both the live skeleton and a hidden Suspense S: clone (search-chrome invariant 17).

Fix: 78c7d1c7 — assert only the visible skeleton ([aria-label="Loading answer"]:visible), matching the rest of ui-smoke.

Proof:

  • Local: that journey 3/3 green
  • Hosted on tip with the harden: Production UI and PR required both SUCCESS (run 30308513222)

A docs-only ledger closeout (c844560d) just pushed and will re-run CI; product code is unchanged.

Open in Web Open in Cursor 

cursoragent and others added 2 commits July 27, 2026 21:40
Production UI failed when getByLabel("Loading answer") matched both the
live skeleton and a hidden Suspense S: clone (search-chrome invariant 17).
Use the same :visible locator pattern as the rest of ui-smoke.

Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
@cursor
cursor Bot disabled auto-merge July 27, 2026 21:50
BigSimmo and others added 3 commits July 27, 2026 21:50
@cursor

cursor Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Follow-up on tip 980b4298: synced origin/main and taught useRailOverflow to remeasure on childList mutations so the fade mask stays honest when scope chips/controls change. Temporarily disabled auto-merge so this polish could land without racing squash; re-arm when CI is green if desired.

cursoragent and others added 2 commits July 27, 2026 22:07
…rden

Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
BigSimmo added a commit that referenced this pull request Jul 27, 2026
* docs(ledger): record open-PR review and Bugbot sweep

Append review outcomes for the current open PR queue after #1277
merged, including merge-tree classification and pr-bugbot findings.

* docs(ledger): record open-PR recommendation implementation

Capture closeouts for the unsafe audit cluster and the follow-up fixes
pushed to #1273, #1275, #1280, and #1281.

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
@BigSimmo
BigSimmo enabled auto-merge July 28, 2026 02:45
@BigSimmo
BigSimmo merged commit 0944f3b into main Jul 28, 2026
19 checks passed
@BigSimmo
BigSimmo deleted the claude/top-search-design-mockups-w53znc branch July 28, 2026 03:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants