Skip to content

fix(editor): stop autocomplete popup from stealing keyboard focus (#1885) - #1893

Merged
datlechin merged 1 commit into
mainfrom
fix/1885-autocomplete-focus-steal
Jul 16, 2026
Merged

fix(editor): stop autocomplete popup from stealing keyboard focus (#1885)#1893
datlechin merged 1 commit into
mainfrom
fix/1885-autocomplete-focus-steal

Conversation

@datlechin

Copy link
Copy Markdown
Member

Fixes #1885.

Problem

In a new SQL editor tab (Cmd+T), typing the first couple of characters made the editor lose keyboard focus: the caret disappeared, further keystrokes went nowhere, and you had to click back into the editor. Reported on 0.57.1, macOS 14.2.1.

Root cause

The autocomplete panel's SwiftUI content used a List, which on macOS is backed by a focusable NSTableView. When the panel first appeared (about 600ms after the first keystroke: 50ms debounce plus the first schema fetch), AppKit tried to focus that table and called makeKeyWindow() on the panel. The panel refuses key status (canBecomeKey = false), but on macOS 14 the editor window had already resigned key by then, leaving no key window at all. Keystrokes were silently dropped. The panel then closed itself through its own parent-resigned-key observer, so the popup was never even visible. Frame-by-frame analysis of the issue's screen recording and CodeEditSourceEditor upstream issue #351 (the same makeKeyWindow console warning) both confirm the chain.

Fix

  • SuggestionContentView: replace List with ScrollView + LazyVStack. Selection, arrow keys, and taps were already model-driven, so the popup content is now display-only chrome with no focus participation. Rows keep the same layout, gestures, and highlight, and gain VoiceOver combine/selected traits.
  • SuggestionController.showWindow(attachedTo:): drop super.showWindow(nil) (its default implementation routes through makeKeyAndOrderFront); order front only. Detach the panel from a previous parent window before re-attaching, and detach on close, matching Apple's CustomMenus sample.
  • SuggestionViewModel.showCompletions: after the async completion fetch, re-validate that the editor is still first responder in the key window before presenting, and resolve screen coordinates from the freshly validated window. A popup must never present over a surface the user has left.

Tests

  • SuggestionPanelFocusTests (package): panel flag invariants, a structural walk asserting no NSTableView exists anywhere in the popup hierarchy (fails if a focusable control is ever reintroduced), first-responder retention across showWindow(attachedTo:), re-parenting, and detach-on-close.
  • SuggestionShowCompletionsGuardTests (package): the async guard drops presentation when focus moved during the fetch, and still presents when it did not. The positive-path test skips when the environment cannot make a window key (headless runners).
  • EditorAutocompleteFocusUITests (UI automation): Open Sample Database, Cmd+T, type select at full speed, assert all six characters land in the editor.

All 23 pre-existing suggestion tests still pass. swiftlint --strict clean on changed files.

@datlechin
datlechin merged commit 167fd67 into main Jul 16, 2026
3 checks passed
@datlechin
datlechin deleted the fix/1885-autocomplete-focus-steal branch July 16, 2026 17:18

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a20f12d9c2

ℹ️ 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".

Comment on lines +124 to +129
guard let window = textView.view.window,
window.isKeyWindow,
let responder = window.firstResponder as? NSView,
responder.isDescendant(of: textView.view) else {
Self.logger.debug("showCompletions: editor lost focus while completions were loading")
return

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reset stale completions when dropping unfocused presentations

When the async request finishes after focus has moved away, this new guard returns without clearing activeTextView/delegate or resetting the delegate's completion session. In the SQL editor, SQLCompletionAdapter.completionSuggestionsRequested has already cached its final session before returning; if the user clicks back into the same editor and keeps typing, cursorsUpdated sees the same active text view, gets filtered items from completionOnCursorMove, and returns without invoking the presentIfNot path, so the popup stays hidden until something else resets the session. Please route this drop path through the same cleanup as closing/dismissing the request.

Useful? React with 👍 / 👎.

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.

SQL editor lost focus after tying 'se'.

1 participant