feat(mockups): three directions for the services filter surface - #1828
Conversation
The services filter sheet is titled "Filter services" and contains no filter. Every chip calls applyServiceQuery(), which pushes a new route and replaces the query, so choosing "Crisis" while reading "16 services · lithium level timing" discards that search and its results. It is a preset switcher wearing a funnel icon. Adds /mockups/services-filter-refined: a reproduction of what ships today with its defects annotated, then three directions, each at desktop and phone. - A · Refine in place — facet groups with live counts above a visually separated "Start a new search" block. Expressible in the current ResultFilterSheet props (the unused ResultFilterOption.hint field already exists for counts); upgrades all 7 modes, nothing forks. - B · Presets and filters, split — two tabs so the reader picks a verb before a value, plus a persistent active-filter row under the band. - C · Directory-grade refine — a persistent desktop rail instead of a popover, and a phone sheet with find-a-filter and collapsible groups. Every count is real, computed live from the 219 services in data/services-snapshot.json via ~1KB of base64 facet bitmasks (OR within a group, AND across groups). Those tags are already populated and entirely unused by the UI today. Recorded caveats: cost_funding is 87 distinct free-text values so a "No cost" facet needs normalising first and is deliberately absent rather than faked; age_groups:mixed (202/219) and setting_flags:public (207/219) are omitted because a facet that never excludes anything is a row of dead pixels; and the phone radiogroup / desktop aria-pressed disagreement must be resolved deliberately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011btGFwWKYFWDs5McQkqz9J
|
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: 53 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 (7)
📝 WalkthroughWalkthroughAdded a runnable services filter mockup route. The page includes live facet counts, shared filter state, responsive previews, three redesign directions, route-specific mockup chrome handling, and supporting documentation. ChangesServices filter mockup
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011btGFwWKYFWDs5McQkqz9J
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b64f25597
ℹ️ 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".
|
@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. The only repository destination is BigSimmo/Database, and the only branch destination is the pull request head branch claude/filter-popup-design-mockups-x6sbjv at starting commit 17ca6be; never publish fixes to a detached or synthetic work branch and never create a stacked pull request. Use the authenticated GitHub connector to commit each approved fix to BigSimmo/Database:claude/filter-popup-design-mockups-x6sbjv, then verify that the pull request head contains the published commit before reporting success. 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. For a fixed thread, reply with as the first line and as the second line. For a no-code disposition, use followed by . These result markers authorize the workflow to close that exact thread only after it verifies a fixed commit is the pull request head; a local-only commit is not a fix. If publication or verification fails, do not use either result marker, do not claim success, and leave the thread open with the blocker. If human input or new authorization is required, do the same. 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. Delightful! 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". |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/components/services-filter-refined-mockups.tsx (3)
554-561: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
text-whiteis a hardcoded design value.Every other color in this file resolves through a
var(--…)token. Line 560 uses the literaltext-whitefor the commit button foreground. On the accent background this also fixes the foreground independently of theme, so a light-accent theme loses contrast.Use the surface token that the checkbox indicator at Line 335 already pairs with
--clinical-accent.Guidelines require Tailwind 4
@themetokens fromsrc/app/globals.cssrather than hardcoded design values.♻️ Proposed fix
- : "bg-[color:var(--clinical-accent)] text-white hover:brightness-110", + : "bg-[color:var(--clinical-accent)] text-[color:var(--surface)] hover:brightness-110",🤖 Prompt for 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. In `@src/components/services-filter-refined-mockups.tsx` around lines 554 - 561, Replace the hardcoded text-white class in the commit button’s cn configuration with the existing surface token used by the checkbox indicator alongside --clinical-accent, using the corresponding Tailwind 4 `@theme` token from globals.css so the foreground adapts to themed accent backgrounds.Source: Coding guidelines
953-985: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
SegmentSwitchdeclares a tablist without the rest of the tab contract.The container uses
role="tablist"and the children userole="tab". Three pieces are missing:aria-controlson each tab, a matchingrole="tabpanel"on the renderedDirectionBBodyoutput, and arrow-key navigation with rovingtabindex. A screen reader announces "tab 1 of 2" and then finds no associated panel.This matters more than usual here. The study argues that the shipped sheet mixes
role="radiogroup"andaria-pressedfor the same control, and the anatomy entry at Line 1422 promises "one contract at both widths". A comp that introduces its own incomplete role contract weakens that argument.The simplest fix keeps the visual design and drops the tab roles, since the switch selects a body rather than paging through peer panels.
♻️ Option 1 — use a radiogroup contract, which matches "pick one of two modes"
<div - role="tablist" + role="radiogroup" aria-label="Filter mode" className="inline-flex w-full rounded-xl border border-[color:var(--border)] bg-[color:var(--surface-subtle)] p-1" > {segments.map((segment) => ( <button key={segment.value} type="button" - role="tab" - aria-selected={value === segment.value} + role="radio" + aria-checked={value === segment.value} onClick={() => onChange(segment.value)}♻️ Option 2 — complete the tab contract
Give each tab an id and
aria-controls, then wrap theDirectionBBodyoutput in arole="tabpanel"with the matchingaria-labelledby, and addArrowLeft/ArrowRighthandling with rovingtabindex.🤖 Prompt for 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. In `@src/components/services-filter-refined-mockups.tsx` around lines 953 - 985, Update SegmentSwitch to remove the incomplete tablist/tab roles and use a radiogroup-style contract for selecting one mode, preserving the existing visual behavior and onChange flow. Align each option with the component’s established single-choice accessibility pattern, such as the existing aria-pressed or radiogroup semantics, without adding tab panels or keyboard-navigation logic.
232-243: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
skipGroupis never passed by any caller.
maskForSelectionacceptsskipGroup, butuseFacetCountsat Lines 249 and 255 calls it with one argument only. The parameter is dead.The parameter also hints at a counting strategy the code does not use. The anatomy entry at Line 1404 says counts are "computed against the other groups' selections", which describes the
skipGroupapproach. The implementation instead adds the candidate facet to the selection and recounts, which answers "what would I get if I added this?" — the question the same sentence ends with. Remove the parameter, or use it and align the prose.🤖 Prompt for 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. In `@src/components/services-filter-refined-mockups.tsx` around lines 232 - 243, Remove the unused skipGroup parameter from maskForSelection and delete the related group-skipping branch, updating both useFacetCounts call sites to match. Keep the implementation and anatomy prose aligned with the existing candidate-addition counting behavior rather than introducing the unused “other groups” strategy.
🤖 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 `@src/components/services-filter-refined-mockups.tsx`:
- Around line 1226-1236: Fix the Direction C footer counts in the component’s
preview strip so facet values represent true subsets of the current selection,
not “if added” union counts. Add a helper alongside useFacetCounts that
intersects maskForSelection(selected) with DECODED[facetId] and returns its
popcount, then use it for the confidence:High and setting:digital_phone values
while preserving the existing total count and labels.
- Around line 456-487: Update ResultsPreview to render the “+ more” span only
when remaining is greater than 0; keep the existing count and SAMPLE_RESULTS
rendering behavior unchanged.
---
Nitpick comments:
In `@src/components/services-filter-refined-mockups.tsx`:
- Around line 554-561: Replace the hardcoded text-white class in the commit
button’s cn configuration with the existing surface token used by the checkbox
indicator alongside --clinical-accent, using the corresponding Tailwind 4 `@theme`
token from globals.css so the foreground adapts to themed accent backgrounds.
- Around line 953-985: Update SegmentSwitch to remove the incomplete tablist/tab
roles and use a radiogroup-style contract for selecting one mode, preserving the
existing visual behavior and onChange flow. Align each option with the
component’s established single-choice accessibility pattern, such as the
existing aria-pressed or radiogroup semantics, without adding tab panels or
keyboard-navigation logic.
- Around line 232-243: Remove the unused skipGroup parameter from
maskForSelection and delete the related group-skipping branch, updating both
useFacetCounts call sites to match. Keep the implementation and anatomy prose
aligned with the existing candidate-addition counting behavior rather than
introducing the unused “other groups” strategy.
🪄 Autofix
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: ef55859b-a3d0-4cb7-9cc4-c5119b81e3bb
📒 Files selected for processing (6)
docs/branch-review-ledger.mddocs/site-map.mdmockups/README.mdsrc/app/mockups/mockups-layout-client.tsxsrc/app/mockups/services-filter-refined/page.tsxsrc/components/services-filter-refined-mockups.tsx
Babysit snapshot (@2026-08-12)
Dispositions
No new PR-policy or clinical/RAG text changes were needed (design-scratch route only). |
Codex babysit sweep (review-and-fix)Snapshot
Findings and fixes
Checks on this head
Local proof
Ledger
Merge left to you. |
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #9764 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
Babysit snapshot — scope
|
There was a problem hiding this comment.
Pull request overview
Adds a new design-scratch mockup route (/mockups/services-filter-refined) to study three alternative “services filter” sheet directions (desktop + phone), including live facet counts derived from a compact bitmask index, plus documentation/sitemap updates to record the study.
Changes:
- Add
ServicesFilterRefinedMockupsPagewith three filter-surface directions and a “today” reproduction/annotation. - Add the
/mockups/services-filter-refinedroute and suppress shared mockups chrome for it. - Document the study in
mockups/README.mdand regeneratedocs/site-map.md.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/services-filter-refined-mockups.tsx | New mockup page implementing the three design directions, including live facet-count recomputation. |
| src/app/mockups/services-filter-refined/page.tsx | New mockup route entrypoint and metadata. |
| src/app/mockups/mockups-layout-client.tsx | Suppress shared mockup chrome for the new route. |
| mockups/README.md | Records the new services filter surface study and its rationale/caveats. |
| docs/site-map.md | Adds the new mockup route to the generated site map. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Babysit snapshot (scope: babysit)
Actionable issues reviewed and fixed
Required checks run locally
Thread dispositions
Residual risk
Merge left to you. |
Babysit PR summary
|
|
Review sweep complete for PR #1828 (HEAD a858ea9).
|
babysit summary
|
Babysit review summary
|
Round one offered three directions. Asked which to build, the answer was a sequence rather than a winner, plus one bolder move flagged as a product judgement. This draws those three threads so they can be compared directly, at desktop and phone. - 1 · Stop the bleed (ship first) — no filtering at all. The sheet is renamed to what it is, each shortcut shows the literal query it will run, and the current search sits at the top as the thing you are about to lose. Today that loss is invisible until it has happened. The footer commits to "Keep <query>" rather than "Done". No new state, no facets, no new component, and nothing that has to be unpicked later. - 2 · The recommendation — direction A's multi-select facets with live counts and a committed "Show N services", plus direction B's persistent active-filter pill row. Phone shows the sheet open, desktop shows it closed, because the closed state is what the pill row exists for. - 3 · Presets evicted (your call) — the sheet becomes purely a filter and the presets move to the composer as AnswerSuggestionChips, a production component whose own prop docs name composer rows as a use. Deletes sheet code rather than adding a surface; the risk is product, not engineering. Both surfaces are shown together because the sheet alone would flatter the option. The facet engine, chips, band and sheet shell are imported from the round-one study rather than copied, so the ~1KB bitmask table is not duplicated and the two pages cannot drift to different numbers for the same catalogue. That reuse is why the whole second study costs 7.1 KiB gzip against the mockups budget where the first cost 12.1 KiB. Adds an optional maxHeight to PhoneSheetShell so a study can leave the surface behind the sheet visible when that surface is part of the argument. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011btGFwWKYFWDs5McQkqz9J
…ps-x6sbjv' into claude/filter-popup-design-mockups-x6sbjv # Conflicts: # src/components/services-filter-refined-mockups.tsx
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011btGFwWKYFWDs5McQkqz9J
…esign-mockups-x6sbjv
Summary
Adds
/mockups/services-filter-refined— a design study for the sheet reached from the Filter control in the services results band, with three directions each shown at desktop and phone, plus a reproduction of what ships today with its defects annotated.applyServiceQuery()(services-navigator-page.tsx:604), which pushes a new route and replaces the query — so choosing "Crisis" while reading "16 services · lithium level timing" discards that search and its results. It is a preset switcher wearing a funnel icon. Secondary defects: no per-option counts, four unrelated categories flattened into one chip row, a dead band below the fold, "Done" as a low-emphasis outlined button doing the primary job, and a phonerole="radiogroup"contradicting the desktop rail'saria-pressedtoggles.ResultFilterSheetprops — the unusedResultFilterOption.hintfield (result-filter-control.tsx:39) already exists for counts. Upgrades all 7 modes at once; nothing forks.data/services-snapshot.jsonvia ~1KB of base64 facet bitmasks (OR within a group, AND across groups). ThoseCatalogServiceTagsare already populated and entirely unused by the UI today. Verified against the snapshot: Crisis ∩ Regional WA = 12, of which 10 are High confidence and 9 are digital/phone.mockups-layout-client.tsx, records the study inmockups/README.md, and regeneratesdocs/site-map.md.Data caveats recorded in the study itself: a "No cost" facet is not free (
cost_fundingis 87 distinct free-text values; ~69 of 219 match a free-ish pattern, so it is deliberately absent rather than faked);age_groups: mixed(202/219) andsetting_flags: public(207/219) are omitted as facets because an option that never excludes anything is a row of dead pixels; and the radiogroup /aria-presseddisagreement must be resolved deliberately rather than inherited.This is design scratch only — no production route, component or behaviour changes.
Verification
npm run verify:pr-localGreen through
check:runtime,check:installed-lock-parity,format:changed,sitemap:check,docs:check-index,docs:check-inventory,docs:check-scripts,docs:check-links,check:branch-review-ledger,check:outstanding-issues,lint,typecheck. Then:The single failure is pre-existing and environment-caused, not from this diff:
tests/pr-handoff-stop.test.ts > emits handoff context only when the marker file existschmods a git dir to0o555and expects the marker write to fail, but this container runs as uid 0, and root bypasses directory permissions on Linux. Confirmed by checking out pristineorigin/main(046feb3) in a separate worktree and running the same file there — it fails identically, with none of this branch's changes present. The file is not in this diff.The two steps
verify:pr-localdid not reach were then run directly:npm run check:bundle-budget— run afterrm -rf .next && npm run build, since a cached.nextmakes this check report a false pass. A prior mockup route was deleted for failing exactly this gate.npm run verify:uiwas not run because this adds only a design-scratch route that 404s in production and touches no production component or shared chrome. Proof was taken directly against the route instead — rendered atnpm run ensure's URL, screenshotted at desktop 1280×900 and phone 390×844, and checked at the blocking 320px narrow breakpoint with 0px horizontal overflow.Risk and rollout
src/app/mockups/**404s in production viamockupsEnabled()(src/lib/env.ts:376) and is blocked again at the proxy (src/proxy.ts:129);robots.tsdisallows it; eslint forbids production source importing*-mockups. The only shared file touched ismockups-layout-client.tsx, and the edit is one additional pathname branch that cannot affect other mockup routes.Notes
The mockups deliberately go no further than a study. Every direction here needs real filter state, URL round-tripping, and a services facet index modelled on
buildSmartDocumentTagFacetIndexinsrc/lib/document-tags.tsbefore any of it could ship — services has no filter state today, because the chip is the query. Directions B and C also add a persistent row under the results band, which touches phone chrome reserve and would needdocs/search-chrome-behaviour.mdplusnpm run verify:phone-chromebefore being trusted.No Clinical Governance Preflight or
RAG impact:line:classifyPullRequestFilesreturnsclinicalRisk: falseandoperationalRisk: falsefor this diff, and it touches no RAG-ranking surface.Generated by Claude Code
Summary by CodeRabbit
New Features
/mockups/services-filter-refined.Documentation