diff --git a/CHANGELOG.md b/CHANGELOG.md index 86825658a..b822d45d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Fixed the SQL editor in a new tab losing keyboard focus after the first couple of characters, which stopped further typing until you clicked back into the editor. (#1885) - Cancelling a SQL Server connection now stops waiting right away instead of staying stuck until the attempt finished on its own. A connection attempt also gives up after the login timeout with a clear message rather than hanging, and for Windows Authentication the message points at the usual Kerberos causes (KDC unreachable, missing SPN, or clock skew). (#1889) - Fixed MySQL and MariaDB queries ending in ORDER BY that could show an empty result with no error. A failure part way through reading rows was treated as a normal end of results, so a real server error looked like a table with no rows. TablePro now shows the error the server reported. (#1884) - Fixed the query result row cap returning a single row when it was set to unlimited. (#1884) diff --git a/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/CodeSuggestion/Model/SuggestionViewModel.swift b/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/CodeSuggestion/Model/SuggestionViewModel.swift index 268cf6d5f..8a56f7b9c 100644 --- a/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/CodeSuggestion/Model/SuggestionViewModel.swift +++ b/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/CodeSuggestion/Model/SuggestionViewModel.swift @@ -95,7 +95,7 @@ final class SuggestionViewModel: ObservableObject { self.delegate = nil itemsRequestTask?.cancel() - guard let targetParentWindow = textView.view.window else { + guard textView.view.window != nil else { Self.logger.warning("showCompletions: textView.view.window is nil") return } @@ -121,22 +121,31 @@ final class SuggestionViewModel: ObservableObject { try await MainActor.run { try Task.checkCancellation() + 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 + } + guard let cursorPosition = textView.resolveCursorPosition(completionItems.windowPosition), let cursorRect = textView.textView.layoutManager.rectForOffset( cursorPosition.range.location - ), - let cursorRect = textView.view.window?.convertToScreen( - textView.textView.convert(cursorRect, to: nil) ) else { Self.logger.warning("showCompletions: cursor rect resolution failed") return } + let screenCursorRect = window.convertToScreen( + textView.textView.convert(cursorRect, to: nil) + ) + self.items = completionItems.items self.selectedIndex = 0 self.syntaxHighlightedCache = [:] self.notifySelection() - showWindowOnParent(targetParentWindow, cursorRect) + showWindowOnParent(window, screenCursorRect) } } catch { return diff --git a/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/CodeSuggestion/View/SuggestionContentView.swift b/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/CodeSuggestion/View/SuggestionContentView.swift index c63e7f4a1..257d70701 100644 --- a/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/CodeSuggestion/View/SuggestionContentView.swift +++ b/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/CodeSuggestion/View/SuggestionContentView.swift @@ -9,6 +9,8 @@ import AppKit import SwiftUI struct SuggestionContentView: View { + static let rowHeight: CGFloat = 26 + @ObservedObject var model: SuggestionViewModel var body: some View { @@ -38,43 +40,20 @@ struct SuggestionContentView: View { } } + /// Rows live in a plain `ScrollView`/`LazyVStack`, never a `List`. `List` is backed by a + /// focusable `NSTableView`, and AppKit's attempt to focus it when the panel appears asked + /// the panel to become key, resigning the editor window's key status with no successor + /// (issue #1885). Selection, arrow keys, and taps are all model-driven, so the popup's + /// content must stay display-only chrome with no focus participation at all. private var suggestionList: some View { ScrollViewReader { proxy in - List { - ForEach(Array(model.items.enumerated()), id: \.offset) { index, item in - CodeSuggestionLabelView( - suggestion: item, - labelColor: model.themeTextColor, - secondaryLabelColor: model.themeTextColor.withAlphaComponent(0.5), - font: model.activeTextView?.font ?? .systemFont(ofSize: 12), - isSelected: index == model.selectedIndex - ) - .lineLimit(1) - .truncationMode(.tail) - .contentShape(Rectangle()) - .onTapGesture(count: 1) { - model.selectedIndex = index - } - .onTapGesture(count: 2) { - model.selectedIndex = index - if let selectedItem = model.selectedItem { - model.applySelectedItem(item: selectedItem) - } + ScrollView { + LazyVStack(spacing: 0) { + ForEach(Array(model.items.enumerated()), id: \.offset) { index, item in + suggestionRow(index: index, item: item) } - .listRowInsets(EdgeInsets()) - .listRowBackground( - RoundedRectangle(cornerRadius: 5) - .fill(index == model.selectedIndex - ? Color(nsColor: .selectedContentBackgroundColor) - : Color.clear) - .padding(.horizontal, SuggestionController.WINDOW_PADDING) - ) - .listRowSeparator(.hidden) - .id(index) } } - .listStyle(.plain) - .scrollContentBackground(.hidden) .padding(.vertical, SuggestionController.WINDOW_PADDING) .frame(height: listMaxHeight) .onChange(of: model.selectedIndex) { newIndex in @@ -85,6 +64,40 @@ struct SuggestionContentView: View { } } + private func suggestionRow(index: Int, item: CodeSuggestionEntry) -> some View { + CodeSuggestionLabelView( + suggestion: item, + labelColor: model.themeTextColor, + secondaryLabelColor: model.themeTextColor.withAlphaComponent(0.5), + font: model.activeTextView?.font ?? .systemFont(ofSize: 12), + isSelected: index == model.selectedIndex + ) + .lineLimit(1) + .truncationMode(.tail) + .frame(maxWidth: .infinity, alignment: .leading) + .frame(height: Self.rowHeight) + .background( + RoundedRectangle(cornerRadius: 5) + .fill(index == model.selectedIndex + ? Color(nsColor: .selectedContentBackgroundColor) + : Color.clear) + .padding(.horizontal, SuggestionController.WINDOW_PADDING) + ) + .contentShape(Rectangle()) + .onTapGesture(count: 1) { + model.selectedIndex = index + } + .onTapGesture(count: 2) { + model.selectedIndex = index + if let selectedItem = model.selectedItem { + model.applySelectedItem(item: selectedItem) + } + } + .accessibilityElement(children: .combine) + .accessibilityAddTraits(index == model.selectedIndex ? .isSelected : []) + .id(index) + } + private var contentWidth: CGFloat { let font = model.activeTextView?.font ?? NSFont.systemFont(ofSize: 12) let iconWidth = font.pointSize + 6 @@ -101,9 +114,8 @@ struct SuggestionContentView: View { } private var listMaxHeight: CGFloat { - let rowHeight: CGFloat = 26 let visibleRows = min(CGFloat(model.items.count), SuggestionController.MAX_VISIBLE_ROWS) - return rowHeight * visibleRows + SuggestionController.WINDOW_PADDING * 2 + return Self.rowHeight * visibleRows + SuggestionController.WINDOW_PADDING * 2 } private var noCompletionsView: some View { diff --git a/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/CodeSuggestion/Window/SuggestionController.swift b/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/CodeSuggestion/Window/SuggestionController.swift index e935a6f8e..8343a21b0 100644 --- a/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/CodeSuggestion/Window/SuggestionController.swift +++ b/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/CodeSuggestion/Window/SuggestionController.swift @@ -143,9 +143,15 @@ public final class SuggestionController: NSWindowController { } } - /// Opens the window as a child of another window. + /// Opens the window as a child of another window. The panel is ordered front without ever + /// being made key or main: `orderFront(_:)` is documented not to change either status, and + /// `NSWindowController.showWindow(_:)` is deliberately avoided because its default + /// implementation routes through `makeKeyAndOrderFront(_:)`. public func showWindow(attachedTo parentWindow: NSWindow) { guard let window = window else { return } + if let currentParent = window.parent, currentParent !== parentWindow { + currentParent.removeChildWindow(window) + } parentWindow.addChildWindow(window, ordered: .above) if let existingObserver = windowResignObserver { @@ -179,7 +185,6 @@ public final class SuggestionController: NSWindowController { } setupEventMonitors() - super.showWindow(nil) window.orderFront(nil) } @@ -204,6 +209,10 @@ public final class SuggestionController: NSWindowController { model.willClose() removeEventMonitors() + if let window, let parent = window.parent { + parent.removeChildWindow(window) + } + if let observer = windowResignObserver { NotificationCenter.default.removeObserver(observer) windowResignObserver = nil diff --git a/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/CodeSuggestion/SuggestionPanelFocusTests.swift b/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/CodeSuggestion/SuggestionPanelFocusTests.swift new file mode 100644 index 000000000..f084fb256 --- /dev/null +++ b/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/CodeSuggestion/SuggestionPanelFocusTests.swift @@ -0,0 +1,114 @@ +import AppKit +@testable import CodeEditSourceEditor +import SwiftUI +import XCTest + +final class SuggestionPanelFocusTests: XCTestCase { + @MainActor + func test_panel_neverAcceptsKeyOrMainStatus() throws { + let controller = SuggestionController() + let window = try XCTUnwrap(controller.window) + + XCTAssertFalse(window.canBecomeKey) + XCTAssertFalse(window.canBecomeMain) + XCTAssertTrue(window.styleMask.contains(.nonactivatingPanel)) + XCTAssertEqual((window as? NSPanel)?.becomesKeyOnlyIfNeeded, true) + XCTAssertEqual(window.tabbingMode, .disallowed) + } + + @MainActor + func test_contentViewHierarchy_containsNoFocusableTableView() throws { + let controller = SuggestionController() + let window = try XCTUnwrap(controller.window) + + controller.model.items = [PanelStubEntry(label: "SELECT"), PanelStubEntry(label: "SET")] + window.setContentSize(NSSize(width: 300, height: 200)) + window.orderFrontRegardless() + defer { window.close() } + RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.2)) + + let contentView = try XCTUnwrap(window.contentView) + XCTAssertNil(firstFocusableTableView(in: contentView)) + } + + @MainActor + func test_showWindowAttachedTo_preservesParentFirstResponder() throws { + let (parentWindow, textViewController) = Mock.windowedTextViewController(theme: Mock.theme()) + let controller = SuggestionController() + defer { + controller.close() + parentWindow.close() + } + + parentWindow.orderFrontRegardless() + let textView = try XCTUnwrap(textViewController.textView) + XCTAssertTrue(parentWindow.makeFirstResponder(textView)) + + controller.model.items = [PanelStubEntry(label: "SELECT")] + controller.showWindow(attachedTo: parentWindow) + RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.2)) + + XCTAssertTrue(parentWindow.firstResponder === textView) + } + + @MainActor + func test_showWindowAttachedTo_reattachesFromPreviousParent() throws { + let (firstParent, _) = Mock.windowedTextViewController(theme: Mock.theme()) + let (secondParent, _) = Mock.windowedTextViewController(theme: Mock.theme()) + let controller = SuggestionController() + let window = try XCTUnwrap(controller.window) + defer { + controller.close() + firstParent.close() + secondParent.close() + } + + controller.showWindow(attachedTo: firstParent) + XCTAssertTrue(window.parent === firstParent) + + controller.showWindow(attachedTo: secondParent) + XCTAssertTrue(window.parent === secondParent) + XCTAssertFalse(firstParent.childWindows?.contains(window) ?? false) + } + + @MainActor + func test_close_detachesPanelFromParent() throws { + let (parentWindow, _) = Mock.windowedTextViewController(theme: Mock.theme()) + let controller = SuggestionController() + let window = try XCTUnwrap(controller.window) + defer { parentWindow.close() } + + controller.showWindow(attachedTo: parentWindow) + XCTAssertTrue(window.parent === parentWindow) + + controller.close() + + XCTAssertNil(window.parent) + XCTAssertFalse(parentWindow.childWindows?.contains(window) ?? false) + } + + @MainActor + private func firstFocusableTableView(in view: NSView) -> NSTableView? { + if let tableView = view as? NSTableView { + return tableView + } + for subview in view.subviews { + if let found = firstFocusableTableView(in: subview) { + return found + } + } + return nil + } +} + +private struct PanelStubEntry: CodeSuggestionEntry { + var label: String + var detail: String? { nil } + var documentation: String? { nil } + var pathComponents: [String]? { nil } + var targetPosition: CursorPosition? { nil } + var sourcePreview: String? { nil } + var image: Image { Image(systemName: "circle") } + var imageColor: Color { .gray } + var deprecated: Bool { false } +} diff --git a/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/CodeSuggestion/SuggestionShowCompletionsGuardTests.swift b/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/CodeSuggestion/SuggestionShowCompletionsGuardTests.swift new file mode 100644 index 000000000..1afbfe4dc --- /dev/null +++ b/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/CodeSuggestion/SuggestionShowCompletionsGuardTests.swift @@ -0,0 +1,106 @@ +import AppKit +@testable import CodeEditSourceEditor +import SwiftUI +import XCTest + +final class SuggestionShowCompletionsGuardTests: XCTestCase { + @MainActor + func test_showCompletions_dropsPresentationWhenFirstResponderChangedDuringFetch() async throws { + let (window, textViewController) = Mock.windowedTextViewController(theme: Mock.theme()) + defer { window.close() } + window.orderFrontRegardless() + let textView = try XCTUnwrap(textViewController.textView) + XCTAssertTrue(window.makeFirstResponder(textView)) + + let model = SuggestionViewModel() + let delegate = DelayedStubDelegate(items: [GuardStubEntry(label: "SELECT")]) + + var presentationCount = 0 + model.showCompletions( + textView: textViewController, + delegate: delegate, + cursorPosition: CursorPosition(range: NSRange(location: 0, length: 0)) + ) { _, _ in + presentationCount += 1 + } + + window.makeFirstResponder(nil) + await model.itemsRequestTask?.value + + XCTAssertEqual(presentationCount, 0) + } + + @MainActor + func test_showCompletions_presentsWhenEditorStillFocused() async throws { + let (window, textViewController) = Mock.windowedTextViewController(theme: Mock.theme()) + defer { window.close() } + window.makeKeyAndOrderFront(nil) + RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.1)) + guard window.isKeyWindow else { + throw XCTSkip("Window cannot become key in this environment") + } + + let textView = try XCTUnwrap(textViewController.textView) + XCTAssertTrue(window.makeFirstResponder(textView)) + + let model = SuggestionViewModel() + let delegate = DelayedStubDelegate(items: [GuardStubEntry(label: "SELECT")]) + + var presentedWindows: [NSWindow] = [] + model.showCompletions( + textView: textViewController, + delegate: delegate, + cursorPosition: CursorPosition(range: NSRange(location: 0, length: 0)) + ) { parentWindow, _ in + presentedWindows.append(parentWindow) + } + + await model.itemsRequestTask?.value + + XCTAssertEqual(presentedWindows.count, 1) + XCTAssertTrue(presentedWindows.first === window) + } +} + +@MainActor +private final class DelayedStubDelegate: CodeSuggestionDelegate { + let items: [CodeSuggestionEntry] + + init(items: [CodeSuggestionEntry]) { + self.items = items + } + + func completionSuggestionsRequested( + textView: TextViewController, + cursorPosition: CursorPosition, + isManualTrigger: Bool + ) async -> (windowPosition: CursorPosition, items: [CodeSuggestionEntry])? { + try? await Task.sleep(for: .milliseconds(50)) + return (windowPosition: cursorPosition, items: items) + } + + func completionOnCursorMove( + textView: TextViewController, + cursorPosition: CursorPosition + ) -> [CodeSuggestionEntry]? { + nil + } + + func completionWindowApplyCompletion( + item: CodeSuggestionEntry, + textView: TextViewController, + cursorPosition: CursorPosition? + ) {} +} + +private struct GuardStubEntry: CodeSuggestionEntry { + var label: String + var detail: String? { nil } + var documentation: String? { nil } + var pathComponents: [String]? { nil } + var targetPosition: CursorPosition? { nil } + var sourcePreview: String? { nil } + var image: Image { Image(systemName: "circle") } + var imageColor: Color { .gray } + var deprecated: Bool { false } +} diff --git a/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/Mock.swift b/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/Mock.swift index 58b6b9544..b25e4ae8a 100644 --- a/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/Mock.swift +++ b/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/Mock.swift @@ -66,6 +66,20 @@ enum Mock { ) } + @MainActor + static func windowedTextViewController(theme: EditorTheme) -> (NSWindow, TextViewController) { + let controller = textViewController(theme: theme) + let window = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: 400, height: 300), + styleMask: [.titled, .closable, .resizable], + backing: .buffered, + defer: false + ) + window.isReleasedWhenClosed = false + window.contentViewController = controller + return (window, controller) + } + static func theme() -> EditorTheme { EditorTheme( text: EditorTheme.Attribute(color: .textColor), diff --git a/TablePro/Views/Editor/SQLEditorView.swift b/TablePro/Views/Editor/SQLEditorView.swift index 35686f293..8e0524109 100644 --- a/TablePro/Views/Editor/SQLEditorView.swift +++ b/TablePro/Views/Editor/SQLEditorView.swift @@ -63,6 +63,7 @@ struct SQLEditorView: View { completionDelegate: completionAdapter ) .accessibilityLabel(String(localized: "SQL query editor")) + .accessibilityIdentifier("sql-editor-textview") .onChange(of: editorState.cursorPositions) { _, newValue in guard let positions = newValue else { return } // Skip cursor propagation when the editor doesn't have focus diff --git a/TableProUITests/EditorAutocompleteFocusUITests.swift b/TableProUITests/EditorAutocompleteFocusUITests.swift new file mode 100644 index 000000000..2ef6ea562 --- /dev/null +++ b/TableProUITests/EditorAutocompleteFocusUITests.swift @@ -0,0 +1,60 @@ +import XCTest + +final class EditorAutocompleteFocusUITests: XCTestCase { + override func setUpWithError() throws { + continueAfterFailure = false + } + + override func tearDownWithError() throws { + XCUIApplication().terminate() + } + + func testTypingInNewTabKeepsEditorFocusWhileAutocompleteAppears() throws { + let app = XCUIApplication() + app.launchEnvironment["TABLEPRO_UI_TESTING"] = "1" + app.launch() + + let menuBar = app.menuBars.firstMatch + XCTAssertTrue(menuBar.waitForExistence(timeout: 10)) + menuBar.menuBarItems["File"].click() + let openSample = menuBar.menuItems["Open Sample Database"] + XCTAssertTrue(openSample.waitForExistence(timeout: 5)) + openSample.click() + + let firstEditor = editorTextView(in: app) + XCTAssertTrue(firstEditor.waitForExistence(timeout: 15)) + + app.typeKey("t", modifierFlags: .command) + + let editor = editorTextView(in: app) + XCTAssertTrue(editor.waitForExistence(timeout: 10)) + XCTAssertTrue(waitForValue("", in: editor, timeout: 5), "New tab editor should start empty") + + app.typeText("select") + + XCTAssertTrue( + waitForValue("select", in: editor, timeout: 5), + "All typed characters must land in the editor; got '\(editor.value as? String ?? "nil")'" + ) + } + + private func editorTextView(in app: XCUIApplication) -> XCUIElement { + let window = app.windows.firstMatch + let identified = window.textViews.matching(identifier: "sql-editor-textview").firstMatch + if identified.exists { + return identified + } + return window.textViews.firstMatch + } + + private func waitForValue(_ expected: String, in element: XCUIElement, timeout: TimeInterval) -> Bool { + let deadline = Date(timeIntervalSinceNow: timeout) + while Date() < deadline { + if (element.value as? String) == expected { + return true + } + RunLoop.current.run(until: Date(timeIntervalSinceNow: 0.1)) + } + return (element.value as? String) == expected + } +}