Add searchable grouped mode picker - #2591
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 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. Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_e7ef737b-334c-451b-8066-c4613787d3e7) |
![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: 68113e01d4
ℹ️ 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 Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…esign-20260901-fix # Conflicts: # src/components/clinical-dashboard/master-search-header.tsx
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_94d13c7b-31fc-4a76-aade-ba75099e78ef) |
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #15508 (success). That run's conclusion is an aggregate and did not exercise Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_2fd6934a-ccb0-4abf-8d95-8eb40ef64273) |
…ale-filter focus - tests/audit-navigation-auth-regressions.test.ts asserted the desktop trigger's old aria-haspopup value and old min-height class, which this PR intentionally replaced with the searchable dialog's aria-haspopup="dialog" and min-h-11 rows. Update both assertions, plus a third (unrelated to either review comment) that still expected the pre-refactor inline setUsesPhoneSearchLayout(currentUsesPhoneSearchLayout()) call instead of the local-variable form this PR introduced. - openModeMenuWithFocus (reached via ArrowDown/ArrowUp on the closed trigger) reset modeMenuQuery to "" but then scheduled focusModeOption(nextIndex) via rAF, which re-derives its bounds from activeModeMenuOptions — a value still closed over from before the query reset took effect. Dismissing the dialog while filtered to zero or one match therefore left focus stuck on the trigger (zero matches, modulo by zero) or jumped it to the first item of the next render's full list instead of the intended mode (one match, modulo wraps every index to 0). Focus directly by the already-valid nextIndex instead of re-deriving it against a list that hasn't caught up yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UTZh9kpgzN6RteWQ1CTyBv
…901' into codex/mode-switcher-design-20260901-fix
…ode pick The new searchable mode dialog is taller/wider than the flat menu it replaced, so options render at different on-screen coordinates than before. This test's click on the "DSM" option left Playwright's virtual pointer stranded at that coordinate, which now lands exactly over a command-dropdown suggestion once the dropdown opens on the search input's focus. Chromium fires a hover for whatever ends up under a stationary pointer when the DOM shifts beneath it, so that suggestion became the dropdown's active item — hijacking the Enter keypress to select it instead of submitting the search, leaving the URL stuck at `?mode=dsm` instead of navigating to `/dsm/search?q=bipolar`. A real mouse user must move the pointer to the input to focus it in the first place, so clicking the input before `.fill()` (which only focuses programmatically) reproduces that pointer relocation and removes the coincidental hover. Confirmed via targeted diagnostics that this is a Playwright-only interaction gap, not a navigation regression: canAsk and the submit button were never disabled, and the composer's dropdown active-item state was the sole reason submission never fired. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UTZh9kpgzN6RteWQ1CTyBv
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_b3171105-0937-474d-b071-76fd244fa2cf) |
…ode search box check:design-system-contract failed on the searchable mode dialog: the desktop menu-option row and the search-box wrapper both shrank to min-h-11 (44px), under the project's 48px interactive tap floor (AGENTS.md/CLAUDE.md warn specifically against this — it previously caused a ui-smoke flake). Restore min-h-12 on both. The search-box wrapper also combined a `border` utility with `focus-within:ring-1`, an edge-ownership conflict (border and ring compete for the same edge) the same gate forbids. Drop the ring and keep the existing border-color transition on focus, matching the border-only focus idiom already used elsewhere in this codebase. Also drop the redundant `aria-live="polite"` from the match-count `role="status"` paragraph — the status role already implies aria-live politeness, and the gate's live-region scanner exists specifically to catch this duplication (per the precedent recorded in design-system-contract-utils.mjs for LoadingPanel's spinner). Update the one test assertion pinned to the old min-h-11 value. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015uRvqm44emrLDMyPF7Pame
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_2b592a90-3933-40ad-a0bc-82baa33489ab) |
|
Not this PR's: Re-running the failed job once to confirm. Generated by Claude Code |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_4c858a33-11b2-4238-a8ef-2f30536c99b7) |
|
Same shape as the Queuing one re-run of the failed job. Generated by Claude Code |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_0a93da8b-557f-4574-aef9-9e51f8cc7f65) |
…cation (#2613) * docs: queue outstanding-issue for recurring shared-shell testid duplication Playwright strict-mode "resolved to 2 elements" failures on service-actions-trigger (PR #2536) and sources-topics-main (PR #2591, tests/ui-sources.spec.ts:88) share the same shape: one element nested inside GlobalSearchShell's mobile-composer-reserve-pad wrapper, one outside it. Confirmed unrelated to either PR's own diff. Queuing for a focused investigation/fix rather than patching a "one owner" shared shell component inside an unrelated PR. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015uRvqm44emrLDMyPF7Pame * docs: log a third occurrence of the shared-shell testid duplication bug Same strict-mode "resolved to 2 elements" shape seen on PR #2613's queued issue, now also on caring-contacts-phone-dock (PR #2600). Three unrelated PRs, three different testids — updates the priority signal for the existing queued investigation without duplicating it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015uRvqm44emrLDMyPF7Pame * docs: correct an unverified detail in the third-occurrence ledger entry The earlier note claimed the duplicate caring-contacts-phone-dock element was nested inside mobile-composer-reserve-pad by analogy with the first two occurrences. The actual job log for this instance only shows two sibling matches for the same testid; the nesting detail was not verified and is removed. Cancels and replaces the prior entry. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015uRvqm44emrLDMyPF7Pame --------- Co-authored-by: Claude <noreply@anthropic.com>
2 participants
Summary
Verification
npm run test:e2e -- tests/ui-smoke.spec.ts --project=chromium --grep "desktop mode options close|phone mode menu groups|app mode menu supports keyboard"
Risk and rollout
Clinical Governance Preflight
Not applicable: this is a client-side mode-navigation and presentation change. It does not alter retrieval, ranking, ingestion, source rendering, document access, privacy, or clinical output.
Notes
Note
Medium Risk
Shared header mode switching and focus/ARIA behavior changed on desktop; clinical routing is unchanged but regressions could affect keyboard users and overlay dismissal.
Overview
Replaces the desktop flat mode dropdown in
master-search-headerwith a searchable dialog: a "Find a mode" field, match/mode counts, grouped Find / Diagnose / Care sections (viaphoneModeGroups), empty-state copy, and a keyboard hint footer. The trigger now always usesaria-haspopup="dialog"; the overlay isrole="dialog"with the option list stillrole="menu".Filtering and focus: Label substring search drives
desktopModeMenuOptions; arrow-key navigation and Home/End use the filtered list. Opening the menu clears the query, focuses the search box on click (or the highlighted option on ArrowOpen), and fixes a stale-filter bug by focusing by index inrequestAnimationFrameinstead of callingfocusModeOptionbefore React commits the reset.Phone keeps the bottom sheet; desktop gets the new picker only.
actionMenuModeOptionsdropsuseMemo(inline map). Desktop row styling tightens slightly (min-h-12).Tests and audit contracts are updated for dialog semantics, grouping, filter/clear,
dismissOverlayByHeaderClickfor outside dismiss, and a real click on the global search input before Enter (avoids hover on command dropdown after the taller dialog).Reviewed by Cursor Bugbot for commit dbdb962. Configure here.