From ce0262f2a3b70f8a8c1e1a33de3ef519eb94d68a Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 15 Aug 2026 07:16:14 +0700 Subject: [PATCH 1/4] fix(connections): make the rail's close command close the connection --- CHANGELOG.md | 2 +- TablePro/Core/Menu/FileMenuBuilder.swift | 4 + .../ConnectionCloseAction.swift | 73 ++++++++++++++++ ...nSplitViewController+FileMenuActions.swift | 7 ++ ...inSplitViewController+MenuValidation.swift | 2 +- .../WorkspaceRailViewController.swift | 83 +++++++++++++------ .../MainContentCommandActions+BulkClose.swift | 8 -- .../ConnectionCloseActionTests.swift | 45 ++++++++++ .../Services/WorkspaceRailStoreTests.swift | 24 ++++++ docs/features/workspace-rail.mdx | 4 +- 10 files changed, 216 insertions(+), 36 deletions(-) create mode 100644 TablePro/Core/Services/Infrastructure/ConnectionCloseAction.swift create mode 100644 TableProTests/Core/Services/Infrastructure/ConnectionCloseActionTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 00a3c6a15..1cb48b483 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,7 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Closing a connection removes it from the workspace rail, ends its session, and closes its tunnel. It stayed listed as connected, could not be clicked, and its database connection stayed open. -- Close Workspace on a rail row acts on the connection the row names. It acted on the connection on screen instead, closing its tabs and losing unsaved editor work. +- The workspace rail's close command ends the connection: every tab across every database it has open, its session, and every row it holds in the rail. It used to close only the tabs of one database and leave the row you clicked exactly where it was, which read as doing nothing. Disconnect still ends only the session and keeps the row. **File > Close Connection** does the same from the menu bar. - Closing a window disconnects every connection it was showing, not just one of them. - Hiding a database in the sidebar's database filter takes effect straight away. The tree kept listing every database until some other change happened to rebuild it. - An alert opens on the window you were working in. It could attach itself to a floating panel such as the Quick Switcher, which takes the alert with it when it closes. diff --git a/TablePro/Core/Menu/FileMenuBuilder.swift b/TablePro/Core/Menu/FileMenuBuilder.swift index 43b4e0c23..aa2eb383f 100644 --- a/TablePro/Core/Menu/FileMenuBuilder.swift +++ b/TablePro/Core/Menu/FileMenuBuilder.swift @@ -72,6 +72,10 @@ enum FileMenuBuilder { shortcut: .closeAllTabs, keyboard: keyboard ), + MenuItemFactory.item( + String(localized: "Close Connection"), + action: #selector(MainSplitViewController.closeConnection(_:)) + ), MenuItemFactory.item( String(localized: "Reopen Closed Tab"), action: #selector(AppDelegate.reopenClosedTab(_:)), diff --git a/TablePro/Core/Services/Infrastructure/ConnectionCloseAction.swift b/TablePro/Core/Services/Infrastructure/ConnectionCloseAction.swift new file mode 100644 index 000000000..6616c4016 --- /dev/null +++ b/TablePro/Core/Services/Infrastructure/ConnectionCloseAction.swift @@ -0,0 +1,73 @@ +// +// ConnectionCloseAction.swift +// TablePro +// + +import AppKit +import Foundation + +/// The one path a user-requested connection close takes, whatever surface asked for it. A peer of +/// `ConnectionDisconnectAction`, and the two are deliberately different: Disconnect ends the +/// session and leaves the connection in place to reconnect, Close ends the connection. +/// +/// The rail used to offer this as "Close Workspace", wired to the tab strip's bulk-close family, so +/// it closed a subset of one connection's tabs and left the row it was invoked on exactly where it +/// was. No platform or competitor precedent puts that scope on a list of open sessions: the HIG has +/// no close verb for a sidebar row at all, the one app shipping the literal string is Xcode's File +/// menu where it means the whole project session, and Finder's Locations rows, the true analogue of +/// a list of live remote sessions, end the session and drop the row. +@MainActor +internal enum ConnectionCloseAction { + internal enum Decision: Equatable { + case closeImmediately + case confirmUnsavedWork + } + + /// Pure so the case that used to fail silently is pinned by a test: a connection with no session + /// has nothing to lose, and asking about it produced an alert nobody could answer. + internal static func decision(hasSession: Bool, hasUnsavedWork: Bool) -> Decision { + guard hasSession, hasUnsavedWork else { return .closeImmediately } + return .confirmUnsavedWork + } + + internal static func close(connectionId: UUID) async { + let coordinator = WindowManager.shared.coordinator(for: connectionId) + let decision = decision( + hasSession: coordinator != nil, + hasUnsavedWork: coordinator?.hasAnyUnsavedWork() ?? false + ) + guard decision == .confirmUnsavedWork else { + WindowManager.shared.closeWindow(for: connectionId) + return + } + + /// Shown, then asked. A data-loss alert over a connection the user cannot see names work + /// they have no way to look at before answering. + let presentingWindow = reveal(connectionId: connectionId) + switch await AlertHelper.confirmSaveChanges( + message: String(localized: "Your changes will be lost if you don't save them."), + window: presentingWindow + ) { + case .save: + coordinator?.commandActions?.saveChanges() + case .dontSave: + WindowManager.shared.closeWindow(for: connectionId) + case .cancel: + break + } + } + + /// `hasAnyUnsavedWork` is coordinator state, so it answers for a connection whose content has + /// never been on screen. Acting on that answer needs the connection in front of the user first. + @discardableResult + private static func reveal(connectionId: UUID) -> NSWindow? { + guard let window = WindowManager.shared.window(for: connectionId), + let host = window.contentViewController as? MainSplitViewController else { return nil } + if let group = window.tabGroup, group.selectedWindow !== window { + group.selectedWindow = window + } + window.makeKeyAndOrderFront(nil) + host.workspaces.select(connectionId) + return window + } +} diff --git a/TablePro/Core/Services/Infrastructure/MainSplitViewController+FileMenuActions.swift b/TablePro/Core/Services/Infrastructure/MainSplitViewController+FileMenuActions.swift index c61e80108..3fb04873f 100644 --- a/TablePro/Core/Services/Infrastructure/MainSplitViewController+FileMenuActions.swift +++ b/TablePro/Core/Services/Infrastructure/MainSplitViewController+FileMenuActions.swift @@ -37,6 +37,13 @@ extension MainSplitViewController { actions.closeTab() } + /// The contextual menu on a rail row offers this too, and the HIG requires every context-menu + /// command to be reachable from the menu bar. + @objc func closeConnection(_ sender: Any?) { + guard let connectionId = workspaces.selectedConnectionId else { return } + Task { await ConnectionCloseAction.close(connectionId: connectionId) } + } + @objc func selectNextEditorTab(_ sender: Any?) { commandActions?.selectTab(offsetBy: 1) } diff --git a/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift b/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift index aa3a82d7c..832078c6a 100644 --- a/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift +++ b/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift @@ -92,7 +92,7 @@ extension MainSplitViewController: NSMenuItemValidation { /// ours to enable and disable now. case #selector(newEditorTab(_:)): return context.isConnected - case #selector(closeEditorTab(_:)): + case #selector(closeEditorTab(_:)), #selector(closeConnection(_:)): return context.hasSelectedWorkspace case #selector(selectNextEditorTab(_:)), #selector(selectPreviousEditorTab(_:)): return context.isConnected diff --git a/TablePro/Core/Services/Infrastructure/WorkspaceRailViewController.swift b/TablePro/Core/Services/Infrastructure/WorkspaceRailViewController.swift index 392d33d29..687b4553d 100644 --- a/TablePro/Core/Services/Infrastructure/WorkspaceRailViewController.swift +++ b/TablePro/Core/Services/Infrastructure/WorkspaceRailViewController.swift @@ -29,6 +29,26 @@ internal protocol WorkspaceRailHost: AnyObject { func selectHostedConnection(_ connectionId: UUID) } +/// Middle-click closes the row under the pointer, which is what a list of open things does +/// everywhere it exists: browser tabs, and every database client surveyed. `NSTableView` routes no +/// action for the tertiary button, so the row is resolved from the click point the same way +/// `menu(for:)` resolves one. +@MainActor +internal final class WorkspaceRailTableView: NSTableView { + internal var onMiddleClick: ((Int) -> Void)? + + override internal func otherMouseUp(with event: NSEvent) { + guard event.buttonNumber == 2 else { + super.otherMouseUp(with: event) + return + } + let point = convert(event.locationInWindow, from: nil) + let clicked = row(at: point) + guard clicked >= 0 else { return } + onMiddleClick?(clicked) + } +} + @MainActor internal final class WorkspaceRailViewController: NSViewController { private static let logger = Logger(subsystem: "com.TablePro", category: "WorkspaceRail") @@ -39,7 +59,7 @@ internal final class WorkspaceRailViewController: NSViewController { internal weak var host: (any WorkspaceRailHost)? private let scrollView = NSScrollView() - private let tableView = NSTableView() + private let tableView = WorkspaceRailTableView() /// `rowSizeStyle` is the only route to the sidebar icon size preference, but any value /// other than `.custom` makes the table impose the system row height and ignore @@ -93,6 +113,7 @@ internal final class WorkspaceRailViewController: NSViewController { tableView.menu = contextMenu() tableView.registerForDraggedTypes([Self.reorderType]) tableView.setDraggingSourceOperationMask(.move, forLocal: true) + tableView.onMiddleClick = { [weak self] row in self?.closeConnection(atRow: row) } tableView.setAccessibilityIdentifier("workspace-rail") tableView.setAccessibilityLabel(String(localized: "Open Workspaces")) @@ -393,16 +414,19 @@ internal final class WorkspaceRailViewController: NSViewController { return menu } - /// The connection the row names, not the one the window is showing. Resolving through the - /// window handed this to the selected connection's coordinator, so Close Workspace on a row for - /// another connection closed the visible connection's tabs in a container of the same name, - /// taking its unsaved editor work with them. @objc - private func closeWorkspace(_ sender: NSMenuItem) { - guard let workspace = sender.representedObject as? WorkspaceID, - let coordinator = WindowManager.shared.coordinator(for: workspace.connectionId) - else { return } - coordinator.commandActions?.closeWorkspace(container: workspace.container) + private func closeConnection(_ sender: NSMenuItem) { + guard let workspace = sender.representedObject as? WorkspaceID else { return } + close(connectionId: workspace.connectionId) + } + + private func closeConnection(atRow row: Int) { + guard entries.indices.contains(row) else { return } + close(connectionId: entries[row].workspace.connectionId) + } + + private func close(connectionId: UUID) { + Task { await ConnectionCloseAction.close(connectionId: connectionId) } } /// Ends the session, which every workspace of the connection shares, so the other rows for it @@ -429,29 +453,38 @@ internal final class WorkspaceRailViewController: NSViewController { // MARK: - NSMenuDelegate extension WorkspaceRailViewController: NSMenuDelegate { + /// Lighter action first, the one that ends the connection last, which is the order Finder uses + /// on a Locations row and Mail on an account. Close carries the connection's own name because + /// a row can be one of several a connection has open, and the command takes all of them. internal func menuNeedsUpdate(_ menu: NSMenu) { menu.removeAllItems() let row = tableView.clickedRow guard entries.indices.contains(row) else { return } + let entry = entries[row] + + if ConnectionMenuPolicy.showsDisconnect(status: entry.status) { + addItem( + to: menu, + title: String(localized: "Disconnect"), + action: #selector(disconnectWorkspace(_:)), + workspace: entry.workspace + ) + menu.addItem(.separator()) + } - let item = NSMenuItem( - title: String(localized: "Close Workspace"), - action: #selector(closeWorkspace(_:)), - keyEquivalent: "" + addItem( + to: menu, + title: String(format: String(localized: "Close “%@”"), entry.connection.name), + action: #selector(closeConnection(_:)), + workspace: entry.workspace ) + } + + private func addItem(to menu: NSMenu, title: String, action: Selector, workspace: WorkspaceID) { + let item = NSMenuItem(title: title, action: action, keyEquivalent: "") item.target = self - item.representedObject = entries[row].workspace + item.representedObject = workspace menu.addItem(item) - - guard ConnectionMenuPolicy.showsDisconnect(status: entries[row].status) else { return } - let disconnectItem = NSMenuItem( - title: String(localized: "Disconnect"), - action: #selector(disconnectWorkspace(_:)), - keyEquivalent: "" - ) - disconnectItem.target = self - disconnectItem.representedObject = entries[row].workspace - menu.addItem(disconnectItem) } } diff --git a/TablePro/Views/Main/MainContentCommandActions+BulkClose.swift b/TablePro/Views/Main/MainContentCommandActions+BulkClose.swift index 207994ee3..ef8b71407 100644 --- a/TablePro/Views/Main/MainContentCommandActions+BulkClose.swift +++ b/TablePro/Views/Main/MainContentCommandActions+BulkClose.swift @@ -13,12 +13,6 @@ extension MainContentCommandActions { /// that is not always the selected one. case others(anchor: UUID) case otherDatabases - case container(String) - } - - /// Closes a workspace from the rail: every tab of this connection sitting in that container. - func closeWorkspace(container: String) { - Task { await runBatchClose(kind: .container(container)) } } func closeAllTabs() { @@ -117,8 +111,6 @@ extension MainContentCommandActions { case .otherDatabases: let current = browsedContainerName return tabs.filter { WorkspaceAnchoring.containerName(of: $0, target: target) != current } - case .container(let container): - return tabs.filter { WorkspaceAnchoring.containerName(of: $0, target: target) == container } } } } diff --git a/TableProTests/Core/Services/Infrastructure/ConnectionCloseActionTests.swift b/TableProTests/Core/Services/Infrastructure/ConnectionCloseActionTests.swift new file mode 100644 index 000000000..89ffe2ea4 --- /dev/null +++ b/TableProTests/Core/Services/Infrastructure/ConnectionCloseActionTests.swift @@ -0,0 +1,45 @@ +// +// ConnectionCloseActionTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import Testing + +@Suite("Connection close decision") +@MainActor +struct ConnectionCloseActionTests { + /// The case the old command failed on. A connection the window hosts but that has no session + /// yet still has a rail row, and Close on it resolved no coordinator and returned in silence. + /// There is nothing to lose there, so it closes without asking rather than doing nothing. + @Test("A connection with no session closes without asking") + func sessionlessClosesImmediately() { + #expect( + ConnectionCloseAction.decision(hasSession: false, hasUnsavedWork: false) == .closeImmediately + ) + } + + /// Unsaved work reported for a connection that has no session cannot be acted on, so it must + /// not gate the close behind an alert whose Save button has nothing to call. + @Test("Unsaved work without a session still closes without asking") + func sessionlessIgnoresUnsavedWork() { + #expect( + ConnectionCloseAction.decision(hasSession: false, hasUnsavedWork: true) == .closeImmediately + ) + } + + @Test("A clean connection closes without asking") + func cleanSessionClosesImmediately() { + #expect( + ConnectionCloseAction.decision(hasSession: true, hasUnsavedWork: false) == .closeImmediately + ) + } + + @Test("A connection with unsaved work asks first") + func unsavedWorkIsConfirmed() { + #expect( + ConnectionCloseAction.decision(hasSession: true, hasUnsavedWork: true) == .confirmUnsavedWork + ) + } +} diff --git a/TableProTests/Core/Services/WorkspaceRailStoreTests.swift b/TableProTests/Core/Services/WorkspaceRailStoreTests.swift index 7153ae3f4..2f6a37fbb 100644 --- a/TableProTests/Core/Services/WorkspaceRailStoreTests.swift +++ b/TableProTests/Core/Services/WorkspaceRailStoreTests.swift @@ -59,6 +59,30 @@ struct WorkspaceRailStoreTests { #expect(entries.isEmpty) } + /// Close acts on the connection, so every row it owns has to go in one pass. A connection with + /// tabs in two databases has two rows, and leaving either behind is what made the old + /// container-scoped close read as doing nothing. + @Test("Closing a connection removes every row it owns, not just one") + func closingAConnectionDropsAllOfItsRows() { + let connection = TestFixtures.makeConnection(database: "app") + let session = makeSession(connection, browseDatabase: "app") + let tabs = [connection.id: [tableTab(database: "app"), tableTab(database: "logs")]] + + let before = resolve( + openConnectionIds: [connection.id], + sessions: [connection.id: session], + tabs: tabs + ) + #expect(Set(before.map(\.container)) == ["app", "logs"]) + + let after = resolve( + openConnectionIds: [], + sessions: [connection.id: session], + tabs: tabs + ) + #expect(after.isEmpty) + } + @Test("An entry shows the database being browsed, not the connection's saved default") func entryShowsBrowsedContainer() throws { let connection = TestFixtures.makeConnection(database: "saved_default") diff --git a/docs/features/workspace-rail.mdx b/docs/features/workspace-rail.mdx index 62cea1574..4dd5a80c1 100644 --- a/docs/features/workspace-rail.mdx +++ b/docs/features/workspace-rail.mdx @@ -52,7 +52,9 @@ The rail takes the keyboard too. Click into it and the arrow keys move the highl ## Closing -Right-click a workspace and choose **Close Workspace** to close every tab in it. Tabs bound to another database stay open, because they are not part of the workspace you are closing. Unsaved work is confirmed first, the same as closing a window. +Right-click a connection and choose **Close "\"** to close it: every tab across every database it has open, its session, and every row it has in the rail. Unsaved work is confirmed first, the same as closing a window. If the window has no other connection open, the window closes too. + +**File > Close Connection** does the same for the connection on screen. ## Disconnecting From b09804062f98a898478064f22cee0288faef1b2d Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 15 Aug 2026 07:41:30 +0700 Subject: [PATCH 2/4] feat(connections): move a connection to its own window and retire workspace from the ui --- CHANGELOG.md | 5 +- TablePro/Core/Menu/ViewMenuBuilder.swift | 6 +-- .../ConnectionWorkspaceHandoff.swift | 35 ++++++++++++ ...nSplitViewController+FileMenuActions.swift | 6 +++ ...inSplitViewController+MenuValidation.swift | 2 +- .../MainSplitViewController.swift | 15 ++++++ .../Infrastructure/WindowManager.swift | 43 ++++++++++++++- .../WorkspaceRailViewController.swift | 18 ++++++- .../Models/UI/KeyboardShortcutModels.swift | 6 +-- .../Views/Settings/GeneralSettingsView.swift | 2 +- docs/customization/settings.mdx | 2 +- docs/features/keyboard-shortcuts.mdx | 6 +-- docs/features/tabs.mdx | 8 +-- docs/features/workspace-rail.mdx | 54 ++++++++++--------- 14 files changed, 162 insertions(+), 46 deletions(-) create mode 100644 TablePro/Core/Services/Infrastructure/ConnectionWorkspaceHandoff.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cb48b483..4a57e164a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,8 +34,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Closing a connection removes it from the workspace rail, ends its session, and closes its tunnel. It stayed listed as connected, could not be clicked, and its database connection stayed open. -- The workspace rail's close command ends the connection: every tab across every database it has open, its session, and every row it holds in the rail. It used to close only the tabs of one database and leave the row you clicked exactly where it was, which read as doing nothing. Disconnect still ends only the session and keeps the row. **File > Close Connection** does the same from the menu bar. +- Closing a connection removes it from the connections strip, ends its session, and closes its tunnel. It stayed listed as connected, could not be clicked, and its database connection stayed open. +- **Open in New Window** on a connection in the strip moves it out of the shared window with its tabs, its session and its unsaved work. +- The strip's close command ends the connection: every tab across every database it has open, its session, and every row it holds in the rail. It used to close only the tabs of one database and leave the row you clicked exactly where it was, which read as doing nothing. Disconnect still ends only the session and keeps the row. **File > Close Connection** does the same from the menu bar. - Closing a window disconnects every connection it was showing, not just one of them. - Hiding a database in the sidebar's database filter takes effect straight away. The tree kept listing every database until some other change happened to rebuild it. - An alert opens on the window you were working in. It could attach itself to a floating panel such as the Quick Switcher, which takes the alert with it when it closes. diff --git a/TablePro/Core/Menu/ViewMenuBuilder.swift b/TablePro/Core/Menu/ViewMenuBuilder.swift index 27bfdf08e..a27046026 100644 --- a/TablePro/Core/Menu/ViewMenuBuilder.swift +++ b/TablePro/Core/Menu/ViewMenuBuilder.swift @@ -25,7 +25,7 @@ enum ViewMenuBuilder { keyboard: keyboard ), MenuItemFactory.item( - String(localized: "Show Workspace Rail"), + String(localized: "Show Connections"), action: #selector(MainSplitViewController.toggleWorkspaceRail(_:)), shortcut: .toggleWorkspaceRail, keyboard: keyboard @@ -84,13 +84,13 @@ enum ViewMenuBuilder { ), MenuItemFactory.separator, MenuItemFactory.item( - String(localized: "Show Previous Workspace"), + String(localized: "Show Previous Connection"), action: #selector(MainSplitViewController.showPreviousWorkspace(_:)), shortcut: .showPreviousWorkspace, keyboard: keyboard ), MenuItemFactory.item( - String(localized: "Show Next Workspace"), + String(localized: "Show Next Connection"), action: #selector(MainSplitViewController.showNextWorkspace(_:)), shortcut: .showNextWorkspace, keyboard: keyboard diff --git a/TablePro/Core/Services/Infrastructure/ConnectionWorkspaceHandoff.swift b/TablePro/Core/Services/Infrastructure/ConnectionWorkspaceHandoff.swift new file mode 100644 index 000000000..8083c89e6 --- /dev/null +++ b/TablePro/Core/Services/Infrastructure/ConnectionWorkspaceHandoff.swift @@ -0,0 +1,35 @@ +// +// ConnectionWorkspaceHandoff.swift +// TablePro +// + +import Foundation + +/// Carries a live `ConnectionWorkspace` from the window it is leaving to the one being built for +/// it, so Open in New Window moves the connection rather than reopening it. +/// +/// Rebuilding it in the new window would mean a second `SessionStateFactory.create`, and everything +/// the user has in that connection lives on the state it would replace: the open tabs, the change +/// manager holding unsaved cell edits, the coordinator, and the undo stack. The workspace is handed +/// over whole instead, keyed by the payload the new window is created with, mirroring how +/// `SessionStateFactory.registerPending` hands a freshly built state to a window that does not +/// exist yet. +@MainActor +internal enum ConnectionWorkspaceHandoff { + private static var pending: [UUID: ConnectionWorkspace] = [:] + + internal static func register(_ workspace: ConnectionWorkspace, for payloadId: UUID) { + pending[payloadId] = workspace + } + + internal static func consume(for payloadId: UUID) -> ConnectionWorkspace? { + pending.removeValue(forKey: payloadId) + } + + /// A window that failed to build never consumes its handoff, and the workspace it was carrying + /// is the user's open work. Returning it lets the caller put it back where it came from. + @discardableResult + internal static func reclaim(for payloadId: UUID) -> ConnectionWorkspace? { + pending.removeValue(forKey: payloadId) + } +} diff --git a/TablePro/Core/Services/Infrastructure/MainSplitViewController+FileMenuActions.swift b/TablePro/Core/Services/Infrastructure/MainSplitViewController+FileMenuActions.swift index 3fb04873f..73e5951bb 100644 --- a/TablePro/Core/Services/Infrastructure/MainSplitViewController+FileMenuActions.swift +++ b/TablePro/Core/Services/Infrastructure/MainSplitViewController+FileMenuActions.swift @@ -29,6 +29,12 @@ extension MainSplitViewController { /// itself. Leaving it to `commandActions` made the shortcut inert on exactly the pane a /// user most wants to dismiss. @objc func closeEditorTab(_ sender: Any?) { + /// Close is interpreted by whatever holds the keyboard. With the connections strip focused + /// it means the connection highlighted there, which is the row the user is looking at. + if railOwnsFocus { + closeConnection(sender) + return + } guard let actions = commandActions else { guard let connectionId = workspaces.selectedConnectionId else { return } WindowManager.shared.closeWindow(for: connectionId) diff --git a/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift b/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift index 832078c6a..175895fe8 100644 --- a/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift +++ b/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift @@ -242,7 +242,7 @@ extension MainSplitViewController: NSMenuItemValidation { case #selector(toggleInspector(_:)): setTitle(isInspectorVisible ? "Hide Inspector" : "Show Inspector", on: menuItem) case #selector(toggleWorkspaceRail(_:)): - setTitle(isWorkspaceRailEnabled ? "Hide Workspace Rail" : "Show Workspace Rail", on: menuItem) + setTitle(isWorkspaceRailEnabled ? "Hide Connections" : "Show Connections", on: menuItem) case #selector(undo(_:)): setResolvedTitle(commandActions?.resolvedUndoTitle ?? String(localized: "Undo"), on: menuItem) case #selector(redo(_:)): diff --git a/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift b/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift index f49e5f225..52f782afc 100644 --- a/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift +++ b/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift @@ -153,6 +153,12 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi return existing } + /// A connection moving here from another window arrives whole. Building a new workspace for + /// it would replace the session state its open tabs, unsaved edits and undo stack live on. + if let payloadId = payload?.id, let moved = ConnectionWorkspaceHandoff.consume(for: payloadId) { + return workspaces.insert(moved) + } + let resolvedConnection = DatabaseManager.shared.activeSessions[connectionId]?.connection ?? ConnectionStorage.shared.loadConnections().first { $0.id == connectionId } @@ -1019,6 +1025,15 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi applyDefaultCollapseStateIfNoAutosave() } + /// Whether the connections strip holds the keyboard. A close command is interpreted by the view + /// that has focus, so with the strip focused it means the connection it has highlighted rather + /// than the editor's front tab. + internal var railOwnsFocus: Bool { + guard let navigationSidebar, navigationSidebar.isRailVisible, + let responder = view.window?.firstResponder as? NSView else { return false } + return responder.isDescendant(of: navigationSidebar.railController.view) + } + /// A collapsed pane keeps whatever first responder it held, which would leave the window /// typing into a search field nobody can see. private func resignFirstResponderInsideChrome() { diff --git a/TablePro/Core/Services/Infrastructure/WindowManager.swift b/TablePro/Core/Services/Infrastructure/WindowManager.swift index f6b318684..108f49c31 100644 --- a/TablePro/Core/Services/Infrastructure/WindowManager.swift +++ b/TablePro/Core/Services/Infrastructure/WindowManager.swift @@ -77,7 +77,42 @@ internal final class WindowManager { return candidates.indices.contains(index) ? candidates[index] : nil } - private func openInNewWindow(payload: EditorTabPayload, activate: Bool, autoConnect: Bool) { + /// Moves a connection the window already hosts into a window of its own, carrying its session, + /// tabs and unsaved work with it. The reverse of the single-window model's default, and the + /// workflow it took away: `NSWindow`'s own Move Tab to New Window cannot express it, because a + /// connection is not a window tab here. + /// + /// Refused for a window's last connection, where it would close the window and open an + /// identical one. The rail hides the command in that case rather than dimming it. + internal func canMoveToNewWindow(connectionId: UUID) -> Bool { + guard let host = hosts().first(where: { $0.workspaces.contains(connectionId) }) else { return false } + return host.workspaces.count > 1 + } + + internal func moveToNewWindow(connectionId: UUID) { + guard canMoveToNewWindow(connectionId: connectionId), + let host = hosts().first(where: { $0.workspaces.contains(connectionId) }), + let workspace = host.workspaces.remove(connectionId) else { return } + host.applySelectedWorkspace() + + let payload = EditorTabPayload(connectionId: connectionId, intent: .restoreOrDefault) + ConnectionWorkspaceHandoff.register(workspace, for: payload.id) + openInNewWindow(payload: payload, activate: true, autoConnect: false, isHandoff: true) + + /// The new window consumes the handoff while it builds. Anything still pending means it + /// never got there, and the connection would otherwise be hosted by no window at all. + if let stranded = ConnectionWorkspaceHandoff.reclaim(for: payload.id) { + host.workspaces.insert(stranded) + host.applySelectedWorkspace() + } + } + + private func openInNewWindow( + payload: EditorTabPayload, + activate: Bool, + autoConnect: Bool, + isHandoff: Bool = false + ) { let t0 = Date() Self.lifecycleLogger.info( "[open] WindowManager.openTab start payloadId=\(payload.id, privacy: .public) connId=\(payload.connectionId, privacy: .public) intent=\(String(describing: payload.intent), privacy: .public) skipAutoExecute=\(payload.skipAutoExecute) activate=\(activate)" @@ -85,7 +120,11 @@ internal final class WindowManager { let resolvedConnection = DatabaseManager.shared.activeSessions[payload.connectionId]?.connection let preCreatedSessionState: SessionStateFactory.SessionState? - if let resolvedConnection { + /// A handoff brings its own state. Building a second one here would register it pending and + /// leave it there, holding a coordinator for a connection that already has one. + if isHandoff { + preCreatedSessionState = nil + } else if let resolvedConnection { let state = SessionStateFactory.create(connection: resolvedConnection, payload: payload) SessionStateFactory.registerPending(state, for: payload.id) preCreatedSessionState = state diff --git a/TablePro/Core/Services/Infrastructure/WorkspaceRailViewController.swift b/TablePro/Core/Services/Infrastructure/WorkspaceRailViewController.swift index 687b4553d..234c8df50 100644 --- a/TablePro/Core/Services/Infrastructure/WorkspaceRailViewController.swift +++ b/TablePro/Core/Services/Infrastructure/WorkspaceRailViewController.swift @@ -115,7 +115,7 @@ internal final class WorkspaceRailViewController: NSViewController { tableView.setDraggingSourceOperationMask(.move, forLocal: true) tableView.onMiddleClick = { [weak self] row in self?.closeConnection(atRow: row) } tableView.setAccessibilityIdentifier("workspace-rail") - tableView.setAccessibilityLabel(String(localized: "Open Workspaces")) + tableView.setAccessibilityLabel(String(localized: "Open Connections")) scrollView.documentView = tableView scrollView.hasVerticalScroller = true @@ -414,6 +414,12 @@ internal final class WorkspaceRailViewController: NSViewController { return menu } + @objc + private func openInNewWindow(_ sender: NSMenuItem) { + guard let workspace = sender.representedObject as? WorkspaceID else { return } + WindowManager.shared.moveToNewWindow(connectionId: workspace.connectionId) + } + @objc private func closeConnection(_ sender: NSMenuItem) { guard let workspace = sender.representedObject as? WorkspaceID else { return } @@ -462,6 +468,16 @@ extension WorkspaceRailViewController: NSMenuDelegate { guard entries.indices.contains(row) else { return } let entry = entries[row] + if WindowManager.shared.canMoveToNewWindow(connectionId: entry.workspace.connectionId) { + addItem( + to: menu, + title: String(localized: "Open in New Window"), + action: #selector(openInNewWindow(_:)), + workspace: entry.workspace + ) + menu.addItem(.separator()) + } + if ConnectionMenuPolicy.showsDisconnect(status: entry.status) { addItem( to: menu, diff --git a/TablePro/Models/UI/KeyboardShortcutModels.swift b/TablePro/Models/UI/KeyboardShortcutModels.swift index 43d5afd76..22a9106ee 100644 --- a/TablePro/Models/UI/KeyboardShortcutModels.swift +++ b/TablePro/Models/UI/KeyboardShortcutModels.swift @@ -241,9 +241,9 @@ enum ShortcutAction: String, Codable, CaseIterable, Identifiable { case .focusSidebarSearch: return String(localized: "Focus Sidebar Filter") case .showPreviousTab: return String(localized: "Show Previous Tab") case .showNextTab: return String(localized: "Show Next Tab") - case .toggleWorkspaceRail: return String(localized: "Toggle Workspace Rail") - case .showPreviousWorkspace: return String(localized: "Show Previous Workspace") - case .showNextWorkspace: return String(localized: "Show Next Workspace") + case .toggleWorkspaceRail: return String(localized: "Toggle Connections") + case .showPreviousWorkspace: return String(localized: "Show Previous Connection") + case .showNextWorkspace: return String(localized: "Show Next Connection") case .aiExplainQuery: return String(localized: "Explain with AI") case .aiOptimizeQuery: return String(localized: "Optimize with AI") } diff --git a/TablePro/Views/Settings/GeneralSettingsView.swift b/TablePro/Views/Settings/GeneralSettingsView.swift index cd3651a40..170d3fcac 100644 --- a/TablePro/Views/Settings/GeneralSettingsView.swift +++ b/TablePro/Views/Settings/GeneralSettingsView.swift @@ -52,7 +52,7 @@ struct GeneralSettingsView: View { } Section("Sidebar") { - Toggle("Show workspace rail", isOn: $settings.showWorkspaceRail) + Toggle("Show connections", isOn: $settings.showWorkspaceRail) .help("Adds a narrow strip on the window's leading edge listing every connection and database you have open, so one click switches to it.") Toggle("Show recent tables", isOn: $settings.showRecentTables) diff --git a/docs/customization/settings.mdx b/docs/customization/settings.mdx index 1ffe84e33..9bb53d0e0 100644 --- a/docs/customization/settings.mdx +++ b/docs/customization/settings.mdx @@ -47,7 +47,7 @@ Reopened tabs restore their SQL, cursor position, sort, filters, page, and colum | Setting | Default | Description | |---------|---------|-------------| -| **Show workspace rail** | On | Adds a narrow strip on the window's leading edge listing every connection and database you have open, so one click switches to it | +| **Show connections** | On | Adds a narrow strip on the window's leading edge listing every connection and database you have open, so one click switches to it | | **Show recent tables** | Off | Adds a Recent section at the top of the sidebar with the last 10 tables opened per connection and database | | **Show object icons** | On | Shows a type icon before each object name in the sidebar. Turn it off for a plain list of names. Also on the **View** menu, and under **View Options** in the sidebar right-click menu | | **Show object comments** | On | Shows database object comments next to tables in the sidebar and in grid column headers | diff --git a/docs/features/keyboard-shortcuts.mdx b/docs/features/keyboard-shortcuts.mdx index 5473fc31b..b281a0f33 100644 --- a/docs/features/keyboard-shortcuts.mdx +++ b/docs/features/keyboard-shortcuts.mdx @@ -173,9 +173,9 @@ See [Filtering](/features/filtering) for the filter panel itself. | Action | Shortcut | |--------|----------| | Toggle sidebar (listed as Toggle Table Browser in **Settings > Keyboard**) | `Cmd+0` | -| Toggle workspace rail | `Cmd+Option+0` | -| Show previous workspace | `Ctrl+Cmd+Up` | -| Show next workspace | `Ctrl+Cmd+Down` | +| Toggle connections | `Cmd+Option+0` | +| Show previous connection | `Ctrl+Cmd+Up` | +| Show next connection | `Ctrl+Cmd+Down` | | Focus sidebar filter | `Ctrl+Cmd+Option+F` | | Move from the sidebar filter into the object list | `Down` | | Move between objects in the sidebar | `Up` / `Down` | diff --git a/docs/features/tabs.mdx b/docs/features/tabs.mdx index 6841ac30f..d67af49cb 100644 --- a/docs/features/tabs.mdx +++ b/docs/features/tabs.mdx @@ -5,7 +5,7 @@ description: Each tab keeps its own SQL, results, sorting, and filters, and come Every open connection lives in one window, and each connection has its own set of tabs. A tab strip appears above the editor as soon as a connection holds more than one tab, and lists only that connection's tabs. With a single tab there is no strip, so a window that behaves the way it always did gains no chrome. -Each tab is an independent workspace with its own SQL, results, sorting, and filter state. Tabs persist across app restarts. +Each tab keeps its own SQL, results, sorting, and filter state. Tabs persist across app restarts. Tab strip with query and table tabs @@ -73,7 +73,7 @@ Each tab keeps its full state when you switch away: SQL, cursor position, result ## Windows and Connections -One window hosts every connection you have open. Picking a connection in the [workspace rail](/features/workspace-rail) switches that window to it and returns you to the tab you last used there, rather than raising a second window. Opening a table or query on a connection you already have open adds a tab to that connection instead of opening another window. +One window hosts every connection you have open. Picking a connection in the [connections strip](/features/workspace-rail) switches that window to it and returns you to the tab you last used there, rather than raising a second window. Opening a table or query on a connection you already have open adds a tab to that connection instead of opening another window. New windows open at 1200x800; size and position are remembered across launches. @@ -94,9 +94,9 @@ Both dim when the window is not part of a tab group. These are macOS window tabs Reconnecting puts the tabs back as they were. -Disconnecting asks first only when there are unsaved changes or a query still running. It applies to the whole connection, so every window and workspace on it shows the same Reconnect screen. Nothing reconnects on its own afterwards: a connection you disconnected is not reopened at the next launch, and clicking back into it leaves it disconnected until you ask. +Disconnecting asks first only when there are unsaved changes or a query still running. It applies to the whole connection, so every window and entry on it shows the same Reconnect screen. Nothing reconnects on its own afterwards: a connection you disconnected is not reopened at the next launch, and clicking back into it leaves it disconnected until you ask. -You can also disconnect by right-clicking a workspace in the [workspace rail](/features/workspace-rail), or a connection in the connection list. +You can also disconnect by right-clicking an entry in the [connections strip](/features/workspace-rail), or a connection in the connection list. ## Database Binding diff --git a/docs/features/workspace-rail.mdx b/docs/features/workspace-rail.mdx index 4dd5a80c1..4a1242d8b 100644 --- a/docs/features/workspace-rail.mdx +++ b/docs/features/workspace-rail.mdx @@ -1,34 +1,34 @@ --- -title: Workspace Rail +title: Connections description: Switch between every connection and database you have open from a narrow strip on the leading edge of the window --- -The workspace rail is a narrow strip on the leading edge of the window. It lists every workspace you have open, so you can move between them with one click instead of going back through the connection list, the Window menu, or the database picker. +The connections strip is a narrow column on the leading edge of the window. It lists every connection and database you have open, so you can move between them with one click instead of going back through the connection list, the Window menu, or the database picker. -A workspace is a connection plus the database you are browsing in it. Open two connections and you get two entries. Switch one of them to a second database and you get three. +Each entry is a connection plus the database you are browsing in it. Open two connections and you get two entries. Switch one of them to a second database and you get three. - - Workspace rail - Workspace rail + + Connections strip + Connections strip -The rail appears once you have a second workspace open, since a rail listing one has nothing to switch to. It is on by default; turn it off from **View** > **Hide Workspace Rail**, with `Cmd+Option+0`, or in **Settings** > **General** > **Sidebar**. +The strip appears once you have a second entry open, since a list of one has nothing to switch to. It is on by default; turn it off from **View** > **Hide Connections**, with `Cmd+Option+0`, or in **Settings** > **General** > **Sidebar**. ## What each entry shows -One icon per workspace, top to bottom. The icon is the database engine's symbol, tinted with the connection's colour, so you can tell staging from production at a glance without hovering. +One icon per entry, top to bottom. The icon is the database engine's symbol, tinted with the connection's colour, so you can tell staging from production at a glance without hovering. The icon changes shape, not just colour, when a connection is not healthy: a warning triangle when it failed, and a disconnected symbol when it has no live session. Colour alone would be invisible to anyone who cannot distinguish red from grey. -Under each icon is the database that workspace browses. Long names are shortened in the middle, so both ends stay recognisable. Hover an icon for the full connection name, host, and database. VoiceOver reads the same information plus the connection's state. +Under each icon is the database that entry browses. Long names are shortened in the middle, so both ends stay recognisable. Hover an icon for the full connection name, host, and database. VoiceOver reads the same information plus the connection's state. -The workspace you are looking at is highlighted. Every window highlights its own, so the rail always tells you where you are. +The entry you are looking at is highlighted. Every window highlights its own, so the strip always tells you where you are. -On servers that group objects by schema rather than by database, an entry stands for a schema. On a single-file or single-database engine, like SQLite, DuckDB, or BigQuery, a connection has one workspace and its entry is labelled with the connection's name. +On servers that group objects by schema rather than by database, an entry stands for a schema. On a single-file or single-database engine, like SQLite, DuckDB, or BigQuery, a connection has one entry, labelled with the connection's name. -## When a workspace appears +## When an entry appears -Switching database adds an entry only when the one you are leaving has work in it. A workspace has work when it has a table open, a query you have typed, a file you opened, or edits you have not saved. +Switching database adds an entry only when the one you are leaving has work in it. An entry has work when it has a table open, a query you have typed, a file you opened, or edits you have not saved. | You are on | You switch to `logs` | Result | |---|---|---| @@ -36,7 +36,7 @@ Switching database adds an entry only when the one you are leaving has work in i | `app`, with only an empty query tab | | `app` becomes `logs` | | `app`, and `logs` is already listed | | the existing `logs` entry activates | -An entry goes away when its last tab closes and you are no longer browsing it. Nothing is closed for you, and the rail never removes a workspace you still have work in. The set is per session, so relaunching starts from the databases your restored tabs use. +An entry goes away when its last tab closes and you are no longer browsing it. Nothing is closed for you, and the strip never removes an entry you still have work in. The set is per session, so relaunching starts from the databases your restored tabs use. ## Switching @@ -44,37 +44,41 @@ Click an entry to go to it. Moving between two connections switches the window t One window hosts every connection you have open. Each connection keeps its own set of tabs, and the tab strip above the editor lists only the tabs of the connection you are in. Switching reads as the window changing content, because that is what it is. -Open tabs are never closed or retargeted by a switch. A tab keeps the database it was opened against and keeps querying it, whichever workspace you are in. +Open tabs are never closed or retargeted by a switch. A tab keeps the database it was opened against and keeps querying it, whichever entry you are in. -Clicking the workspace you are already in does nothing. +Clicking the entry you are already in does nothing. -The rail takes the keyboard too. Click into it and the arrow keys move the highlight, typing jumps to a name, and Return opens the workspace you land on. +The strip takes the keyboard too. Click into it and the arrow keys move the highlight, typing jumps to a name, and Return opens the entry you land on. + +## Moving to its own window + +Right-click a connection and choose **Open in New Window** to move it out of the shared window, carrying its tabs, its session and anything you have not saved. The item appears only when the window has another connection left to show, since moving the last one would close the window and open an identical one. ## Closing -Right-click a connection and choose **Close "\"** to close it: every tab across every database it has open, its session, and every row it has in the rail. Unsaved work is confirmed first, the same as closing a window. If the window has no other connection open, the window closes too. +Right-click a connection and choose **Close "\"** to close it: every tab across every database it has open, its session, and every entry it has in the strip. Unsaved work is confirmed first, the same as closing a window. If the window has no other connection open, the window closes too. -**File > Close Connection** does the same for the connection on screen. +Middle-click an entry to close its connection without opening the menu. **File** > **Close Connection** does the same for the connection on screen. ## Disconnecting -Right-click a workspace and choose **Disconnect** to end its connection. The item only appears while the connection is live. Unlike Close Workspace, this covers every workspace of that connection, and no window closes: the connection shows a Reconnect screen in place of its tabs, with the tabs saved before the session ends. See [disconnecting](/features/tabs#disconnecting). +Right-click an entry and choose **Disconnect** to end its session while keeping it open. The item only appears while the connection is live. Unlike Close, no window closes and nothing is removed from the strip: the connection shows a Reconnect screen in place of its tabs, with the tabs saved before the session ends. See [disconnecting](/features/tabs#disconnecting). ## Reordering Drag entries to arrange them. The order is shared by every window and remembered across launches. -Once you have arranged the rail, a workspace keeps its position when you close it, and returns to the same slot when you reopen it. Until then, workspaces are listed in the order their connections opened, and the databases of one connection are grouped together by name. +Once you have arranged the strip, an entry keeps its position when you close it, and returns to the same slot when you reopen it. Until then, entries are listed in the order their connections opened, and the databases of one connection are grouped together by name. ## Keys | Action | Shortcut | |--------|----------| -| Toggle workspace rail | `Cmd+Option+0` | -| Show previous workspace | `Ctrl+Cmd+Up` | -| Show next workspace | `Ctrl+Cmd+Down` | +| Toggle connections | `Cmd+Option+0` | +| Show previous connection | `Ctrl+Cmd+Up` | +| Show next connection | `Ctrl+Cmd+Down` | -Show Previous and Show Next Workspace move through the rail in the order it displays, not the order things were opened. Change any of these in **Settings** > **Keyboard**. If you had already assigned `Ctrl+Cmd+Up` or `Ctrl+Cmd+Down` to something else, your assignment wins and the workspace command stays unbound until you free the chord again. +Show Previous and Show Next Connection move through the strip in the order it displays, not the order things were opened. Change any of these in **Settings** > **Keyboard**. If you had already assigned `Ctrl+Cmd+Up` or `Ctrl+Cmd+Down` to something else, your assignment wins and the command stays unbound until you free the chord again. ## Related From d9666178ec574147b0085e959d3e92e85b013723 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 15 Aug 2026 07:53:52 +0700 Subject: [PATCH 3/4] test(connections): drive the close command through the menu bar in ui automation --- TableProUITests/ConnectionCloseUITests.swift | 78 +++++++++++++++++++ .../SingleWindowMenuContractUITests.swift | 26 +++++++ 2 files changed, 104 insertions(+) create mode 100644 TableProUITests/ConnectionCloseUITests.swift diff --git a/TableProUITests/ConnectionCloseUITests.swift b/TableProUITests/ConnectionCloseUITests.swift new file mode 100644 index 000000000..99aac91be --- /dev/null +++ b/TableProUITests/ConnectionCloseUITests.swift @@ -0,0 +1,78 @@ +import XCTest + +/// Closing a connection used to be a command that could not be seen to do anything: it closed a +/// subset of one connection's tabs and left the row that invoked it exactly where it was, and on a +/// connection with no session it resolved no coordinator and returned in silence. +/// +/// These drive the command through the menu bar rather than the connections strip, because the +/// strip only appears once a second connection is open and the sample database is a single-file +/// SQLite connection. The command is the same one the strip's row invokes. +final class ConnectionCloseUITests: XCTestCase { + override func setUpWithError() throws { + continueAfterFailure = false + } + + override func tearDownWithError() throws { + XCUIApplication().terminate() + } + + /// The regression test for the reported bug: choosing Close must have an observable effect. + /// + /// The observable is the command's own enablement, which is `hasSelectedWorkspace`: true while + /// a window is showing a connection and false once none is. Asserting on the editor view + /// instead made the test depend on which kind of tab the sample database happens to open with, + /// which is not what this is testing. The sample connection has no unsaved work, so it takes + /// the close-without-asking path. + func testClosingTheOnlyConnectionLeavesNoConnectionShowing() throws { + let app = launchWithSampleDatabase() + + XCTAssertTrue( + waitForCloseConnection(in: app, toBeEnabled: true, timeout: 30), + "Opening the sample database must leave a connection showing" + ) + closeConnectionItem(in: app).click() + + XCTAssertTrue( + waitForCloseConnection(in: app, toBeEnabled: false, timeout: 20), + "Closing the connection must leave none showing, not leave the window as it was" + ) + } + + /// Opens the File menu and reports the item's enablement, closing the menu again so the next + /// poll starts from the same state. A menu item only answers `isEnabled` while its menu is up. + private func waitForCloseConnection( + in app: XCUIApplication, + toBeEnabled expected: Bool, + timeout: TimeInterval + ) -> Bool { + let deadline = Date().addingTimeInterval(timeout) + while Date() < deadline { + let item = closeConnectionItem(in: app) + guard item.waitForExistence(timeout: 5) else { continue } + let isEnabled = item.isEnabled + app.typeKey(.escape, modifierFlags: []) + if isEnabled == expected { return true } + } + return false + } + + private func closeConnectionItem(in app: XCUIApplication) -> XCUIElement { + let menuBar = app.menuBars.firstMatch + menuBar.menuBarItems["File"].click() + return menuBar.menuItems["Close Connection"] + } + + private func launchWithSampleDatabase() -> XCUIApplication { + let app = XCUIApplication() + app.launchEnvironment["TABLEPRO_UI_TESTING"] = "1" + app.launch() + + let menuBar = app.menuBars.firstMatch + XCTAssertTrue(menuBar.waitForExistence(timeout: 10)) + menuBar.menuBarItems["Help"].click() + let openSample = menuBar.menuItems["Open Sample Database"] + XCTAssertTrue(openSample.waitForExistence(timeout: 5)) + openSample.click() + return app + } +} diff --git a/TableProUITests/SingleWindowMenuContractUITests.swift b/TableProUITests/SingleWindowMenuContractUITests.swift index 1d37d21df..194036d64 100644 --- a/TableProUITests/SingleWindowMenuContractUITests.swift +++ b/TableProUITests/SingleWindowMenuContractUITests.swift @@ -55,6 +55,32 @@ final class SingleWindowMenuContractUITests: XCTestCase { } } + /// The connections strip offers Close on a row, and the HIG requires every context-menu command + /// to be reachable from the menu bar too. + func testFileMenuOffersCloseConnection() throws { + let app = launchApp() + + XCTAssertTrue( + app.menuBars.menuItems["Close Connection"].waitForExistence(timeout: 5), + "File menu must mirror the connections strip's Close command" + ) + } + + /// The rail is named for what it lists. "Workspace" was a term the product invented for a + /// window-like thing, which the HIG's Windows guidance rules out. + func testViewMenuNamesTheConnectionsStrip() throws { + let app = launchApp() + + XCTAssertTrue( + app.menuBars.menuItems["Show Connections"].waitForExistence(timeout: 5), + "View menu must name the strip after what it lists" + ) + XCTAssertFalse( + app.menuBars.menuItems["Show Workspace Rail"].exists, + "The invented noun must be gone from the menu bar" + ) + } + /// Launching shows the welcome window and nothing else. A second main window appearing here /// is the shape of the bug the single-window model exists to prevent. func testLaunchOpensExactlyOneWindow() throws { From d8a0ef85e56897f80537a042663e2383eec4ea94 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 15 Aug 2026 08:41:10 +0700 Subject: [PATCH 4/4] fix(hig): identify the connection form's name and file path fields for accessibility --- .../ConnectionForm/Panes/GeneralPaneView.swift | 2 ++ .../SingleWindowMenuContractUITests.swift | 13 ++++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/TablePro/Views/ConnectionForm/Panes/GeneralPaneView.swift b/TablePro/Views/ConnectionForm/Panes/GeneralPaneView.swift index 5cd5b8720..48d604c47 100644 --- a/TablePro/Views/ConnectionForm/Panes/GeneralPaneView.swift +++ b/TablePro/Views/ConnectionForm/Panes/GeneralPaneView.swift @@ -38,6 +38,7 @@ struct GeneralPaneView: View { prompt: Text(String(localized: "Connection name")) ) .focused($nameFocused) + .accessibilityIdentifier("connection-form-name") } connectionSection @@ -70,6 +71,7 @@ struct GeneralPaneView: View { text: $coordinator.network.database, prompt: Text(filePathPrompt) ) + .accessibilityIdentifier("connection-form-file-path") Button(String(localized: "Browse...")) { browseForFile() } diff --git a/TableProUITests/SingleWindowMenuContractUITests.swift b/TableProUITests/SingleWindowMenuContractUITests.swift index 194036d64..3eaa6e310 100644 --- a/TableProUITests/SingleWindowMenuContractUITests.swift +++ b/TableProUITests/SingleWindowMenuContractUITests.swift @@ -68,17 +68,20 @@ final class SingleWindowMenuContractUITests: XCTestCase { /// The rail is named for what it lists. "Workspace" was a term the product invented for a /// window-like thing, which the HIG's Windows guidance rules out. + /// Either verb, because the item toggles with the strip's visibility and this is about the noun. + /// Asserting only on "Show" made the test depend on how many connections happened to be + /// restored at launch, which another suite could change out from under it. func testViewMenuNamesTheConnectionsStrip() throws { let app = launchApp() + let menuItems = app.menuBars.menuItems XCTAssertTrue( - app.menuBars.menuItems["Show Connections"].waitForExistence(timeout: 5), + menuItems["Show Connections"].waitForExistence(timeout: 5) + || menuItems["Hide Connections"].exists, "View menu must name the strip after what it lists" ) - XCTAssertFalse( - app.menuBars.menuItems["Show Workspace Rail"].exists, - "The invented noun must be gone from the menu bar" - ) + XCTAssertFalse(menuItems["Show Workspace Rail"].exists, "The invented noun must be gone") + XCTAssertFalse(menuItems["Hide Workspace Rail"].exists, "The invented noun must be gone") } /// Launching shows the welcome window and nothing else. A second main window appearing here