fix(editor): dismiss autocomplete popup on inert clicks and constrain it to the editor#1833
Conversation
… it to the editor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ecb50f5e7
ℹ️ 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".
| let maxTopLeftY = upperLimit - edgePadding | ||
| if topLeftY > maxTopLeftY { | ||
| topLeftY = maxTopLeftY | ||
| } | ||
| return (topLeftY - windowSize.height, false) |
There was a problem hiding this comment.
Keep the top clamp from pushing the popup below the editor
When the caret is near the top of a short editor and the popup height still technically fits below it, this top-edge padding clamp can lower topLeftY and then return topLeftY - windowSize.height without rechecking lowerLimit. For example, with an editor from y=100...250, a 140pt popup, and a caret at y=245, the unclamped popup would fit at y=105...245, but clamping the top to upperLimit - 22 returns an origin of 88, so the resized popup overlaps the status/buttons below the editor—the case this change is trying to prevent during resize re-placement.
Useful? React with 👍 / 👎.
Fixes #1831. Follow-up to the still-unreleased #1815.
Problem
Two defects in the SQL editor autocomplete popup, both introduced by #1815:
Clicks swallowed. Clicking the popup's non-interactive areas (background, padding, rounded corners, divider, preview pane, "No Completions" label) did nothing: the popup stayed open and the click never reached the editor. The window server routes every mouse-down inside the panel's rectangle to the panel window regardless of what SwiftUI clips away, and the mouse monitor only closed the panel for clicks outside it, so inert-area clicks fell through to nothing.
Popup drifts over chrome. The panel was positioned once, at first appearance. Every keystroke re-filtered the list and resized the window from its top-left with no re-clamp, so the right edge drifted past the editor and the bottom edge drifted down over the status-bar Add/Columns buttons. The initial horizontal clamp was also screen-only, never editor-aware.
Fix
onBackgroundTaphook wired to the controller'sclose(), plus an outer.contentShape(Rectangle()).onTapGestureon the popup so any inert click dismisses it. Row selection is unchanged: the inner row gestures still take precedence. Right and middle clicks inside the panel also dismiss (via the mouse monitor), so a click meant for the editor's context menu is no longer lost.SuggestionWindowPlacementhelper: the horizontal position is now constrained to the editor frame (not just the screen), a panel wider than the editor left-aligns instead of inverting, and the vertical flip logic is preserved. The helper is re-applied on every resize from a stored cursor anchor, so the panel re-derives its position instead of drifting from its previous frame. This collapses the two divergent placement paths into one.Behavior now matches VS Code, Xcode, and the macOS system completion: click a suggestion to insert it, click anything else to dismiss. Double-click-to-insert is unchanged.
Considered and rejected rewriting the popup as an
NSPopover .transient: it would touch every editor host site through the sharedSuggestionController.sharedsingleton and change the popup's visual chrome for no correctness gain over this fix.Files
SuggestionViewModel.swift:onBackgroundTaphook.SuggestionController.swift: wire the hook, store/clear the placement anchor, dismiss on right/other-mouse-down inside the panel.SuggestionWindowPlacement.swift(new): pure placement math.SuggestionController+Window.swift:constrainWindowToScreenEdges/updateWindowSizedelegate to a sharedapplyPlacementthat re-clamps on every resize.SuggestionContentView.swift: outer dead-zone dismiss gesture.All in the vendored
LocalPackages/CodeEditSourceEditorpackage, so no PluginKit ABI or version-bump implications.Testing
swift test --package-path LocalPackages/CodeEditSourceEditor: 14 executed, 0 failures.test_onBackgroundTap_closesControllerAndClearsModelState: the dismiss hook clears controller and model state.SuggestionWindowPlacementTests: 6 pure-geometry tests (cursor near the right edge stays within the editor; width and height growth across resizes stay in bounds; wider-than-editor left-aligns; flips above the cursor;nileditor frame falls back to the screen). These fail against the old screen-only clamp.SuggestionControllerPlacementTests: integration test that resize re-clamps X;XCTSkips in a headless environment with no screen.Note: these package tests are not wired into
TablePro.xcscheme's test action, so CI's app-scheme test run does not execute them. Wiring them in can be a separate change.Docs
docs/features/autocomplete.mdxdocuments completion content and Escape-to-dismiss, not click dismissal or positioning, so it needs no change (consistent with #1815).