Skip to content

fix(hig): dismiss search sheets and popovers with Escape (#1490)#1502

Merged
datlechin merged 1 commit into
mainfrom
fix/sheet-escape-focus
May 30, 2026
Merged

fix(hig): dismiss search sheets and popovers with Escape (#1490)#1502
datlechin merged 1 commit into
mainfrom
fix/sheet-escape-focus

Conversation

@datlechin

Copy link
Copy Markdown
Member

Part of #1490 (keyboard, focus, accessibility).

Problem

NativeSearchField (the shared NSSearchField wrapper) always returned true for cancelOperation, even when the field was empty, so Escape was swallowed and never reached the enclosing sheet or popover. Pressing Escape in the database switcher or quick switcher did nothing; you had to Tab to Cancel or click. The same swallow blocked Escape from closing the search popovers (column visibility, connection switcher, type picker). DatabaseSwitcherSheet also had no container Escape handler.

Fix (standard NSSearchField behavior)

  • NativeSearchField.cancelOperation: when the field has text, clear it and consume the key (return true); when the field is empty, return false so Escape propagates up the responder chain to the enclosing sheet or popover. This is the documented search-field convention: first Escape clears, second Escape dismisses.
  • DatabaseSwitcherSheet: add .onExitCommand { dismiss() }. (QuickSwitcherSheet already had one, so it starts working once the search field stops swallowing Escape.)

No custom focus machinery; this restores the native responder-chain behavior the search field was short-circuiting.

Scope note

NativeSearchField is shared across ~17 call sites. This change also lets Escape-when-empty close the four search popovers (column visibility, database/connection switcher, type picker) and propagate normally in settings and inline panels. That is an improvement and matches native behavior; a propagated Escape with no handler is a no-op, so nothing should regress.

Verify

  • Database switcher / quick switcher: type, press Escape -> text clears; press Escape again (empty) -> sheet dismisses.
  • Column visibility / connection switcher / type picker popovers: Escape when empty closes the popover.
  • A search field inside settings/inline panels: Escape when empty does nothing harmful.

Out of scope (separate #1490 follow-ups)

Initial-focus gaps (MCPTokenRevealSheet, LicenseActivationSheet), SQLReviewSheet Escape-from-editor (needs verifying whether the editor releases Escape), accessibility hints on dialog toggles, and the AlertHelper.runModal() fallback.

Focus/responder behavior is not unit-testable here (consistent with the codebase); verification is manual.

@datlechin datlechin merged commit 945032b into main May 30, 2026
4 checks passed
@datlechin datlechin deleted the fix/sheet-escape-focus branch May 30, 2026 06:33
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.

1 participant