fix(windows): leave a connection window's first focus to its tab so Cmd+W closes the tab

CHANGELOG.md

@@ -82,6 +82,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Rows saved, added or deleted on iPhone and iPad refused by a MySQL, MariaDB, PostgreSQL or Redshift server that starts sessions read-only.
 - Copying objects into a connection that already has a transaction open committing it.
 - Replace-copy into a remote libSQL target failing at `BEGIN`.
+- `Cmd+W` closing the whole connection instead of the current tab until something in the window was clicked.
 
 ### Security
 

TablePro/Core/Services/Infrastructure/MainSplitViewController+Focus.swift

@@ -17,6 +17,19 @@ import TableProTextEngine
 /// a window hosting several connections has several editors registered at once, and a window-wide
 /// registry cannot tell them apart.
 internal extension MainSplitViewController {
+    /// What the window names as its `initialFirstResponder`: the container the selected tab's content
+    /// is shown in.
+    ///
+    /// AppKit picks a first responder once, as the window is first placed on screen, and only from
+    /// the views that exist at that moment. The editor, the grid and the object list are SwiftUI and
+    /// do not exist yet, so left to itself AppKit took the first key view it could find, which was
+    /// the connections strip, and Command W then closed the connection instead of the tab. The
+    /// container takes no focus itself, so the window keeps it, and the content adopts it once it is
+    /// built (`SQLEditorCoordinator`, `SidebarOutlineView`), whatever else the window holds by then.
+    var initialFirstResponderContainer: NSView {
+        detailPaneHost.view
+    }
+
     @discardableResult
     func focusQueryEditor() -> Bool {
         guard let textView = mountedQueryEditor, let window = view.window else { return false }

TablePro/Core/Services/Infrastructure/MainSplitViewController.swift

@@ -85,7 +85,7 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan
     private var navigationSidebar: NavigationSidebarViewController!
     /// Stable containers, one per split item. The pane they show is the selected workspace's own,
     /// so switching connection is a view swap and every other connection's tree stays built.
-    private var detailPaneHost: WorkspacePaneHost!
+    internal private(set) var detailPaneHost: WorkspacePaneHost!
     private var inspectorPaneHost: WorkspacePaneHost!
 
     /// The editor tab strip's band. It is a titlebar accessory rather than a split item, so it is

TablePro/Core/Services/Infrastructure/NavigationSidebarViewController.swift

@@ -71,6 +71,7 @@ internal final class NavigationSidebarViewController: NSViewController {
         ])
 
         separator.isHidden = true
+        rail.isHidden = true
     }
 
     /// The width the sidebar needs on top of the object browser's own minimum. Read from the
@@ -81,10 +82,24 @@ internal final class NavigationSidebarViewController: NSViewController {
         railWidthConstraint.constant + separatorWidthConstraint.constant
     }
 
+    /// A collapsed strip is hidden, not only zero points wide. AppKit counts a zero-width view as
+    /// visible, so the strip's list stayed a key view: it was what the window focused when first
+    /// shown, Tab could land on it, and its own Close then took the whole connection on a Command W
+    /// meant for a tab. Hiding it takes it out of the key view loop, the responder chain and the
+    /// accessibility tree.
+    ///
+    /// It is shown before it grows and hidden once it has shrunk, so the animation stays visible.
+    /// The keyboard moves on as the collapse starts, because a list that is about to vanish must
+    /// not keep answering keys for the whole of the animation.
     internal func setRailVisible(_ visible: Bool, animated: Bool, alongside: (() -> Void)? = nil) {
         guard isRailVisible != visible else { return }
         isRailVisible = visible
         separator.isHidden = !visible
+        if visible {
+            railController.view.isHidden = false
+        } else {
+            handKeyboardOnFromRail()
+        }
         applyRailWidth(animated: animated, alongside: alongside)
     }
 
@@ -96,19 +111,44 @@ internal final class NavigationSidebarViewController: NSViewController {
         let separatorWidth: CGFloat = isRailVisible ? 1 : 0
         guard railWidthConstraint.constant != width else {
             alongside?()
+            hideRailIfCollapsed()
             return
         }
         guard animated, view.window != nil else {
             railWidthConstraint.constant = width
             separatorWidthConstraint.constant = separatorWidth
             alongside?()
+            hideRailIfCollapsed()
             return
         }
         NSAnimationContext.runAnimationGroup { context in
             context.duration = 0.15
             railWidthConstraint.animator().constant = width
             separatorWidthConstraint.animator().constant = separatorWidth
             alongside?()
+        } completionHandler: { [weak self] in
+            self?.hideRailIfCollapsed()
         }
     }
+
+    /// Asked again when a collapse finishes, because the strip can have been shown again while it
+    /// was still shrinking.
+    private func hideRailIfCollapsed() {
+        guard !isRailVisible else { return }
+        railController.view.isHidden = true
+    }
+
+    /// Moves the keyboard to the next key view, which is what AppKit does itself when a focused
+    /// view is hidden, only at the start of the collapse rather than the end. Leaving it with the
+    /// window instead would make every key beep until the next click. The window takes it only
+    /// when nothing else in the loop can.
+    private func handKeyboardOnFromRail() {
+        guard let window = view.window,
+              let responder = window.firstResponder as? NSView,
+              responder.isDescendant(of: railController.view)
+        else { return }
+        window.selectKeyView(following: responder)
+        guard let next = window.firstResponder as? NSView, next.isDescendant(of: railController.view) else { return }
+        window.makeFirstResponder(nil)
+    }
 }

TablePro/Core/Services/Infrastructure/TabWindowController.swift

@@ -21,6 +21,16 @@ private final class EditorWindow: NSWindow, NSDraggingDestination {
         super.performClose(sender)
     }
 
+    /// The window's first focus belongs to the tab it shows. AppKit reads `initialFirstResponder`
+    /// once, as the window is first placed on screen, so it is named as soon as the content that
+    /// owns the answer is installed.
+    override var contentViewController: NSViewController? {
+        didSet {
+            initialFirstResponder = (contentViewController as? MainSplitViewController)?
+                .initialFirstResponderContainer
+        }
+    }
+
     /// Hiding the toolbar is what drops the content pane's top safe area, so the titlebar has to be
     /// reconsidered every time the user sends this from View > Show Toolbar.
     override func toggleToolbarShown(_ sender: Any?) {

TableProTests/Core/Services/Infrastructure/ConnectionWindowInitialFocusTests.swift

@@ -0,0 +1,151 @@
+import AppKit
+import Foundation
+@testable import TablePro
+import Testing
+
+/// Command W closed the whole connection instead of the current tab in a window nobody had clicked
+/// into yet, whether it was restored at launch or opened fresh.
+///
+/// AppKit gives a window its first responder once, as the window is first placed on screen, and
+/// only from the views that exist at that moment. The editor, the grid and the object list are
+/// SwiftUI and did not exist yet, so the pick fell to the connections strip's list, which was
+/// collapsed to zero width but never hidden. The strip answers Close itself, so Command W took the
+/// connection.
+@Suite("Connection window initial focus", .serialized)
+@MainActor
+struct ConnectionWindowInitialFocusTests {
+    @Test("A connections strip that has never been shown is not a key view")
+    func unshownStripIsNotAKeyView() {
+        let host = SidebarHost()
+        defer { host.tearDown() }
+
+        #expect(host.rail.isHidden)
+        #expect(host.rail.firstKeyViewDescendant == nil)
+    }
+
+    @Test("A shown connections strip can take the keyboard")
+    func shownStripIsAKeyView() {
+        let host = SidebarHost()
+        defer { host.tearDown() }
+
+        host.sidebar.setRailVisible(true, animated: false)
+
+        #expect(!host.rail.isHidden)
+        #expect(host.rail.firstKeyViewDescendant != nil)
+    }
+
+    @Test("A collapsed connections strip leaves the key view loop")
+    func collapsedStripLeavesTheKeyViewLoop() {
+        let host = SidebarHost()
+        defer { host.tearDown() }
+
+        host.sidebar.setRailVisible(true, animated: false)
+        host.sidebar.setRailVisible(false, animated: false)
+
+        #expect(host.rail.isHidden)
+        #expect(host.rail.firstKeyViewDescendant == nil)
+    }
+
+    /// The strip collapses whenever the app-wide entry count drops to one, which closing another
+    /// connection is enough to do, so the list can be holding the keyboard when it goes.
+    @Test("Collapsing the connections strip lets go of the keyboard at once")
+    func collapsingStripLetsGoOfTheKeyboard() throws {
+        let host = SidebarHost()
+        defer { host.tearDown() }
+
+        host.sidebar.setRailVisible(true, animated: false)
+        let list = try #require(host.rail.firstKeyViewDescendant)
+        #expect(host.window.makeFirstResponder(list))
+
+        host.sidebar.setRailVisible(false, animated: true)
+
+        let responder = host.window.firstResponder as? NSView
+        #expect(responder?.isDescendant(of: host.rail) != true)
+    }
+
+    /// Leaving the keyboard with the window itself would make every key beep until the next click,
+    /// so it goes where AppKit sends it when a focused view is hidden: the next key view.
+    @Test("Collapsing the connections strip hands the keyboard to the next key view")
+    func collapsingStripHandsTheKeyboardOn() throws {
+        let host = SidebarHost()
+        defer { host.tearDown() }
+        let field = NSTextField(frame: NSRect(x: 400, y: 200, width: 120, height: 22))
+        host.sidebar.view.addSubview(field)
+
+        host.sidebar.setRailVisible(true, animated: false)
+        let list = try #require(host.rail.firstKeyViewDescendant)
+        #expect(host.window.makeFirstResponder(list))
+
+        host.sidebar.setRailVisible(false, animated: true)
+
+        #expect(host.window.firstResponder !== host.window)
+        let responder = host.window.firstResponder as? NSView
+        #expect(responder?.isDescendant(of: host.rail) == false)
+    }
+
+    /// The strip on screen at first show is the case hiding it cannot reach: two restored
+    /// connections, or a connection opened while another is already open.
+    @Test("A connection window leaves its first focus to the tab content, with the strip on screen")
+    func firstFocusIsLeftForTheContent() throws {
+        let connection = TestFixtures.makeConnection(name: "Initial focus")
+        let workspace = ConnectionWorkspace(
+            connectionId: connection.id,
+            payload: nil,
+            autoConnect: false,
+            payloadConnection: connection,
+            session: nil,
+            sessionState: nil,
+            trailingPaneState: nil,
+            phase: .connecting
+        )
+        let window = TabWindowController.makeEditorWindow()
+        window.isReleasedWhenClosed = false
+        let split = MainSplitViewController(payload: nil, sessionState: nil, adopting: workspace)
+        window.contentViewController = split
+        defer {
+            window.orderOut(nil)
+            window.contentViewController = nil
+            workspace.teardown()
+        }
+
+        let previous = AppSettingsManager.shared.general.showWorkspaceRail
+        AppSettingsManager.shared.general.showWorkspaceRail = true
+        defer { AppSettingsManager.shared.general.showWorkspaceRail = previous }
+        split.applyRailVisibility(workspaceCount: 2)
+        try #require(split.isWorkspaceRailVisible, "The strip has to be on screen for this to test anything")
+
+        window.orderFront(nil)
+
+        #expect(window.initialFirstResponder === split.initialFirstResponderContainer)
+        #expect(window.firstResponder === window)
+    }
+
+    @MainActor
+    private struct SidebarHost {
+        let sidebar: NavigationSidebarViewController
+        let window: NSWindow
+
+        var rail: NSView {
+            sidebar.railController.view
+        }
+
+        init() {
+            sidebar = NavigationSidebarViewController()
+            window = NSWindow(
+                contentRect: NSRect(x: 0, y: 0, width: 600, height: 400),
+                styleMask: [.titled],
+                backing: .buffered,
+                defer: false
+            )
+            window.isReleasedWhenClosed = false
+            window.keepsKeyViewLoopCurrent()
+            window.contentViewController = sidebar
+            window.orderFront(nil)
+        }
+
+        func tearDown() {
+            window.orderOut(nil)
+            window.contentViewController = nil
+        }
+    }
+}