fix(connections): keep the connections strip and its commands when a connection is down (#2551)

Author: datlechin

CHANGELOG.md

@@ -72,6 +72,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Connections strip not scrolling to the entry you switch to.
 - Background connections stuck on the session preparation screen after connecting. (#2545)
 - Blank sidebar, grid and inspector on a connection opened into a window that was already on screen.
+- Connections strip hidden, with Switch Connection and the Show Next and Previous Connection items disabled, whenever the connection on screen was not connected.
+- Choosing a connection from the connection list re-fronting its window without switching to it.
 - Grid cells left at the old column positions until the next click, after a resize, an auto-fit, a reorder, hiding a column, or a row-number width change. (#2449, #2446)
 - The row-number column draggable out of first place, which walked it to the far right on the next refresh.
 - A time entered into a date cell discarded when the stored value carried no time.

TablePro/Core/Services/Infrastructure/ConnectionWindowPaneResolver.swift

@@ -12,6 +12,16 @@ internal enum ConnectionWindowPane: Equatable {
     case empty
 }
 
+/// What the window's one sidebar item holds. `railOnly` is the state that exists because the
+/// workspace rail and the object browser share that item and answer to different owners.
+internal enum SidebarChromeMode: Equatable {
+    case revealed
+    case railOnly
+    case hidden
+
+    internal var showsObjectBrowser: Bool { self == .revealed }
+}
+
 internal enum ConnectionWindowPaneResolver {
     internal static func pane(
         phase: ConnectionWindowPhase,
@@ -33,8 +43,8 @@ internal enum ConnectionWindowPaneResolver {
         }
     }
 
-    /// A sidebar and an inspector with nothing to put in them are not chrome, they are two empty
-    /// columns that promise a session the window does not have yet.
+    /// An object browser and an inspector with nothing to put in them are not chrome, they are two
+    /// empty columns that promise a session the window does not have yet.
     internal static func hidesChrome(for pane: ConnectionWindowPane) -> Bool {
         switch pane {
         case .content:
@@ -44,6 +54,21 @@ internal enum ConnectionWindowPaneResolver {
         }
     }
 
+    /// How much of the window's sidebar survives the pane it is standing next to.
+    ///
+    /// The rule above is right about the object browser and wrong about the workspace rail, which
+    /// lists every connection the window hosts and belongs to the window rather than to any one of
+    /// them. They share a split item because AppKit grants full-height sidebar layout to exactly one
+    /// leading sidebar, so collapsing for an empty object browser took the switcher with it and left
+    /// the window's other connections with no way in.
+    internal static func sidebarChromeMode(
+        for pane: ConnectionWindowPane,
+        hasRail: Bool
+    ) -> SidebarChromeMode {
+        guard hidesChrome(for: pane) else { return .revealed }
+        return hasRail ? .railOnly : .hidden
+    }
+
     /// The tab strip's band is a list of tabs, so it appears only when there is a list worth
     /// showing: content behind it, and more than one tab in it. A window with a single tab keeps
     /// the chrome it always had, which is what the system does too.

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

@@ -229,9 +229,15 @@ extension MainSplitViewController: NSMenuItemValidation {
         }
     }
 
+    /// The workspace-rail facts come from the window in both branches. They are true of the window,
+    /// not of the connection it happens to be showing, and reading them off a connection that has
+    /// no coordinator left disabled the only menu route to the window's other connections.
     var menuValidationContext: MenuValidationContext {
         guard let actions = commandActions else {
-            return MenuValidationContext(hasSelectedWorkspace: workspaces.selectedConnectionId != nil)
+            return MenuValidationContext(
+                hasSelectedWorkspace: workspaces.selectedConnectionId != nil,
+                canToggleWorkspaceRail: canToggleWorkspaceRail
+            )
         }
         return MenuValidationContext(
             hasSelectedWorkspace: workspaces.selectedConnectionId != nil,
@@ -259,7 +265,7 @@ extension MainSplitViewController: NSMenuItemValidation {
             canNavigateForward: actions.canNavigateForward,
             canSaveAsFavorite: actions.canSaveAsFavorite,
             canSwitchSidebarLayout: actions.canSwitchSidebarLayout,
-            canToggleWorkspaceRail: actions.canToggleWorkspaceRail,
+            canToggleWorkspaceRail: canToggleWorkspaceRail,
             canShowTableStructure: actions.canShowTableStructure,
             canEditViewDefinition: actions.canEditViewDefinition,
             canCreateDatabase: actions.canCreateDatabase,

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

@@ -10,12 +10,16 @@ extension MainSplitViewController {
         toggleWorkspaceRail()
     }
 
+    /// Switching which connection the window shows is the window's own business, so it acts on the
+    /// registry directly the way `toggleWorkspaceRail(_:)` already does. Routing it through the
+    /// selected connection's `commandActions` made it a no-op exactly when it was needed: that
+    /// connection losing its session is what leaves the others unreachable.
     @objc func showPreviousWorkspace(_ sender: Any?) {
-        commandActions?.showPreviousWorkspace()
+        activateWorkspace(offsetBy: -1)
     }
 
     @objc func showNextWorkspace(_ sender: Any?) {
-        commandActions?.showNextWorkspace()
+        activateWorkspace(offsetBy: 1)
     }
 
     @objc func setResultView(_ sender: Any?) {

TablePro/Core/Services/Infrastructure/MainSplitViewController.swift

@@ -104,8 +104,6 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi
     var tabStripObservationIsArmed = false
     var tabStripObservedManager: ObjectIdentifier?
 
-    private var chromeState: ChromeState = .unapplied
-
     // MARK: - Panel Layout State
 
     /// One name for the window's split view, because one `NSSplitView` can only carry one.
@@ -253,6 +251,10 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi
         navigationSidebar.railController.host = self
         navigationSidebar.railController.onLayoutChange = { [weak self] _ in
             self?.navigationSidebar.applyRailWidth(animated: false)
+            /// The row size is a setting, so the rail's own width changes under a sidebar already
+            /// narrowed to it. Reapplying the clamp is what moves both thicknesses onto the new
+            /// allowance rather than clipping the rail against the old one.
+            self?.reapplySidebarClampIfNarrowed()
             self?.recomputeWindowMinSize()
         }
         sidebarSplitItem = NSSplitViewItem(sidebarWithViewController: navigationSidebar)
@@ -935,6 +937,11 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi
         navigationSidebar.setRailVisible(visible, animated: view.window != nil) { [weak self] in
             self?.recomputeWindowMinSize()
         }
+        /// The rail appearing or going is a chrome change of its own, and it happens without the
+        /// selected connection's phase moving: a sibling opening or closing is enough. Without this
+        /// a window whose connection is down keeps a fully collapsed sidebar when the rail arrives,
+        /// or an empty clamped column after it leaves.
+        applyPaneChrome()
     }
 
     func activateWorkspace(offsetBy offset: Int) {
@@ -943,8 +950,25 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi
 
     // MARK: - Sidebar
 
+    /// Whether the object browser is off screen, which is the question every caller is really
+    /// asking: the toolbar's segment, the Show/Hide Sidebar title and the reveal actions. A sidebar
+    /// narrowed to the workspace rail is an open split item with no object browser in it, so the
+    /// item's own flag is not the answer on its own.
     var isSidebarCollapsed: Bool {
-        sidebarSplitItem?.isCollapsed ?? true
+        guard sidebarChromeMode.showsObjectBrowser else { return true }
+        return sidebarSplitItem?.isCollapsed ?? true
+    }
+
+    var isSidebarUserCollapsible: Bool {
+        sidebarSplitItem?.canCollapse ?? false
+    }
+
+    var sidebarThicknessRange: (min: CGFloat, max: CGFloat) {
+        (sidebarSplitItem?.minimumThickness ?? 0, sidebarSplitItem?.maximumThickness ?? 0)
+    }
+
+    var railAllowance: CGFloat {
+        navigationSidebar?.railAllowance ?? 0
     }
 
     /// Every collapse route reaches AppKit's own `toggleSidebar(_:)`: the View menu sends the
@@ -957,13 +981,15 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi
     }
 
     func focusSidebarSearch() {
+        guard sidebarChromeMode.showsObjectBrowser else { return }
         if sidebarSplitItem?.isCollapsed == true {
             sidebarSplitItem?.animator().isCollapsed = false
         }
         navigationSidebar.objectBrowser.focusSearchField()
     }
 
     func presentDatabaseFilter() {
+        guard sidebarChromeMode.showsObjectBrowser else { return }
         guard let connectionId = currentSession?.connection.id else { return }
         if sidebarSplitItem?.isCollapsed == true {
             sidebarSplitItem?.isCollapsed = false
@@ -981,7 +1007,11 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi
         state.databaseFilterSelected = []
     }
 
+    /// Refused while the sidebar is narrowed to the workspace rail. The item is open, so the
+    /// collapse branch below would read it as showing and collapse it, taking the rail and every
+    /// route to the window's other connections with it.
     func setSidebarTab(_ tab: SidebarTab) {
+        guard sidebarChromeMode.showsObjectBrowser else { return }
         guard let connectionId = currentSession?.connection.id else { return }
         let sidebarState = SharedSidebarState.forConnection(connectionId)
 
@@ -1062,8 +1092,12 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi
         )
     }
 
+    /// Inert while the sidebar is clamped to the rail. Seven other call sites reach
+    /// `recomputeWindowMinSize`, and any of them writing the object browser's minimum back over the
+    /// clamp would leave a minimum above the maximum.
     private func applySidebarMinimumThickness() {
         guard let sidebarSplitItem else { return }
+        guard appliedSidebarMode ?? .revealed == .revealed else { return }
         let resolved = Self.resolveSidebarMinimumThickness(
             railAllowance: navigationSidebar?.railAllowance ?? 0
         )
@@ -1108,52 +1142,99 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi
 
     // MARK: - Pane Chrome
 
-    private enum ChromeState {
-        case unapplied
-        case hidden
-        case revealed
-    }
-
     private var userPaneLayout: ChromePaneLayout?
 
+    /// What this window's sidebar is currently showing, and `nil` before the first application.
+    /// The resolver decides the mode; this records which one has been put on screen.
+    private var appliedSidebarMode: SidebarChromeMode?
+
+    internal var sidebarChromeMode: SidebarChromeMode {
+        ConnectionWindowPaneResolver.sidebarChromeMode(
+            for: currentPane,
+            hasRail: navigationSidebar?.isRailVisible ?? false
+        )
+    }
+
     /// A split item's collapse state is written into the autosave record, which is how the
     /// inspector remembers being hidden. Collapsing the sidebar for a phase the user did not
     /// choose would persist that as their layout and lose the width they set, so autosaving is
-    /// switched off for the whole span the chrome is hidden and switched back on to restore it.
+    /// switched off for the whole span the chrome is not revealed and switched back on to restore
+    /// it. The same is true of a clamp, which writes its narrow width into the record the way a
+    /// collapse writes the collapsed flag.
     func applyPaneChrome() {
-        if ConnectionWindowPaneResolver.hidesChrome(for: currentPane) {
-            hideWindowChrome()
-        } else {
-            revealWindowChrome()
-        }
+        applySidebarChromeMode(sidebarChromeMode)
         applyTabStripVisibility()
         toolbarOwner?.managedToolbar.validateVisibleItems()
         recomputeWindowMinSize()
     }
 
-    private func hideWindowChrome() {
-        guard chromeState != .hidden else { return }
-        chromeState = .hidden
+    private func applySidebarChromeMode(_ mode: SidebarChromeMode) {
+        guard appliedSidebarMode != mode else { return }
+        let previous = appliedSidebarMode
+        appliedSidebarMode = mode
+
+        guard mode != .revealed else {
+            revealWindowChrome()
+            return
+        }
+
+        /// Captured on the way out of `revealed` and never again, because the geometry a
+        /// `railOnly` to `hidden` step would see is the clamp, not the width the user chose.
+        if previous == nil || previous == .revealed {
+            resignFirstResponderInsideChrome()
+            splitView.autosaveName = nil
+            userPaneLayout = ChromePaneLayout(
+                isSidebarCollapsed: sidebarSplitItem.isCollapsed,
+                isInspectorCollapsed: inspectorSplitItem.isCollapsed
+            )
+        }
 
-        resignFirstResponderInsideChrome()
-        splitView.autosaveName = nil
-        userPaneLayout = ChromePaneLayout(
-            isSidebarCollapsed: sidebarSplitItem.isCollapsed,
-            isInspectorCollapsed: inspectorSplitItem.isCollapsed
-        )
-        sidebarSplitItem.isCollapsed = true
         inspectorSplitItem.isCollapsed = true
+        switch mode {
+        case .railOnly:
+            sidebarSplitItem.isCollapsed = false
+            clampSidebarToRail()
+        case .hidden:
+            releaseSidebarClamp()
+            sidebarSplitItem.isCollapsed = true
+        case .revealed:
+            break
+        }
         view.window?.recalculateKeyViewLoop()
     }
 
-    /// Autosaving is off while the chrome is hidden, so the record still holds what the user had.
-    /// AppKit will not re-apply it though: assigning an autosave name to a split view that has
+    /// Narrowed rather than collapsed, so the rail stays on screen while the object browser it
+    /// shares a split item with goes. Measured: clamping and later releasing returns the item to
+    /// the width the user set, but a `setPosition` while the clamp holds discards it, which is why
+    /// nothing else may write the sidebar's thickness for the span.
+    private func clampSidebarToRail() {
+        let allowance = navigationSidebar?.railAllowance ?? 0
+        sidebarSplitItem.minimumThickness = allowance
+        sidebarSplitItem.maximumThickness = allowance
+        /// A clamp is not a lock. AppKit still collapses a collapsible item on a divider
+        /// double-click or a drag to the edge, which no menu or toolbar validation sees, and the
+        /// mode is already applied so nothing would open it again.
+        sidebarSplitItem.canCollapse = false
+    }
+
+    internal func reapplySidebarClampIfNarrowed() {
+        guard appliedSidebarMode == .railOnly else { return }
+        clampSidebarToRail()
+    }
+
+    private func releaseSidebarClamp() {
+        sidebarSplitItem.canCollapse = true
+        sidebarSplitItem.maximumThickness = Self.sidebarMaxThickness
+        applySidebarMinimumThickness()
+    }
+
+    /// Autosaving is off while the chrome is not revealed, so the record still holds what the user
+    /// had. AppKit will not re-apply it though: assigning an autosave name to a split view that has
     /// already laid out restores nothing. The state captured on the way in is therefore what gives
     /// the panes back. Forcing the sidebar open here instead reopened a sidebar the user had
     /// deliberately hidden, every time a connection dropped and came back.
     private func revealWindowChrome() {
-        guard chromeState != .revealed else { return }
-        chromeState = .revealed
+        releaseSidebarClamp()
 
         /// Only a reveal that follows a hide has something to put back. A first reveal is a window
         /// opening on a live connection, where the panes are already where the user's autosaved

TablePro/Core/Services/Infrastructure/MainWindowToolbar+Validation.swift

@@ -24,6 +24,11 @@ extension MainWindowToolbar: NSToolbarItemValidation {
         let supportsServerDashboard: Bool
         let canNavigateBack: Bool
         let canNavigateForward: Bool
+        /// Separate from `connected` because a connection that is still dialing counts as alive
+        /// while its sidebar is narrowed to the workspace rail, and a segment that toggles an
+        /// object browser the window is not showing has nothing to toggle. Defaulted, because a
+        /// context built for a connected pane is describing a window that has one.
+        var showsObjectBrowser = true
     }
 
     /// Listed exhaustively so a new state has to choose a side instead of inheriting "alive".
@@ -46,8 +51,10 @@ extension MainWindowToolbar: NSToolbarItemValidation {
             return true
         case Self.database:
             return context.connected && !context.fileBased && context.supportsContainerSwitching
-        case Self.refresh, Self.quickSwitcher, Self.newTab, Self.exportTables, Self.sidebarToggle:
+        case Self.refresh, Self.quickSwitcher, Self.newTab, Self.exportTables:
             return context.connected
+        case Self.sidebarToggle:
+            return context.connected && context.showsObjectBrowser
         case Self.addRow:
             return context.connected && context.canAddRow
         case Self.restorePreviousValues:
@@ -86,10 +93,16 @@ extension MainWindowToolbar: NSToolbarItemValidation {
             supportsImport: PluginManager.shared.supportsImport(for: state.databaseType),
             supportsServerDashboard: coordinator?.commandActions?.supportsServerDashboard ?? false,
             canNavigateBack: coordinator?.canNavigateBack ?? false,
-            canNavigateForward: coordinator?.canNavigateForward ?? false
+            canNavigateForward: coordinator?.canNavigateForward ?? false,
+            showsObjectBrowser: coordinator?.splitViewController?.sidebarChromeMode.showsObjectBrowser ?? false
         )
     }
 
+    /// No subject disables the whole toolbar, Switch Connection included. Every item here needs the
+    /// coordinator that presents it, so enabling one without a subject would leave a live-looking
+    /// button that does nothing. The routes to a window's other connections that survive a
+    /// connection going down are the connections strip and the View menu, neither of which asks a
+    /// coordinator anything.
     func validateToolbarItem(_ item: NSToolbarItem) -> Bool {
         guard let context = validationContext() else { return false }
         return Self.isEnabled(itemIdentifier: item.itemIdentifier, context: context)

TablePro/Core/Services/Infrastructure/SidebarContainerViewController.swift

@@ -49,10 +49,17 @@ internal final class SidebarContainerViewController: NSViewController {
         view.addSubview(hostingView)
         searchField.nextKeyView = hostingView
 
+        /// The insets are a margin, not an invariant, so they yield rather than break when the
+        /// window narrows the sidebar to the workspace rail and leaves this view no width at all.
+        let searchLeading = searchField.leadingAnchor.constraint(equalTo: view.leadingAnchor, constant: 10)
+        let searchTrailing = searchField.trailingAnchor.constraint(equalTo: view.trailingAnchor, constant: -10)
+        searchLeading.priority = .defaultHigh
+        searchTrailing.priority = .defaultHigh
+
         NSLayoutConstraint.activate([
             searchField.topAnchor.constraint(equalTo: view.safeAreaLayoutGuide.topAnchor, constant: 5),
-            searchField.leadingAnchor.constraint(equalTo: view.leadingAnchor, constant: 10),
-            searchField.trailingAnchor.constraint(equalTo: view.trailingAnchor, constant: -10),
+            searchLeading,
+            searchTrailing,
 
             hostingView.topAnchor.constraint(equalTo: searchField.bottomAnchor, constant: 5),
             hostingView.leadingAnchor.constraint(equalTo: view.leadingAnchor),