fix: stop Escape focus restore from closing the app-mode menu - #822
Conversation
📝 WalkthroughWalkthroughChangesFocus restoration
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/ui-smoke.spec.ts`:
- Around line 166-171: Remove the catch that suppresses failures from the
scope-command-popover wait in the deferred focus restore flow. Let waitFor({
state: "hidden", timeout: uiAssertionTimeoutMs }) fail when the popover remains
visible, so opening the mode menu only occurs after the dismissal precondition
succeeds.
🪄 Autofix (Beta)
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 Plus
Run ID: 68cee0a8-6428-48d8-9e43-8c8944253ae9
📒 Files selected for processing (4)
src/components/clinical-dashboard/master-search-header.tsxsrc/components/use-dismissable-layer.tstests/restore-focus-unless-moved.dom.test.tsxtests/ui-smoke.spec.ts
Uh oh!
There was an error while loading. Please reload this page.
Scope dismiss deferred a focus restore onto the answer-options trigger. When a mode-menu open landed in the same frame window, that restore stole focus, blur-dismissed the menu, and flaked the Documents mode switch in Production UI CI. Skip restore when the mode menu is open or focus already moved, and wait for the scope popover to hide before opening the menu. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Only wait for the scope popover when it is visible, and let that wait throw if dismissal times out so the mode-menu open cannot recreate the Escape focus-restore race. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
f0febf2 to
2b8ddb8CompareUh oh!
There was an error while loading. Please reload this page.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
* fix: stop Escape focus restore from closing the app-mode menu Scope dismiss deferred a focus restore onto the answer-options trigger. When a mode-menu open landed in the same frame window, that restore stole focus, blur-dismissed the menu, and flaked the Documents mode switch in Production UI CI. Skip restore when the mode menu is open or focus already moved, and wait for the scope popover to hide before opening the menu. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix(test): fail closed when scope popover does not dismiss Only wait for the scope popover when it is visible, and let that wait throw if dismissal times out so the mode-menu open cannot recreate the Escape focus-restore race. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Summary
onBlurbefore the Documents item was visible.restoreFocusUnlessMoved()so Escape/scope-close restore is skipped when focus already moved (for example into the mode menu).Verification
npm run test -- tests/restore-focus-unless-moved.dom.test.tsx(3 passed)tests/ui-smoke.spec.tsfordashboard defers source and administration requests until their surfaces openanddocument search mode lists matching documents(2 passed; 6/6 stress reruns of the formerly flaky test)Risk and rollout
Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
switchToDocumentSearchMode/ Documents menuitemradio not found.Summary by CodeRabbit
Bug Fixes
Tests