fix(editor): keep SQL editor focus after dismissing autocomplete with Escape (#1845)#1848
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![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: 7f400932c1
ℹ️ 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".
| reclaimFirstResponder() | ||
| } | ||
|
|
||
| return wasFocused || vimHandled |
There was a problem hiding this comment.
When the editor focus cache is stale, this returns true solely because wasEditorFocused was previously true, so the Clear Selection menu skips the cancelOperation fallback even if focus has already moved to the data grid. wasEditorFocused is updated later via the window update observer, so a quick click from the editor into a selected grid followed by Escape can be swallowed here and even reclaim editor focus instead of clearing the grid selection, contradicting the intended grid-focused behavior.
Useful? React with 👍 / 👎.
1 participant
Problem
Fixes #1845. In the SQL editor, pressing Escape to dismiss the autocomplete popup also drops keyboard focus, so the caret disappears and you cannot keep typing without clicking back into the editor.
Root cause
Bare Escape is registered as a global SwiftUI menu key equivalent for "Clear Selection" (a data grid action). A menu key equivalent preempts the key window's local event monitors, so the completion popup's own Escape handler (which would close the popup and keep focus) never runs. With Vim off, the "Clear Selection" action falls through to
NSApp.sendAction(#selector(NSResponder.cancelOperation(_:)), to: nil, from: nil), a target-agnostic broadcast that drops the editor's first responder, and nothing re-asserts it. This is the same class as the existing "Window Close (Cmd+W)" invariant, and it is whyhandleVimEscapeFromMenualready existed.Fix
Extend the existing
EditorEventRoutermenu-escape routing so the menu Escape defers to the focused editor before falling back to the grid. When the editor is focused, the menu now consumes Escape (dismissing the popup if open) and skips the broadcast, with an explicitmakeFirstResponderand cursor rebuild as a backstop. Focus stays in the editor, matching nativeNSTextViewcompletion behavior and every peer editor (VS Code, Xcode, JetBrains, Postico, Sequel Ace, Sublime).Grid-focused Escape is unchanged: it still clears the grid selection.
Changes
isShowingCompletions/dismissCompletions()on the vendoredTextViewController, with a per-controller identity check so one tab cannot dismiss another's popup.SQLEditorCoordinator.handleEscapeFromMenu()dismisses the popup, routes to Vim, and reclaims first responder and the caret when the editor was focused.EditorEventRouter.handleVimEscapeFromMenu()renamed tohandleEscapeFromMenu(); the "Clear Selection" call site updated.handleShowCompletionsreturned the unconsumed event when closing a visible popup (reachable via Ctrl+Space).dismissCompletions()API.Tests
TextViewControllerSuggestionsTestscoversisShowingCompletionsidentity, anddismissCompletions()return value and popup close, against the realSuggestionController.SQLEditorCoordinatorEscapeMenuTestscovershandleEscapeFromMenu()returning false when no editor is focused, so the grid keeps Escape.The full end-to-end flow (type, popup shows, Esc, keep typing) needs accessibility identifiers on the editor and the vendored popup and races the completion debounce, so it is not covered by a UI test yet. Manual check: open a query tab, type a keyword, press Esc, keep typing. The caret stays and characters land.
Notes
App and vendored-package code only. No PluginKit ABI change.