From faf7efa2a44eb89cf3c42898a33ae1baf7156bf8 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Mon, 21 Sep 2026 13:14:00 +0700 Subject: [PATCH 1/9] feat(toolbar): add the context resolvers the connection window's chrome is rebuilt on --- .../Infrastructure/MainWindowToolbar.swift | 57 +-- .../Toolbar/ActionsMenuSpec.swift | 57 +++ .../ConnectionActionsMenuResolver.swift | 244 +++++++++++ .../Toolbar/ToolbarContextResolver.swift | 142 +++++++ .../Policy/AgentModeSafeModeFloor.swift | 18 +- TablePro/Models/UI/PendingChangeKind.swift | 78 ++++ TablePro/Models/UI/ToolbarContext.swift | 139 +++++++ .../UI/TrailingPaneSurfaceResolver.swift | 47 +++ .../ConnectionActionsMenuResolverTests.swift | 265 ++++++++++++ .../ToolbarContextResolverTests.swift | 382 ++++++++++++++++++ .../Models/PendingChangeKindTests.swift | 179 ++++++++ .../TrailingPaneSurfaceResolverTests.swift | 105 +++++ 12 files changed, 1684 insertions(+), 29 deletions(-) create mode 100644 TablePro/Core/Services/Infrastructure/Toolbar/ActionsMenuSpec.swift create mode 100644 TablePro/Core/Services/Infrastructure/Toolbar/ConnectionActionsMenuResolver.swift create mode 100644 TablePro/Core/Services/Infrastructure/Toolbar/ToolbarContextResolver.swift create mode 100644 TablePro/Models/UI/PendingChangeKind.swift create mode 100644 TablePro/Models/UI/ToolbarContext.swift create mode 100644 TablePro/Models/UI/TrailingPaneSurfaceResolver.swift create mode 100644 TableProTests/Core/Services/Infrastructure/ConnectionActionsMenuResolverTests.swift create mode 100644 TableProTests/Core/Services/Infrastructure/ToolbarContextResolverTests.swift create mode 100644 TableProTests/Models/PendingChangeKindTests.swift create mode 100644 TableProTests/Models/TrailingPaneSurfaceResolverTests.swift diff --git a/TablePro/Core/Services/Infrastructure/MainWindowToolbar.swift b/TablePro/Core/Services/Infrastructure/MainWindowToolbar.swift index 5b66986f2..d0605ea49 100644 --- a/TablePro/Core/Services/Infrastructure/MainWindowToolbar.swift +++ b/TablePro/Core/Services/Infrastructure/MainWindowToolbar.swift @@ -289,38 +289,45 @@ internal final class MainWindowToolbar: NSObject, NSToolbarDelegate { // MARK: - Identifiers - static let connectionGroup = NSToolbarItem.Identifier("com.TablePro.toolbar.connectionGroup") - static let connection = NSToolbarItem.Identifier("com.TablePro.toolbar.connection") - static let database = NSToolbarItem.Identifier("com.TablePro.toolbar.database") - static let refresh = NSToolbarItem.Identifier("com.TablePro.toolbar.refresh") - static let saveChanges = NSToolbarItem.Identifier("com.TablePro.toolbar.saveChanges") - static let addRow = NSToolbarItem.Identifier("com.TablePro.toolbar.addRow") - static let safeMode = NSToolbarItem.Identifier("com.TablePro.toolbar.safeMode") - static let quickSwitcher = NSToolbarItem.Identifier("com.TablePro.toolbar.quickSwitcher") - static let newTab = NSToolbarItem.Identifier("com.TablePro.toolbar.newTab") - static let previewSQL = NSToolbarItem.Identifier("com.TablePro.toolbar.previewSQL") - static let results = NSToolbarItem.Identifier("com.TablePro.toolbar.results") + /// `nonisolated` throughout: these are immutable strings that name a command, and + /// `ToolbarContextResolver` reads them from off the main actor to answer which items a context + /// shows. Isolating them to this class was incidental to the class being `@MainActor`. + nonisolated static let connectionGroup = NSToolbarItem.Identifier("com.TablePro.toolbar.connectionGroup") + nonisolated static let connection = NSToolbarItem.Identifier("com.TablePro.toolbar.connection") + nonisolated static let database = NSToolbarItem.Identifier("com.TablePro.toolbar.database") + nonisolated static let refresh = NSToolbarItem.Identifier("com.TablePro.toolbar.refresh") + nonisolated static let saveChanges = NSToolbarItem.Identifier("com.TablePro.toolbar.saveChanges") + nonisolated static let addRow = NSToolbarItem.Identifier("com.TablePro.toolbar.addRow") + nonisolated static let safeMode = NSToolbarItem.Identifier("com.TablePro.toolbar.safeMode") + nonisolated static let quickSwitcher = NSToolbarItem.Identifier("com.TablePro.toolbar.quickSwitcher") + nonisolated static let newTab = NSToolbarItem.Identifier("com.TablePro.toolbar.newTab") + nonisolated static let previewSQL = NSToolbarItem.Identifier("com.TablePro.toolbar.previewSQL") + nonisolated static let results = NSToolbarItem.Identifier("com.TablePro.toolbar.results") /// `.toggleInspector` is macOS 14. The identifier only has to be stable and unique, and /// AppKit's own inspector behaviour is not used here, so 13 gets an app-owned one. - static let inspector: NSToolbarItem.Identifier = { + nonisolated static let inspector: NSToolbarItem.Identifier = { if #available(macOS 14.0, *) { return .toggleInspector } return NSToolbarItem.Identifier("com.TablePro.toolbar.inspector") }() - static let assistant = NSToolbarItem.Identifier("com.TablePro.toolbar.assistant") - static let dashboard = NSToolbarItem.Identifier("com.TablePro.toolbar.dashboard") - static let history = NSToolbarItem.Identifier("com.TablePro.toolbar.history") - static let exportTables = NSToolbarItem.Identifier("com.TablePro.toolbar.export") - static let importTables = NSToolbarItem.Identifier("com.TablePro.toolbar.import") - static let refreshSaveGroup = NSToolbarItem.Identifier("com.TablePro.toolbar.refreshSaveGroup") - static let editorGroup = NSToolbarItem.Identifier("com.TablePro.toolbar.editorGroup") - static let restorePreviousValues = NSToolbarItem.Identifier("com.TablePro.toolbar.restorePreviousValues") - static let exportImportGroup = NSToolbarItem.Identifier("com.TablePro.toolbar.exportImportGroup") - static let sidebarToggle = NSToolbarItem.Identifier("com.TablePro.toolbar.sidebarToggle") - static let backForwardGroup = NSToolbarItem.Identifier("com.TablePro.toolbar.backForwardGroup") - static let navigateBack = NSToolbarItem.Identifier("com.TablePro.toolbar.navigateBack") - static let navigateForward = NSToolbarItem.Identifier("com.TablePro.toolbar.navigateForward") + nonisolated static let assistant = NSToolbarItem.Identifier("com.TablePro.toolbar.assistant") + nonisolated static let dashboard = NSToolbarItem.Identifier("com.TablePro.toolbar.dashboard") + nonisolated static let history = NSToolbarItem.Identifier("com.TablePro.toolbar.history") + nonisolated static let exportTables = NSToolbarItem.Identifier("com.TablePro.toolbar.export") + nonisolated static let importTables = NSToolbarItem.Identifier("com.TablePro.toolbar.import") + nonisolated static let refreshSaveGroup = NSToolbarItem.Identifier("com.TablePro.toolbar.refreshSaveGroup") + nonisolated static let editorGroup = NSToolbarItem.Identifier("com.TablePro.toolbar.editorGroup") + nonisolated static let restorePreviousValues = NSToolbarItem + .Identifier("com.TablePro.toolbar.restorePreviousValues") + nonisolated static let exportImportGroup = NSToolbarItem.Identifier("com.TablePro.toolbar.exportImportGroup") + nonisolated static let sidebarToggle = NSToolbarItem.Identifier("com.TablePro.toolbar.sidebarToggle") + nonisolated static let backForwardGroup = NSToolbarItem.Identifier("com.TablePro.toolbar.backForwardGroup") + nonisolated static let navigateBack = NSToolbarItem.Identifier("com.TablePro.toolbar.navigateBack") + nonisolated static let navigateForward = NSToolbarItem.Identifier("com.TablePro.toolbar.navigateForward") + /// The pull-down that carries the long tail of a context's commands. One control whose menu + /// changes with the tab, instead of one permanent titlebar slot per command. + nonisolated static let actions = NSToolbarItem.Identifier("com.TablePro.toolbar.actions") // MARK: - NSToolbarDelegate diff --git a/TablePro/Core/Services/Infrastructure/Toolbar/ActionsMenuSpec.swift b/TablePro/Core/Services/Infrastructure/Toolbar/ActionsMenuSpec.swift new file mode 100644 index 000000000..73d52d569 --- /dev/null +++ b/TablePro/Core/Services/Infrastructure/Toolbar/ActionsMenuSpec.swift @@ -0,0 +1,57 @@ +// +// ActionsMenuSpec.swift +// TablePro +// + +import AppKit + +/// A submenu whose leaves are resolved when it opens rather than when the menu is built. +/// +/// The resolver stays pure by naming the submenu and stopping there. Import formats come from the +/// connection's driver and the mode list from the window, and both are questions with an answer +/// that can change between two openings of the same menu; `NSMenuDelegate.menuNeedsUpdate` is where +/// they are asked, measured to fire exactly once per real open. +internal enum ActionsSubmenuKind: Equatable, Hashable, Sendable { + case importFormats + case mode +} + +/// One command in the Actions pull-down. +/// +/// Carries no target. Every entry is built with `target = nil` so AppKit routes it through the +/// responder chain and `MainSplitViewController.validateMenuItem` decides it, which is the same +/// path the menu bar already takes. Giving an entry an explicit target would hand validation to +/// `MainWindowToolbar.validateMenuItem`, whose unrecognised-action arm returns true, and ship every +/// entry enabled. +internal struct ActionsMenuEntry: Equatable { + internal let title: String + internal let selector: Selector + internal let shortcut: ShortcutAction? + /// What the command is about, for an entry that names one of several values of the same + /// command. `setContentModeFromMenu:` needs it for both the action and the checkmark. + internal let representedValue: String? + internal let submenu: ActionsSubmenuKind? + + internal init( + title: String, + selector: Selector, + shortcut: ShortcutAction? = nil, + representedValue: String? = nil, + submenu: ActionsSubmenuKind? = nil + ) { + self.title = title + self.selector = selector + self.shortcut = shortcut + self.representedValue = representedValue + self.submenu = submenu + } +} + +/// A run of related commands, drawn with a separator between one section and the next. +internal struct ActionsMenuSection: Equatable { + internal let entries: [ActionsMenuEntry] + + internal init(_ entries: [ActionsMenuEntry]) { + self.entries = entries + } +} diff --git a/TablePro/Core/Services/Infrastructure/Toolbar/ConnectionActionsMenuResolver.swift b/TablePro/Core/Services/Infrastructure/Toolbar/ConnectionActionsMenuResolver.swift new file mode 100644 index 000000000..2d0b4b10e --- /dev/null +++ b/TablePro/Core/Services/Infrastructure/Toolbar/ConnectionActionsMenuResolver.swift @@ -0,0 +1,244 @@ +// +// ConnectionActionsMenuResolver.swift +// TablePro +// + +import AppKit + +/// What the Actions pull-down offers in a context. +/// +/// This is where the commands that used to each own a permanent slot in the titlebar now live. One +/// control whose menu changes, rather than seventeen buttons of which most are dim on most tabs. +/// +/// Every entry has a menu-bar twin carrying the same title, and that is a rule rather than a +/// coincidence: a pull-down that grows commands of its own becomes a second, undiscoverable menu +/// bar. A test pins it. +/// +/// Nothing here reads a global. What the driver supports, what the tab is and what the window is +/// doing all arrive in the context, so the whole table is decided by a value and testable without a +/// session. +internal enum ConnectionActionsMenuResolver { + internal static func sections(_ context: ToolbarContext) -> [ActionsMenuSection] { + switch context.contentMode { + case .agent: + return [modeSection(context), connectionSection(context)].compactMap(\.self) + case .browse: + return browseSections(context) + } + } + + private static func browseSections(_ context: ToolbarContext) -> [ActionsMenuSection] { + guard context.isConnected else { + return [modeSection(context), connectionSection(context)].compactMap(\.self) + } + return [ + rowSection(context), + historySection(context), + dataSection(context), + objectSection(context), + windowSection(context), + editorSection(context), + modeSection(context), + connectionSection(context), + ].compactMap(\.self) + } + + // MARK: - Sections + + /// The commands that act on the rows in front of the user. Add Row asks the same question the + /// grid does, so it follows the results mode rather than the tab kind alone. + private static func rowSection(_ context: ToolbarContext) -> ActionsMenuSection? { + var entries: [ActionsMenuEntry] = [] + if context.tabKind == .table, context.resultsMode == .data { + entries.append( + ActionsMenuEntry( + title: String(localized: "Add Row"), + selector: NSSelectorFromString("addRow:"), + shortcut: .addRow + ) + ) + } + if context.tabKind == .table || context.tabKind == .query { + entries.append( + ActionsMenuEntry( + title: String(localized: "Restore Previous Values…"), + selector: NSSelectorFromString("restorePreviousValues:") + ) + ) + } + return entries.isEmpty ? nil : ActionsMenuSection(entries) + } + + /// Back and Forward walk a table's own browse history, which only a table tab has. + private static func historySection(_ context: ToolbarContext) -> ActionsMenuSection? { + guard context.tabKind == .table else { return nil } + return ActionsMenuSection([ + ActionsMenuEntry( + title: String(localized: "Back"), + selector: NSSelectorFromString("navigateBack:"), + shortcut: .navigateBack + ), + ActionsMenuEntry( + title: String(localized: "Forward"), + selector: NSSelectorFromString("navigateForward:"), + shortcut: .navigateForward + ), + ]) + } + + /// What goes in and what comes out. Preview SQL sits beside the commit verb's own tabs because + /// it answers the question the commit raises. + private static func dataSection(_ context: ToolbarContext) -> ActionsMenuSection? { + var entries: [ActionsMenuEntry] = [] + if context.tabKind == .table || context.tabKind == .query || context.tabKind == .createTable { + entries.append( + ActionsMenuEntry( + title: String(localized: "Preview SQL"), + selector: NSSelectorFromString("previewSQL:"), + shortcut: .previewSQL + ) + ) + } + if context.tabKind == .query { + /// The one context with a results pane to collapse. The shipped rule offered this on + /// the five kinds that have none. + entries.append( + ActionsMenuEntry( + title: String(localized: "Show Results"), + selector: NSSelectorFromString("toggleResults:"), + shortcut: .toggleResults + ) + ) + } + if context.tabKind == .table || context.tabKind == .query { + entries.append( + ActionsMenuEntry( + title: String(localized: "Export Results…"), + selector: NSSelectorFromString("exportQueryResults:") + ) + ) + } + entries.append( + ActionsMenuEntry( + title: String(localized: "Export Tables…"), + selector: NSSelectorFromString("exportTables:"), + shortcut: .export + ) + ) + if context.supportsImport { + /// A submenu rather than a leaf, because the menu bar's own item always takes the first + /// format and the toolbar was until now the only route to any of the others. The leaves + /// are filled when it opens. + entries.append( + ActionsMenuEntry( + title: String(localized: "Import Data…"), + selector: NSSelectorFromString("importData:"), + shortcut: .importData, + submenu: .importFormats + ) + ) + } + return entries.isEmpty ? nil : ActionsMenuSection(entries) + } + + /// What the selected object is made of. + private static func objectSection(_ context: ToolbarContext) -> ActionsMenuSection? { + guard context.tabKind == .table || context.tabKind == .objectSource else { return nil } + return ActionsMenuSection([ + ActionsMenuEntry( + title: String(localized: "Show DDL"), + selector: NSSelectorFromString("showObjectDDL:") + ), + ActionsMenuEntry( + title: String(localized: "Copy DDL"), + selector: NSSelectorFromString("copyObjectDDL:") + ), + ]) + } + + /// The places in this connection the window can go. Each of these opens a tab or a drawer, so + /// they belong together and away from the commands that change data. + private static func windowSection(_ context: ToolbarContext) -> ActionsMenuSection? { + var entries: [ActionsMenuEntry] = [ + ActionsMenuEntry( + title: String(localized: "Show Query History"), + selector: NSSelectorFromString("toggleQueryHistory:"), + shortcut: .toggleHistory + ), + ActionsMenuEntry( + title: String(localized: "Users & Roles"), + selector: NSSelectorFromString("showUsersAndRoles:") + ), + ActionsMenuEntry( + title: String(localized: "Query Insights"), + selector: NSSelectorFromString("showQueryInsights:") + ), + ] + if context.supportsServerDashboard { + entries.append( + ActionsMenuEntry( + title: String(localized: "Server Dashboard"), + selector: NSSelectorFromString("showServerDashboard:") + ) + ) + } + return ActionsMenuSection(entries) + } + + private static func editorSection(_ context: ToolbarContext) -> ActionsMenuSection? { + ActionsMenuSection([ + ActionsMenuEntry( + title: String(localized: "New Tab"), + selector: NSSelectorFromString("newEditorTab:"), + shortcut: .newTab + ), + ActionsMenuEntry( + title: String(localized: "Open Quickly…"), + selector: NSSelectorFromString("openQuickSwitcher:"), + shortcut: .quickSwitcher + ), + ]) + } + + /// The mode control's new home, now that it no longer holds two permanent segments in the + /// titlebar. Absent with AI off, where Agent mode does not exist. + private static func modeSection(_ context: ToolbarContext) -> ActionsMenuSection? { + guard context.isAIEnabled else { return nil } + return ActionsMenuSection([ + ActionsMenuEntry( + title: String(localized: "Mode"), + selector: NSSelectorFromString("setContentModeFromMenu:"), + submenu: .mode + ), + ]) + } + + /// The route out of a window whose connection went away, and the reason the pull-down answers + /// in every phase rather than only over a live session. + private static func connectionSection(_ context: ToolbarContext) -> ActionsMenuSection? { + var entries: [ActionsMenuEntry] = [ + ActionsMenuEntry( + title: String(localized: "Switch Connection…"), + selector: NSSelectorFromString("switchConnection:"), + shortcut: .switchConnection + ), + ] + if !context.isConnected { + /// Declared with no sender, and the menu bar spells it the same way. A selector with a + /// colon reaches nothing and AppKit draws the entry disabled. + entries.append( + ActionsMenuEntry( + title: String(localized: "Reconnect"), + selector: NSSelectorFromString("retryConnection") + ) + ) + } + entries.append( + ActionsMenuEntry( + title: String(localized: "Close Connection"), + selector: NSSelectorFromString("closeConnection:") + ) + ) + return ActionsMenuSection(entries) + } +} diff --git a/TablePro/Core/Services/Infrastructure/Toolbar/ToolbarContextResolver.swift b/TablePro/Core/Services/Infrastructure/Toolbar/ToolbarContextResolver.swift new file mode 100644 index 000000000..29bb83d84 --- /dev/null +++ b/TablePro/Core/Services/Infrastructure/Toolbar/ToolbarContextResolver.swift @@ -0,0 +1,142 @@ +// +// ToolbarContextResolver.swift +// TablePro +// + +import AppKit + +/// Which of the connection window's toolbar items a context shows, and which of them answer. +/// +/// Two questions with deliberately different inputs, and keeping them apart is what stops the +/// titlebar reflowing while the user types. +/// +/// `hidden` is a function of `ToolbarContext.VisibilityKey` alone: the tab kind, the results mode, +/// the content mode and the driver's capabilities. Those change on a tab switch, a mode switch or a +/// connection switch and at no other time, which are the three moments a native app's toolbar is +/// expected to change shape. `isEnabled` carries everything transient, so a staged edit, a running +/// query or a reconnect backoff dims a control and never moves one. +/// +/// Version-free on purpose. `NSToolbarItem.isHidden` is macOS 15, and the caller is what decides +/// whether to apply the set or fall back to dimming; the answer itself does not depend on the OS. +/// +/// Exhaustive over `TabType` with no `default:` arm. CLAUDE.md's "every switch needs `default:`" +/// rule is about `DatabaseType`, which is an open string-based struct; `TabType` is closed, and a +/// ninth kind must not compile without choosing what its toolbar shows. +internal enum ToolbarContextResolver { + /// The identifiers this context takes out of the titlebar entirely. + /// + /// Only ever names an item from the default set. An item the user dragged in from the + /// customization palette is opt-in, so it stays where they put it and dims instead; the toolbar + /// enforces that separately, and a test pins that this set never reaches past the default list. + /// + /// Never names both subitems of the centred group at once: measured on macOS 27, hiding both + /// makes the group vanish while `group.isHidden` stays false, and a popover anchored on it then + /// lands at the window's centre. + internal static func hidden(_ context: ToolbarContext) -> Set { + var hidden: Set = [] + + /// A file-based engine has one database and it is the file already named beside it, so the + /// second capsule has never been clickable on SQLite or DuckDB. + if context.isFileBased || !context.supportsContainerSwitching { + hidden.insert(MainWindowToolbar.database) + } + + switch context.contentMode { + case .agent: + /// No grid and no object browser are on screen, and the commit control's gate is frozen + /// because the browse content tree is not mounted to write it. + hidden.insert(MainWindowToolbar.refresh) + hidden.insert(MainWindowToolbar.saveChanges) + return hidden + case .browse: + hidden.formUnion(browseHidden(context)) + return hidden + } + } + + private static func browseHidden(_ context: ToolbarContext) -> Set { + guard let tabKind = context.tabKind else { return [] } + switch tabKind { + case .createTable: + /// A definition that is not on the server yet has nothing to reload. + return [MainWindowToolbar.refresh] + case .erDiagram, .serverDashboard, .insights, .objectSource: + /// None of these four can stage a change, so the commit control could only ever be dim. + return [MainWindowToolbar.saveChanges] + case .query, .table, .usersRoles: + return [] + } + } + + /// Whether an item answers in this context. + /// + /// Every identifier the toolbar vends has an arm. The `default:` returns false rather than true + /// because the old unconditional arm is what left Query History live and inert over a window + /// that had never connected, and left every identifier nobody had thought about enabled. + internal static func isEnabled( + _ identifier: NSToolbarItem.Identifier, + context: ToolbarContext + ) -> Bool { + switch identifier { + case MainWindowToolbar.connection, MainWindowToolbar.connectionGroup: + /// Switch Connection is the window's command, so it answers before a session exists. + /// It is the route back from a connection that failed. + return true + case MainWindowToolbar.database: + return context.isConnected && !context.isFileBased && context.supportsContainerSwitching + case MainWindowToolbar.actions: + /// The menu gates its own entries through the responder chain, so this answers only for + /// the window having something to be about at all. A closing window opens no menu. + return context.hasSelectedWorkspace && context.pane != .empty + case MainWindowToolbar.refresh: + return context.isConnected && context.contentMode == .browse + case MainWindowToolbar.saveChanges: + return context.pendingChange != nil && context.isConnected && !context.blocksAllWrites + case MainWindowToolbar.safeMode: + /// Safe Mode is what stands between a stray keystroke and a live table, so it answers + /// for as long as the session does. A window with no session has nothing to write it to. + return context.isConnected + case MainWindowToolbar.inspector: + /// Not `isConnected`. A connection that drops with the pane open must still be able to + /// close it, which is the state the old rule disabled on the app's minimum OS. + return context.canToggleTrailingPane + case MainWindowToolbar.addRow: + return context.isConnected && context.canAddRow + case MainWindowToolbar.restorePreviousValues: + return context.isConnected && context.canRestorePreviousValues + case MainWindowToolbar.navigateBack, MainWindowToolbar.backForwardGroup: + return context.isConnected && context.canNavigateBack + case MainWindowToolbar.navigateForward: + return context.isConnected && context.canNavigateForward + case MainWindowToolbar.previewSQL: + return context.isConnected && context.hasDataPendingChanges + case MainWindowToolbar.results: + /// The results pane belongs to the query editor. The shipped rule was `!isTableTab`, + /// which enabled it on the five kinds that have no results pane at all and then wrote a + /// collapse flag with no tab-kind guard behind it. + return context.isConnected && context.tabKind == .query + case MainWindowToolbar.history: + /// The drawer is not mounted in Agent mode, and toggling it there flipped a persisted + /// flag that sprang the drawer open on the way back to browsing. + return context.isConnected && context.contentMode == .browse + case MainWindowToolbar.dashboard: + return context.isConnected && context.supportsServerDashboard + case MainWindowToolbar.exportTables, MainWindowToolbar.newTab, MainWindowToolbar.quickSwitcher: + return context.isConnected + case MainWindowToolbar.importTables: + return context.isConnected && !context.blocksAllWrites && context.supportsImport + case MainWindowToolbar.assistant: + return context.isConnected && context.isAIEnabled + default: + return false + } + } + + /// The identifiers `hidden` is allowed to name, which is the default set and nothing else. + /// Pinned by a test so a later context cannot start hiding a button the user placed. + internal static let hideableIdentifiers: Set = [ + MainWindowToolbar.database, + MainWindowToolbar.refresh, + MainWindowToolbar.saveChanges, + ] +} diff --git a/TablePro/Core/Services/Policy/AgentModeSafeModeFloor.swift b/TablePro/Core/Services/Policy/AgentModeSafeModeFloor.swift index de767e730..ca68d6422 100644 --- a/TablePro/Core/Services/Policy/AgentModeSafeModeFloor.swift +++ b/TablePro/Core/Services/Policy/AgentModeSafeModeFloor.swift @@ -35,16 +35,26 @@ internal enum AgentModeSafeModeFloor { .contains { $0.resolvedContentMode == .agent } } - /// The level this connection should run at right now. - internal static func level(for connection: DatabaseConnection) -> SafeModeLevel { - let floor = SafeModeFloor.resolve( + /// The floor that applies to this connection right now, and why. + /// + /// Named rather than computed inline, because the reason is what the user needs: the Safe Mode + /// menu offers only the levels a floor allows and prints its explanation under them, and the + /// toolbar's padlock carries the same sentence in its tooltip. Agent mode used to raise the + /// floor silently, so choosing a weaker level appeared to do nothing and nothing said why. + internal static func effectiveFloor(for connection: DatabaseConnection) -> SafeModeFloor? { + SafeModeFloor.resolve( isEngineReadOnly: PluginMetadataRegistry.shared .snapshot(for: connection.type)?.capabilities.isEngineReadOnly ?? false, opensRemoteDatabaseFile: connection.opensRemoteDatabaseFile, managedMinimum: ManagedPolicyResolver.minimumSafeModeLevel(policy: ManagedPolicyReader.shared), isAgentModeActive: isActive(for: connection.id) ) - return floor?.raising(connection.preferredSafeModeLevel) ?? connection.preferredSafeModeLevel + } + + /// The level this connection should run at right now. + internal static func level(for connection: DatabaseConnection) -> SafeModeLevel { + effectiveFloor(for: connection)?.raising(connection.preferredSafeModeLevel) + ?? connection.preferredSafeModeLevel } /// Recomputes the live session's level after a mode change. diff --git a/TablePro/Models/UI/PendingChangeKind.swift b/TablePro/Models/UI/PendingChangeKind.swift new file mode 100644 index 000000000..3e8b5935e --- /dev/null +++ b/TablePro/Models/UI/PendingChangeKind.swift @@ -0,0 +1,78 @@ +// +// PendingChangeKind.swift +// TablePro +// + +import Foundation + +/// What the window's Save command would commit, and the verb it says. +/// +/// One value rather than five booleans read in five places. `updateToolbarPendingState()` folded +/// four of the five sources into `hasPendingChanges` and never read the fifth, so a Users & Roles +/// tab with staged principals left both the toolbar's commit button and Cmd+S dim while +/// `saveChanges()` already carried the branch that would have applied them. +/// +/// The tab decides the verb, because two kinds can be staged at once and only one of them is the +/// one the user is looking at. +internal enum PendingChangeKind: Equatable, Hashable, Sendable { + case data + case structure + case createTable + case principals + case file + + /// Nil when nothing is staged for this tab, which is what leaves the commit control dim. + internal static func resolve( + tabType: TabType?, + hasDataChanges: Bool, + hasStructureChanges: Bool, + hasCreateTablePending: Bool, + hasPrincipalChanges: Bool, + isFileDirty: Bool + ) -> PendingChangeKind? { + guard let tabType else { + return contentKind( + hasDataChanges: hasDataChanges, + hasStructureChanges: hasStructureChanges, + isFileDirty: isFileDirty + ) + } + switch tabType { + case .createTable: + /// A definition that is not yet committable is not a pending change: the tab's own + /// validity gate is what `hasCreateTablePending` already answers. + return hasCreateTablePending ? .createTable : nil + case .usersRoles: + return hasPrincipalChanges ? .principals : nil + case .query, .table, .erDiagram, .serverDashboard, .insights, .objectSource: + return contentKind( + hasDataChanges: hasDataChanges, + hasStructureChanges: hasStructureChanges, + isFileDirty: isFileDirty + ) + } + } + + /// Structure outranks data, and data outranks a dirty file, because a structure edit rewrites + /// the table the staged rows are going into and has to be named first. + private static func contentKind( + hasDataChanges: Bool, + hasStructureChanges: Bool, + isFileDirty: Bool + ) -> PendingChangeKind? { + if hasStructureChanges { return .structure } + if hasDataChanges { return .data } + return isFileDirty ? .file : nil + } + + internal var commitTitle: String { + switch self { + case .data, .structure, .file: + String(localized: "Save Changes") + case .createTable: + String(localized: "Create Table") + case .principals: + String(localized: "Apply Changes") + } + } +} diff --git a/TablePro/Models/UI/ToolbarContext.swift b/TablePro/Models/UI/ToolbarContext.swift new file mode 100644 index 000000000..842a4b845 --- /dev/null +++ b/TablePro/Models/UI/ToolbarContext.swift @@ -0,0 +1,139 @@ +// +// ToolbarContext.swift +// TablePro +// + +import Foundation + +/// Everything the connection window's toolbar is allowed to know about what the window is showing. +/// +/// The toolbar used to carry one tab-shaped fact, `isTableTab`, and no content mode at all, so a +/// single list of identifiers was vended for every tab kind, every pane and both content modes and +/// the only lever anyone had was dimming. Every feature that arrived then had to buy a permanent +/// slot in the titlebar. +/// +/// Nothing global is read inside this struct or inside the resolvers that take it. It is built once +/// per change by the toolbar and passed down, so the resolvers stay pure and testable with no host +/// app and no session. +internal struct ToolbarContext: Equatable { + /// What the window's detail pane is drawing. `nil` when no tab is selected, which is a real + /// state on a window that has just opened. + internal let tabKind: TabType? + internal let resultsMode: ResultsViewMode? + internal let contentMode: ConnectionWorkspaceContentMode + internal let pane: ConnectionWindowPane + + /// True whenever the session is alive, which includes a query in flight. A running query is not + /// a reason to disable Refresh, and the menu bar derives its own answer from the window phase + /// rather than from execution. + internal let isConnected: Bool + /// A connection is on screen, whether or not it has finished connecting. + internal let hasSelectedWorkspace: Bool + internal let isTrailingPaneOpen: Bool + internal let canToggleTrailingPane: Bool + + internal let pendingChange: PendingChangeKind? + /// Not a projection of `pendingChange`. The two are computed from different inputs: a dirty + /// query file raises the commit control and leaves this false, which is exactly the distinction + /// Preview SQL is gated on. + internal let hasDataPendingChanges: Bool + internal let blocksAllWrites: Bool + + internal let canAddRow: Bool + internal let canRestorePreviousValues: Bool + internal let canNavigateBack: Bool + internal let canNavigateForward: Bool + + internal let isFileBased: Bool + internal let supportsContainerSwitching: Bool + internal let supportsImport: Bool + internal let supportsServerDashboard: Bool + + internal let isAIEnabled: Bool + internal let hasAgentSession: Bool + + /// What this engine calls the thing the centre's second capsule names, and what it calls its + /// query language. Both are words the Actions menu puts in front of the user. + internal let containerEntityName: String + internal let queryLanguageName: String + + /// The subset of the context that may move an item in or out of the titlebar. + /// + /// This is the whole anti-reflow rule in one type. `isHidden` is written only from these + /// fields, so the item set can change on a tab switch, a mode switch or a connection switch and + /// on nothing else; everything transient rides `isEnabled` instead. A keystroke in a cell + /// editor therefore costs one struct comparison and writes nothing. + internal struct VisibilityKey: Equatable { + internal let tabKind: TabType? + internal let resultsMode: ResultsViewMode? + internal let contentMode: ConnectionWorkspaceContentMode + internal let isFileBased: Bool + internal let supportsContainerSwitching: Bool + internal let supportsImport: Bool + internal let supportsServerDashboard: Bool + internal let isAIEnabled: Bool + } + + internal var visibilityKey: VisibilityKey { + VisibilityKey( + tabKind: tabKind, + resultsMode: resultsMode, + contentMode: contentMode, + isFileBased: isFileBased, + supportsContainerSwitching: supportsContainerSwitching, + supportsImport: supportsImport, + supportsServerDashboard: supportsServerDashboard, + isAIEnabled: isAIEnabled + ) + } + + internal init( + tabKind: TabType? = nil, + resultsMode: ResultsViewMode? = nil, + contentMode: ConnectionWorkspaceContentMode = .browse, + pane: ConnectionWindowPane = .empty, + isConnected: Bool = false, + hasSelectedWorkspace: Bool = false, + isTrailingPaneOpen: Bool = false, + canToggleTrailingPane: Bool = false, + pendingChange: PendingChangeKind? = nil, + hasDataPendingChanges: Bool = false, + blocksAllWrites: Bool = false, + canAddRow: Bool = false, + canRestorePreviousValues: Bool = false, + canNavigateBack: Bool = false, + canNavigateForward: Bool = false, + isFileBased: Bool = false, + supportsContainerSwitching: Bool = false, + supportsImport: Bool = false, + supportsServerDashboard: Bool = false, + isAIEnabled: Bool = false, + hasAgentSession: Bool = false, + containerEntityName: String = "", + queryLanguageName: String = "" + ) { + self.tabKind = tabKind + self.resultsMode = resultsMode + self.contentMode = contentMode + self.pane = pane + self.isConnected = isConnected + self.hasSelectedWorkspace = hasSelectedWorkspace + self.isTrailingPaneOpen = isTrailingPaneOpen + self.canToggleTrailingPane = canToggleTrailingPane + self.pendingChange = pendingChange + self.hasDataPendingChanges = hasDataPendingChanges + self.blocksAllWrites = blocksAllWrites + self.canAddRow = canAddRow + self.canRestorePreviousValues = canRestorePreviousValues + self.canNavigateBack = canNavigateBack + self.canNavigateForward = canNavigateForward + self.isFileBased = isFileBased + self.supportsContainerSwitching = supportsContainerSwitching + self.supportsImport = supportsImport + self.supportsServerDashboard = supportsServerDashboard + self.isAIEnabled = isAIEnabled + self.hasAgentSession = hasAgentSession + self.containerEntityName = containerEntityName + self.queryLanguageName = queryLanguageName + } +} diff --git a/TablePro/Models/UI/TrailingPaneSurfaceResolver.swift b/TablePro/Models/UI/TrailingPaneSurfaceResolver.swift new file mode 100644 index 000000000..187f2baf6 --- /dev/null +++ b/TablePro/Models/UI/TrailingPaneSurfaceResolver.swift @@ -0,0 +1,47 @@ +// +// TrailingPaneSurfaceResolver.swift +// TablePro +// + +import Foundation + +/// Which surface the window's trailing pane is drawing, and which surfaces the user may pick. +/// +/// The single answer, because four readings of the same question disagreed and only one of them +/// knew Agent mode imposes the result pane. Show Inspector therefore titled itself Hide Inspector +/// over a column the inspector does not own, collapsed it, and left no command that could bring it +/// back; Show Assistant persisted a browse preference the mode overrode on the next read. +internal enum TrailingPaneSurfaceResolver { + /// The content mode is resolved first, so a window left in Agent mode with the AI setting off + /// cannot ask for a surface the pane will never draw. + internal static func resolve( + stored: TrailingPaneSurface, + contentMode: ConnectionWorkspaceContentMode, + isAIEnabled: Bool + ) -> TrailingPaneSurface { + switch ConnectionWorkspaceContentMode.resolved(contentMode, isAIEnabled: isAIEnabled) { + case .agent: + return .agentResult + case .browse: + /// The result pane belongs to Agent mode, so browsing never draws it however the stored + /// value got there. `TrailingPaneSurface.resolved` passes it through, because its own + /// job is the AI setting rather than the mode. + guard stored.isUserSelectable else { return .inspector } + return TrailingPaneSurface.resolved(stored, isAIEnabled: isAIEnabled) + } + } + + /// What the pane's header offers. Empty in Agent mode, where the mode chose for the user, which + /// is what makes the header draw a plain title rather than a picker with one segment in it. + internal static func selectable( + contentMode: ConnectionWorkspaceContentMode, + isAIEnabled: Bool + ) -> [TrailingPaneSurface] { + switch ConnectionWorkspaceContentMode.resolved(contentMode, isAIEnabled: isAIEnabled) { + case .agent: + return [] + case .browse: + return isAIEnabled ? [.inspector, .assistant] : [.inspector] + } + } +} diff --git a/TableProTests/Core/Services/Infrastructure/ConnectionActionsMenuResolverTests.swift b/TableProTests/Core/Services/Infrastructure/ConnectionActionsMenuResolverTests.swift new file mode 100644 index 000000000..578e0bfa8 --- /dev/null +++ b/TableProTests/Core/Services/Infrastructure/ConnectionActionsMenuResolverTests.swift @@ -0,0 +1,265 @@ +// +// ConnectionActionsMenuResolverTests.swift +// TableProTests +// + +import AppKit +@testable import TablePro +import Testing + +@Suite("Connection actions menu resolver") +struct ConnectionActionsMenuResolverTests { + private static let tabKinds: [TabType] = [ + .query, .table, .createTable, .erDiagram, .serverDashboard, .usersRoles, .insights, .objectSource, + ] + + /// Every title the pull-down may use, and every one of them is a title the menu bar already + /// carries. The rule this pins is that the pull-down is a second route to commands that exist, + /// never a place for commands that exist nowhere else: a menu nobody can find from the menu bar + /// is a menu that cannot be searched, rebound or discovered. + private static let menuBarTitles: Set = [ + String(localized: "Add Row"), + String(localized: "Restore Previous Values…"), + String(localized: "Back"), + String(localized: "Forward"), + String(localized: "Preview SQL"), + String(localized: "Show Results"), + String(localized: "Export Results…"), + String(localized: "Export Tables…"), + String(localized: "Import Data…"), + String(localized: "Show DDL"), + String(localized: "Copy DDL"), + String(localized: "Show Query History"), + String(localized: "Users & Roles"), + String(localized: "Query Insights"), + String(localized: "Server Dashboard"), + String(localized: "New Tab"), + String(localized: "Open Quickly…"), + String(localized: "Mode"), + String(localized: "Switch Connection…"), + String(localized: "Reconnect"), + String(localized: "Close Connection"), + ] + + private static func context( + tabKind: TabType? = .table, + resultsMode: ResultsViewMode? = .data, + contentMode: ConnectionWorkspaceContentMode = .browse, + isConnected: Bool = true, + supportsImport: Bool = true, + supportsServerDashboard: Bool = true, + isAIEnabled: Bool = true + ) -> ToolbarContext { + ToolbarContext( + tabKind: tabKind, + resultsMode: resultsMode, + contentMode: contentMode, + pane: isConnected ? .content : .unavailable(.notConnected), + isConnected: isConnected, + hasSelectedWorkspace: true, + supportsImport: supportsImport, + supportsServerDashboard: supportsServerDashboard, + isAIEnabled: isAIEnabled + ) + } + + private static func entries(_ context: ToolbarContext) -> [ActionsMenuEntry] { + ConnectionActionsMenuResolver.sections(context).flatMap(\.entries) + } + + private static func titles(_ context: ToolbarContext) -> [String] { + entries(context).map(\.title) + } + + // MARK: - House rules + + @Test("Every entry the pull-down can emit has a menu-bar twin with the same title") + func everyEntryHasAMenuBarTwin() { + for tabKind in Self.tabKinds + [nil] { + for contentMode in ConnectionWorkspaceContentMode.allCases { + for isConnected in [true, false] { + for title in Self.titles( + Self.context(tabKind: tabKind, contentMode: contentMode, isConnected: isConnected) + ) { + #expect(Self.menuBarTitles.contains(title), "\(title) has no menu-bar twin") + } + } + } + } + } + + /// A section is a run drawn between two separators. Past about six entries a run stops reading + /// as a group and becomes a list, which is what the pull-down exists to avoid. + @Test("No section runs longer than six entries") + func sectionsStayShort() { + for tabKind in Self.tabKinds + [nil] { + for contentMode in ConnectionWorkspaceContentMode.allCases { + for isConnected in [true, false] { + let sections = ConnectionActionsMenuResolver.sections( + Self.context(tabKind: tabKind, contentMode: contentMode, isConnected: isConnected) + ) + for section in sections { + #expect(section.entries.count <= 6) + #expect(section.entries.isEmpty == false) + } + } + } + } + } + + @Test("A command never appears twice in one menu") + func titlesAreUniqueWithinAMenu() { + for tabKind in Self.tabKinds + [nil] { + for contentMode in ConnectionWorkspaceContentMode.allCases { + let titles = Self.titles(Self.context(tabKind: tabKind, contentMode: contentMode)) + #expect(Set(titles).count == titles.count) + } + } + } + + /// The window always has a way out of itself, whatever it is showing and whether or not the + /// connection is up. This is what lets the pull-down stay enabled in every phase. + @Test("Every context offers a route to another connection") + func everyContextOffersSwitchConnection() { + for tabKind in Self.tabKinds + [nil] { + for contentMode in ConnectionWorkspaceContentMode.allCases { + for isConnected in [true, false] { + let titles = Self.titles( + Self.context(tabKind: tabKind, contentMode: contentMode, isConnected: isConnected) + ) + #expect(titles.contains(String(localized: "Switch Connection…"))) + #expect(titles.contains(String(localized: "Close Connection"))) + } + } + } + } + + // MARK: - Capability gates + + @Test("The import submenu is offered only by an engine that has one") + func importSubmenuFollowsTheDriver() { + let withImport = Self.entries(Self.context(supportsImport: true)) + let without = Self.entries(Self.context(supportsImport: false)) + + #expect(withImport.contains { $0.submenu == .importFormats }) + #expect(without.contains { $0.submenu == .importFormats } == false) + } + + @Test("Server Dashboard is offered only by an engine that has one") + func dashboardFollowsTheDriver() { + #expect( + Self.titles(Self.context(supportsServerDashboard: true)) + .contains(String(localized: "Server Dashboard")) + ) + #expect( + Self.titles(Self.context(supportsServerDashboard: false)) + .contains(String(localized: "Server Dashboard")) == false + ) + } + + /// Agent mode is the AI feature, so with the setting off there is no mode to choose between. + @Test("The Mode submenu is offered only while AI is on") + func modeSubmenuFollowsTheSetting() { + #expect(Self.entries(Self.context(isAIEnabled: true)).contains { $0.submenu == .mode }) + #expect(Self.entries(Self.context(isAIEnabled: false)).contains { $0.submenu == .mode } == false) + } + + @Test("Reconnect is offered only when there is nothing connected") + func reconnectFollowsTheSession() { + #expect( + Self.titles(Self.context(isConnected: false)).contains(String(localized: "Reconnect")) + ) + #expect( + Self.titles(Self.context(isConnected: true)).contains(String(localized: "Reconnect")) == false + ) + } + + // MARK: - Per-context content + + /// Add Row asks the same question the grid does, so it follows the results mode rather than the + /// tab kind alone. + @Test("Add Row is offered on a table tab showing data and nowhere else", arguments: ResultsViewMode.allCases) + func addRowFollowsTheResultsMode(mode: ResultsViewMode) { + let onTable = Self.titles(Self.context(tabKind: .table, resultsMode: mode)) + #expect(onTable.contains(String(localized: "Add Row")) == (mode == .data)) + } + + @Test("Add Row is never offered outside a table tab", arguments: tabKinds.filter { $0 != .table }) + func addRowIsTableOnly(tabKind: TabType) { + #expect( + Self.titles(Self.context(tabKind: tabKind)).contains(String(localized: "Add Row")) == false + ) + } + + @Test("Show Results is offered on a query tab and nowhere else", arguments: tabKinds) + func showResultsIsQueryOnly(tabKind: TabType) { + let offered = Self.titles(Self.context(tabKind: tabKind)).contains(String(localized: "Show Results")) + #expect(offered == (tabKind == .query)) + } + + /// Back and Forward walk a table's own browse history, which only a table tab has. They were + /// two permanent hit targets in the titlebar and dim on the other seven kinds. + @Test("Back and Forward are offered on a table tab and nowhere else", arguments: tabKinds) + func navigationIsTableOnly(tabKind: TabType) { + let titles = Self.titles(Self.context(tabKind: tabKind)) + #expect(titles.contains(String(localized: "Back")) == (tabKind == .table)) + #expect(titles.contains(String(localized: "Forward")) == (tabKind == .table)) + } + + @Test("An unsaved definition offers its preview but nothing that reads rows") + func createTableOffersPreviewOnly() { + let titles = Self.titles(Self.context(tabKind: .createTable)) + #expect(titles.contains(String(localized: "Preview SQL"))) + #expect(titles.contains(String(localized: "Add Row")) == false) + #expect(titles.contains(String(localized: "Export Results…")) == false) + } + + /// Agent mode has no grid, no object browser and no tab to act on, so the pull-down carries the + /// window's own commands and stops. + @Test("Agent mode offers only the mode and the connection") + func agentModeIsMinimal() { + let titles = Self.titles(Self.context(contentMode: .agent)) + #expect(titles.contains(String(localized: "Mode"))) + #expect(titles.contains(String(localized: "Switch Connection…"))) + #expect(titles.contains(String(localized: "Close Connection"))) + #expect(titles.contains(String(localized: "Add Row")) == false) + #expect(titles.contains(String(localized: "Show Query History")) == false) + #expect(titles.contains(String(localized: "New Tab")) == false) + } + + /// A window whose connection went away offers the three commands that can do something about + /// it, and nothing that needs a session. + @Test("A window with no session offers only what can be done without one") + func disconnectedIsMinimal() { + let titles = Self.titles(Self.context(isConnected: false)) + #expect(titles.contains(String(localized: "Switch Connection…"))) + #expect(titles.contains(String(localized: "Reconnect"))) + #expect(titles.contains(String(localized: "Close Connection"))) + #expect(titles.contains(String(localized: "Export Tables…")) == false) + #expect(titles.contains(String(localized: "Show Query History")) == false) + } + + // MARK: - Selectors + + /// Every selector is spelled exactly as the class declares it. A selector with a colon the + /// implementation does not have reaches nothing and AppKit draws the entry disabled, which + /// fails quietly. + @Test("Reconnect takes no sender, as the menu bar spells it") + func reconnectTakesNoSender() { + let reconnect = Self.entries(Self.context(isConnected: false)) + .first { $0.title == String(localized: "Reconnect") } + #expect(reconnect?.selector == NSSelectorFromString("retryConnection")) + } + + @Test("Every entry carries a selector and a title") + func everyEntryIsComplete() { + for tabKind in Self.tabKinds + [nil] { + for contentMode in ConnectionWorkspaceContentMode.allCases { + for entry in Self.entries(Self.context(tabKind: tabKind, contentMode: contentMode)) { + #expect(entry.title.isEmpty == false) + #expect(NSStringFromSelector(entry.selector).isEmpty == false) + } + } + } + } +} diff --git a/TableProTests/Core/Services/Infrastructure/ToolbarContextResolverTests.swift b/TableProTests/Core/Services/Infrastructure/ToolbarContextResolverTests.swift new file mode 100644 index 000000000..2e2f716b2 --- /dev/null +++ b/TableProTests/Core/Services/Infrastructure/ToolbarContextResolverTests.swift @@ -0,0 +1,382 @@ +// +// ToolbarContextResolverTests.swift +// TableProTests +// + +import AppKit +@testable import TablePro +import Testing + +@Suite("Toolbar context resolver") +struct ToolbarContextResolverTests { + /// `TabType` is not `CaseIterable`, so the list is written out. A ninth kind fails the + /// exhaustive switch in the resolver before it can fail here. + private static let tabKinds: [TabType] = [ + .query, .table, .createTable, .erDiagram, .serverDashboard, .usersRoles, .insights, .objectSource, + ] + + private static let panes: [ConnectionWindowPane] = [ + .connecting, .unavailable(.notConnected), .content, .empty, + ] + + /// The hit targets in the default set: one sidebar toggle, the centred pair, the three content + /// commands, Safe Mode and the trailing-pane toggle. Spacers and tracking separators take no + /// click and are not counted. + private static let defaultHitTargets: [NSToolbarItem.Identifier] = [ + .toggleSidebar, + MainWindowToolbar.connection, + MainWindowToolbar.database, + MainWindowToolbar.refresh, + MainWindowToolbar.saveChanges, + MainWindowToolbar.actions, + MainWindowToolbar.safeMode, + MainWindowToolbar.inspector, + ] + + private static func context( + tabKind: TabType? = .table, + resultsMode: ResultsViewMode? = .data, + contentMode: ConnectionWorkspaceContentMode = .browse, + pane: ConnectionWindowPane = .content, + isFileBased: Bool = false, + supportsContainerSwitching: Bool = true, + isAIEnabled: Bool = true + ) -> ToolbarContext { + ToolbarContext( + tabKind: tabKind, + resultsMode: resultsMode, + contentMode: contentMode, + pane: pane, + isConnected: pane == .content, + hasSelectedWorkspace: true, + canToggleTrailingPane: pane == .content, + isFileBased: isFileBased, + supportsContainerSwitching: supportsContainerSwitching, + isAIEnabled: isAIEnabled + ) + } + + private static func visibleCount(_ context: ToolbarContext) -> Int { + let hidden = ToolbarContextResolver.hidden(context) + return defaultHitTargets.filter { !hidden.contains($0) }.count + } + + // MARK: - The ceiling + + /// The whole point of the revamp: no context may put more than eight things to click in the + /// titlebar. The guard test this replaces counted identifiers rather than hit targets, which is + /// how a two-segment control was added to a full titlebar and passed. + @Test("No reachable context exceeds eight hit targets") + func hitTargetCeiling() { + for tabKind in Self.tabKinds + [nil] { + for contentMode in ConnectionWorkspaceContentMode.allCases { + for pane in Self.panes { + for isFileBased in [true, false] { + let context = Self.context( + tabKind: tabKind, + contentMode: contentMode, + pane: pane, + isFileBased: isFileBased + ) + #expect(Self.visibleCount(context) <= 8) + } + } + } + } + } + + @Test("Agent mode shows six") + func agentModeShowsSix() { + #expect(Self.visibleCount(Self.context(contentMode: .agent)) == 6) + } + + @Test("A file-based connection drops the container capsule") + func fileBasedShowsSeven() { + #expect(Self.visibleCount(Self.context(isFileBased: true)) == 7) + } + + // MARK: - What may be hidden + + /// An item the user dragged in from the customization palette is opt-in, so it stays where they + /// put it and dims. Only the default set may be taken off screen. + @Test("The hidden set never reaches past the default set") + func hiddenStaysInsideTheDefaultSet() { + for tabKind in Self.tabKinds + [nil] { + for contentMode in ConnectionWorkspaceContentMode.allCases { + for isFileBased in [true, false] { + for supportsContainerSwitching in [true, false] { + let hidden = ToolbarContextResolver.hidden( + Self.context( + tabKind: tabKind, + contentMode: contentMode, + isFileBased: isFileBased, + supportsContainerSwitching: supportsContainerSwitching + ) + ) + #expect(hidden.isSubset(of: ToolbarContextResolver.hideableIdentifiers)) + } + } + } + } + } + + /// The window's own identity, the pull-down that carries everything displaced, the control that + /// says whether a keystroke can reach a live table, and the two pane toggles. None of these has + /// a context in which it means nothing. + @Test("The permanent controls are never hidden") + func permanentControlsAreNeverHidden() { + let permanent: Set = [ + .toggleSidebar, + MainWindowToolbar.connection, + MainWindowToolbar.actions, + MainWindowToolbar.safeMode, + MainWindowToolbar.inspector, + ] + for tabKind in Self.tabKinds + [nil] { + for contentMode in ConnectionWorkspaceContentMode.allCases { + for pane in Self.panes { + let hidden = ToolbarContextResolver.hidden( + Self.context(tabKind: tabKind, contentMode: contentMode, pane: pane) + ) + #expect(hidden.isDisjoint(with: permanent)) + } + } + } + } + + /// Measured on macOS 27: hiding both subitems makes the group vanish while `group.isHidden` + /// stays false, and a popover anchored on it then opens at the window's centre. + @Test("The centred group never loses both of its capsules") + func centredGroupKeepsACapsule() { + for tabKind in Self.tabKinds + [nil] { + for contentMode in ConnectionWorkspaceContentMode.allCases { + for isFileBased in [true, false] { + let hidden = ToolbarContextResolver.hidden( + Self.context(tabKind: tabKind, contentMode: contentMode, isFileBased: isFileBased) + ) + let both = hidden.contains(MainWindowToolbar.connection) + && hidden.contains(MainWindowToolbar.database) + #expect(both == false) + } + } + } + } + + // MARK: - The anti-reflow rule + + /// `isHidden` is written only from the slow-moving subset, so the item set can change on a tab + /// switch, a mode switch or a connection switch and on nothing else. A keystroke in a cell + /// editor costs one struct comparison and moves nothing. + @Test("Visibility ignores everything transient") + func visibilityIgnoresTransientState() { + let quiet = ToolbarContext( + tabKind: .table, + resultsMode: .data, + contentMode: .browse, + pane: .content, + isConnected: true, + hasSelectedWorkspace: true, + canToggleTrailingPane: true, + supportsContainerSwitching: true + ) + let busy = ToolbarContext( + tabKind: .table, + resultsMode: .data, + contentMode: .browse, + pane: .connecting, + isConnected: false, + hasSelectedWorkspace: true, + isTrailingPaneOpen: true, + canToggleTrailingPane: false, + pendingChange: .data, + hasDataPendingChanges: true, + blocksAllWrites: true, + canAddRow: true, + canRestorePreviousValues: true, + canNavigateBack: true, + canNavigateForward: true, + supportsContainerSwitching: true, + hasAgentSession: true + ) + + #expect(quiet.visibilityKey == busy.visibilityKey) + #expect(ToolbarContextResolver.hidden(quiet) == ToolbarContextResolver.hidden(busy)) + } + + // MARK: - Per-kind sets + + @Test("An unsaved definition has nothing to reload") + func createTableHidesRefresh() { + let hidden = ToolbarContextResolver.hidden(Self.context(tabKind: .createTable)) + #expect(hidden.contains(MainWindowToolbar.refresh)) + #expect(hidden.contains(MainWindowToolbar.saveChanges) == false) + } + + @Test( + "The four kinds that can never stage a change lose the commit control", + arguments: [TabType.erDiagram, .serverDashboard, .insights, .objectSource] + ) + func readOnlyKindsHideSaveChanges(tabKind: TabType) { + let hidden = ToolbarContextResolver.hidden(Self.context(tabKind: tabKind)) + #expect(hidden.contains(MainWindowToolbar.saveChanges)) + #expect(hidden.contains(MainWindowToolbar.refresh) == false) + } + + @Test("The three kinds that can stage a change keep both content commands", arguments: [ + TabType.query, .table, .usersRoles, + ]) + func editableKindsKeepBoth(tabKind: TabType) { + let hidden = ToolbarContextResolver.hidden(Self.context(tabKind: tabKind)) + #expect(hidden.contains(MainWindowToolbar.saveChanges) == false) + #expect(hidden.contains(MainWindowToolbar.refresh) == false) + } + + @Test("Agent mode has no grid to reload and nothing mounted to commit") + func agentModeHidesBothContentCommands() { + let hidden = ToolbarContextResolver.hidden(Self.context(contentMode: .agent)) + #expect(hidden.contains(MainWindowToolbar.refresh)) + #expect(hidden.contains(MainWindowToolbar.saveChanges)) + } + + @Test("A window with no selected tab keeps the full set") + func noSelectedTabKeepsEverything() { + #expect(ToolbarContextResolver.hidden(Self.context(tabKind: nil)).isEmpty) + } + + @Test("The results mode never moves an item", arguments: ResultsViewMode.allCases) + func resultsModeNeverMovesAnything(mode: ResultsViewMode) { + #expect( + ToolbarContextResolver.hidden(Self.context(resultsMode: mode)) + == ToolbarContextResolver.hidden(Self.context(resultsMode: .data)) + ) + } + + @Test("The container capsule goes when the engine has nothing to switch to") + func containerCapsuleVisibility() { + #expect( + ToolbarContextResolver.hidden(Self.context(isFileBased: false, supportsContainerSwitching: true)) + .contains(MainWindowToolbar.database) == false + ) + #expect( + ToolbarContextResolver.hidden(Self.context(isFileBased: true, supportsContainerSwitching: true)) + .contains(MainWindowToolbar.database) + ) + #expect( + ToolbarContextResolver.hidden(Self.context(isFileBased: false, supportsContainerSwitching: false)) + .contains(MainWindowToolbar.database) + ) + } + + // MARK: - Enablement + + /// Switch Connection is the window's command and the route back from a connection that failed, + /// so it answers in every phase including the one with no session at all. + @Test("Only the connection chooser answers over an empty pane") + func emptyPaneDisablesEverythingButTheConnection() { + let context = Self.context(pane: .empty) + for identifier in Self.defaultHitTargets where identifier != MainWindowToolbar.connection { + guard identifier != .toggleSidebar else { continue } + #expect(ToolbarContextResolver.isEnabled(identifier, context: context) == false) + } + #expect(ToolbarContextResolver.isEnabled(MainWindowToolbar.connection, context: context)) + } + + /// The pull-down offers Switch Connection, Reconnect and Close Connection here, so it is the + /// route out of a window whose connection went away. + @Test("The pull-down answers while connecting and while unavailable", arguments: [ + ConnectionWindowPane.connecting, .unavailable(.notConnected), + ]) + func actionsAnswersWithoutASession(pane: ConnectionWindowPane) { + #expect(ToolbarContextResolver.isEnabled(MainWindowToolbar.actions, context: Self.context(pane: pane))) + } + + /// The shipped rule was `connected && !isTableTab`, which enabled the command on the five kinds + /// that have no results pane at all and then wrote a collapse flag with no tab-kind guard. + @Test("Show Results answers on a query tab and nowhere else", arguments: tabKinds) + func showResultsIsQueryOnly(tabKind: TabType) { + let enabled = ToolbarContextResolver.isEnabled( + MainWindowToolbar.results, + context: Self.context(tabKind: tabKind) + ) + #expect(enabled == (tabKind == .query)) + } + + /// The drawer is not mounted in Agent mode, and toggling it there flipped a persisted flag that + /// sprang it open on the way back to browsing. + @Test("Query History answers while browsing a live session and nowhere else") + func queryHistoryGating() { + #expect(ToolbarContextResolver.isEnabled(MainWindowToolbar.history, context: Self.context())) + #expect( + ToolbarContextResolver.isEnabled( + MainWindowToolbar.history, + context: Self.context(contentMode: .agent) + ) == false + ) + #expect( + ToolbarContextResolver.isEnabled( + MainWindowToolbar.history, + context: Self.context(pane: .connecting) + ) == false + ) + } + + /// A connection that drops with the pane open must still be able to close it, which is the + /// state the old `connected` rule disabled on the app's minimum OS. + @Test("The trailing-pane toggle follows whether the pane can be toggled") + func trailingPaneToggleFollowsItsOwnRule() { + var context = Self.context(pane: .unavailable(.notConnected)) + #expect(ToolbarContextResolver.isEnabled(MainWindowToolbar.inspector, context: context) == false) + + context = ToolbarContext( + tabKind: .table, + contentMode: .browse, + pane: .unavailable(.notConnected), + isConnected: false, + hasSelectedWorkspace: true, + isTrailingPaneOpen: true, + canToggleTrailingPane: true + ) + #expect(ToolbarContextResolver.isEnabled(MainWindowToolbar.inspector, context: context)) + } + + /// The old switch ended in `default: return true`, so every identifier nobody had thought about + /// was live, including over a window with no coordinator and no session. + @Test("An identifier the toolbar does not vend does not answer") + func unknownIdentifiersDoNotAnswer() { + #expect( + ToolbarContextResolver.isEnabled( + NSToolbarItem.Identifier("com.TablePro.toolbar.nothingAtAll"), + context: Self.context() + ) == false + ) + } + + @Test("The commit control answers only over staged work a safe mode allows") + func commitControlGating() { + var context = Self.context() + #expect(ToolbarContextResolver.isEnabled(MainWindowToolbar.saveChanges, context: context) == false) + + context = ToolbarContext( + tabKind: .table, + contentMode: .browse, + pane: .content, + isConnected: true, + hasSelectedWorkspace: true, + pendingChange: .data, + supportsContainerSwitching: true + ) + #expect(ToolbarContextResolver.isEnabled(MainWindowToolbar.saveChanges, context: context)) + + context = ToolbarContext( + tabKind: .table, + contentMode: .browse, + pane: .content, + isConnected: true, + hasSelectedWorkspace: true, + pendingChange: .data, + blocksAllWrites: true, + supportsContainerSwitching: true + ) + #expect(ToolbarContextResolver.isEnabled(MainWindowToolbar.saveChanges, context: context) == false) + } +} diff --git a/TableProTests/Models/PendingChangeKindTests.swift b/TableProTests/Models/PendingChangeKindTests.swift new file mode 100644 index 000000000..67476858a --- /dev/null +++ b/TableProTests/Models/PendingChangeKindTests.swift @@ -0,0 +1,179 @@ +// +// PendingChangeKindTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import Testing + +@Suite("Pending change kind") +struct PendingChangeKindTests { + private static let contentKinds: [TabType] = [ + .query, .table, .erDiagram, .serverDashboard, .insights, .objectSource, + ] + + @Test("Nothing staged leaves no kind", arguments: contentKinds + [.createTable, .usersRoles]) + func nothingStaged(tabType: TabType) { + #expect( + PendingChangeKind.resolve( + tabType: tabType, + hasDataChanges: false, + hasStructureChanges: false, + hasCreateTablePending: false, + hasPrincipalChanges: false, + isFileDirty: false + ) == nil + ) + } + + @Test("Each source names its own kind", arguments: contentKinds) + func eachSource(tabType: TabType) { + let data = PendingChangeKind.resolve( + tabType: tabType, + hasDataChanges: true, + hasStructureChanges: false, + hasCreateTablePending: false, + hasPrincipalChanges: false, + isFileDirty: false + ) + let structure = PendingChangeKind.resolve( + tabType: tabType, + hasDataChanges: false, + hasStructureChanges: true, + hasCreateTablePending: false, + hasPrincipalChanges: false, + isFileDirty: false + ) + let file = PendingChangeKind.resolve( + tabType: tabType, + hasDataChanges: false, + hasStructureChanges: false, + hasCreateTablePending: false, + hasPrincipalChanges: false, + isFileDirty: true + ) + + #expect(data == .data) + #expect(structure == .structure) + #expect(file == .file) + } + + /// The defect this type exists for: the commit control was dim on a Users & Roles tab with + /// staged principals, because the one function that fed it never read that flag. + @Test("Staged principals are a pending change on a Users & Roles tab") + func principalsAreAPendingChange() { + #expect( + PendingChangeKind.resolve( + tabType: .usersRoles, + hasDataChanges: false, + hasStructureChanges: false, + hasCreateTablePending: false, + hasPrincipalChanges: true, + isFileDirty: false + ) == .principals + ) + } + + @Test("A committable definition is a pending change on a Create Table tab") + func createTableIsAPendingChange() { + #expect( + PendingChangeKind.resolve( + tabType: .createTable, + hasDataChanges: true, + hasStructureChanges: true, + hasCreateTablePending: true, + hasPrincipalChanges: true, + isFileDirty: true + ) == .createTable + ) + } + + /// A Create Table tab whose definition is not yet valid stages nothing, whatever else is true + /// elsewhere in the window. + @Test("An incomplete definition stages nothing, whatever else is set") + func incompleteDefinitionStagesNothing() { + #expect( + PendingChangeKind.resolve( + tabType: .createTable, + hasDataChanges: true, + hasStructureChanges: true, + hasCreateTablePending: false, + hasPrincipalChanges: true, + isFileDirty: true + ) == nil + ) + } + + @Test("Structure outranks data, and data outranks a dirty file", arguments: contentKinds) + func precedence(tabType: TabType) { + let structureOverData = PendingChangeKind.resolve( + tabType: tabType, + hasDataChanges: true, + hasStructureChanges: true, + hasCreateTablePending: false, + hasPrincipalChanges: false, + isFileDirty: true + ) + let dataOverFile = PendingChangeKind.resolve( + tabType: tabType, + hasDataChanges: true, + hasStructureChanges: false, + hasCreateTablePending: false, + hasPrincipalChanges: false, + isFileDirty: true + ) + + #expect(structureOverData == .structure) + #expect(dataOverFile == .data) + } + + /// Principals belong to their own tab. A stale flag left by a tab the user closed must not + /// light the commit control somewhere else. + @Test("Staged principals stage nothing outside a Users & Roles tab", arguments: contentKinds) + func principalsAreTabScoped(tabType: TabType) { + #expect( + PendingChangeKind.resolve( + tabType: tabType, + hasDataChanges: false, + hasStructureChanges: false, + hasCreateTablePending: false, + hasPrincipalChanges: true, + isFileDirty: false + ) == nil + ) + } + + @Test("A window with no selected tab still answers for its content sources") + func noSelectedTab() { + #expect( + PendingChangeKind.resolve( + tabType: nil, + hasDataChanges: true, + hasStructureChanges: false, + hasCreateTablePending: false, + hasPrincipalChanges: false, + isFileDirty: false + ) == .data + ) + #expect( + PendingChangeKind.resolve( + tabType: nil, + hasDataChanges: false, + hasStructureChanges: false, + hasCreateTablePending: false, + hasPrincipalChanges: false, + isFileDirty: false + ) == nil + ) + } + + @Test("Each kind names the verb its tab commits with") + func commitTitles() { + #expect(PendingChangeKind.data.commitTitle == PendingChangeKind.structure.commitTitle) + #expect(PendingChangeKind.data.commitTitle == PendingChangeKind.file.commitTitle) + #expect(PendingChangeKind.createTable.commitTitle != PendingChangeKind.data.commitTitle) + #expect(PendingChangeKind.principals.commitTitle != PendingChangeKind.data.commitTitle) + #expect(PendingChangeKind.principals.commitTitle != PendingChangeKind.createTable.commitTitle) + } +} diff --git a/TableProTests/Models/TrailingPaneSurfaceResolverTests.swift b/TableProTests/Models/TrailingPaneSurfaceResolverTests.swift new file mode 100644 index 000000000..d3c1ec545 --- /dev/null +++ b/TableProTests/Models/TrailingPaneSurfaceResolverTests.swift @@ -0,0 +1,105 @@ +// +// TrailingPaneSurfaceResolverTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import Testing + +@Suite("Trailing pane surface resolver") +struct TrailingPaneSurfaceResolverTests { + @Test( + "Agent mode draws the result pane whatever the user last chose", + arguments: TrailingPaneSurface.allCases + ) + func agentModeImposesTheResult(stored: TrailingPaneSurface) { + #expect( + TrailingPaneSurfaceResolver.resolve(stored: stored, contentMode: .agent, isAIEnabled: true) + == .agentResult + ) + } + + @Test("Browsing draws the surface the user chose") + func browsingHonoursTheStoredSurface() { + #expect( + TrailingPaneSurfaceResolver.resolve(stored: .inspector, contentMode: .browse, isAIEnabled: true) + == .inspector + ) + #expect( + TrailingPaneSurfaceResolver.resolve(stored: .assistant, contentMode: .browse, isAIEnabled: true) + == .assistant + ) + } + + /// The assistant is the one surface a setting can take away, and the stored value is restored + /// per connection without anything asking whether the surface still exists. + @Test("With AI off, every surface resolves to the inspector", arguments: TrailingPaneSurface.allCases) + func aiOffFallsBackToTheInspector(stored: TrailingPaneSurface) { + #expect( + TrailingPaneSurfaceResolver.resolve(stored: stored, contentMode: .browse, isAIEnabled: false) + == .inspector + ) + } + + /// Agent mode is the AI feature, so a window left in it with the setting off is browsing, and + /// its pane must not go on drawing a result the mode no longer imposes. + @Test("Agent mode with AI off resolves as browsing", arguments: TrailingPaneSurface.allCases) + func agentModeWithAIOffResolvesAsBrowsing(stored: TrailingPaneSurface) { + #expect( + TrailingPaneSurfaceResolver.resolve(stored: stored, contentMode: .agent, isAIEnabled: false) + == .inspector + ) + } + + @Test("The header offers both surfaces while browsing with AI on") + func selectableWhileBrowsing() { + #expect( + TrailingPaneSurfaceResolver.selectable(contentMode: .browse, isAIEnabled: true) + == [.inspector, .assistant] + ) + #expect( + TrailingPaneSurfaceResolver.selectable(contentMode: .browse, isAIEnabled: false) == [.inspector] + ) + } + + /// Empty rather than one segment: the mode chose, so the header draws a plain title. + @Test("The header offers nothing to choose in Agent mode") + func selectableInAgentMode() { + #expect(TrailingPaneSurfaceResolver.selectable(contentMode: .agent, isAIEnabled: true).isEmpty) + } + + @Test("A resolved surface is one the header offers, unless the mode imposed it") + func resolvedSurfaceIsOffered() { + for mode in ConnectionWorkspaceContentMode.allCases { + for aiEnabled in [true, false] { + for stored in TrailingPaneSurface.allCases { + let resolved = TrailingPaneSurfaceResolver.resolve( + stored: stored, + contentMode: mode, + isAIEnabled: aiEnabled + ) + let offered = TrailingPaneSurfaceResolver.selectable( + contentMode: mode, + isAIEnabled: aiEnabled + ) + let imposed = offered.isEmpty && resolved == .agentResult + #expect(imposed || offered.contains(resolved)) + } + } + } + } + + /// The result pane belongs to a mode rather than to a command, so it never reaches the stored + /// per-connection preference and never appears in the header's choices. + @Test("The result surface is never user selectable") + func resultIsNeverSelectable() { + #expect(TrailingPaneSurface.agentResult.isUserSelectable == false) + for mode in ConnectionWorkspaceContentMode.allCases { + #expect( + TrailingPaneSurfaceResolver.selectable(contentMode: mode, isAIEnabled: true) + .contains(.agentResult) == false + ) + } + } +} From 353d588e1b844b82e8425fff02ce9f871378004d Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Mon, 21 Sep 2026 13:23:36 +0700 Subject: [PATCH 2/9] fix(connections): stage principal edits for Save, and keep each window's pane state and preferences its own --- CHANGELOG.md | 2 + .../Infrastructure/ConnectionWorkspace.swift | 9 ++ .../MainSplitViewController+ContentMode.swift | 31 ++++--- .../MainSplitViewController.swift | 8 ++ .../Core/Storage/ConnectionLocalState.swift | 21 ++++- .../Connection/ConnectionToolbarState.swift | 5 ++ TablePro/Models/UI/RowInspectorState.swift | 2 +- TablePro/Models/UI/TrailingPaneState.swift | 6 +- .../Extensions/MainContentView+Bindings.swift | 4 + .../Extensions/MainContentView+Setup.swift | 30 ++++--- TablePro/Views/Main/MainContentView.swift | 3 +- .../BrowseCollapseStateOwnershipTests.swift | 85 +++++++++++++++++++ .../ConnectionLocalStatePurgeTests.swift | 44 ++++++++++ .../Views/Main/TriggerStructTests.swift | 14 ++- 14 files changed, 233 insertions(+), 31 deletions(-) create mode 100644 TableProTests/Core/Services/Infrastructure/BrowseCollapseStateOwnershipTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 97446365e..907973485 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Save Changes** and ⌘S dim on a Users & Roles tab with staged changes. +- A deleted connection's inspector and assistant choice left behind, and inherited by a new connection with its id. - A column resized moments before its table was dropped saving the layout back over the clear. - A connection left pointing at a database that was dropped. - A dropped table's saved filters, column layout, highlight rules, value formats and label columns coming back on a table recreated with its name. diff --git a/TablePro/Core/Services/Infrastructure/ConnectionWorkspace.swift b/TablePro/Core/Services/Infrastructure/ConnectionWorkspace.swift index 590176135..9db495bea 100644 --- a/TablePro/Core/Services/Infrastructure/ConnectionWorkspace.swift +++ b/TablePro/Core/Services/Infrastructure/ConnectionWorkspace.swift @@ -47,6 +47,15 @@ internal final class ConnectionWorkspace { /// on a table. internal var contentMode: ConnectionWorkspaceContentMode = .browse + /// What Browse had collapsed, so entering Agent mode can reveal its three columns and leaving + /// can put the window back the way the user had it. + /// + /// On the workspace rather than in a static keyed by connection id, because two windows can + /// host the same connection and each has its own collapsed sidebar and inspector. Shared, the + /// second window to enter Agent mode overwrote what the first had recorded, and the first then + /// left the mode with the second window's layout. + internal var browseCollapseState: (sidebar: Bool, inspector: Bool)? + /// Each workspace owns its undo stack. Routing through `NSWindow.undoManager` was correct /// while a window meant one connection; sharing one window between several would let an /// undo in one connection roll back an edit made in another. diff --git a/TablePro/Core/Services/Infrastructure/MainSplitViewController+ContentMode.swift b/TablePro/Core/Services/Infrastructure/MainSplitViewController+ContentMode.swift index 4e92e9d79..cfca1051d 100644 --- a/TablePro/Core/Services/Infrastructure/MainSplitViewController+ContentMode.swift +++ b/TablePro/Core/Services/Infrastructure/MainSplitViewController+ContentMode.swift @@ -26,10 +26,6 @@ internal extension MainSplitViewController { setContentMode(mode, for: workspace.connectionId) } - /// What Browse had collapsed, so entering Agent mode can reveal its columns and leaving can put - /// the window back the way the user had it. - private static var browseCollapseState: [UUID: (sidebar: Bool, inspector: Bool)] = [:] - func setContentMode(_ mode: ConnectionWorkspaceContentMode, for connectionId: UUID) { guard let workspace = workspaces.workspace(for: connectionId) else { return } let resolved = ConnectionWorkspaceContentMode.resolved( @@ -57,22 +53,33 @@ internal extension MainSplitViewController { /// /// A fresh window starts with the inspector collapsed, and the user may have collapsed the /// sidebar, so swapping the hosted roots alone gave a first-time Agent mode with no Result - /// column and sometimes no Sessions column either. What Browse had is remembered and put back. - private func applyColumnVisibility( + /// column and sometimes no Sessions column either. What Browse had is remembered on the + /// workspace and put back. + /// + /// Returns without touching the window when the workspace is not the one on screen, which is + /// why `applySelectedWorkspace` calls it again: a connection put into Agent mode while another + /// was selected reached the window with its columns still collapsed and nothing to reveal them. + internal func applyColumnVisibility( for connectionId: UUID, mode: ConnectionWorkspaceContentMode ) { - guard workspaces.selectedConnectionId == connectionId else { return } + guard workspaces.selectedConnectionId == connectionId, + let workspace = workspaces.workspace(for: connectionId) else { return } switch mode { case .agent: - Self.browseCollapseState[connectionId] = ( - sidebar: sidebarSplitItem.isCollapsed, - inspector: inspectorSplitItem.isCollapsed - ) + /// Recorded once per entry into the mode. Recording again on a later selection would + /// save the mode's own revealed columns as the layout to go back to. + if workspace.browseCollapseState == nil { + workspace.browseCollapseState = ( + sidebar: sidebarSplitItem.isCollapsed, + inspector: inspectorSplitItem.isCollapsed + ) + } sidebarSplitItem.animator().isCollapsed = false inspectorSplitItem.animator().isCollapsed = false case .browse: - guard let previous = Self.browseCollapseState.removeValue(forKey: connectionId) else { return } + guard let previous = workspace.browseCollapseState else { return } + workspace.browseCollapseState = nil sidebarSplitItem.animator().isCollapsed = previous.sidebar inspectorSplitItem.animator().isCollapsed = previous.inspector } diff --git a/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift b/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift index f2fb0bb14..979ee9ac4 100644 --- a/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift +++ b/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift @@ -608,6 +608,14 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan /// costs a key comparison when nothing has moved, which is what the record is for. syncSelectedPanes() showSelectedPanes() + + /// A workspace put into Agent mode while another connection was on screen never reached + /// the branch that reveals its columns, because that branch only ever touched the selected + /// one. Selection is where it gets them, which is the same repair-on-selection shape the + /// pane render key already relies on. + if let selected = workspaces.selected { + applyColumnVisibility(for: selected.connectionId, mode: selected.resolvedContentMode) + } applyDetailMinimumThicknessForSelection() applyPaneChrome() applyWindowTitle() diff --git a/TablePro/Core/Storage/ConnectionLocalState.swift b/TablePro/Core/Storage/ConnectionLocalState.swift index 2ded7b2ac..9c4fa1fd0 100644 --- a/TablePro/Core/Storage/ConnectionLocalState.swift +++ b/TablePro/Core/Storage/ConnectionLocalState.swift @@ -28,7 +28,8 @@ internal enum ConnectionLocalState { appSettings: AppSettingsStorage = .shared, tableScopedStores: [any TableScopedSettingsStore] = TableScopedSettingsRegistry.stores, sqlFavorites: SQLFavoriteManager = .shared, - queryHistory: QueryHistoryManager = .shared + queryHistory: QueryHistoryManager = .shared, + defaults: UserDefaults = AppStorageEnvironment.shared.defaults ) { guard !connectionIds.isEmpty else { return } @@ -43,6 +44,7 @@ internal enum ConnectionLocalState { QueryInsightsPreferencesStorage.remove(for: connectionId) MCPServerStore.shared.forgetConnection(connectionId) } + purgeTrailingPaneKeys(connectionIds, defaults: defaults) for store in tableScopedStores { store.purgeConnections(connectionIds, leavesTombstones: origin == .local) @@ -94,6 +96,23 @@ internal enum ConnectionLocalState { } } + /// The trailing pane's two keys, which are written straight onto the defaults object rather + /// than through a store with a `remove(for:)` of its own, so `purge`'s list of stores never + /// reached them. A deleted connection left the surface it was last showing and its inspector's + /// view mode behind, and a connection later given the same id inherited both. + /// + /// Separate from `purge` for the reason `purgeAsyncStores` is: `purge` reaches nine shared + /// singletons and a test cannot call it, while this takes the one thing it writes to. + internal static func purgeTrailingPaneKeys( + _ connectionIds: Set, + defaults: UserDefaults = AppStorageEnvironment.shared.defaults + ) { + for connectionId in connectionIds { + defaults.removeObject(forKey: TrailingPaneState.surfaceKey(connectionId)) + defaults.removeObject(forKey: RowInspectorState.viewModeKey(connectionId)) + } + } + /// The in-memory registries go first. A live `SharedSidebarState` for this connection rewrites /// its own defaults keys on the next mutation, so removing the keys under it achieves nothing. private static func purgeLiveState(_ connectionId: UUID) { diff --git a/TablePro/Models/Connection/ConnectionToolbarState.swift b/TablePro/Models/Connection/ConnectionToolbarState.swift index 699741f87..5a1832747 100644 --- a/TablePro/Models/Connection/ConnectionToolbarState.swift +++ b/TablePro/Models/Connection/ConnectionToolbarState.swift @@ -114,6 +114,11 @@ final class ConnectionToolbarState: ObservableObject { /// Whether there are pending changes (data grid or file) @Published var hasPendingChanges: Bool = false + /// What the commit would commit, which is what decides the verb it says. `hasPendingChanges` + /// stays as the answer to "is anything staged" that its thirty-odd readers already ask; this is + /// the same question answered with the kind attached, written by the same one function. + @Published var pendingChange: PendingChangeKind? + /// Whether there are pending data grid changes (for SQL preview button) @Published var hasDataPendingChanges: Bool = false diff --git a/TablePro/Models/UI/RowInspectorState.swift b/TablePro/Models/UI/RowInspectorState.swift index bbfb3da93..8a77e45e6 100644 --- a/TablePro/Models/UI/RowInspectorState.swift +++ b/TablePro/Models/UI/RowInspectorState.swift @@ -40,7 +40,7 @@ internal final class RowInspectorState: ObservableObject { /// the reader's expansions and the rows already fetched for them. internal let jsonViewModel = JSONRowInspectorViewModel() - internal init(connectionId: UUID? = nil, defaults: UserDefaults = .standard) { + internal init(connectionId: UUID? = nil, defaults: UserDefaults = AppStorageEnvironment.shared.defaults) { self.connectionId = connectionId self.defaults = defaults if let connectionId, diff --git a/TablePro/Models/UI/TrailingPaneState.swift b/TablePro/Models/UI/TrailingPaneState.swift index f514889df..813f1ba1a 100644 --- a/TablePro/Models/UI/TrailingPaneState.swift +++ b/TablePro/Models/UI/TrailingPaneState.swift @@ -31,9 +31,13 @@ internal final class TrailingPaneState: ObservableObject { internal let inspector: RowInspectorState internal let assistant: AssistantState + /// `AppStorageEnvironment.shared.defaults` rather than `.standard`, which is what every other + /// per-connection preference already resolves through. Both of this object's keys escaped the + /// UI-test sandbox, so a test run read and wrote the surface and inspector mode of whoever was + /// running it. internal init( connectionId: UUID? = nil, - defaults: UserDefaults = .standard, + defaults: UserDefaults = AppStorageEnvironment.shared.defaults, sessionRegistry: AgentSessionRegistry = .shared ) { self.connectionId = connectionId diff --git a/TablePro/Views/Main/Extensions/MainContentView+Bindings.swift b/TablePro/Views/Main/Extensions/MainContentView+Bindings.swift index 8e2305b87..117c7707f 100644 --- a/TablePro/Views/Main/Extensions/MainContentView+Bindings.swift +++ b/TablePro/Views/Main/Extensions/MainContentView+Bindings.swift @@ -258,4 +258,8 @@ struct PendingChangeTrigger: Equatable { let hasStructureChanges: Bool let isFileDirty: Bool let hasCreateTablePending: Bool + /// The fifth source. Without it staging a principal edit changes nothing this value can see, so + /// `updateToolbarPendingState()` is never re-run and the commit control stays dim however many + /// changes the Users & Roles tab holds. + let hasPrincipalChanges: Bool } diff --git a/TablePro/Views/Main/Extensions/MainContentView+Setup.swift b/TablePro/Views/Main/Extensions/MainContentView+Setup.swift index 20142a668..46655d4f3 100644 --- a/TablePro/Views/Main/Extensions/MainContentView+Setup.swift +++ b/TablePro/Views/Main/Extensions/MainContentView+Setup.swift @@ -201,20 +201,24 @@ extension MainContentView { // MARK: - Command Actions Setup + /// One resolution of what is staged, so the commit control, its verb and Preview SQL's gate + /// cannot disagree. The arm this replaces never read `hasPrincipalChanges`, so a Users & Roles + /// tab with staged principals left both the toolbar's commit button and Cmd+S dim over work + /// `saveChanges()` already knew how to apply. func updateToolbarPendingState() { - if tabManager.selectedTab?.tabType == .createTable { - toolbarState.hasDataPendingChanges = false - toolbarState.hasPendingChanges = toolbarState.hasCreateTablePending - return - } - let hasDataChanges = - changeManager.hasChanges - || !pendingTruncates.isEmpty - || !pendingDeletes.isEmpty - || toolbarState.hasStructureChanges - let hasFileChanges = tabManager.selectedTab?.content.isFileDirty ?? false - toolbarState.hasDataPendingChanges = hasDataChanges - toolbarState.hasPendingChanges = hasDataChanges || hasFileChanges + let kind = PendingChangeKind.resolve( + tabType: tabManager.selectedTab?.tabType, + hasDataChanges: changeManager.hasChanges || !pendingTruncates.isEmpty || !pendingDeletes.isEmpty, + hasStructureChanges: toolbarState.hasStructureChanges, + hasCreateTablePending: toolbarState.hasCreateTablePending, + hasPrincipalChanges: toolbarState.hasPrincipalChanges, + isFileDirty: tabManager.selectedTab?.content.isFileDirty ?? false + ) + toolbarState.pendingChange = kind + toolbarState.hasPendingChanges = kind != nil + /// Preview SQL asks a narrower question than the commit control: a dirty query file and + /// staged principals both raise the commit and neither has grid SQL to show. + toolbarState.hasDataPendingChanges = kind == .data || kind == .structure } /// Update window title, proxy icon, and dirty dot based on the selected tab. diff --git a/TablePro/Views/Main/MainContentView.swift b/TablePro/Views/Main/MainContentView.swift index 94b7a3b17..0409dec41 100644 --- a/TablePro/Views/Main/MainContentView.swift +++ b/TablePro/Views/Main/MainContentView.swift @@ -308,7 +308,8 @@ struct MainContentView: View { pendingDeletes: pendingDeletes, hasStructureChanges: toolbarState.hasStructureChanges, isFileDirty: tabManager.selectedTab?.content.isFileDirty ?? false, - hasCreateTablePending: toolbarState.hasCreateTablePending + hasCreateTablePending: toolbarState.hasCreateTablePending, + hasPrincipalChanges: toolbarState.hasPrincipalChanges ) } diff --git a/TableProTests/Core/Services/Infrastructure/BrowseCollapseStateOwnershipTests.swift b/TableProTests/Core/Services/Infrastructure/BrowseCollapseStateOwnershipTests.swift new file mode 100644 index 000000000..83c8273b2 --- /dev/null +++ b/TableProTests/Core/Services/Infrastructure/BrowseCollapseStateOwnershipTests.swift @@ -0,0 +1,85 @@ +// +// BrowseCollapseStateOwnershipTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import Testing + +/// What Browse had collapsed belongs to the workspace, not to the connection. +/// +/// A tab torn into its own window leaves one connection hosted by two workspaces, and each window +/// has its own collapsed sidebar and inspector. Keyed by connection id in a static, the second +/// window to enter Agent mode overwrote what the first had recorded, and the first then came out of +/// the mode with the second window's layout. +@Suite("Browse collapse state ownership") +@MainActor +struct BrowseCollapseStateOwnershipTests { + private static let connectionId = UUID(uuidString: "00000000-0000-0000-0000-0000000000D4") + + private func makeWorkspace(_ connectionId: UUID) -> ConnectionWorkspace { + ConnectionWorkspace( + connectionId: connectionId, + payload: nil, + autoConnect: false, + payloadConnection: nil, + session: nil, + sessionState: nil, + trailingPaneState: nil, + phase: .idle + ) + } + + @Test("A fresh workspace has recorded nothing") + func startsEmpty() throws { + let connectionId = try #require(Self.connectionId) + #expect(makeWorkspace(connectionId).browseCollapseState == nil) + } + + @Test("Two workspaces for one connection keep separate records") + func twoWorkspacesDoNotShareARecord() throws { + let connectionId = try #require(Self.connectionId) + let first = makeWorkspace(connectionId) + let second = makeWorkspace(connectionId) + + first.browseCollapseState = (sidebar: false, inspector: true) + second.browseCollapseState = (sidebar: true, inspector: false) + + #expect(first.browseCollapseState?.sidebar == false) + #expect(first.browseCollapseState?.inspector == true) + #expect(second.browseCollapseState?.sidebar == true) + #expect(second.browseCollapseState?.inspector == false) + } + + /// Leaving the mode hands the layout back and forgets it, so a second entry records what Browse + /// has at that moment rather than replaying what it had the first time. + @Test("Clearing one workspace's record leaves the other's standing") + func clearingIsScopedToItsWorkspace() throws { + let connectionId = try #require(Self.connectionId) + let first = makeWorkspace(connectionId) + let second = makeWorkspace(connectionId) + first.browseCollapseState = (sidebar: true, inspector: true) + second.browseCollapseState = (sidebar: false, inspector: false) + + first.browseCollapseState = nil + + #expect(first.browseCollapseState == nil) + #expect(second.browseCollapseState?.sidebar == false) + #expect(second.browseCollapseState?.inspector == false) + } + + /// The mode is per connection too, so one workspace can sit in Agent mode with its columns + /// revealed while another in the same window stays on a table with its inspector closed. + @Test("The content mode is per workspace as well") + func contentModeIsPerWorkspace() throws { + let connectionId = try #require(Self.connectionId) + let browsing = makeWorkspace(connectionId) + let agent = makeWorkspace(connectionId) + + agent.contentMode = .agent + + #expect(browsing.contentMode == .browse) + #expect(agent.contentMode == .agent) + } +} diff --git a/TableProTests/Core/Storage/ConnectionLocalStatePurgeTests.swift b/TableProTests/Core/Storage/ConnectionLocalStatePurgeTests.swift index d7b019812..ce5aaa73b 100644 --- a/TableProTests/Core/Storage/ConnectionLocalStatePurgeTests.swift +++ b/TableProTests/Core/Storage/ConnectionLocalStatePurgeTests.swift @@ -215,6 +215,50 @@ struct ConnectionLocalStatePurgeTests { #expect(!(await storage.planSnapshots(matching: keptIdentity, excluding: nil, limit: 10)).isEmpty) } + @Test("Purging a connection takes its trailing pane preferences with it") + @MainActor + func purgeClearsTrailingPaneKeys() throws { + let suite = "tablepro-purge-pane-\(UUID().uuidString)" + let defaults = try #require(UserDefaults(suiteName: suite)) + defer { defaults.removePersistentDomain(forName: suite) } + let deleted = UUID() + + defaults.set(TrailingPaneSurface.assistant.rawValue, forKey: TrailingPaneState.surfaceKey(deleted)) + defaults.set(InspectorViewMode.json.rawValue, forKey: RowInspectorState.viewModeKey(deleted)) + + ConnectionLocalState.purgeTrailingPaneKeys([deleted], defaults: defaults) + + #expect(defaults.string(forKey: TrailingPaneState.surfaceKey(deleted)) == nil) + #expect(defaults.string(forKey: RowInspectorState.viewModeKey(deleted)) == nil) + } + + /// A connection deleted on one device must not take another connection's pane with it, and the + /// keys are per connection precisely so it cannot. + @Test("Purging one connection leaves another connection's trailing pane alone") + @MainActor + func purgeLeavesOtherTrailingPanesAlone() throws { + let suite = "tablepro-purge-pane-\(UUID().uuidString)" + let defaults = try #require(UserDefaults(suiteName: suite)) + defer { defaults.removePersistentDomain(forName: suite) } + let deleted = UUID() + let kept = UUID() + + defaults.set(TrailingPaneSurface.assistant.rawValue, forKey: TrailingPaneState.surfaceKey(deleted)) + defaults.set(TrailingPaneSurface.assistant.rawValue, forKey: TrailingPaneState.surfaceKey(kept)) + defaults.set(InspectorViewMode.json.rawValue, forKey: RowInspectorState.viewModeKey(kept)) + + ConnectionLocalState.purgeTrailingPaneKeys([deleted], defaults: defaults) + + #expect(defaults.string(forKey: TrailingPaneState.surfaceKey(deleted)) == nil) + #expect( + defaults.string(forKey: TrailingPaneState.surfaceKey(kept)) + == TrailingPaneSurface.assistant.rawValue + ) + #expect( + defaults.string(forKey: RowInspectorState.viewModeKey(kept)) == InspectorViewMode.json.rawValue + ) + } + /// `ConnectionLocalState` exists because this list used to be written out at each delete site, /// and they drifted: the query history clear reached the two local sites and never the remote /// one. Anything reaching these stores itself is that drift starting again. diff --git a/TableProTests/Views/Main/TriggerStructTests.swift b/TableProTests/Views/Main/TriggerStructTests.swift index 30ac60229..402879bfe 100644 --- a/TableProTests/Views/Main/TriggerStructTests.swift +++ b/TableProTests/Views/Main/TriggerStructTests.swift @@ -92,7 +92,8 @@ struct PendingChangeTriggerTests { pendingDeletes: Set = [], hasStructureChanges: Bool = false, isFileDirty: Bool = false, - hasCreateTablePending: Bool = false + hasCreateTablePending: Bool = false, + hasPrincipalChanges: Bool = false ) -> PendingChangeTrigger { PendingChangeTrigger( hasDataChanges: hasDataChanges, @@ -100,10 +101,19 @@ struct PendingChangeTriggerTests { pendingDeletes: pendingDeletes, hasStructureChanges: hasStructureChanges, isFileDirty: isFileDirty, - hasCreateTablePending: hasCreateTablePending + hasCreateTablePending: hasCreateTablePending, + hasPrincipalChanges: hasPrincipalChanges ) } + /// The fifth source, and the reason the commit control was dim on a Users & Roles tab however + /// many principal edits were staged: without this field the trigger never changed, so the one + /// function that recomputes what is pending was never re-run. + @Test("Different hasPrincipalChanges produces unequal triggers") + func differentHasPrincipalChanges() { + #expect(makeTrigger(hasPrincipalChanges: true) != makeTrigger(hasPrincipalChanges: false)) + } + @Test("Same values are equal") func sameValuesAreEqual() { let truncate = TestFixtures.makeTableRef(name: "t1") From 54e9414bddc5f2f67457615cf4dc433a3b45eb71 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Mon, 21 Sep 2026 16:11:44 +0700 Subject: [PATCH 3/9] feat(toolbar): cut the connection window toolbar to eight controls that follow the tab and the mode --- CHANGELOG.md | 9 + .../Core/Menu/ContentModeMenuDelegate.swift | 44 ++ TablePro/Core/Menu/FileMenuBuilder.swift | 12 + .../Core/Menu/ImportFormatMenuDelegate.swift | 71 +++ .../Menu/SessionContextMenuDelegate.swift | 5 +- TablePro/Core/Menu/ViewMenuBuilder.swift | 12 +- .../ServerDashboardQueryProviderFactory.swift | 50 ++- .../Core/Services/Export/ImportRouting.swift | 8 + .../MainSplitViewController+ContentMode.swift | 4 +- ...itViewController+DatabaseMenuActions.swift | 1 + ...nSplitViewController+FileMenuActions.swift | 9 + ...inSplitViewController+MenuValidation.swift | 4 +- ...nSplitViewController+ViewMenuActions.swift | 4 +- .../MainSplitViewController.swift | 69 +-- .../MainWindowToolbar+Actions.swift | 8 +- .../MainWindowToolbar+ContentMode.swift | 102 ----- .../MainWindowToolbar+Context.swift | 75 ++++ .../MainWindowToolbar+Delegate.swift | 98 ++-- .../MainWindowToolbar+Items.swift | 144 +++--- .../MainWindowToolbar+Validation.swift | 154 +------ .../Infrastructure/MainWindowToolbar.swift | 422 +++++++++--------- .../SidebarContainerViewController.swift | 109 ++++- .../Infrastructure/StatefulToolbarItem.swift | 30 +- .../Toolbar/ActionsMenuSpec.swift | 62 ++- .../ConnectionActionsMenuDelegate.swift | 98 ++++ .../ConnectionActionsMenuResolver.swift | 25 +- .../Toolbar/ToolbarContextResolver.swift | 85 ++-- .../Toolbar/ToolbarVisibility.swift | 28 ++ .../TransportRateToolbarItem.swift | 86 ---- .../Core/Transport/TransportRateLabel.swift | 69 --- TablePro/Models/UI/PendingChangeKind.swift | 19 +- TablePro/Models/UI/ToolbarContext.swift | 78 +++- TablePro/Resources/Localizable.xcstrings | 60 ++- .../CompareEndpointToolbarController.swift | 4 +- .../Views/Components/PopoverPresenter.swift | 14 +- .../MainContentCommandActions+Switchers.swift | 8 +- .../Main/MainContentCommandActions.swift | 2 +- .../Views/Sidebar/SidebarScopeControl.swift | 64 +++ .../Toolbar/ToolbarSwitcherPresenter.swift | 74 ++- ...erDashboardQueryProviderFactoryTests.swift | 40 ++ .../ConnectionActionsMenuDelegateTests.swift | 204 +++++++++ .../ConnectionActionsMenuResolverTests.swift | 74 ++- .../ConnectionWindowChromeTests.swift | 73 ++- .../Infrastructure/ContentModeTests.swift | 60 +-- .../MenuValidationCoverageTests.swift | 46 ++ .../ToolbarContextResolverTests.swift | 198 +++++--- .../Transport/TransportRateLabelTests.swift | 102 ----- .../Models/PendingChangeKindTests.swift | 9 - .../MainWindowToolbarLayoutTests.swift | 153 +++---- ...MainWindowToolbarNativeContractTests.swift | 277 ++++++------ .../MainWindowToolbarShortcutHintTests.swift | 39 +- .../MainWindowToolbarValidationTests.swift | 379 ++++++++-------- .../Services/ToolbarHiddenSetTests.swift | 273 +++++++++++ .../Services/ToolbarSourceAccessTests.swift | 241 ++++++++++ .../Services/ToolbarSwitcherAnchorTests.swift | 151 +++---- .../Sidebar/SidebarScopeControlTests.swift | 227 ++++++++++ .../MainWindowToolbarIdentifierTests.swift | 51 +++ TableProUITests/AgentModeMenuUITests.swift | 19 +- .../ConnectionWindowChromeUITests.swift | 164 +++++++ .../SidebarFavoritesFirstEntryUITests.swift | 7 +- 60 files changed, 3260 insertions(+), 1747 deletions(-) create mode 100644 TablePro/Core/Menu/ContentModeMenuDelegate.swift create mode 100644 TablePro/Core/Menu/ImportFormatMenuDelegate.swift delete mode 100644 TablePro/Core/Services/Infrastructure/MainWindowToolbar+ContentMode.swift create mode 100644 TablePro/Core/Services/Infrastructure/MainWindowToolbar+Context.swift create mode 100644 TablePro/Core/Services/Infrastructure/Toolbar/ConnectionActionsMenuDelegate.swift create mode 100644 TablePro/Core/Services/Infrastructure/Toolbar/ToolbarVisibility.swift delete mode 100644 TablePro/Core/Services/Infrastructure/TransportRateToolbarItem.swift delete mode 100644 TablePro/Core/Transport/TransportRateLabel.swift create mode 100644 TablePro/Views/Sidebar/SidebarScopeControl.swift create mode 100644 TableProTests/Core/ServerDashboard/ServerDashboardQueryProviderFactoryTests.swift create mode 100644 TableProTests/Core/Services/Infrastructure/ConnectionActionsMenuDelegateTests.swift delete mode 100644 TableProTests/Core/Transport/TransportRateLabelTests.swift create mode 100644 TableProTests/Services/ToolbarHiddenSetTests.swift create mode 100644 TableProTests/Services/ToolbarSourceAccessTests.swift create mode 100644 TableProTests/Views/Sidebar/SidebarScopeControlTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 907973485..23bb7f8a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Numbered databases on Valkey 9 clusters with `cluster-databases` above 1. - `DB ` in the Redis editor, running one command on another database. - Explain for Teradata. +- **Actions** menu in the connection window's toolbar, with the commands for the tab you are on and every import format. +- **File > Import > Import Data From**, for choosing the import format from the menu bar. ### Changed @@ -43,16 +45,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - iCloud sync and usage data on iPhone and iPad stay off until you turn them on. - Group rows in the iOS connection list take swipe actions and show even when no connection is saved. - Typed entry beside the stepper for number settings in the connection form. +- Connection window toolbar trimmed to eight controls that follow the tab and the mode. +- **Tables** and **Favorites** chooser moved from the toolbar to the top of the sidebar. ### Removed - **Refresh from iCloud**, **Sync Now** and the toolbar sync button on iPhone and iPad. - **Manage Groups**, the **Clear** button on **Recent**, and the **More** menu's tag filter on iPhone and iPad. +- Throughput readout in the toolbar; the connection switcher still shows it. +- Back, Forward, New Tab, Open Quickly, Add Row, Restore Previous Values and Assistant from the default toolbar; **Customize Toolbar** adds them back. ### Fixed - **Save Changes** and ⌘S dim on a Users & Roles tab with staged changes. - A deleted connection's inspector and assistant choice left behind, and inherited by a new connection with its id. +- Toolbar **Results** button enabled on tabs that have no results pane. +- Toolbar **History** button enabled over a window that never connected. +- Toolbar **Inspector** button dim on macOS 13 over a pane left open when the connection dropped. - A column resized moments before its table was dropped saving the layout back over the clear. - A connection left pointing at a database that was dropped. - A dropped table's saved filters, column layout, highlight rules, value formats and label columns coming back on a table recreated with its name. diff --git a/TablePro/Core/Menu/ContentModeMenuDelegate.swift b/TablePro/Core/Menu/ContentModeMenuDelegate.swift new file mode 100644 index 000000000..c72cbbd67 --- /dev/null +++ b/TablePro/Core/Menu/ContentModeMenuDelegate.swift @@ -0,0 +1,44 @@ +// +// ContentModeMenuDelegate.swift +// TablePro +// + +import AppKit + +/// Browse and Agent as a pair of menu items, filled when the menu opens. +/// +/// Each entry names its mode in `representedObject`, and that is load-bearing twice over: +/// `setContentModeFromMenu(_:)` reads it to know which mode was chosen and does nothing without +/// it, and the window's `validateMenuItem` reads it to put the checkmark on the mode the +/// connection is in. An entry without it validates enabled, acts on nothing and ticks nothing. +/// +/// Driven from the enum rather than a hand copy, the way View > Mode is, so a third mode cannot be +/// offered in one menu and missing from the other. +@MainActor +internal final class ContentModeMenuDelegate: NSObject, NSMenuDelegate { + internal static let action = #selector(MainSplitViewController.setContentModeFromMenu(_:)) + + func menuNeedsUpdate(_ menu: NSMenu) { + menu.removeAllItems() + for mode in ConnectionWorkspaceContentMode.allCases { + menu.addItem(Self.item(for: mode)) + } + } + + internal static func item(for mode: ConnectionWorkspaceContentMode) -> NSMenuItem { + let item = NSMenuItem(title: mode.localizedTitle, action: action, keyEquivalent: "") + item.target = nil + item.representedObject = mode.rawValue + return item + } + + /// Keeps AppKit's key-equivalent search from rebuilding the menu on every modified keystroke. + func menuHasKeyEquivalent( + _ menu: NSMenu, + for event: NSEvent, + target: AutoreleasingUnsafeMutablePointer, + action: UnsafeMutablePointer + ) -> Bool { + false + } +} diff --git a/TablePro/Core/Menu/FileMenuBuilder.swift b/TablePro/Core/Menu/FileMenuBuilder.swift index b5b7829d7..144f928c0 100644 --- a/TablePro/Core/Menu/FileMenuBuilder.swift +++ b/TablePro/Core/Menu/FileMenuBuilder.swift @@ -9,6 +9,7 @@ import AppKit enum FileMenuBuilder { /// Retained for the menu's lifetime, which is the app's: `NSMenu.delegate` is unowned. private static let closeTitleDelegate = CloseCommandMenuDelegate() + private static let importFormatDelegate = ImportFormatMenuDelegate() static func build(keyboard: KeyboardSettings) -> NSMenuItem { let file = MenuItemFactory.menu(String(localized: "File"), items: [ @@ -152,6 +153,7 @@ enum FileMenuBuilder { ) ]) container.submenu?.insertItem(.separator(), at: 0) + container.submenu?.insertItem(importFormatsSubmenu(), at: 0) container.submenu?.insertItem( MenuItemFactory.item( String(localized: "Import Data…"), @@ -164,6 +166,16 @@ enum FileMenuBuilder { return container } + /// Every format the connection imports from. Import Data… above it takes the first one, which + /// left the menu bar with no route to any other: the toolbar's Import item was the only one, and + /// a toolbar item is not a menu-bar command. The Actions pull-down offers the same list under the + /// same title, and the two are filled by the same class when they open. + private static func importFormatsSubmenu() -> NSMenuItem { + let container = MenuItemFactory.submenu(String(localized: "Import Data From"), items: []) + container.submenu?.delegate = importFormatDelegate + return container + } + private static func exportSubmenu(keyboard: KeyboardSettings) -> NSMenuItem { MenuItemFactory.submenu(String(localized: "Export"), items: [ MenuItemFactory.item( diff --git a/TablePro/Core/Menu/ImportFormatMenuDelegate.swift b/TablePro/Core/Menu/ImportFormatMenuDelegate.swift new file mode 100644 index 000000000..db05c61a2 --- /dev/null +++ b/TablePro/Core/Menu/ImportFormatMenuDelegate.swift @@ -0,0 +1,71 @@ +// +// ImportFormatMenuDelegate.swift +// TablePro +// + +import AppKit + +/// The formats the connection's driver imports from, filled when the menu opens. +/// +/// Filled on open rather than when the menu is built, because the list is the driver's and a window +/// changes driver with every connection switch. The menu this replaces was rebuilt by hand on each +/// repoint, and a copy built at any other moment kept the formats of the connection it was built for. +/// +/// Every entry carries no target and names its format in `representedObject`, so AppKit resolves it +/// through the responder chain to the window's controller and validates it there, the way the menu +/// bar's own commands are. The entries this replaces targeted the toolbar object, which is not a +/// responder, so a menu resolved through the chain could not have reached them at all. +/// +/// One class serves every place the list is offered: File > Import > Import Data From, the Actions +/// pull-down's row of the same name, and the Import item a user can add from Customize Toolbar. Each +/// asks the key window when it opens, so none of them can list another connection's formats. +/// +/// Built on the same shape as `SafeModeMenuDelegate`, including the responder-chain lookup that +/// finds the window the chosen format will import into. `NSMenu.delegate` is weak, so whoever builds +/// a menu keeps the delegate alive alongside it. +@MainActor +internal final class ImportFormatMenuDelegate: NSObject, NSMenuDelegate { + internal static let action = #selector(MainSplitViewController.importDataFormat(_:)) + + func menuNeedsUpdate(_ menu: NSMenu) { + menu.removeAllItems() + let controller = NSApp.target(forAction: Self.action, to: nil, from: nil) as? MainSplitViewController + let formats = controller?.commandActions?.availableImportFormats ?? [] + guard !formats.isEmpty else { + menu.addItem(Self.placeholder()) + return + } + for format in formats { + menu.addItem(Self.item(for: format)) + } + } + + internal static func item(for format: ImportFormatOption) -> NSMenuItem { + let item = NSMenuItem(title: format.formatLabel, action: action, keyEquivalent: "") + item.target = nil + item.representedObject = format.id + return item + } + + /// A list with nothing in it opens as a sliver with no text, which reads as a broken command. + /// The row that opens it cannot be dimmed through the responder chain, because AppKit gives a + /// submenu's row its own action, so the list says why it is empty instead, the way Database > + /// Session Context does. That happens with no connection window in front, and with a driver + /// whose import plugins are missing or failed to load. + internal static func placeholder() -> NSMenuItem { + let item = NSMenuItem(title: String(localized: "None Available"), action: nil, keyEquivalent: "") + item.isEnabled = false + return item + } + + /// Keeps AppKit's key-equivalent search from rebuilding the menu on every modified keystroke, + /// which would walk the responder chain for items that carry no key equivalent. + func menuHasKeyEquivalent( + _ menu: NSMenu, + for event: NSEvent, + target: AutoreleasingUnsafeMutablePointer, + action: UnsafeMutablePointer + ) -> Bool { + false + } +} diff --git a/TablePro/Core/Menu/SessionContextMenuDelegate.swift b/TablePro/Core/Menu/SessionContextMenuDelegate.swift index 39813acee..68a8215fd 100644 --- a/TablePro/Core/Menu/SessionContextMenuDelegate.swift +++ b/TablePro/Core/Menu/SessionContextMenuDelegate.swift @@ -12,8 +12,9 @@ import TableProPluginKit /// /// It is a menu rather than a toolbar control because the set is dynamic: a driver may publish /// none, one or several, and `NSToolbar` needs a fixed identifier per item. As hosted SwiftUI -/// inside the connection group these had no overflow entry, no menu command and no shortcut, so a -/// window narrow enough to clip that group left no way to change warehouse or role at all. +/// inside the toolbar group that once held the connection and its database, these had no overflow +/// entry, no menu command and no shortcut, so a window narrow enough to clip that group left no way +/// to change warehouse or role at all. @MainActor final class SessionContextMenuDelegate: NSObject, NSMenuDelegate { private static let action = #selector(MainSplitViewController.switchSessionContext(_:)) diff --git a/TablePro/Core/Menu/ViewMenuBuilder.swift b/TablePro/Core/Menu/ViewMenuBuilder.swift index 1278441f0..fe3183fea 100644 --- a/TablePro/Core/Menu/ViewMenuBuilder.swift +++ b/TablePro/Core/Menu/ViewMenuBuilder.swift @@ -39,9 +39,9 @@ enum ViewMenuBuilder { ), modeSubmenu(keyboard: keyboard), MenuItemFactory.separator, - /// The segmented control in the toolbar was the only route to either of these, so a - /// window whose toolbar was narrow, hidden or customized could not switch what the - /// sidebar lists. The HIG asks that every toolbar item also be a menu-bar command. + /// The sidebar's own scope control is the pointer route to these, and it is on screen + /// only while the sidebar is. These reveal a collapsed sidebar and switch its list in + /// one step, and they write the state the control reads, so the two move together. MenuItemFactory.item( String(localized: "Show Tables"), action: #selector(MainSplitViewController.showTablesSidebarTab(_:)) @@ -172,9 +172,9 @@ enum ViewMenuBuilder { } /// Browse and Agent as a checked pair sharing one selector, the shape the Result View submenu - /// already uses. The toolbar control is the pointer affordance; the HIG asks that every toolbar - /// item also be a menu-bar command, and it is also the only route a UI test can drive, because a - /// synthetic click on a segment inside an `NSToolbarItemGroup` is measured not to select it. + /// already uses. The toolbar's Actions pull-down offers the same pair, built by + /// `ContentModeMenuDelegate` with the same selector and the same `representedObject`, so the + /// checkmark and the action cannot differ between the two. private static func modeSubmenu(keyboard: KeyboardSettings) -> NSMenuItem { let items = ConnectionWorkspaceContentMode.allCases.map { mode -> NSMenuItem in let item = MenuItemFactory.item( diff --git a/TablePro/Core/ServerDashboard/ServerDashboardQueryProviderFactory.swift b/TablePro/Core/ServerDashboard/ServerDashboardQueryProviderFactory.swift index 459da9922..e17ed6f39 100644 --- a/TablePro/Core/ServerDashboard/ServerDashboardQueryProviderFactory.swift +++ b/TablePro/Core/ServerDashboard/ServerDashboardQueryProviderFactory.swift @@ -6,18 +6,29 @@ import Foundation enum ServerDashboardQueryProviderFactory { + /// Whether an engine has a dashboard, answered without building one. + /// + /// The toolbar and the menu bar ask this on every validation pass, and a pass can follow a + /// keystroke. Asking `provider(for:) != nil` instead built a provider to throw away, which on + /// PostgreSQL is an activity catalog and a metric set. Both functions read `DashboardEngine`, so + /// the list of engines cannot drift between the question and the answer. + static func supportsDashboard(for databaseType: DatabaseType) -> Bool { + DashboardEngine(databaseType) != nil + } + static func provider(for databaseType: DatabaseType, serverVersion: String? = nil) -> ServerDashboardQueryProvider? { - switch databaseType { + guard let engine = DashboardEngine(databaseType) else { return nil } + switch engine { case .postgresql: return PostgreSQLDashboardProvider( activityCatalog: PostgreSQLActivityCatalog(serverVersion: PostgreSQLServerVersion(serverVersion)), metricSet: PostgreSQLDashboardMetricSet(databaseType: databaseType) ) - case .redshift, .cockroachdb: + case .postgresqlCompatible: return PostgreSQLDashboardProvider( metricSet: PostgreSQLDashboardMetricSet(databaseType: databaseType) ) - case .mysql, .mariadb: + case .mysql: return MySQLDashboardProvider() case .mssql: return MSSQLDashboardProvider() @@ -29,6 +40,39 @@ enum ServerDashboardQueryProviderFactory { return SQLiteDashboardProvider() case .typesense: return TypesenseDashboardProvider() + } + } +} + +/// The engines that have a dashboard, and which provider each one takes. +private enum DashboardEngine { + case postgresql + case postgresqlCompatible + case mysql + case mssql + case clickhouse + case duckdb + case sqlite + case typesense + + init?(_ databaseType: DatabaseType) { + switch databaseType { + case .postgresql: + self = .postgresql + case .redshift, .cockroachdb: + self = .postgresqlCompatible + case .mysql, .mariadb: + self = .mysql + case .mssql: + self = .mssql + case .clickhouse: + self = .clickhouse + case .duckdb: + self = .duckdb + case .sqlite: + self = .sqlite + case .typesense: + self = .typesense default: return nil } diff --git a/TablePro/Core/Services/Export/ImportRouting.swift b/TablePro/Core/Services/Export/ImportRouting.swift index bbca28eee..cebcdbe01 100644 --- a/TablePro/Core/Services/Export/ImportRouting.swift +++ b/TablePro/Core/Services/Export/ImportRouting.swift @@ -16,6 +16,14 @@ struct ImportFormatOption: Identifiable, Equatable { var standaloneLabel: String { String(format: String(localized: "Import %@\u{2026}"), name) } + + /// The format's name alone, for a menu whose parent already names the command: Import Data From + /// > CSV…, the shape Keynote and Numbers give Export To. Under that parent `submenuLabel` would + /// read "Import Data From > From CSV…". Not localized, because the format's name is the whole of + /// it and a format name is a technical term. + var formatLabel: String { + "\(name)\u{2026}" + } } enum ImportSheetRoute: Equatable { diff --git a/TablePro/Core/Services/Infrastructure/MainSplitViewController+ContentMode.swift b/TablePro/Core/Services/Infrastructure/MainSplitViewController+ContentMode.swift index cfca1051d..0ee74c9ec 100644 --- a/TablePro/Core/Services/Infrastructure/MainSplitViewController+ContentMode.swift +++ b/TablePro/Core/Services/Infrastructure/MainSplitViewController+ContentMode.swift @@ -59,7 +59,7 @@ internal extension MainSplitViewController { /// Returns without touching the window when the workspace is not the one on screen, which is /// why `applySelectedWorkspace` calls it again: a connection put into Agent mode while another /// was selected reached the window with its columns still collapsed and nothing to reveal them. - internal func applyColumnVisibility( + func applyColumnVisibility( for connectionId: UUID, mode: ConnectionWorkspaceContentMode ) { @@ -100,7 +100,7 @@ internal extension MainSplitViewController { showSelectedTrailingPane() applyPaneChrome() applyWindowTitle() - toolbarOwner?.refreshContentMode() + toolbarOwner?.refreshContext() } func startAgentSession(for connectionId: UUID) { diff --git a/TablePro/Core/Services/Infrastructure/MainSplitViewController+DatabaseMenuActions.swift b/TablePro/Core/Services/Infrastructure/MainSplitViewController+DatabaseMenuActions.swift index 8ad5293d9..ff848bf22 100644 --- a/TablePro/Core/Services/Infrastructure/MainSplitViewController+DatabaseMenuActions.swift +++ b/TablePro/Core/Services/Infrastructure/MainSplitViewController+DatabaseMenuActions.swift @@ -24,6 +24,7 @@ extension MainSplitViewController { switcherPresenter.present( from: view.window, anchoredTo: MainWindowToolbar.connection, + hiddenBy: toolbarOwner?.visibility, subject: .connection, contentSize: ConnectionSwitcherPopover.contentSize ) { [selectedConnectionId] dismiss in diff --git a/TablePro/Core/Services/Infrastructure/MainSplitViewController+FileMenuActions.swift b/TablePro/Core/Services/Infrastructure/MainSplitViewController+FileMenuActions.swift index af21b61f5..61d0a0af5 100644 --- a/TablePro/Core/Services/Infrastructure/MainSplitViewController+FileMenuActions.swift +++ b/TablePro/Core/Services/Infrastructure/MainSplitViewController+FileMenuActions.swift @@ -137,6 +137,15 @@ extension MainSplitViewController { commandActions?.importTables(formatId: formatId) } + /// One named format, from a list `ImportFormatMenuDelegate` filled. Its own selector rather + /// than a second reading of `importData(_:)`, because that one is the menu bar's Import Data + /// and carries ⇧⌘I: AppKit ignores a key equivalent on an item that owns a submenu, so the + /// shortcut has to stay on a leaf that needs no format. + @objc func importDataFormat(_ sender: Any?) { + guard let formatId = (sender as? NSMenuItem)?.representedObject as? String else { return } + commandActions?.importTables(formatId: formatId) + } + @objc func backupDatabase(_ sender: Any?) { commandActions?.backupDatabase() } diff --git a/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift b/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift index 587322cff..ed0b1a6d4 100644 --- a/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift +++ b/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift @@ -7,7 +7,7 @@ import AppKit /// Everything the menu bar needs to decide whether a command applies, captured once /// per validation pass. Keeping it a plain value keeps `isEnabled` pure and testable, -/// the same split `MainWindowToolbar+Validation` uses for the toolbar. +/// the same split `ToolbarContextResolver` uses for the toolbar. struct MenuValidationContext: Equatable { /// Comes from the window's own `ConnectionWindowPhase`, never from the presence of a /// coordinator: the coordinator deliberately outlives a lost session so a reconnect keeps @@ -189,7 +189,7 @@ extension MainSplitViewController: NSMenuItemValidation { case #selector(closeAllTabs(_:)): return context.canCloseAllTabs - case #selector(importData(_:)): + case #selector(importData(_:)), #selector(importDataFormat(_:)): return context.isConnected && !context.isReadOnly && context.hasImportFormats case #selector(backupDatabase(_:)): return context.isConnected && context.supportsBackup diff --git a/TablePro/Core/Services/Infrastructure/MainSplitViewController+ViewMenuActions.swift b/TablePro/Core/Services/Infrastructure/MainSplitViewController+ViewMenuActions.swift index cd3559bc3..0359960cf 100644 --- a/TablePro/Core/Services/Infrastructure/MainSplitViewController+ViewMenuActions.swift +++ b/TablePro/Core/Services/Infrastructure/MainSplitViewController+ViewMenuActions.swift @@ -22,8 +22,8 @@ extension MainSplitViewController { activateWorkspace(offsetBy: 1) } - /// Both mode commands. A tolerant sender: AppKit hands the menu item here and the toolbar - /// group's own action hands the group, and neither should be the only one that works. + /// Both routes to a mode, View > Mode and the Actions pull-down, send a menu item that names + /// its mode in `representedObject`. An item without one does nothing. @objc func setContentModeFromMenu(_ sender: Any?) { guard let raw = (sender as? NSMenuItem)?.representedObject as? String, let mode = ConnectionWorkspaceContentMode(rawValue: raw) else { return } diff --git a/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift b/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift index 979ee9ac4..f764f12ca 100644 --- a/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift +++ b/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift @@ -280,6 +280,9 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan self?.navigationSidebar.applyRailWidth(animated: false) self?.recomputeWindowMinSize() } + navigationSidebar.objectBrowser.onScopeSelection = { [weak self] tab in + self?.setSidebarTab(tab) + } sidebarSplitItem = NSSplitViewItem(sidebarWithViewController: navigationSidebar) sidebarSplitItem.canCollapse = true sidebarSplitItem.minimumThickness = Self.sidebarMinThickness @@ -327,13 +330,11 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan applyPaneChrome() } - /// A divider dragged all the way in collapses the sidebar without going through - /// `toggleSidebar(_:)`, so the toolbar has to be reconciled here too or its segment stays lit - /// over a sidebar that is no longer on screen. + /// A divider dragged all the way in collapses a pane without going through a command, so the + /// window's minimum width is settled here as well as on the commands that collapse one. override func splitViewDidResizeSubviews(_ notification: Notification) { super.splitViewDidResizeSubviews(notification) recomputeWindowMinSize() - toolbarOwner?.syncSidebarSelection() } override func viewWillAppear() { @@ -437,6 +438,10 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan } guard repaint else { return } applyWindowTitle() + /// An edit can change the engine, and what the engine can do decides whether the container + /// capsule stands at all. Nothing else would reconsider it for a connection with no + /// session, whose toolbar state never publishes. + toolbarOwner?.refreshContext() } // MARK: - Toolbar @@ -446,8 +451,8 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan /// /// Attaching it only once a coordinator existed is what made a dozen items arrive at once, a /// second and a half into a connect, over a window that had been on screen the whole time. - /// `MainWindowToolbar` already answers with no subject: `validationContext()` returns nil, so - /// every connection-scoped item validates to disabled and only the window's own commands stay + /// `MainWindowToolbar` already answers with no subject: its context says nothing is connected, + /// so every connection-scoped item validates to disabled and only the window's own commands stay /// live, which is the dimmed-not-absent state the HIG asks for. /// /// The subject is set before the toolbar reaches the window, so a window opening onto a live @@ -462,6 +467,13 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan owner.repoint(to: coordinator) guard window.toolbar !== owner.managedToolbar else { return } window.toolbar = owner.managedToolbar + /// The item set every window starts from is the full one, because `isHidden` is never + /// persisted, so the context is applied the moment the items exist rather than on the first + /// change to it. Measured on macOS 27, that moment is this assignment: `NSToolbar.items` is + /// filled before it returns, with the window not yet shown, and a hide written here holds + /// when the window appears. A repoint cannot be relied on for this: a window opening with + /// no session repoints from nothing to nothing, which returns before it reaches the toolbar. + owner.refreshContext(forcing: true) /// The transparency decision reads `window.toolbar?.isVisible`, so a window that gains its /// toolbar after that decision was taken keeps the opaque titlebar chosen for a /// toolbar-less one, over content that is no longer inset below it. @@ -482,7 +494,6 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan /// that loses its session still has to reach the right phase, or switching to it later /// would show content for a connection that is already gone. private func handleConnectionStatusChange() { - defer { toolbarOwner?.syncSidebarSelection() } for workspace in workspaces.workspaces { reconcileStatus(of: workspace) } @@ -623,6 +634,14 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan /// Only this window's rail moved, and only its highlight. Broadcasting instead made every /// rail in the app rebuild its whole entry list to answer a question none of them asked. navigationSidebar?.railController.refreshSelection() + + /// The toolbar's shape follows the workspace, not only its coordinator. Pointing the toolbar + /// reaches it through a repoint, and a switch between two workspaces that both have no + /// coordinator is a repoint from nothing to nothing, which returns before it looks. The + /// engine and the mode still differ between the two, so a down SQLite connection left the + /// container capsule hidden over a down PostgreSQL one. When nothing has moved this costs + /// building the key and one comparison. + toolbarOwner?.refreshContext() } private func applyPhase() { @@ -756,9 +775,11 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan if let selected { bindSidebarChrome(to: selected) } } - /// The filter field lives above the object list and belongs to the window, so it follows the - /// connection on screen rather than being owned by one. + /// The scope control and the filter field live above the object list and belong to the window, + /// so they follow the connection on screen rather than being owned by one. Agent mode draws no + /// object list for either of them to act on, so both stand down while it is on. private func bindSidebarChrome(to workspace: ConnectionWorkspace) { + navigationSidebar.objectBrowser.setChromeHidden(workspace.resolvedContentMode == .agent) /// The pane decides, not the session. A reconnect keeps `sessionState` while the object /// list below the field is empty, and a filter that accepts typing for a list nobody can /// see is a control that answers for nothing. @@ -1101,7 +1122,7 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan } showSelectedTrailingPane() applyPaneChrome() - toolbarOwner?.refreshContentMode() + toolbarOwner?.refreshContext() toolbarOwner?.managedToolbar.validateVisibleItems() } @@ -1186,7 +1207,8 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan // 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. + /// asking: the Show/Hide Sidebar title, the list checkmarks in the View menu and the reveal + /// actions. var isSidebarCollapsed: Bool { sidebarSplitItem?.isCollapsed ?? true } @@ -1203,15 +1225,6 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan navigationSidebar?.railAllowance ?? 0 } - /// Every collapse route reaches AppKit's own `toggleSidebar(_:)`: the View menu sends the - /// selector down the responder chain, and so does the toolbar's sidebar button. Overriding - /// it is the one place that catches them all, so the toolbar's segment can never stay lit - /// over a collapsed sidebar. - override func toggleSidebar(_ sender: Any?) { - super.toggleSidebar(sender) - toolbarOwner?.syncSidebarSelection() - } - func focusSidebarSearch() { expandSidebarIfCollapsed() navigationSidebar.objectBrowser.focusSearchField() @@ -1254,9 +1267,10 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan state.databaseFilterSelected = [] } - /// Which list the sidebar is showing, or nil while it is collapsed. The segmented control and - /// the two View-menu items both read it, so neither can report a selection the sidebar is not - /// showing. + /// Which list the sidebar is showing, or nil while it is collapsed. The two View-menu items read + /// it for their checkmarks, so neither can report a selection the sidebar is not showing. The + /// scope control reads the connection's state directly, because it is only ever on screen with + /// the sidebar open. var selectedSidebarTab: SidebarTab? { guard sidebarSplitItem?.isCollapsed == false else { return nil } guard let connectionId = currentSession?.connection.id else { return nil } @@ -1264,18 +1278,18 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan } /// Selects a list and leaves the sidebar open, which is what a command called "Show Tables" - /// has to do. `setSidebarTab` is a toggle, correctly so for the segmented control it serves: - /// pressing the segment already selected closes the sidebar. A menu item that did that would - /// be a Show command that hides. + /// has to do. `setSidebarTab` is a toggle: pressing the list already shown closes the sidebar. + /// A menu item that did that would be a Show command that hides. func revealSidebarTab(_ tab: SidebarTab) { guard let connectionId = currentSession?.connection.id else { return } SharedSidebarState.forConnection(connectionId).selectedSidebarTab = tab if sidebarSplitItem?.isCollapsed == true { sidebarSplitItem?.animator().isCollapsed = false } - toolbarOwner?.syncSidebarSelection() } + /// What the sidebar's scope control drives. It writes the same state the View menu's two + /// commands write, and the control reads that state back, so the three cannot disagree. func setSidebarTab(_ tab: SidebarTab) { guard let connectionId = currentSession?.connection.id else { return } let sidebarState = SharedSidebarState.forConnection(connectionId) @@ -1288,7 +1302,6 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan } else { sidebarState.selectedSidebarTab = tab } - toolbarOwner?.syncSidebarSelection() } // MARK: - Dynamic Window Minimum Size diff --git a/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Actions.swift b/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Actions.swift index 8532031d3..17d4f4041 100644 --- a/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Actions.swift +++ b/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Actions.swift @@ -74,9 +74,9 @@ extension MainWindowToolbar { coordinator?.commandActions?.exportTables() } - @objc func performImportFormat(_ sender: Any?) { - guard let menuItem = sender as? NSMenuItem, - let formatId = menuItem.representedObject as? String else { return } - coordinator?.commandActions?.importTables(formatId: formatId) + /// The Inspector item on macOS 13, forwarded because the action belongs to the split controller + /// and a toolbar item's explicit target has to respond to its own selector to validate. + @objc func forwardToggleInspector(_ sender: Any?) { + host?.toggleInspector(sender) } } diff --git a/TablePro/Core/Services/Infrastructure/MainWindowToolbar+ContentMode.swift b/TablePro/Core/Services/Infrastructure/MainWindowToolbar+ContentMode.swift deleted file mode 100644 index 51eb0dcc7..000000000 --- a/TablePro/Core/Services/Infrastructure/MainWindowToolbar+ContentMode.swift +++ /dev/null @@ -1,102 +0,0 @@ -// -// MainWindowToolbar+ContentMode.swift -// TablePro -// - -import AppKit - -/// The Browse / Agent control, and the overflow menu it owns. -/// -/// Two things this gets right that the sidebar control next to it had to be fixed for. Each segment -/// names itself through its image's `accessibilityDescription`, which is measured to be the only -/// channel an expanded group publishes a segment name on. And the overflow menu form is built here -/// rather than left to AppKit, with each item carrying its segment in `tag`, so choosing a mode from -/// the overflow acts. AppKit's own generated form forwards the group as the sender on macOS 27, but -/// that is undocumented; owning the form makes the OS version stop mattering. -internal extension MainWindowToolbar { - static let contentModeItem = NSToolbarItem.Identifier("contentMode") - - static var contentModes: [ConnectionWorkspaceContentMode] { - ConnectionWorkspaceContentMode.allCases - } - - static func makeContentModeGroup(target: AnyObject?, action: Selector) -> NSToolbarItemGroup { - let modes = contentModes - let images = modes.compactMap { - NSImage(systemSymbolName: $0.symbolName, accessibilityDescription: $0.localizedTitle) - } - let group = NSToolbarItemGroup( - itemIdentifier: contentModeItem, - images: images, - selectionMode: .selectOne, - labels: modes.map(\.localizedTitle), - target: target, - action: action - ) - group.label = String(localized: "Mode") - group.paletteLabel = group.label - group.controlRepresentation = .expanded - /// Not navigational: that flag lets AppKit lift an item out of its declared slot and pin it - /// to the leading edge, which is where the sidebar control ended up before it was cleared. - group.isNavigational = false - group.menuFormRepresentation = makeContentModeMenuForm(target: target, action: action) - return group - } - - /// A "Mode" root with one item per mode, each carrying its index in `tag`. - static func makeContentModeMenuForm(target: AnyObject?, action: Selector) -> NSMenuItem { - let root = NSMenuItem(title: String(localized: "Mode"), action: nil, keyEquivalent: "") - let submenu = NSMenu(title: root.title) - for (index, mode) in contentModes.enumerated() { - let item = NSMenuItem(title: mode.localizedTitle, action: action, keyEquivalent: "") - item.target = target - item.tag = index - submenu.addItem(item) - } - root.submenu = submenu - return root - } - - func makeContentModeToolbarItem(claimsSlot: Bool) -> NSToolbarItem { - let group = Self.makeContentModeGroup(target: self, action: #selector(contentModeChanged(_:))) - bindMenuForm(action: #selector(contentModeChanged(_:)), to: Self.contentModeItem) - guard claimsSlot else { return group } - contentModeGroup = group - refreshContentMode() - return group - } - - /// The Inspector item on macOS 13, forwarded because the action belongs to the split controller - /// and a toolbar item's explicit target has to respond to its own selector to validate. - @objc func forwardToggleInspector(_ sender: Any?) { - modeHost?.toggleInspector(sender) - } - - /// The window's own controller, not the connection's coordinator. - /// - /// A workspace that is still connecting, or disconnected, has no `MainContentCoordinator`, so - /// reaching the split controller through one made the mode control inert in the one state agent - /// mode exists to cover. - var modeHost: MainSplitViewController? { - windowController ?? coordinator?.splitViewController - } - - @objc func contentModeChanged(_ sender: Any?) { - guard let index = Self.segmentIndex(from: sender, group: contentModeGroup), - Self.contentModes.indices.contains(index) else { return } - modeHost?.setContentMode(Self.contentModes[index]) - } - - /// Pushed from the split view controller when the mode changes, and the tick in the overflow - /// menu follows the same pass the segments do rather than being a second channel that can drift. - func refreshContentMode() { - guard let group = contentModeGroup else { return } - let mode = modeHost?.contentMode ?? .browse - let index = Self.contentModes.firstIndex(of: mode) ?? 0 - group.selectedIndex = index - for (itemIndex, item) in (group.menuFormRepresentation?.submenu?.items ?? []).enumerated() { - item.state = itemIndex == index ? .on : .off - } - managedToolbar.validateVisibleItems() - } -} diff --git a/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Context.swift b/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Context.swift new file mode 100644 index 000000000..0f6f9c545 --- /dev/null +++ b/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Context.swift @@ -0,0 +1,75 @@ +// +// MainWindowToolbar+Context.swift +// TablePro +// + +import AppKit +import TableProPluginKit + +internal extension MainWindowToolbar { + /// The window's own controller, not the connection's coordinator. + /// + /// A workspace that is still connecting, or disconnected, has no `MainContentCoordinator`, so + /// reaching the split controller through one made the window's own commands inert in exactly + /// the state a user reaches for them. + var host: MainSplitViewController? { + windowController ?? coordinator?.splitViewController + } + + /// The engine the window is showing, read from the workspace's connection record and never from + /// the coordinator. + /// + /// The coordinator goes with the session, so a capability read through it would take the + /// container capsule out of the titlebar and put it back on every dropped connection, which is + /// the titlebar moving on something transient. + private var databaseType: DatabaseType? { + host?.workspaces.selected?.connection?.type ?? coordinator?.connection.type + } + + /// The eight slow-moving facts, read on their own and never through a whole context. + /// + /// This is what every tab-manager publish pays, and typing publishes one per keystroke, so it + /// reads only what the key holds: the selected tab, the window's mode, four locked reads of the + /// plugin metadata registry and a switch over the engine for the dashboard. Everything + /// transient, and every question that walks the coordinator, is left to `currentContext()`. + func currentVisibilityKey() -> ToolbarContext.VisibilityKey { + let tab = coordinator?.tabManager.selectedTab + let databaseType = self.databaseType + return ToolbarContext.VisibilityKey( + tabKind: tab?.tabType, + resultsMode: tab?.display.resultsViewMode, + contentMode: host?.contentMode ?? .browse, + isFileBased: databaseType.map { PluginManager.shared.connectionMode(for: $0) == .fileBased } ?? false, + supportsContainerSwitching: databaseType.map { PluginManager.shared.supportsContainerSwitching(for: $0) } + ?? false, + supportsImport: databaseType.map { PluginManager.shared.supportsImport(for: $0) } ?? false, + supportsServerDashboard: databaseType.map { ServerDashboardQueryProviderFactory.supportsDashboard(for: $0) } + ?? false, + isAIEnabled: AppSettingsManager.shared.ai.enabled + ) + } + + /// Everything the two resolvers are allowed to know, read once per question. + /// + /// `isConnected` is the session's own liveness, which counts a reconnect in progress as up. + /// The health monitor writes `.connecting` on every attempt of a backoff while the window keeps + /// showing the session's tabs and rows, and dimming the row for that would be noise. + func currentContext() -> ToolbarContext { + let host = self.host + let state = coordinator?.toolbarState + return ToolbarContext( + key: currentVisibilityKey(), + pane: host?.currentPane ?? .empty, + isConnected: state.map { Self.hasLiveSession($0.connectionState) } ?? false, + hasSelectedWorkspace: host?.hasSelectedWorkspace ?? false, + canToggleTrailingPane: host?.canToggleTrailingPane ?? false, + pendingChange: state?.pendingChange, + hasDataPendingChanges: state?.hasDataPendingChanges ?? false, + blocksAllWrites: state?.safeModeLevel.blocksAllWrites ?? false, + canAddRow: coordinator?.canAddRow ?? false, + canRestorePreviousValues: coordinator?.canRewindSelectedTab ?? false, + canNavigateBack: coordinator?.canNavigateBack ?? false, + canNavigateForward: coordinator?.canNavigateForward ?? false + ) + } +} diff --git a/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Delegate.swift b/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Delegate.swift index d6b92da58..ea3685623 100644 --- a/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Delegate.swift +++ b/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Delegate.swift @@ -7,6 +7,13 @@ import AppKit import os extension MainWindowToolbar { + /// Builds a new item on every call and keeps none of them. + /// + /// Customize Toolbar asks again for every allowed identifier, and a delegate that handed back + /// a cached instance handed back the one the context had hidden: measured on macOS 27, a + /// dragged-in item that was the same instance arrived with `isHidden` still true, took its slot + /// and drew nothing. The palette's stale visibility state rides the item instance as well, so + /// an instance shared across vends would carry it into every later arrangement. internal func toolbar( _ toolbar: NSToolbar, itemForItemIdentifier itemIdentifier: NSToolbarItem.Identifier, @@ -15,15 +22,19 @@ extension MainWindowToolbar { Self.lifecycleLogger.info( "[open] toolbar delegate buildItem id=\(itemIdentifier.rawValue, privacy: .public) hasCoordinator=\(self.coordinator != nil)" ) - guard let item = buildItem(itemIdentifier, willBeInsertedIntoToolbar: flag) else { return nil } + guard let item = buildItem(itemIdentifier) else { return nil } applyVisibilityPriority(to: item) return item } - private func buildItem( - _ itemIdentifier: NSToolbarItem.Identifier, - willBeInsertedIntoToolbar flag: Bool - ) -> NSToolbarItem? { + /// An item the palette is about to drop in has to take the context the window is in, and the + /// hideable ones are the ones that care. Measured, opening and closing the palette without a + /// change posts nothing, so this costs nothing when the user only looks. + internal func toolbarWillAddItem(_ notification: Notification) { + scheduleVisibilityReapply() + } + + private func buildItem(_ itemIdentifier: NSToolbarItem.Identifier) -> NSToolbarItem? { switch itemIdentifier { case Self.inspector: /// AppKit builds `.toggleInspector` itself and never asks the delegate for it, so this @@ -43,10 +54,18 @@ extension MainWindowToolbar { shortcut: .toggleInspector, description: String(localized: "Toggle Inspector") ) - case Self.sidebarToggle: - return makeSidebarToggleItem(claimsSlot: Self.claimsItemSlot(willBeInsertedIntoToolbar: flag)) - case Self.contentModeItem: - return makeContentModeToolbarItem(claimsSlot: Self.claimsItemSlot(willBeInsertedIntoToolbar: flag)) + case Self.connection: + return makeConnectionItem() + case Self.database: + return makeDatabaseItem() + case Self.refresh: + return makeRefreshItem() + case Self.saveChanges: + return makeSaveChangesItem() + case Self.actions: + return makeActionsItem() + case Self.safeMode: + return makeSafeModeItem() case Self.backForwardGroup: /// `isNavigational` is what puts back and forward on the leading edge of the content /// title area, where Finder and Safari keep them, instead of in the slot the identifier @@ -55,44 +74,14 @@ extension MainWindowToolbar { /// Both subitems are installed unconditionally and stay installed. Availability is /// `isEnabled`, written by `validateToolbarItem(_:)`, never presence: measured on three /// running Apple apps, Xcode, Finder in column view and System Settings all keep the - /// 75pt capsule and dim the direction that has nowhere to go. Emptying the group - /// instead put the pair behind state that is `@ObservationIgnored`, so once hidden it - /// did not come back until the user switched tabs. + /// 75pt capsule and dim the direction that has nowhere to go. let group = makeNativeGroup( id: itemIdentifier, label: String(localized: "Navigation"), - subitems: [subitemNavigateBack(), subitemNavigateForward()] + subitems: [makeNavigateBackItem(), makeNavigateForwardItem()] ) group.isNavigational = true return group - case Self.connectionGroup: - /// Native, like every other group here. As a view-backed group it drew a hosted SwiftUI - /// row and its subitems were inert: the header is explicit that a property set on the - /// parent, "such as label or view, apply to the entire item", so neither subitem - /// reached the overflow menu, the customization palette or `validate()`. - /// - /// Not navigational, unlike back and forward. `isNavigational` asks AppKit to lift an - /// item to the leading edge of the content area, which is the opposite of what - /// `centeredItemIdentifiers` asks for, and this group is the centred one. - return makeNativeGroup( - id: itemIdentifier, - label: String(localized: "Connection"), - subitems: [subitemConnection(), subitemDatabase()] - ) - case TransportRateToolbarItem.identifier: - /// Beside the centred pair, never inside it. A group is laid out around its own - /// midpoint, so a readout inside this one pushed the two capsules off centre by half - /// the readout's width; measured as its own adjacent item, the group sits where it - /// sits with no readout at all and the figure lands 6.0pt past its trailing edge. - return transportRateGroup - case Self.safeMode: - return subitemSafeMode() - case Self.editorGroup: - return makeNativeGroup( - id: itemIdentifier, - label: String(localized: "Editor"), - subitems: [subitemNewTab(), subitemQuickSwitcher()] - ) case Self.previewSQL: return menuOnlyItem( id: itemIdentifier, @@ -142,21 +131,18 @@ extension MainWindowToolbar { shortcut: .toggleHistory, description: String(localized: "Toggle Query History") ) - case Self.refreshSaveGroup: - return makeNativeGroup( - id: itemIdentifier, - label: String(localized: "Table Actions"), - subitems: [ - subitemRefresh(), subitemSaveChanges(), subitemAddRow(), - subitemRestorePreviousValues(), - ] - ) - case Self.exportImportGroup: - return makeNativeGroup( - id: itemIdentifier, - label: String(localized: "Export & Import"), - subitems: [subitemExport(), subitemImport()] - ) + case Self.exportTables: + return makeExportItem() + case Self.importTables: + return makeImportItem() + case Self.addRow: + return makeAddRowItem() + case Self.restorePreviousValues: + return makeRestorePreviousValuesItem() + case Self.newTab: + return makeNewTabItem() + case Self.quickSwitcher: + return makeQuickSwitcherItem() default: return nil } diff --git a/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Items.swift b/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Items.swift index b885e5a33..4f8d9c487 100644 --- a/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Items.swift +++ b/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Items.swift @@ -6,7 +6,7 @@ import AppKit extension MainWindowToolbar { - // MARK: - Subitem Builders + // MARK: - Item Builders /// The name of the driver's own query language, so the Preview tooltip says "Preview MQL" on /// MongoDB rather than a generic word the user has to translate. @@ -45,14 +45,6 @@ extension MainWindowToolbar { coordinator?.connection.name ?? "" } - /// Whether the centred group carries a throughput readout at all, and whether there is a second - /// reading worth taking. Read from the connection's configuration rather than from the registry, - /// so it holds for the whole session: the group's shape is settled when the connection is - /// adopted and never changes under a running tunnel. - var carriesMeasuredTransport: Bool { - coordinator?.connection.activeTunnelKind?.carriesMeasuredBytes == true - } - /// The container this control switches, and only that. It briefly read "app › public" on a /// schema-grouped engine while the click still opened the database chooser, which makes the /// word the user aimed at the one thing the control cannot change. The schema has its own @@ -61,7 +53,13 @@ extension MainWindowToolbar { coordinator?.toolbarState.currentDatabase ?? "" } - func subitemConnection() -> NSToolbarItem { + /// The verb the selected tab commits with, for an item vended now. `refreshCommitVerb(for:)` + /// keeps a live one in step, from the same tab kind. + var commitVerb: String { + ToolbarContextResolver.commitVerb(for: coordinator?.tabManager.selectedTab?.tabType) + } + + func makeConnectionItem() -> NSToolbarItem { menuOnlyItem( id: Self.connection, label: String(localized: "Connection"), @@ -78,26 +76,23 @@ extension MainWindowToolbar { /// `NSMenuToolbarItem` plus a glyph that follows the level. `StatefulToolbarItem.validate()` /// re-reads `symbolProvider` on every validation pass, and `observeItemState` puts /// `safeModeLevel` on the list of things that trigger one. - func subitemSafeMode() -> NSToolbarItem { + func makeSafeModeItem() -> NSToolbarItem { let label = String(localized: "Safe Mode") let item = SafeModeToolbarItem(itemIdentifier: Self.safeMode) item.label = label item.paletteLabel = label item.isBordered = true item.levelProvider = { [weak self] in self?.coordinator?.toolbarState.safeModeLevel ?? .silent } - item.isEnabledProvider = { [weak self] in - guard let self, let context = validationContext() else { return false } - return Self.isEnabled(itemIdentifier: Self.safeMode, context: context) - } + item.isEnabledProvider = enablement(of: Self.safeMode) /// The same class the Database menu's submenu uses, so the two lists cannot describe /// different levels, and the checkmark is resolved when the menu opens rather than when /// the item was built. `NSMenu.delegate` is weak, so the toolbar holds this one. - item.menu = safeModeMenu() + item.menu = menu(delegate: safeModeMenuDelegate) /// The overflow entry names the list, not the control, for the same reason the Database /// menu's container does: one of the levels inside it is itself called Safe Mode. let menuItem = NSMenuItem(title: String(localized: "Safe Mode Level"), action: nil, keyEquivalent: "") - menuItem.submenu = safeModeMenu() + menuItem.submenu = menu(delegate: safeModeMenuDelegate) item.menuFormRepresentation = menuItem /// No `toolTip` here. `levelProvider` already wrote one naming the current level, and /// overwriting it with the bare label was permanent: `applyLevel` returns early once the @@ -105,12 +100,43 @@ extension MainWindowToolbar { return item } - private func safeModeMenu() -> NSMenu { + /// The long tail of what a context can do, in one control whose menu changes with the tab. + /// + /// `ellipsis.circle` is the glyph Finder gives its own Action pull-down. The menu is built by + /// `ConnectionActionsMenuDelegate` when it opens. The overflow entry is AppKit's own and is + /// left to it: measured on macOS 27, an `NSMenuToolbarItem` answers `menuFormRepresentation` + /// with a fresh item titled with its label over this same menu, whatever was assigned, so a + /// narrow window's overflow offers exactly what the control would. + func makeActionsItem() -> NSToolbarItem { + let label = String(localized: "Actions") + let item = StatefulMenuToolbarItem(itemIdentifier: Self.actions) + item.label = label + item.paletteLabel = label + item.isBordered = true + item.image = NSImage(systemSymbolName: "ellipsis.circle", accessibilityDescription: label) + item.toolTip = String(localized: "Commands for the current tab and connection") + item.isEnabledProvider = enablement(of: Self.actions) + item.menu = menu(delegate: actionsMenuDelegate) + return item + } + + /// A menu filled by its delegate when it opens. `NSMenu.delegate` is weak, so the delegate is + /// one the toolbar keeps. + private func menu(delegate: any NSMenuDelegate) -> NSMenu { let menu = NSMenu() - menu.delegate = safeModeMenuDelegate + menu.delegate = delegate return menu } + /// The enablement a menu-owning item asks for on each validation pass, answered by the same + /// resolver and from the same pass context as every other item. + private func enablement(of identifier: NSToolbarItem.Identifier) -> @MainActor () -> Bool { + { [weak self] in + guard let self else { return false } + return ToolbarContextResolver.isEnabled(identifier, context: self.validationContext()) + } + } + /// What this driver calls the thing a connection browses, so the item reads "Open Keyspace" on /// Cassandra rather than a word that does not exist there. var containerEntityName: String { @@ -119,7 +145,7 @@ extension MainWindowToolbar { } ?? String(localized: "Database") } - func subitemDatabase() -> NSToolbarItem { + func makeDatabaseItem() -> NSToolbarItem { let containerName = containerEntityName return menuOnlyItem( id: Self.database, @@ -132,7 +158,7 @@ extension MainWindowToolbar { ) } - func subitemNewTab() -> NSToolbarItem { + func makeNewTabItem() -> NSToolbarItem { menuOnlyItem( id: Self.newTab, label: String(localized: "New Tab"), @@ -143,7 +169,7 @@ extension MainWindowToolbar { ) } - func subitemQuickSwitcher() -> NSToolbarItem { + func makeQuickSwitcherItem() -> NSToolbarItem { menuOnlyItem( id: Self.quickSwitcher, label: String(localized: "Open Quickly"), @@ -153,7 +179,7 @@ extension MainWindowToolbar { ) } - func subitemRefresh() -> NSToolbarItem { + func makeRefreshItem() -> NSToolbarItem { menuOnlyItem( id: Self.refresh, label: String(localized: "Refresh"), @@ -166,7 +192,7 @@ extension MainWindowToolbar { /// No text label on either button: the HIG asks for the standard chevrons and says not to /// label a Back control. `chevron.backward` and `chevron.forward` mirror in a right-to-left /// layout, which `chevron.left` and `chevron.right` do not. - func subitemNavigateBack() -> NSToolbarItem { + func makeNavigateBackItem() -> NSToolbarItem { menuOnlyItem( id: Self.navigateBack, label: String(localized: "Back"), @@ -176,7 +202,7 @@ extension MainWindowToolbar { ) } - func subitemNavigateForward() -> NSToolbarItem { + func makeNavigateForwardItem() -> NSToolbarItem { menuOnlyItem( id: Self.navigateForward, label: String(localized: "Forward"), @@ -186,10 +212,13 @@ extension MainWindowToolbar { ) } - func subitemSaveChanges() -> NSToolbarItem { + /// Labelled with the verb the tab commits with, and re-labelled by `refreshCommitVerb(for:)` when + /// the tab kind moves, so the palette, the overflow entry and the tooltip never offer to save a + /// table definition that is about to be created. + func makeSaveChangesItem() -> NSToolbarItem { menuOnlyItem( id: Self.saveChanges, - label: String(localized: "Save Changes"), + label: commitVerb, symbol: "checkmark.circle.fill", action: #selector(performSaveChanges(_:)), shortcut: .saveChanges @@ -197,10 +226,9 @@ extension MainWindowToolbar { } /// A row insert is a change to the data, so it belongs with the other data commands rather than - /// in the status bar, which reports what is on screen. It ships as a subitem of an existing group - /// so a toolbar the user already customized picks it up: `autosavesConfiguration` restores the - /// saved identifier list, and a brand new top-level identifier would never appear for them. - func subitemAddRow() -> NSToolbarItem { + /// in the status bar, which reports what is on screen. Offered by Customize Toolbar and by the + /// Actions pull-down on a table tab showing data. + func makeAddRowItem() -> NSToolbarItem { menuOnlyItem( id: Self.addRow, label: String(localized: "Add Row"), @@ -210,13 +238,10 @@ extension MainWindowToolbar { ) } - /// Rides in the Table Actions group for the same reason Add Row does: a brand new top-level - /// identifier never appears for anyone whose toolbar configuration is already saved. - /// /// It stays enabled without a license. The point of it being here is that someone who has just /// saved the wrong thing finds it, and finding it is what makes the licence worth buying; a /// dimmed item they never notice sells nothing and helps nobody. - func subitemRestorePreviousValues() -> NSToolbarItem { + func makeRestorePreviousValuesItem() -> NSToolbarItem { menuOnlyItem( id: Self.restorePreviousValues, label: String(localized: "Restore Previous Values"), @@ -226,7 +251,7 @@ extension MainWindowToolbar { ) } - func subitemExport() -> NSToolbarItem { + func makeExportItem() -> NSToolbarItem { menuOnlyItem( id: Self.exportTables, label: String(localized: "Export"), @@ -240,45 +265,22 @@ extension MainWindowToolbar { /// `NSMenuToolbarItem` is the toolbar control that opens a menu. A plain `NSToolbarItem` with a /// submenu on its `menuFormRepresentation` only shows that menu in the overflow list. /// - /// It carries no action on purpose. Given one, AppKit splits the control into a body that sends - /// the action and a separate chevron that opens the menu, so clicking the item itself does - /// nothing whenever the driver offers more than one format. With no action the whole control - /// opens the menu, and a single-format driver simply gets a one-item menu. - func subitemImport() -> NSToolbarItem { + /// The formats come from `ImportFormatMenuDelegate` when the menu opens, the same instance the + /// Actions pull-down's Import Data submenu uses, so the two lists cannot differ. The overflow + /// entry is AppKit's, over this same menu, for the reason `makeActionsItem` gives. + func makeImportItem() -> NSToolbarItem { let label = String(localized: "Import") - let item = NSMenuToolbarItem(itemIdentifier: Self.importTables) + let item = StatefulMenuToolbarItem(itemIdentifier: Self.importTables) item.label = label item.paletteLabel = label item.isBordered = true item.image = NSImage(systemSymbolName: "square.and.arrow.down", accessibilityDescription: label) - item.menu = buildImportSubmenu() - - let menuItem = NSMenuItem(title: label, action: nil, keyEquivalent: "") - menuItem.image = item.image - menuItem.submenu = buildImportSubmenu() - item.menuFormRepresentation = menuItem - bindMenuForm(action: #selector(performImportFormat(_:)), to: Self.importTables) - + item.isEnabledProvider = enablement(of: Self.importTables) + item.menu = menu(delegate: importFormatMenuDelegate) bindShortcut(.importData, description: String(localized: "Import Data"), to: item) return item } - func buildImportSubmenu() -> NSMenu { - let menu = NSMenu() - guard let databaseType = coordinator?.connection.type else { return menu } - for format in PluginManager.shared.importFormatOptions(for: databaseType) { - let menuItem = NSMenuItem( - title: format.submenuLabel, - action: #selector(performImportFormat(_:)), - keyEquivalent: "" - ) - menuItem.target = self - menuItem.representedObject = format.id - menu.addItem(menuItem) - } - return menu - } - // MARK: - Helpers /// The label is what the customization palette and the overflow menu show, so it stays short. @@ -353,15 +355,7 @@ extension MainWindowToolbar { /// and container titles took the whole content width and every command went to the overflow /// menu. A truncated container name is a worse loss than Refresh and Save. func applyVisibilityPriority(to item: NSToolbarItem) { - guard item.itemIdentifier != Self.connectionGroup else { return } + guard item.itemIdentifier != Self.connection, item.itemIdentifier != Self.database else { return } item.visibilityPriority = .high } - - /// One slot per identifier, and the slot belongs to the item that is actually in the toolbar. - /// AppKit asks the delegate again with `willBeInsertedIntoToolbar: false` to build the palette - /// copies shown by Customize Toolbar, and a palette copy that took the slot left every later - /// `syncSidebarSelection()` writing into a discarded group. - static func claimsItemSlot(willBeInsertedIntoToolbar: Bool) -> Bool { - willBeInsertedIntoToolbar - } } diff --git a/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Validation.swift b/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Validation.swift index 582186b31..383d1cb55 100644 --- a/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Validation.swift +++ b/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Validation.swift @@ -4,36 +4,13 @@ // import AppKit -import TableProPluginKit extension MainWindowToolbar: NSToolbarItemValidation { - struct ValidationContext { - /// True whenever the session is alive, which includes a query in flight. A running - /// query is not a reason to disable Refresh or New Tab, and the menu bar already - /// derives its own `isConnected` from the window phase rather than from execution. - let connected: Bool - let isTableTab: Bool - let canAddRow: Bool - let canRestorePreviousValues: Bool - let hasPendingChanges: Bool - let hasDataPendingChanges: Bool - let blocksAllWrites: Bool - let fileBased: Bool - let supportsContainerSwitching: Bool - let supportsImport: Bool - let supportsServerDashboard: Bool - let canNavigateBack: Bool - let canNavigateForward: Bool - /// A connection is on screen, whether or not it has finished connecting. The mode control is - /// the one item that answers to this rather than to `connected`. - var hasSelectedWorkspace: Bool = false - } - /// Listed exhaustively so a new state has to choose a side instead of inheriting "alive". /// /// `.connecting` counts because the health monitor writes it on every reconnect attempt, and /// the window keeps showing the session's tabs and rows throughout. Graying the whole toolbar - /// out for the length of a backoff would take Sidebar Toggle with it. + /// out for the length of a backoff would dim every control for a blip that repairs itself. static func hasLiveSession(_ state: ToolbarConnectionState) -> Bool { switch state { case .connected, .connecting: @@ -43,113 +20,27 @@ extension MainWindowToolbar: NSToolbarItemValidation { } } - static func isEnabled(itemIdentifier: NSToolbarItem.Identifier, context: ValidationContext) -> Bool { - switch itemIdentifier { - case Self.connection, Self.history: - return true - case Self.assistant: - /// The View menu command already gates on the window showing content. Without the same - /// gate here the button stays live over a disconnected session and uncollapses the - /// assistant beside a connection that cannot answer. - return context.connected && AppSettingsManager.shared.ai.enabled - case Self.database: - return context.connected && !context.fileBased && context.supportsContainerSwitching - case Self.safeMode: - /// Safe mode is what stands between a stray keystroke and a live table, so the control - /// that sets it answers for as long as the session does. A window with no session has - /// nothing to protect and nothing to write it to. - return context.connected - case Self.inspector: - return context.connected - case Self.refresh, Self.quickSwitcher, Self.newTab, Self.exportTables, Self.sidebarToggle: - return context.connected - /// Reachable while a connection is still dialling, unlike the rest of these. Agent mode - /// draws the prompt the user typed, so a control gated on `connected` would be dead in the - /// one state that surface exists for. - case Self.contentModeItem: - return context.hasSelectedWorkspace && AppSettingsManager.shared.ai.enabled - case Self.addRow: - return context.connected && context.canAddRow - case Self.restorePreviousValues: - return context.connected && context.canRestorePreviousValues - case Self.navigateBack: - return context.connected && context.canNavigateBack - case Self.navigateForward: - return context.connected && context.canNavigateForward - case Self.saveChanges: - return context.hasPendingChanges && context.connected && !context.blocksAllWrites - case Self.previewSQL: - return context.hasDataPendingChanges && context.connected - case Self.results: - return context.connected && !context.isTableTab - case Self.dashboard: - return context.connected && context.supportsServerDashboard - case Self.importTables: - return context.connected && !context.blocksAllWrites && context.supportsImport - default: - return true - } + /// Every item answers from `ToolbarContextResolver`, which decides with no window and no + /// session, so the answer a test pins is the answer the titlebar draws. + func validateToolbarItem(_ item: NSToolbarItem) -> Bool { + ToolbarContextResolver.isEnabled(item.itemIdentifier, context: validationContext()) } +} - func validationContext() -> ValidationContext? { - guard let state = coordinator?.toolbarState else { - /// A workspace that is still connecting has no coordinator, and returning nil here - /// disabled every item including the mode control, whose whole point is to be reachable - /// in exactly that state. - guard let host = windowController, host.hasSelectedWorkspace else { return nil } - return ValidationContext( - connected: false, - isTableTab: false, - canAddRow: false, - canRestorePreviousValues: false, - hasPendingChanges: false, - hasDataPendingChanges: false, - blocksAllWrites: false, - fileBased: false, - supportsContainerSwitching: false, - supportsImport: false, - supportsServerDashboard: false, - canNavigateBack: false, - canNavigateForward: false, - hasSelectedWorkspace: true - ) +/// The connection window's toolbar, which tells its delegate where a validation pass starts and +/// ends so the pass can be answered from one context. +/// +/// Measured on macOS 27: AppKit's own passes, the ones a window update runs, go through +/// `validateVisibleItems()`, so this override brackets them as well as the ones the app asks for. +/// There is no delegate callback for either edge of a pass, which is why this is a subclass. +@MainActor +internal final class ContextValidatedToolbar: NSToolbar { + override internal func validateVisibleItems() { + guard let owner = delegate as? MainWindowToolbar else { + super.validateVisibleItems() + return } - return ValidationContext( - connected: Self.hasLiveSession(state.connectionState), - isTableTab: state.isTableTab, - canAddRow: coordinator?.canAddRow ?? false, - canRestorePreviousValues: coordinator?.canRewindSelectedTab ?? false, - hasPendingChanges: state.hasPendingChanges, - hasDataPendingChanges: state.hasDataPendingChanges, - blocksAllWrites: state.safeModeLevel.blocksAllWrites, - fileBased: PluginManager.shared.connectionMode(for: state.databaseType) == .fileBased, - supportsContainerSwitching: PluginManager.shared.supportsContainerSwitching(for: state.databaseType), - supportsImport: PluginManager.shared.supportsImport(for: state.databaseType), - supportsServerDashboard: coordinator?.commandActions?.supportsServerDashboard ?? false, - canNavigateBack: coordinator?.canNavigateBack ?? false, - canNavigateForward: coordinator?.canNavigateForward ?? false, - hasSelectedWorkspace: modeHost?.hasSelectedWorkspace ?? false - ) - } - - /// Switch Connection is the window's, so it answers before a subject is required. Every other - /// item here needs the coordinator that presents it, and enabling one of those without a - /// subject would leave a live-looking button that does nothing, so no subject still disables - /// the rest of the toolbar. - /// - /// The sidebar item is not an exception, however window-owned the sidebar itself now is: it is - /// the Tables/Favorites segmented control, `sidebarSegmentChanged` reaches - /// `coordinator?.splitViewController`, and the tab it selects is per-connection state that a - /// window with no session has nowhere to write. Show/Hide Sidebar is the command that answers - /// in every phase, and it lives in the View menu and on the divider rather than here. - static func isWindowScoped(_ itemIdentifier: NSToolbarItem.Identifier) -> Bool { - itemIdentifier == Self.connection - } - - func validateToolbarItem(_ item: NSToolbarItem) -> Bool { - if Self.isWindowScoped(item.itemIdentifier) { return true } - guard let context = validationContext() else { return false } - return Self.isEnabled(itemIdentifier: item.itemIdentifier, context: context) + owner.withinValidationPass { super.validateVisibleItems() } } } @@ -159,11 +50,12 @@ extension MainWindowToolbar: NSToolbarItemValidation { /// New Tab, Open Quickly, Export, Database, Results and Dashboard live in the overflow menu of a /// narrow window while the same buttons were disabled on a wide one, and clicking one did nothing. /// The mapping now comes from the factory that built the item, so it cannot fall behind again. +/// +/// The Actions pull-down never reaches here. Its entries carry no target, so AppKit resolves them +/// through the responder chain to the window's controller, and that is the one validator they get. extension MainWindowToolbar: NSMenuItemValidation { func validateMenuItem(_ menuItem: NSMenuItem) -> Bool { guard let itemIdentifier = itemIdentifier(forMenuFormAction: menuItem.action) else { return true } - if Self.isWindowScoped(itemIdentifier) { return true } - guard let context = validationContext() else { return false } - return Self.isEnabled(itemIdentifier: itemIdentifier, context: context) + return ToolbarContextResolver.isEnabled(itemIdentifier, context: validationContext()) } } diff --git a/TablePro/Core/Services/Infrastructure/MainWindowToolbar.swift b/TablePro/Core/Services/Infrastructure/MainWindowToolbar.swift index d0605ea49..aaaa92909 100644 --- a/TablePro/Core/Services/Infrastructure/MainWindowToolbar.swift +++ b/TablePro/Core/Services/Infrastructure/MainWindowToolbar.swift @@ -10,13 +10,23 @@ import os @MainActor internal final class MainWindowToolbar: NSObject, NSToolbarDelegate { private var itemStateObservation: AnyCancellable? + private var tabStateObservation: AnyCancellable? nonisolated internal static let lifecycleLogger = Logger(subsystem: "com.TablePro", category: "NativeTabLifecycle") - /// The autosave name. Bumping it discards every saved arrangement, so it moves only when the - /// default set changes enough that replaying a stored one would be worse than resetting it. The - /// v3 move drops the hosted status item and four commands from the default, and a v2 list still - /// names identifiers the delegate no longer vends. v4 adds the throughput readout: a stored v3 - /// arrangement does not name it, so a reader who had customized the toolbar would never see it. + /// The autosave name, and deliberately not bumped when the default set changes. + /// + /// Measured on macOS 27 across separate process launches under one identifier: AppKit diffs the + /// stored `TB Default Item Identifiers` against the current default list and splices a new + /// identifier in at its position in that list rather than at the end, so a stored + /// `[alpha, charlie]` against a new default `[alpha, bravo, charlie, delta]` came back + /// `[alpha, bravo, charlie]`, and an identifier the delegate stops vending is pruned from the + /// record on the next launch. A user who customized keeps their arrangement and still receives + /// new items where they belong; a bump would throw that away along with their display mode. + /// The record is six keys and `isHidden` is not among them, since it is runtime state that is + /// never persisted, so a context-sensitive item set is no reason to bump either. + /// + /// Bump only for a change that diff cannot express, and say which. Neither fact was measured on + /// macOS 13 or 14. internal static let toolbarIdentifier = NSToolbar.Identifier("com.TablePro.main.toolbar.v4") /// Which connection the toolbar is about. Every item reads this rather than capturing a @@ -53,37 +63,42 @@ internal final class MainWindowToolbar: NSObject, NSToolbarDelegate { /// `shortcutBindings` is: a second hand-written table drifts. private var menuFormIdentifiers: [Selector: NSToolbarItem.Identifier] = [:] - private(set) var sidebarGroup: NSToolbarItemGroup? - internal var contentModeGroup: NSToolbarItemGroup? - - /// The throughput readout. One item per toolbar rather than one per vend: the ticker writes - /// into it directly, so it has to be the instance the toolbar is actually showing. - internal let transportRateItem = TransportRateToolbarItem() - - /// A group holding nothing but the readout, and the reason it exists is that emptying it is how - /// the readout leaves the toolbar. Measured: an item whose view is hidden keeps its 75pt, and a - /// view constrained to zero width still leaves a 24pt gap, so neither hides it cleanly. - /// `NSToolbarItem.isHidden` does, but it is macOS 15 and the app targets 14. An empty group - /// reclaims the space exactly, on every version, with no availability gate. - internal private(set) lazy var transportRateGroup: NSToolbarItemGroup = { - let group = NSToolbarItemGroup(itemIdentifier: TransportRateToolbarItem.identifier) - let label = String(localized: "Throughput") - group.label = label - group.paletteLabel = label - group.subitems = [] - return group - }() - private var transportSampler = TransportRateSampler() - private var transportTicker: Task? + /// What the app last took out of the titlebar, written by `apply(_:)` and by nothing else. + /// + /// The switcher reads this to decide whether its anchor is reachable, because the palette + /// poisons AppKit's own answer for good: see `ToolbarVisibility`. + internal private(set) var visibility = ToolbarVisibility() + + /// The slow-moving half of the context the visibility and the commit verb were last applied + /// from. A keystroke builds a fresh key and compares it with this, and writes nothing unless it + /// moved, which is what keeps the titlebar from moving while the user types. + private var appliedVisibilityKey: ToolbarContext.VisibilityKey? - private static let transportSampleInterval = Duration.seconds(1) + /// The context of the validation pass in progress, and nil between passes. See + /// `validationContext()`. + private var validationPassContext: ToolbarContext? + + /// Coalesces the passes Customize Toolbar asks for, so a drop of several items is one pass. + private var isVisibilityReapplyScheduled = false /// `NSMenu.delegate` is weak, and the safe-mode control's menu is built here rather than by the /// menu bar, so this toolbar is what keeps its delegate alive. internal let safeModeMenuDelegate = SafeModeMenuDelegate() + /// The import formats, filled when the menu opens. One instance serves the Actions pull-down's + /// submenu and the Import item a user can add from Customize Toolbar, so the two cannot list + /// different formats. + internal let importFormatMenuDelegate = ImportFormatMenuDelegate() + + /// Kept for the same reason as the two above. The Actions menu is built on every open from the + /// context the toolbar is pointed at, so it holds the toolbar weakly and asks it. + internal private(set) lazy var actionsMenuDelegate = ConnectionActionsMenuDelegate( + importFormats: importFormatMenuDelegate, + context: { [weak self] in self?.currentContext() ?? ToolbarContext() } + ) + override internal convenience init() { - self.init(managedToolbar: NSToolbar(identifier: Self.toolbarIdentifier)) + self.init(managedToolbar: ContextValidatedToolbar(identifier: Self.toolbarIdentifier)) } internal init(managedToolbar: NSToolbar) { @@ -97,7 +112,7 @@ internal final class MainWindowToolbar: NSObject, NSToolbarDelegate { self.managedToolbar.displayMode = .iconOnly self.managedToolbar.allowsUserCustomization = true self.managedToolbar.autosavesConfiguration = true - self.managedToolbar.centeredItemIdentifiers = [Self.connectionGroup] + self.managedToolbar.centeredItemIdentifiers = [Self.connection, Self.database] /// The hop off `AppSettingsManager.keyboard`'s own `didSet` matters: without it the toolbar /// items are mutated re-entrantly, part way through the settings write that triggered them. AppEvents.shared.keyboardSettingsChanged @@ -166,9 +181,8 @@ internal final class MainWindowToolbar: NSObject, NSToolbarDelegate { itemStateObservationGeneration += 1 self.coordinator = coordinator observeItemState() - restartTransportTicker() refreshConnectionScopedItems() - syncSidebarSelection() + refreshContext(forcing: true) managedToolbar.validateVisibleItems() } @@ -179,74 +193,47 @@ internal final class MainWindowToolbar: NSObject, NSToolbarDelegate { /// connection was released has no coordinator to reach it through. windowController?.switcherPresenter.dismiss() itemStateObservationGeneration += 1 - transportTicker?.cancel() - transportTicker = nil - transportRateItem.apply(rate: nil) - sidebarGroup = nil + itemStateObservation = nil + tabStateObservation = nil coordinator = nil } - /// Runs only for a connection whose transport the app carries the bytes for, so an ordinary - /// direct connection costs nothing at all. It writes into the readout's own field rather than - /// asking the toolbar to revalidate: the field is a fixed width, so a new figure is a redraw - /// inside one view and never a layout pass. - private func restartTransportTicker() { - transportTicker?.cancel() - transportSampler = TransportRateSampler() - transportRateItem.apply(rate: nil) - guard carriesMeasuredTransport else { - transportTicker = nil - return - } - - transportTicker = Task { @MainActor [weak self] in - while !Task.isCancelled { - try? await Task.sleep(for: Self.transportSampleInterval) - guard !Task.isCancelled, let self else { return } - self.sampleTransportRate() - } - } - } - - private func sampleTransportRate() { - guard let connection = coordinator?.connection else { return } - let totals = TransportActivityRegistry.shared.totals(for: connection.id) - transportRateItem.apply(rate: totals.flatMap { transportSampler.sample($0, at: .now) }) - } - - /// Emptying and refilling the readout's own group is the one structural change AppKit - /// re-measures. It happens when a connection is adopted, never under a running tunnel, so - /// nothing moves while a figure is ticking. - private func syncTransportRateVisibility() { - let carries = !transportRateGroup.subitems.isEmpty - guard carries != carriesMeasuredTransport else { return } - transportRateGroup.subitems = carriesMeasuredTransport ? [transportRateItem] : [] - } - /// What a validation pass depends on beyond the responder chain. `validateVisibleItems()` is /// also what re-runs `StatefulToolbarItem.validate()`, so the safe-mode glyph tracks the level /// through the same channel rather than through an observer of its own. + /// + /// The tab manager is watched for the visibility key alone. It publishes on every edit to a + /// tab, typing included, so its observer builds the key from its eight inputs, compares it and + /// returns: no whole context, no validation pass and no write. private func observeItemState() { + itemStateObservation = nil + tabStateObservation = nil + guard let coordinator else { return } let generation = itemStateObservationGeneration - let coordinatorIdentifier = coordinator.map { ObjectIdentifier($0) } - /// Wakes for any change on the toolbar state rather than only the four properties the - /// tracked closure read. `validateVisibleItems()` is idempotent, so the wider wake set - /// costs a revalidation pass and nothing else. - guard let toolbarState = coordinator?.toolbarState else { return } - itemStateObservation = toolbarState.onMainActorChange { [weak self] in - guard let self, - generation == self.itemStateObservationGeneration, - coordinatorIdentifier == self.coordinator.map({ ObjectIdentifier($0) }) - else { return } + let coordinatorIdentifier = ObjectIdentifier(coordinator) + /// Wakes for any change on the toolbar state rather than only the properties the items + /// read. `validateVisibleItems()` is idempotent and builds one context for the whole pass, + /// so the wider wake set costs a revalidation pass and nothing else. + itemStateObservation = coordinator.toolbarState.onMainActorChange { [weak self] in + guard let self, self.isObserving(generation, coordinatorIdentifier) else { return } + self.refreshContext() self.managedToolbar.validateVisibleItems() } + tabStateObservation = coordinator.tabManager.onMainActorChange { [weak self] in + guard let self, self.isObserving(generation, coordinatorIdentifier) else { return } + self.refreshContext() + } } - /// Items carrying text or a menu derived from the connection. Validation cannot repoint them: - /// a menu built at construction keeps whatever it was built with, and a label is not part of - /// what `validate()` reconsiders. They are pushed here instead. + /// A callback queued before a repoint must not act on the connection that replaced its own. + private func isObserving(_ generation: Int, _ coordinatorIdentifier: ObjectIdentifier) -> Bool { + generation == itemStateObservationGeneration + && coordinatorIdentifier == coordinator.map { ObjectIdentifier($0) } + } + + /// Items carrying text derived from the connection. Validation cannot repoint them: a label is + /// not part of what `validate()` reconsiders. They are pushed here instead. private func refreshConnectionScopedItems() { - syncTransportRateVisibility() for item in allItems() { switch item.itemIdentifier { case Self.connection: @@ -264,15 +251,29 @@ internal final class MainWindowToolbar: NSObject, NSToolbarDelegate { case Self.previewSQL: updateShortcutDescription(previewDescription, for: Self.previewSQL) applyShortcutBinding(to: item) - case Self.importTables: - (item as? NSMenuToolbarItem)?.menu = buildImportSubmenu() - item.menuFormRepresentation?.submenu = buildImportSubmenu() default: continue } } } + /// The commit control says what its tab commits: Create Table on a definition tab, Apply + /// Changes on Users & Roles, Save Changes everywhere else. The palette, the overflow menu and the + /// tooltip all read the label, so all three move with it. + /// + /// Run only when the visibility key moves, because the verb is a function of the tab kind the + /// key carries. It used to follow the staged change, which a Create Table draft raises and + /// drops as it becomes valid and invalid, so the label flipped while the user typed and, with + /// labels shown, every flip changed the item's width and reflowed the titlebar. + private func refreshCommitVerb(for tabKind: TabType?) { + let verb = ToolbarContextResolver.commitVerb(for: tabKind) + for item in managedToolbar.items where item.itemIdentifier == Self.saveChanges && item.label != verb { + apply(label: verb, to: item) + updateShortcutDescription(verb, for: Self.saveChanges) + applyShortcutBinding(to: item) + } + } + /// The overflow menu keeps the title it was built with, so a label change has to reach it too. private func apply(label: String, to item: NSToolbarItem) { item.label = label @@ -287,12 +288,96 @@ internal final class MainWindowToolbar: NSObject, NSToolbarDelegate { } } + // MARK: - Validation + + /// The context a validation question is answered from: the running pass's when there is one, + /// and a fresh one otherwise. + /// + /// A pass asks once per visible item, and the context walks the coordinator for Add Row, Restore + /// Previous Values and both navigation directions, so building it per item paid that walk once + /// for every item on every pass AppKit runs. AppKit also asks outside a pass, measured on + /// macOS 27: an item is validated once as it joins the toolbar, before any pass, and those + /// questions take the fresh read. + internal func validationContext() -> ToolbarContext { + validationPassContext ?? currentContext() + } + + /// Runs one validation pass over a context built once for all of it. A pass that starts inside + /// another reuses the outer one's, because nothing can move between two items of one pass. + internal func withinValidationPass(_ validate: () -> Void) { + guard validationPassContext == nil else { + validate() + return + } + validationPassContext = currentContext() + defer { validationPassContext = nil } + validate() + } + + // MARK: - Visibility + + /// Re-applies the titlebar's shape and the commit verb when the slow-moving half of the context + /// has moved, and otherwise returns after building the key and comparing it. + /// + /// No whole context is built here. Visibility is a function of the key alone, so the coordinator + /// walk the enablement questions need is never paid for a keystroke. + /// + /// `forcing` is for the moments the shape has to be re-established whatever the key says: a + /// repoint to a different connection, and the toolbar first reaching its window. Every launch + /// starts from an all-visible toolbar, measured, because `isHidden` is never persisted. + internal func refreshContext(forcing: Bool = false) { + let key = currentVisibilityKey() + guard forcing || key != appliedVisibilityKey else { return } + appliedVisibilityKey = key + refreshCommitVerb(for: key.tabKind) + apply(ToolbarContextResolver.visibility(for: key)) + } + + /// Writes the resolver's set onto the items the app placed, and never touches one the user + /// added from the palette. + /// + /// `isHidden` and nothing else. `insertItem` and `removeItem` write the saved arrangement to + /// disk on the spot, and every connection window shares this toolbar's identifier, so a context + /// expressed that way would reach every other window and outlive the app. + /// + /// The validation pass is not optional. Measured on macOS 27, the `isHidden` setter runs no + /// validation, and an item shown again keeps whatever `isEnabled` it had when it went away, + /// through the same run-loop turn, the next one and 1.5s later, until something asks. + /// + /// Below macOS 15 there is no `isHidden`, so the full set stands and the contextual items dim. + /// The record is left empty there, because it describes what the app hid and it hid nothing. + internal func apply(_ visibility: ToolbarVisibility) { + guard #available(macOS 15.0, *) else { return } + self.visibility = visibility + for item in managedToolbar.items where ToolbarContextResolver.hideableIdentifiers.contains(item.itemIdentifier) { + item.isHidden = visibility.hides(item.itemIdentifier) + } + managedToolbar.validateVisibleItems() + } + + /// One pass on the next turn rather than now: the item AppKit announces is not in + /// `NSToolbar.items` until the notification returns. A burst of drops is still one pass. + /// + /// Only the item's own arrival needs it. A delegate that vends a fresh item every time hands + /// back one that is visible, so the failure this covers is a hideable item that should be + /// hidden staying visible until the next tab switch. With an instance cached across vends the + /// failure is worse, measured: the palette hands back the same instance still hidden, and it + /// takes a slot and draws nothing. + internal func scheduleVisibilityReapply() { + guard !isVisibilityReapplyScheduled else { return } + isVisibilityReapplyScheduled = true + Task { @MainActor [weak self] in + guard let self else { return } + self.isVisibilityReapplyScheduled = false + self.apply(self.visibility) + } + } + // MARK: - Identifiers /// `nonisolated` throughout: these are immutable strings that name a command, and /// `ToolbarContextResolver` reads them from off the main actor to answer which items a context /// shows. Isolating them to this class was incidental to the class being `@MainActor`. - nonisolated static let connectionGroup = NSToolbarItem.Identifier("com.TablePro.toolbar.connectionGroup") nonisolated static let connection = NSToolbarItem.Identifier("com.TablePro.toolbar.connection") nonisolated static let database = NSToolbarItem.Identifier("com.TablePro.toolbar.database") nonisolated static let refresh = NSToolbarItem.Identifier("com.TablePro.toolbar.refresh") @@ -316,12 +401,8 @@ internal final class MainWindowToolbar: NSObject, NSToolbarDelegate { nonisolated static let history = NSToolbarItem.Identifier("com.TablePro.toolbar.history") nonisolated static let exportTables = NSToolbarItem.Identifier("com.TablePro.toolbar.export") nonisolated static let importTables = NSToolbarItem.Identifier("com.TablePro.toolbar.import") - nonisolated static let refreshSaveGroup = NSToolbarItem.Identifier("com.TablePro.toolbar.refreshSaveGroup") - nonisolated static let editorGroup = NSToolbarItem.Identifier("com.TablePro.toolbar.editorGroup") nonisolated static let restorePreviousValues = NSToolbarItem .Identifier("com.TablePro.toolbar.restorePreviousValues") - nonisolated static let exportImportGroup = NSToolbarItem.Identifier("com.TablePro.toolbar.exportImportGroup") - nonisolated static let sidebarToggle = NSToolbarItem.Identifier("com.TablePro.toolbar.sidebarToggle") nonisolated static let backForwardGroup = NSToolbarItem.Identifier("com.TablePro.toolbar.backForwardGroup") nonisolated static let navigateBack = NSToolbarItem.Identifier("com.TablePro.toolbar.navigateBack") nonisolated static let navigateForward = NSToolbarItem.Identifier("com.TablePro.toolbar.navigateForward") @@ -331,8 +412,10 @@ internal final class MainWindowToolbar: NSObject, NSToolbarDelegate { // MARK: - NSToolbarDelegate - /// Items ahead of `.sidebarTrackingSeparator` lay out in the sidebar's own titlebar strip, - /// so the control that switches what the sidebar shows sits over the pane it drives. + /// Eight controls, and the zones are the panes. AppKit's own sidebar toggle stands alone ahead of + /// `.sidebarTrackingSeparator`, so it lays out in the sidebar's titlebar strip and follows the + /// divider; the connection and its container are centred; Refresh, the commit verb, the Actions + /// pull-down and Safe Mode are the content run; the trailing-pane toggle closes the row. /// /// A tracking separator divides the toolbar into pane-aligned sections; it does not align the /// items inside one. Everything after `.inspectorTrackingSeparator` therefore lays out from @@ -342,53 +425,62 @@ internal final class MainWindowToolbar: NSObject, NSToolbarDelegate { /// WWDC23 session 10054. Keep the toggle last: ahead of the separator it lands in the content /// section and is wrong in both states. /// - /// Four runs, and every item in one belongs to the same job, because adjacent items share one - /// background under Liquid Glass and a run of unrelated singletons reads as scatter. A - /// `NSToolbarItemGroup` is one control however many subitems it holds, so Table Actions and - /// the two editor commands are groups rather than five loose buttons. - /// - /// The centre is what the window is pointed at: the connection and the container, each a - /// titled control that opens its own chooser, which is the shape Xcode gives its scheme and - /// destination. It carries a title rather than a label, so it still reads as words with the - /// toolbar in icon-only mode. The centred item this replaces was an `NSHostingController`, and - /// that is the whole reason it used to disappear: measured, a native centred group is still - /// fully visible at 700pt where the hosted one was dropped at 1000pt, because AppKit can - /// compress a group it draws itself and can only drop a view it does not. + /// The centred pair are two top-level items, not two subitems of one group. Measured on macOS + /// 27, `NSPopover.show(relativeTo:)` on a subitem raised `NSInvalidArgumentException` ("view has + /// no window") whenever its group was hidden or clipped, which Swift cannot catch, while a + /// top-level item raised in none of 16 presentations across the same states and across a + /// Customize Toolbar visit: AppKit anchors it on the titlebar or on the clipped-items indicator + /// instead. As two items they still centre as one adjacent pair, 249pt wide in a 1200pt window + /// against the group's 257pt. Each carries a title rather than a label, so it still reads as + /// words with the toolbar in icon-only mode, which is the shape Xcode gives its scheme and + /// destination. /// /// Nothing here repeats the window title, which names the tab rather than the connection. /// `.inspectorTrackingSeparator` is macOS 14. Without it the divider does not track the /// inspector's edge; the items around it are unchanged. - internal static var defaultItemIdentifiers: [NSToolbarItem.Identifier] { + nonisolated internal static var defaultItemIdentifiers: [NSToolbarItem.Identifier] { var items: [NSToolbarItem.Identifier] = [ - sidebarToggle, + .toggleSidebar, .sidebarTrackingSeparator, - contentModeItem, - backForwardGroup, .flexibleSpace, - connectionGroup, - TransportRateToolbarItem.identifier, + connection, + database, .flexibleSpace, - refreshSaveGroup, - editorGroup, + refresh, + saveChanges, + actions, safeMode, ] if #available(macOS 14.0, *) { items.append(.inspectorTrackingSeparator) } - items.append(contentsOf: [.flexibleSpace, assistant, inspector]) + items.append(contentsOf: [.flexibleSpace, inspector]) return items } - /// `addRow`, `restorePreviousValues`, `quickSwitcher` and `newTab` are absent on purpose: they - /// ride a group as subitems and the delegate vends no standalone item for any of them, so - /// listing one here would offer the customization palette a tile it cannot build. + /// A proper superset of the default list: the commands a user can drag in from Customize + /// Toolbar. Each one is also in the Actions pull-down or the menu bar, so the default set loses + /// nothing by leaving it out, and an item added from here is the user's, so no context hides it. + /// + /// Back and Forward keep their group, because `isNavigational` is what gives the pair the + /// leading-edge placement Finder and Safari use and two loose items cannot have it. /// - internal static let allowedItemIdentifiers: [NSToolbarItem.Identifier] = defaultItemIdentifiers + [ + /// Every identifier here must be one the delegate can build. Asked for one it cannot, the + /// delegate answers nil, and with `autosavesConfiguration` on AppKit prunes it from the saved + /// arrangement. + nonisolated internal static let allowedItemIdentifiers: [NSToolbarItem.Identifier] = defaultItemIdentifiers + [ + backForwardGroup, previewSQL, results, - exportImportGroup, + exportTables, + importTables, dashboard, history, + assistant, + addRow, + restorePreviousValues, + newTab, + quickSwitcher, ] internal func toolbarDefaultItemIdentifiers(_ toolbar: NSToolbar) -> [NSToolbarItem.Identifier] { @@ -399,87 +491,3 @@ internal final class MainWindowToolbar: NSObject, NSToolbarDelegate { Self.allowedItemIdentifiers } } - -// MARK: - Sidebar Toggle - -extension MainWindowToolbar { - private static let sidebarSegmentTabs: [SidebarTab] = [.tables, .favorites] - - /// A one-of-N segmented toolbar control is `NSToolbarItemGroup` with `.selectOne`. - /// Hand-building two `NSButton`s meant faking selection with border tricks and a - /// deprecated bezel, and polling `@Observable` state to keep them in sync. - /// - /// The group must not be navigational. `isNavigational` lets AppKit lift an item out of its - /// declared slot and pin it to the leading edge of the content title area, the way Finder - /// places back and forward, which put this control past the sidebar divider no matter where - /// `defaultItemIdentifiers` listed it. Without the flag it lays out in the sidebar's own - /// titlebar strip, and follows the divider when the sidebar collapses. - internal static func makeSidebarSegmentGroup(target: AnyObject?, action: Selector) -> NSToolbarItemGroup { - /// Measured on macOS 27: an expanded `selectOne` group publishes a radio group whose buttons - /// take their name from each image's `accessibilityDescription` and never from `labels:`. - /// With nil, VoiceOver read the SF Symbol's own description, so these two announced as - /// "List" and "favorite". - let labels = [String(localized: "Tables"), String(localized: "Favorites")] - let images = zip(["list.bullet", "star"], labels).compactMap { - NSImage(systemSymbolName: $0, accessibilityDescription: $1) - } - let group = NSToolbarItemGroup( - itemIdentifier: sidebarToggle, - images: images, - selectionMode: .selectOne, - labels: labels, - target: target, - action: action - ) - group.label = String(localized: "Sidebar") - group.paletteLabel = group.label - group.controlRepresentation = .expanded - return group - } - - /// `sidebarGroup` is the one handle `syncSidebarSelection()` has on the live control, so only - /// the item actually going into the toolbar may claim it. A Customize Toolbar palette copy - /// that took the slot left every later sync writing into a discarded group, and the segments - /// stopped following the sidebar until the window was reopened. - internal func makeSidebarToggleItem(claimsSlot: Bool) -> NSToolbarItem { - let group = Self.makeSidebarSegmentGroup(target: self, action: #selector(sidebarSegmentChanged(_:))) - bindMenuForm(action: #selector(sidebarSegmentChanged(_:)), to: Self.sidebarToggle) - guard claimsSlot else { return group } - sidebarGroup = group - syncSidebarSelection() - return group - } - - /// Reachable from the control and from its overflow menu, and the two send different senders: - /// the group itself, and an `NSMenuItem`. Reading `selectedIndex` off whatever arrived and - /// giving up when it was not a group meant choosing Tables or Favorites from the overflow did - /// nothing at all, which is every ordinary window width where the control lives there. - /// - /// The group is read from `sidebarGroup` rather than from the sender, so both routes resolve the - /// same selection, and a menu item carries its segment in `tag`. - @objc fileprivate func sidebarSegmentChanged(_ sender: Any?) { - guard let index = Self.segmentIndex(from: sender, group: sidebarGroup), - Self.sidebarSegmentTabs.indices.contains(index) else { return } - coordinator?.splitViewController?.setSidebarTab(Self.sidebarSegmentTabs[index]) - } - - /// Which segment a toolbar group's action is about, whichever route sent it. - internal static func segmentIndex(from sender: Any?, group: NSToolbarItemGroup?) -> Int? { - if let menuItem = sender as? NSMenuItem, menuItem.tag >= 0 { - return menuItem.tag - } - if let sent = sender as? NSToolbarItemGroup { - return sent.selectedIndex - } - return group?.selectedIndex - } - - /// Pushed from the split view controller whenever the sidebar tab or its collapsed - /// state changes, instead of an observation loop watching for it. - internal func syncSidebarSelection() { - guard let group = sidebarGroup else { return } - let tab = coordinator?.splitViewController?.selectedSidebarTab - group.selectedIndex = tab.flatMap(Self.sidebarSegmentTabs.firstIndex(of:)) ?? -1 - managedToolbar.validateVisibleItems() - } -} diff --git a/TablePro/Core/Services/Infrastructure/SidebarContainerViewController.swift b/TablePro/Core/Services/Infrastructure/SidebarContainerViewController.swift index e4d8c49e4..4271afc28 100644 --- a/TablePro/Core/Services/Infrastructure/SidebarContainerViewController.swift +++ b/TablePro/Core/Services/Infrastructure/SidebarContainerViewController.swift @@ -9,22 +9,72 @@ import SwiftUI @MainActor internal final class SidebarContainerViewController: NSViewController { + /// Which list the rows below show. Window chrome like the field under it, so it stands through + /// a connection switch and follows the connection on screen. + private let scopeControl = SidebarScopeControl() private let searchField = NSSearchField() /// Sidebar chrome, like the field it shares a row with, so it survives a connection switch and /// writes settings that are not scoped to one. Hidden on the Favorites tab, whose list draws /// none of what these options settle. private let viewOptionsButton = SidebarViewOptionsButton() + private lazy var filterRow = NSStackView(views: [searchField, viewOptionsButton]) /// The filter field is window chrome and stays put; only the object list below it belongs to a /// connection, so that is the part the window swaps. private let listHost = WorkspacePaneHost() private var sidebarState: SharedSidebarState? private var observationTask: Task? private var filterPopover: NSPopover? + /// Exactly one of these is active. A hidden view keeps its constraints, so hiding the two rows + /// alone would leave the list standing under their height. + private var listBelowChrome: NSLayoutConstraint? + private var listAtTop: NSLayoutConstraint? + private var chromeHidden = false + + /// A list the user picked, reported to the window, which owns the sidebar and whether it is + /// open. + internal var onScopeSelection: ((SidebarTab) -> Void)? internal func show(_ controller: NSViewController?) { listHost.show(controller) } + /// Which list the scope control has selected, for a caller that has to read the chrome back. + internal var selectedScope: SidebarTab? { + scopeControl.selectedTab + } + + internal var isScopeEnabled: Bool { + scopeControl.isEnabled + } + + internal var isChromeHidden: Bool { + chromeHidden + } + + /// Agent mode puts its session rail where the object list goes, and neither row above it has + /// anything there to act on: the scope would switch a list that is not drawn, and the field + /// would filter one. Both go, and the rail takes their height. + internal func setChromeHidden(_ hidden: Bool) { + guard chromeHidden != hidden else { return } + chromeHidden = hidden + applyChromeVisibility() + } + + /// Recorded before it is applied, so a mode that arrives before the view loads is still the + /// one the view comes up in. + private func applyChromeVisibility() { + guard isViewLoaded else { return } + scopeControl.isHidden = chromeHidden + filterRow.isHidden = chromeHidden + if chromeHidden { + listBelowChrome?.isActive = false + listAtTop?.isActive = true + } else { + listAtTop?.isActive = false + listBelowChrome?.isActive = true + } + } + /// Whether the filter field answers, and what it currently holds. The object list below it /// belongs to a connection; the field belongs to the window and stands whether or not one is /// up, so both of these have to be true of a window with no session as well as one with. @@ -48,6 +98,13 @@ internal final class SidebarContainerViewController: NSViewController { override func loadView() { view = NSView() + scopeControl.translatesAutoresizingMaskIntoConstraints = false + /// Standing and dimmed until a connection is up, for the reason the field below it is. + scopeControl.isEnabled = false + scopeControl.target = self + scopeControl.action = #selector(scopeChanged(_:)) + view.addSubview(scopeControl) + searchField.translatesAutoresizingMaskIntoConstraints = false /// Standing from the window's first frame, disabled until a connection is up. It used to /// be hidden until then, so the sidebar was a bare column for the length of a connect and @@ -67,7 +124,6 @@ internal final class SidebarContainerViewController: NSViewController { /// A stack view rather than two anchored controls, so hiding the button on the Favorites /// tab takes its width with it: `detachesHiddenViews` removes a hidden arranged subview /// from the layout, where a hidden anchored one would keep its gap beside the field. - let filterRow = NSStackView(views: [searchField, viewOptionsButton]) filterRow.translatesAutoresizingMaskIntoConstraints = false filterRow.orientation = .horizontal filterRow.alignment = .centerY @@ -82,25 +138,35 @@ internal final class SidebarContainerViewController: NSViewController { /// 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 rowLeading = filterRow.leadingAnchor.constraint(equalTo: view.leadingAnchor, constant: 10) - let rowTrailing = filterRow.trailingAnchor.constraint(equalTo: view.trailingAnchor, constant: -10) - rowLeading.priority = .defaultHigh - rowTrailing.priority = .defaultHigh + let insets = [ + scopeControl.leadingAnchor.constraint(equalTo: view.leadingAnchor, constant: 10), + scopeControl.trailingAnchor.constraint(equalTo: view.trailingAnchor, constant: -10), + filterRow.leadingAnchor.constraint(equalTo: view.leadingAnchor, constant: 10), + filterRow.trailingAnchor.constraint(equalTo: view.trailingAnchor, constant: -10), + ] + for inset in insets { + inset.priority = .defaultHigh + } + let listBelowChrome = hostingView.topAnchor.constraint(equalTo: filterRow.bottomAnchor, constant: 5) + self.listBelowChrome = listBelowChrome + listAtTop = hostingView.topAnchor.constraint(equalTo: view.safeAreaLayoutGuide.topAnchor) - NSLayoutConstraint.activate([ - filterRow.topAnchor.constraint(equalTo: view.safeAreaLayoutGuide.topAnchor, constant: 5), - rowLeading, - rowTrailing, + NSLayoutConstraint.activate(insets + [ + scopeControl.topAnchor.constraint(equalTo: view.safeAreaLayoutGuide.topAnchor, constant: 5), + filterRow.topAnchor.constraint(equalTo: scopeControl.bottomAnchor, constant: 6), - hostingView.topAnchor.constraint(equalTo: filterRow.bottomAnchor, constant: 5), + listBelowChrome, hostingView.leadingAnchor.constraint(equalTo: view.leadingAnchor), hostingView.trailingAnchor.constraint(equalTo: view.trailingAnchor), hostingView.bottomAnchor.constraint(equalTo: view.bottomAnchor), ]) + applyChromeVisibility() } + /// Asked of the field's ancestors too, because Agent mode hides the row it sits in rather + /// than the field itself, and focusing a field nobody can see puts the keyboard nowhere. func focusSearchField() { - guard !searchField.isHidden else { return } + guard !searchField.isHiddenOrHasHiddenAncestor else { return } view.window?.makeFirstResponder(searchField) } @@ -126,7 +192,7 @@ internal final class SidebarContainerViewController: NSViewController { /// because that is what it scopes, and because the field is the one piece of sidebar chrome /// that outlives a workspace switch. func presentDatabaseFilter(connectionId: UUID, sidebarState: SharedSidebarState) { - guard !searchField.isHidden else { return } + guard !searchField.isHiddenOrHasHiddenAncestor else { return } filterPopover?.close() filterPopover = PopoverPresenter.show( relativeTo: searchField.bounds, @@ -159,13 +225,18 @@ internal final class SidebarContainerViewController: NSViewController { searchField.placeholderString = String(localized: "Filter") searchField.setAccessibilityLabel(String(localized: "Filter")) viewOptionsButton.isHidden = true + scopeControl.isEnabled = false + scopeControl.selectedTab = nil return } searchField.isEnabled = true + scopeControl.isEnabled = true /// Set here rather than left to the observation task, which runs on the next main-actor /// turn: the button would show over the favorites filter for a turn on the way in, and - /// linger for a turn on the way out, with the stack view re-laying the row each time. + /// linger for a turn on the way out, with the stack view re-laying the row each time. The + /// scope follows for the same reason, or it would name the previous connection's list. viewOptionsButton.isHidden = state.selectedSidebarTab != .tables + scopeControl.selectedTab = state.selectedSidebarTab observationTask = Task { @MainActor [weak self] in guard let self else { return } while !Task.isCancelled { @@ -195,7 +266,10 @@ internal final class SidebarContainerViewController: NSViewController { observationTask?.cancel() } + /// Every later change to the list choice arrives here, so View > Show Tables and Show + /// Favorites, which write the same state the scope control does, move the control with them. private func syncFromState(_ state: SharedSidebarState) { + scopeControl.selectedTab = state.selectedSidebarTab let activeText: String let placeholder: String switch state.selectedSidebarTab { @@ -215,6 +289,15 @@ internal final class SidebarContainerViewController: NSViewController { searchField.placeholderString = placeholder searchField.setAccessibilityLabel(placeholder) } + + /// Only a change reaches the window. `NSSegmentedControl` sends its action for a click on the + /// segment that is already selected as well, measured on macOS 27, and the command it drives + /// collapses the sidebar on a second press of the list it is showing. From a control inside the + /// sidebar that would take the control away with the pane it sits in. + @objc private func scopeChanged(_ sender: SidebarScopeControl) { + guard let tab = sender.selectedTab, tab != sidebarState?.selectedSidebarTab else { return } + onScopeSelection?(tab) + } } extension SidebarContainerViewController: NSSearchFieldDelegate { diff --git a/TablePro/Core/Services/Infrastructure/StatefulToolbarItem.swift b/TablePro/Core/Services/Infrastructure/StatefulToolbarItem.swift index e30398cde..de9488c77 100644 --- a/TablePro/Core/Services/Infrastructure/StatefulToolbarItem.swift +++ b/TablePro/Core/Services/Infrastructure/StatefulToolbarItem.swift @@ -69,30 +69,40 @@ internal final class StatefulToolbarItem: NSToolbarItem { } } +/// A toolbar control that opens a menu and answers for its own enablement. +/// +/// It carries no action on purpose: given one, AppKit splits the control into a body that sends +/// the action and a separate chevron that opens the menu, so a click on the body opens nothing. +/// And `NSToolbarItem`'s own `validate()` only sends `validateToolbarItem(_:)` for an item that +/// has an action, so the toolbar's predicate for this identifier would never be consulted and the +/// control would stay live over a session that had gone. It asks on the validation pass instead, +/// which is also the one channel measured to keep reaching an item while it is hidden. +@MainActor +internal class StatefulMenuToolbarItem: NSMenuToolbarItem { + internal var isEnabledProvider: (@MainActor () -> Bool)? + + override internal func validate() { + super.validate() + guard let isEnabledProvider else { return } + isEnabled = isEnabledProvider() + } +} + /// The safe-mode chooser: one of six levels, and the current one has to be readable without /// opening the menu. `NSMenuToolbarItem` is the toolbar control that opens a menu, and the glyph /// tracks the level through the validation pass `MainWindowToolbar.observeItemState` triggers. @MainActor -internal final class SafeModeToolbarItem: NSMenuToolbarItem { +internal final class SafeModeToolbarItem: StatefulMenuToolbarItem { internal var levelProvider: (@MainActor () -> SafeModeLevel)? { didSet { applyLevel() } } - /// Its own enablement, because AppKit will not ask for it. This item carries no action, and - /// `NSToolbarItem`'s own `validate()` only sends `validateToolbarItem(_:)` for an item that - /// has one, so the toolbar's predicate for this identifier was never consulted and the control - /// stayed live over a session that had gone. - internal var isEnabledProvider: (@MainActor () -> Bool)? - private var symbolSource = ToolbarSymbolSource() private var appliedLevel: SafeModeLevel? override internal func validate() { super.validate() applyLevel() - if let isEnabledProvider { - isEnabled = isEnabledProvider() - } } /// The tooltip carries the level's name because the glyph alone cannot: `lock` and diff --git a/TablePro/Core/Services/Infrastructure/Toolbar/ActionsMenuSpec.swift b/TablePro/Core/Services/Infrastructure/Toolbar/ActionsMenuSpec.swift index 73d52d569..6a212600f 100644 --- a/TablePro/Core/Services/Infrastructure/Toolbar/ActionsMenuSpec.swift +++ b/TablePro/Core/Services/Infrastructure/Toolbar/ActionsMenuSpec.swift @@ -16,34 +16,52 @@ internal enum ActionsSubmenuKind: Equatable, Hashable, Sendable { case mode } -/// One command in the Actions pull-down. +/// One row in the Actions pull-down: a command, or a submenu's own row. /// -/// Carries no target. Every entry is built with `target = nil` so AppKit routes it through the -/// responder chain and `MainSplitViewController.validateMenuItem` decides it, which is the same -/// path the menu bar already takes. Giving an entry an explicit target would hand validation to +/// A command carries no target. Every one is built with `target = nil` so AppKit routes it through +/// the responder chain and `MainSplitViewController.validateMenuItem` decides it, which is the same +/// path the menu bar already takes. Giving a command an explicit target would hand validation to /// `MainWindowToolbar.validateMenuItem`, whose unrecognised-action arm returns true, and ship every /// entry enabled. internal struct ActionsMenuEntry: Equatable { + /// Two roles rather than optional fields, so a submenu's row cannot declare a selector or a + /// shortcut it would never draw. Measured, assigning `submenu` replaces the row's action with + /// `submenuAction:` and its target with the submenu, and AppKit ignores a key equivalent on an + /// item that owns a submenu, so both would be promises the menu does not keep. + internal enum Role: Equatable { + case command(Selector, shortcut: ShortcutAction?) + case submenu(ActionsSubmenuKind) + } + internal let title: String - internal let selector: Selector - internal let shortcut: ShortcutAction? - /// What the command is about, for an entry that names one of several values of the same - /// command. `setContentModeFromMenu:` needs it for both the action and the checkmark. - internal let representedValue: String? - internal let submenu: ActionsSubmenuKind? - - internal init( - title: String, - selector: Selector, - shortcut: ShortcutAction? = nil, - representedValue: String? = nil, - submenu: ActionsSubmenuKind? = nil - ) { + internal let role: Role + + internal init(title: String, selector: Selector, shortcut: ShortcutAction? = nil) { + self.title = title + self.role = .command(selector, shortcut: shortcut) + } + + internal init(title: String, submenu: ActionsSubmenuKind) { self.title = title - self.selector = selector - self.shortcut = shortcut - self.representedValue = representedValue - self.submenu = submenu + self.role = .submenu(submenu) + } + + /// Nil for a submenu's row. + internal var selector: Selector? { + guard case let .command(selector, _) = role else { return nil } + return selector + } + + /// Nil for a submenu's row, and for a command with no chord. + internal var shortcut: ShortcutAction? { + guard case let .command(_, shortcut) = role else { return nil } + return shortcut + } + + /// Nil for a command. + internal var submenu: ActionsSubmenuKind? { + guard case let .submenu(kind) = role else { return nil } + return kind } } diff --git a/TablePro/Core/Services/Infrastructure/Toolbar/ConnectionActionsMenuDelegate.swift b/TablePro/Core/Services/Infrastructure/Toolbar/ConnectionActionsMenuDelegate.swift new file mode 100644 index 000000000..6dd2e665b --- /dev/null +++ b/TablePro/Core/Services/Infrastructure/Toolbar/ConnectionActionsMenuDelegate.swift @@ -0,0 +1,98 @@ +// +// ConnectionActionsMenuDelegate.swift +// TablePro +// + +import AppKit + +/// Builds the Actions pull-down each time it opens, from `ConnectionActionsMenuResolver` and from +/// nothing else. +/// +/// Built on open rather than on every context change. The menu is only read while it is open, and +/// `menuNeedsUpdate` is measured to fire exactly once per real open, so a tab switch costs it +/// nothing and a menu that has been opened once can never describe a tab the window has left. +/// +/// No command carries a target. AppKit resolves each one through the responder chain to the +/// window's controller and asks that controller's `validateMenuItem`, which is the path the menu bar +/// already takes for the same commands, so the pull-down needs no enablement table of its own and +/// cannot disagree with the menu bar. A command targeted at the toolbar would be validated by +/// `MainWindowToolbar.validateMenuItem` instead, which answers true for every action it did not +/// build, and the whole menu would ship enabled. A submenu's own row is the exception AppKit makes +/// itself: measured, assigning `submenu` sets the row's action to `submenuAction:` and its target +/// to the submenu. +@MainActor +internal final class ConnectionActionsMenuDelegate: NSObject, NSMenuDelegate { + private let context: @MainActor () -> ToolbarContext + private let importFormats: ImportFormatMenuDelegate + private let modes = ContentModeMenuDelegate() + + internal init( + importFormats: ImportFormatMenuDelegate, + context: @escaping @MainActor () -> ToolbarContext + ) { + self.importFormats = importFormats + self.context = context + super.init() + } + + func menuNeedsUpdate(_ menu: NSMenu) { + menu.removeAllItems() + for item in items(for: context(), keyboard: AppSettingsManager.shared.keyboard) { + menu.addItem(item) + } + } + + /// The menu for a context, sections divided by separators. Separate from `menuNeedsUpdate` so + /// the whole shape can be read without an open menu or a window. + internal func items(for context: ToolbarContext, keyboard: KeyboardSettings) -> [NSMenuItem] { + var items: [NSMenuItem] = [] + for (index, section) in ConnectionActionsMenuResolver.sections(context).enumerated() { + if index > 0 { items.append(.separator()) } + items.append(contentsOf: section.entries.map { item(for: $0, keyboard: keyboard) }) + } + return items + } + + /// The chord an entry names is drawn from the user's own binding, so a rebind in Settings + /// reaches this menu on its next open. It is shown, not claimed: `menuHasKeyEquivalent` below + /// keeps AppKit's key-equivalent search out of this menu, so the menu bar stays the one owner. + /// + /// A submenu's row takes no action and no key equivalent, because it can draw neither. That is + /// why Import Data… is a leaf of its own beside the format list, carrying ⇧⌘I, the way File > + /// Import draws the same two rows. + private func item(for entry: ActionsMenuEntry, keyboard: KeyboardSettings) -> NSMenuItem { + switch entry.role { + case let .command(selector, shortcut): + let item = NSMenuItem(title: entry.title, action: selector, keyEquivalent: "") + item.target = nil + if let shortcut { + MenuItemFactory.apply(shortcut: shortcut, keyboard: keyboard, to: item) + } + return item + case let .submenu(kind): + let root = NSMenuItem(title: entry.title, action: nil, keyEquivalent: "") + let submenu = NSMenu(title: entry.title) + submenu.delegate = delegate(for: kind) + root.submenu = submenu + return root + } + } + + private func delegate(for kind: ActionsSubmenuKind) -> any NSMenuDelegate { + switch kind { + case .importFormats: + return importFormats + case .mode: + return modes + } + } + + func menuHasKeyEquivalent( + _ menu: NSMenu, + for event: NSEvent, + target: AutoreleasingUnsafeMutablePointer, + action: UnsafeMutablePointer + ) -> Bool { + false + } +} diff --git a/TablePro/Core/Services/Infrastructure/Toolbar/ConnectionActionsMenuResolver.swift b/TablePro/Core/Services/Infrastructure/Toolbar/ConnectionActionsMenuResolver.swift index 2d0b4b10e..c36898436 100644 --- a/TablePro/Core/Services/Infrastructure/Toolbar/ConnectionActionsMenuResolver.swift +++ b/TablePro/Core/Services/Infrastructure/Toolbar/ConnectionActionsMenuResolver.swift @@ -126,17 +126,26 @@ internal enum ConnectionActionsMenuResolver { ) ) if context.supportsImport { - /// A submenu rather than a leaf, because the menu bar's own item always takes the first - /// format and the toolbar was until now the only route to any of the others. The leaves - /// are filled when it opens. + /// The command and the format list are two rows, as they are under File > Import. The + /// leaf is the one ⇧⌘I runs and says so, and it takes the driver's first format; a row + /// that owns a submenu can carry neither the action nor the chord. The list is how any + /// other format is reached, filled when it opens. + /// + /// Gated on the driver's capability, which is a registry read, and not on the formats it + /// actually has: counting those activates every lazily loaded import plugin, and retries + /// the load gate of one that failed it, on every ask. The window's validation answers + /// the rest, dimming the leaf when there is nothing to import, and the list says so in + /// its own placeholder. entries.append( ActionsMenuEntry( title: String(localized: "Import Data…"), selector: NSSelectorFromString("importData:"), - shortcut: .importData, - submenu: .importFormats + shortcut: .importData ) ) + entries.append( + ActionsMenuEntry(title: String(localized: "Import Data From"), submenu: .importFormats) + ) } return entries.isEmpty ? nil : ActionsMenuSection(entries) } @@ -205,11 +214,7 @@ internal enum ConnectionActionsMenuResolver { private static func modeSection(_ context: ToolbarContext) -> ActionsMenuSection? { guard context.isAIEnabled else { return nil } return ActionsMenuSection([ - ActionsMenuEntry( - title: String(localized: "Mode"), - selector: NSSelectorFromString("setContentModeFromMenu:"), - submenu: .mode - ), + ActionsMenuEntry(title: String(localized: "Mode"), submenu: .mode), ]) } diff --git a/TablePro/Core/Services/Infrastructure/Toolbar/ToolbarContextResolver.swift b/TablePro/Core/Services/Infrastructure/Toolbar/ToolbarContextResolver.swift index 29bb83d84..97243aaf1 100644 --- a/TablePro/Core/Services/Infrastructure/Toolbar/ToolbarContextResolver.swift +++ b/TablePro/Core/Services/Infrastructure/Toolbar/ToolbarContextResolver.swift @@ -10,11 +10,12 @@ import AppKit /// Two questions with deliberately different inputs, and keeping them apart is what stops the /// titlebar reflowing while the user types. /// -/// `hidden` is a function of `ToolbarContext.VisibilityKey` alone: the tab kind, the results mode, -/// the content mode and the driver's capabilities. Those change on a tab switch, a mode switch or a +/// `visibility(for:)` and `commitVerb(for:)` take `ToolbarContext.VisibilityKey` or a part of it and +/// nothing else, so the type says what the shape may depend on: the tab kind, the results mode, the +/// content mode and the driver's capabilities. Those change on a tab switch, a mode switch or a /// connection switch and at no other time, which are the three moments a native app's toolbar is /// expected to change shape. `isEnabled` carries everything transient, so a staged edit, a running -/// query or a reconnect backoff dims a control and never moves one. +/// query or a reconnect backoff dims a control and never moves or relabels one. /// /// Version-free on purpose. `NSToolbarItem.isHidden` is macOS 15, and the caller is what decides /// whether to apply the set or fall back to dimming; the answer itself does not depend on the OS. @@ -23,25 +24,57 @@ import AppKit /// rule is about `DatabaseType`, which is an open string-based struct; `TabType` is closed, and a /// ninth kind must not compile without choosing what its toolbar shows. internal enum ToolbarContextResolver { - /// The identifiers this context takes out of the titlebar entirely. + /// The identifiers the app may take out of the titlebar, which is exactly the set it puts there. /// - /// Only ever names an item from the default set. An item the user dragged in from the - /// customization palette is opt-in, so it stays where they put it and dims instead; the toolbar - /// enforces that separately, and a test pins that this set never reaches past the default list. + /// Derived rather than listed, so the rule cannot drift from the toolbar: an item is hideable + /// because the app placed it, and anything else in `NSToolbar.items` was dragged in from the + /// customization palette by the user. That item is opt-in, so it stays where they put it in every + /// context and only dims. Measured on macOS 27 with two palette items inserted into a live + /// toolbar, a pass that hid through this filter wrote neither of them and moved neither. /// - /// Never names both subitems of the centred group at once: measured on macOS 27, hiding both - /// makes the group vanish while `group.isHidden` stays false, and a popover anchored on it then - /// lands at the window's centre. - internal static func hidden(_ context: ToolbarContext) -> Set { + /// The spaces and tracking separators are in it, and deliberately so. Filtering the standard + /// identifiers out by their `NSToolbar` prefix would also declare the sidebar and inspector + /// toggles unhideable, and a space only ever receives `isHidden = false`, which AppKit returns + /// from early at about 2.5ns a write. + internal static let hideableIdentifiers = Set(MainWindowToolbar.defaultItemIdentifiers) + + /// What this context takes out of the titlebar, already confined to what may be taken. + /// + /// The confinement lives here rather than at the call site, so it is a property of the answer + /// and a second caller cannot skip it. + internal static func visibility(for key: ToolbarContext.VisibilityKey) -> ToolbarVisibility { + ToolbarVisibility(hidden: contextualHidden(key).intersection(hideableIdentifiers)) + } + + /// The commit control's label: the verb its tab commits with. + /// + /// Keyed on the tab kind and never on what is staged. The staged change moves with every edit: + /// a Create Table draft is `.createTable` only while it validates, so a label read from it + /// flipped between Save Changes and Create Table as the user typed, and with labels shown each + /// flip changed the item's width and reflowed the titlebar. The kind moves on a tab switch, + /// when the item set may change anyway. Nothing staged still leaves the control dim, which is + /// `isEnabled`'s answer. + internal static func commitVerb(for tabKind: TabType?) -> String { + switch tabKind { + case .createTable: + String(localized: "Create Table") + case .usersRoles: + String(localized: "Apply Changes") + case .query, .table, .erDiagram, .serverDashboard, .insights, .objectSource, nil: + String(localized: "Save Changes") + } + } + + private static func contextualHidden(_ key: ToolbarContext.VisibilityKey) -> Set { var hidden: Set = [] /// A file-based engine has one database and it is the file already named beside it, so the /// second capsule has never been clickable on SQLite or DuckDB. - if context.isFileBased || !context.supportsContainerSwitching { + if key.isFileBased || !key.supportsContainerSwitching { hidden.insert(MainWindowToolbar.database) } - switch context.contentMode { + switch key.contentMode { case .agent: /// No grid and no object browser are on screen, and the commit control's gate is frozen /// because the browse content tree is not mounted to write it. @@ -49,13 +82,13 @@ internal enum ToolbarContextResolver { hidden.insert(MainWindowToolbar.saveChanges) return hidden case .browse: - hidden.formUnion(browseHidden(context)) + hidden.formUnion(browseHidden(key)) return hidden } } - private static func browseHidden(_ context: ToolbarContext) -> Set { - guard let tabKind = context.tabKind else { return [] } + private static func browseHidden(_ key: ToolbarContext.VisibilityKey) -> Set { + guard let tabKind = key.tabKind else { return [] } switch tabKind { case .createTable: /// A definition that is not on the server yet has nothing to reload. @@ -70,15 +103,17 @@ internal enum ToolbarContextResolver { /// Whether an item answers in this context. /// - /// Every identifier the toolbar vends has an arm. The `default:` returns false rather than true - /// because the old unconditional arm is what left Query History live and inert over a window - /// that had never connected, and left every identifier nobody had thought about enabled. + /// Every command the toolbar vends has an arm. The Back and Forward group has none because it is + /// a container: it carries no action, so AppKit never asks for it, and each of its two subitems + /// answers for itself. The `default:` returns false rather than true because the old + /// unconditional arm is what left Query History live and inert over a window that had never + /// connected, and left every identifier nobody had thought about enabled. internal static func isEnabled( _ identifier: NSToolbarItem.Identifier, context: ToolbarContext ) -> Bool { switch identifier { - case MainWindowToolbar.connection, MainWindowToolbar.connectionGroup: + case MainWindowToolbar.connection: /// Switch Connection is the window's command, so it answers before a session exists. /// It is the route back from a connection that failed. return true @@ -104,7 +139,7 @@ internal enum ToolbarContextResolver { return context.isConnected && context.canAddRow case MainWindowToolbar.restorePreviousValues: return context.isConnected && context.canRestorePreviousValues - case MainWindowToolbar.navigateBack, MainWindowToolbar.backForwardGroup: + case MainWindowToolbar.navigateBack: return context.isConnected && context.canNavigateBack case MainWindowToolbar.navigateForward: return context.isConnected && context.canNavigateForward @@ -131,12 +166,4 @@ internal enum ToolbarContextResolver { return false } } - - /// The identifiers `hidden` is allowed to name, which is the default set and nothing else. - /// Pinned by a test so a later context cannot start hiding a button the user placed. - internal static let hideableIdentifiers: Set = [ - MainWindowToolbar.database, - MainWindowToolbar.refresh, - MainWindowToolbar.saveChanges, - ] } diff --git a/TablePro/Core/Services/Infrastructure/Toolbar/ToolbarVisibility.swift b/TablePro/Core/Services/Infrastructure/Toolbar/ToolbarVisibility.swift new file mode 100644 index 000000000..607023d56 --- /dev/null +++ b/TablePro/Core/Services/Infrastructure/Toolbar/ToolbarVisibility.swift @@ -0,0 +1,28 @@ +// +// ToolbarVisibility.swift +// TablePro +// + +import AppKit + +/// Which of the connection window's toolbar items the app has taken out of the titlebar. +/// +/// The app's own record, and the only answer to "is this item on screen" anything may act on. +/// AppKit's readings of the same question are measured unsafe on macOS 27: after one visit to +/// Customize Toolbar, `NSToolbar.visibleItems` and `NSToolbarItem.isVisible` over-report for good, +/// listing 6 items against 2 laid out at a 460pt window and calling a hidden item visible, and no +/// resize, re-toggle or re-insert repaired either. The error always runs toward "on screen", which +/// is the direction that hands `NSPopover` an anchor with no window. +/// +/// Its own file so `ToolbarSwitcherPresenter` can take one without reaching into the resolver. +internal struct ToolbarVisibility: Equatable { + internal let hidden: Set + + internal init(hidden: Set = []) { + self.hidden = hidden + } + + internal func hides(_ identifier: NSToolbarItem.Identifier) -> Bool { + hidden.contains(identifier) + } +} diff --git a/TablePro/Core/Services/Infrastructure/TransportRateToolbarItem.swift b/TablePro/Core/Services/Infrastructure/TransportRateToolbarItem.swift deleted file mode 100644 index 8465aa2df..000000000 --- a/TablePro/Core/Services/Infrastructure/TransportRateToolbarItem.swift +++ /dev/null @@ -1,86 +0,0 @@ -// -// TransportRateToolbarItem.swift -// TablePro -// - -import AppKit - -/// The throughput readout, beside the centred connection group rather than inside it. -/// -/// Bare text with no capsule, which is what Xcode does with the one comparable thing it ships: -/// measured on a running Xcode, its Window Title/Activity readout draws as plain text next to the -/// Back/Forward capsule and wears no platter of its own. A capsule was tried here and it was wrong -/// twice over. `title` on a group subitem is what earns a subitem its own capsule (measured: two -/// titled subitems give two platters, three untitled ones give a single platter spanning all of -/// them, and `controlRepresentation` changes neither), so the centre became three capsules for two -/// controls and one number, and read as scattered. -/// -/// Sitting outside the group is what keeps the pair centred. A group is laid out around its own -/// midpoint, so a readout inside it pushed the connection and database capsules off centre by half -/// the readout's width. Measured at 1400pt: with the readout as a separate adjacent item the group -/// sits at x=647.0, midX=772.8, byte-identical to having no readout at all, and the text lands 6.0pt -/// past the group's trailing edge. -/// -/// The one place in this toolbar that carries a view, and the reason is that the figure has to hold -/// a constant width. A view-less item would carry it in `title`, and a title re-measures: with the -/// figure written into one and `validateVisibleItems()` called, the group went 219pt, 233pt, 232pt, -/// 251pt across `0 kB/s`, `145 kB/s`, `1.2 MB/s` and `888.8 MB/s`, walking its own midpoint 16pt. -/// A group is laid out around that midpoint, so every one of those steps slides the connection name -/// beside it, once a second. -/// -/// A view pinned to a width settles it. Measured across the same four figures, the group frame and -/// the field frame were byte-identical every time: `group.x=396.0 w=248.0`, `field.x=573.0 w=71.0`. -/// -/// It publishes no action, so AppKit never validates it and it has no menu-bar command of its own. -/// That is the cost of a readout, and it is why the item is only in the group at all for a -/// connection whose bytes the app carries. -@MainActor -internal final class TransportRateToolbarItem: NSToolbarItem { - private let field = NSTextField(labelWithString: "") - - internal init() { - super.init(itemIdentifier: Self.identifier) - let label = String(localized: "Throughput") - self.label = label - paletteLabel = label - field.font = NSFont.monospacedDigitSystemFont(ofSize: NSFont.smallSystemFontSize, weight: .regular) - field.textColor = .secondaryLabelColor - field.alignment = .center - field.lineBreakMode = .byClipping - field.setAccessibilityLabel(label) - field.translatesAutoresizingMaskIntoConstraints = false - field.widthAnchor.constraint(equalToConstant: Self.fieldWidth).isActive = true - view = field - overflowEntry.isEnabled = false - menuFormRepresentation = overflowEntry - apply(rate: nil) - } - - internal static let identifier = NSToolbarItem.Identifier("com.TablePro.toolbar.transportRate") - - /// Measured from the widest figure the label can produce rather than typed in, so a change to - /// the format cannot leave the field a few points too narrow and clip its own text. - private static let fieldWidth: CGFloat = { - let font = NSFont.monospacedDigitSystemFont(ofSize: NSFont.smallSystemFontSize, weight: .regular) - let widest = TransportRateLabel.widestCandidates - .map { ($0 as NSString).size(withAttributes: [.font: font]).width } - .max() ?? 0 - return ceil(widest) + 8 - }() - - /// The centred group is the first region AppKit sheds into the overflow menu, so the figure - /// gets an entry of its own there rather than disappearing with the controls beside it. It is - /// disabled because there is nothing to click: it reports, it does not do. - private let overflowEntry = NSMenuItem() - - internal func apply(rate: TransportRate?) { - let text = TransportRateLabel.text(for: rate) - guard text != field.stringValue else { return } - - let spoken = TransportRateLabel.accessibilityValue(for: rate) - field.stringValue = text - field.setAccessibilityValue(spoken) - overflowEntry.title = spoken - toolTip = String(format: String(localized: "%@ through this connection's transport"), spoken) - } -} diff --git a/TablePro/Core/Transport/TransportRateLabel.swift b/TablePro/Core/Transport/TransportRateLabel.swift deleted file mode 100644 index 38740ab71..000000000 --- a/TablePro/Core/Transport/TransportRateLabel.swift +++ /dev/null @@ -1,69 +0,0 @@ -// -// TransportRateLabel.swift -// TablePro -// - -import Foundation - -/// The throughput as one short line: an arrow for the busier direction and a figure. -/// -/// Only one direction is shown. Two rates side by side is more width than the centred toolbar item -/// has, and for a database connection the interesting one is whichever is moving: results coming -/// back, or an import going out. -/// -/// The figure is assembled from a number and a unit rather than handed to `.byteCount`, which -/// cannot produce a shape that holds a constant width. Measured: `allowedUnits: [.kb]` floors -/// nothing, so 512 still comes back "512 bytes", and zero comes back "Zero kB" whatever the units -/// say. Both are four characters wider than the figures around them. The unit is left unlocalized, -/// per the rule that technical terms are; the figure is localized. -/// -/// An idle transport reads `0 kB/s` rather than going blank. Worth knowing before reading much into -/// that figure: polling TablePlus's equivalent readout over an SSH tunnel gave `0 B/s` in 19 of 20 -/// idle samples, and in 37 of 45 taken during two table reloads. A quiet connection is the normal -/// case, not a broken one. -internal enum TransportRateLabel { - /// Every shape the label can take, which is what the toolbar's field measures itself against so - /// its width is settled once and never follows the figure. - internal static let widestCandidates = ["\u{2193}999 kB/s", "\u{2193}999 MB/s", "\u{2193}999 GB/s"] - - private static let bytesPerKilobyte: Double = 1_000 - - internal static func text(for rate: TransportRate?) -> String { - arrow(for: rate) + figureAndUnit(for: rate) - } - - private static func figureAndUnit(for rate: TransportRate?) -> String { - let value = rate.map { max($0.receivedPerSecond, $0.sentPerSecond) } ?? 0 - guard value.isFinite, value > 0 else { return figure(0) + " kB/s" } - - let kilobytes = value / bytesPerKilobyte - guard kilobytes >= 1 else { return figure(0) + " kB/s" } - guard kilobytes >= bytesPerKilobyte else { return figure(kilobytes) + " kB/s" } - - let megabytes = kilobytes / bytesPerKilobyte - guard megabytes >= bytesPerKilobyte else { return figure(megabytes) + " MB/s" } - return figure(megabytes / bytesPerKilobyte) + " GB/s" - } - - /// One fractional digit below ten, none above, so the figure never runs past three characters. - private static func figure(_ value: Double) -> String { - let fractionDigits = value < 10 && value > 0 ? 1 : 0 - return value.formatted(.number.precision(.fractionLength(fractionDigits)).grouping(.never)) - } - - /// Down unless the connection is sending more than it is receiving, which is what an import - /// looks like. An idle transport keeps the down arrow rather than losing a character. - private static func arrow(for rate: TransportRate?) -> String { - guard let rate, rate.sentPerSecond > rate.receivedPerSecond else { return "\u{2193}" } - return "\u{2191}" - } - - /// What VoiceOver reads instead of an arrow it would spell out as a glyph name. - internal static func accessibilityValue(for rate: TransportRate?) -> String { - let figure = figureAndUnit(for: rate) - let sending = (rate?.sentPerSecond ?? 0) > (rate?.receivedPerSecond ?? 0) - return sending - ? String(format: String(localized: "Sending %@"), figure) - : String(format: String(localized: "Receiving %@"), figure) - } -} diff --git a/TablePro/Models/UI/PendingChangeKind.swift b/TablePro/Models/UI/PendingChangeKind.swift index 3e8b5935e..23f2bf68f 100644 --- a/TablePro/Models/UI/PendingChangeKind.swift +++ b/TablePro/Models/UI/PendingChangeKind.swift @@ -5,15 +5,17 @@ import Foundation -/// What the window's Save command would commit, and the verb it says. +/// What the window's Save command would commit, which is also whether it can commit at all. /// /// One value rather than five booleans read in five places. `updateToolbarPendingState()` folded /// four of the five sources into `hasPendingChanges` and never read the fifth, so a Users & Roles /// tab with staged principals left both the toolbar's commit button and Cmd+S dim while /// `saveChanges()` already carried the branch that would have applied them. /// -/// The tab decides the verb, because two kinds can be staged at once and only one of them is the -/// one the user is looking at. +/// The tab decides the kind, because two kinds can be staged at once and only one of them is the +/// one the user is looking at. It decides nothing the user reads: the commit control's label is +/// `ToolbarContextResolver.commitVerb(for:)`, from the tab kind alone, because this value comes and +/// goes with every edit and a label that followed it moved the titlebar while the user typed. internal enum PendingChangeKind: Equatable, Hashable, Sendable { case data case structure @@ -64,15 +66,4 @@ internal enum PendingChangeKind: Equatable, Hashable, Sendable { if hasDataChanges { return .data } return isFileDirty ? .file : nil } - - internal var commitTitle: String { - switch self { - case .data, .structure, .file: - String(localized: "Save Changes") - case .createTable: - String(localized: "Create Table") - case .principals: - String(localized: "Apply Changes") - } - } } diff --git a/TablePro/Models/UI/ToolbarContext.swift b/TablePro/Models/UI/ToolbarContext.swift index 842a4b845..746b3d825 100644 --- a/TablePro/Models/UI/ToolbarContext.swift +++ b/TablePro/Models/UI/ToolbarContext.swift @@ -12,9 +12,11 @@ import Foundation /// the only lever anyone had was dimming. Every feature that arrived then had to buy a permanent /// slot in the titlebar. /// -/// Nothing global is read inside this struct or inside the resolvers that take it. It is built once -/// per change by the toolbar and passed down, so the resolvers stay pure and testable with no host -/// app and no session. +/// Nothing global is read inside this struct or inside the resolvers that take it. The toolbar +/// builds it once per validation pass and once per Actions menu open and passes it down, so the +/// resolvers stay pure and testable with no host app and no session. +/// +/// Every field is one a resolver reads. A fact nothing reads is a fact every pass pays to look up. internal struct ToolbarContext: Equatable { /// What the window's detail pane is drawing. `nil` when no tab is selected, which is a real /// state on a window that has just opened. @@ -29,7 +31,6 @@ internal struct ToolbarContext: Equatable { internal let isConnected: Bool /// A connection is on screen, whether or not it has finished connecting. internal let hasSelectedWorkspace: Bool - internal let isTrailingPaneOpen: Bool internal let canToggleTrailingPane: Bool internal let pendingChange: PendingChangeKind? @@ -50,19 +51,17 @@ internal struct ToolbarContext: Equatable { internal let supportsServerDashboard: Bool internal let isAIEnabled: Bool - internal let hasAgentSession: Bool - - /// What this engine calls the thing the centre's second capsule names, and what it calls its - /// query language. Both are words the Actions menu puts in front of the user. - internal let containerEntityName: String - internal let queryLanguageName: String - /// The subset of the context that may move an item in or out of the titlebar. + /// The subset of the context that may move an item in or out of the titlebar, or change what + /// one says. /// - /// This is the whole anti-reflow rule in one type. `isHidden` is written only from these - /// fields, so the item set can change on a tab switch, a mode switch or a connection switch and - /// on nothing else; everything transient rides `isEnabled` instead. A keystroke in a cell - /// editor therefore costs one struct comparison and writes nothing. + /// This is the whole anti-reflow rule in one type. `isHidden` and the commit control's label are + /// written only from these fields, so the titlebar can change shape on a tab switch, a mode + /// switch or a connection switch and on nothing else; everything transient rides `isEnabled` + /// instead. The toolbar computes this from its eight inputs directly rather than from a whole + /// context, so a keystroke costs the selected-tab lookup, four locked reads of the plugin + /// metadata registry, a switch over the engine for the dashboard and one comparison, and writes + /// nothing. internal struct VisibilityKey: Equatable { internal let tabKind: TabType? internal let resultsMode: ResultsViewMode? @@ -94,7 +93,6 @@ internal struct ToolbarContext: Equatable { pane: ConnectionWindowPane = .empty, isConnected: Bool = false, hasSelectedWorkspace: Bool = false, - isTrailingPaneOpen: Bool = false, canToggleTrailingPane: Bool = false, pendingChange: PendingChangeKind? = nil, hasDataPendingChanges: Bool = false, @@ -107,10 +105,7 @@ internal struct ToolbarContext: Equatable { supportsContainerSwitching: Bool = false, supportsImport: Bool = false, supportsServerDashboard: Bool = false, - isAIEnabled: Bool = false, - hasAgentSession: Bool = false, - containerEntityName: String = "", - queryLanguageName: String = "" + isAIEnabled: Bool = false ) { self.tabKind = tabKind self.resultsMode = resultsMode @@ -118,7 +113,6 @@ internal struct ToolbarContext: Equatable { self.pane = pane self.isConnected = isConnected self.hasSelectedWorkspace = hasSelectedWorkspace - self.isTrailingPaneOpen = isTrailingPaneOpen self.canToggleTrailingPane = canToggleTrailingPane self.pendingChange = pendingChange self.hasDataPendingChanges = hasDataPendingChanges @@ -132,8 +126,44 @@ internal struct ToolbarContext: Equatable { self.supportsImport = supportsImport self.supportsServerDashboard = supportsServerDashboard self.isAIEnabled = isAIEnabled - self.hasAgentSession = hasAgentSession - self.containerEntityName = containerEntityName - self.queryLanguageName = queryLanguageName + } + + /// The whole context over a key the caller already computed, so the eight slow-moving facts + /// are read once per context rather than once for the key and again for the context. + internal init( + key: VisibilityKey, + pane: ConnectionWindowPane, + isConnected: Bool, + hasSelectedWorkspace: Bool, + canToggleTrailingPane: Bool, + pendingChange: PendingChangeKind?, + hasDataPendingChanges: Bool, + blocksAllWrites: Bool, + canAddRow: Bool, + canRestorePreviousValues: Bool, + canNavigateBack: Bool, + canNavigateForward: Bool + ) { + self.init( + tabKind: key.tabKind, + resultsMode: key.resultsMode, + contentMode: key.contentMode, + pane: pane, + isConnected: isConnected, + hasSelectedWorkspace: hasSelectedWorkspace, + canToggleTrailingPane: canToggleTrailingPane, + pendingChange: pendingChange, + hasDataPendingChanges: hasDataPendingChanges, + blocksAllWrites: blocksAllWrites, + canAddRow: canAddRow, + canRestorePreviousValues: canRestorePreviousValues, + canNavigateBack: canNavigateBack, + canNavigateForward: canNavigateForward, + isFileBased: key.isFileBased, + supportsContainerSwitching: key.supportsContainerSwitching, + supportsImport: key.supportsImport, + supportsServerDashboard: key.supportsServerDashboard, + isAIEnabled: key.isAIEnabled + ) } } diff --git a/TablePro/Resources/Localizable.xcstrings b/TablePro/Resources/Localizable.xcstrings index 57ca5456c..b76f1a02c 100644 --- a/TablePro/Resources/Localizable.xcstrings +++ b/TablePro/Resources/Localizable.xcstrings @@ -3831,9 +3831,6 @@ } } } - }, - "%@ through this connection's transport" : { - }, "%@ wants to access '%@' (%@)." : { "localizations" : { @@ -12745,6 +12742,9 @@ }, "A view" : { + }, + "Actions" : { + }, "about %@ row" : { @@ -35943,6 +35943,9 @@ } } } + }, + "Commands for the current tab and connection" : { + }, "Comment Unavailable" : { @@ -63183,6 +63186,7 @@ } }, "Export & Import" : { + "extractionState" : "stale", "localizations" : { "ko" : { "stringUnit" : { @@ -76438,6 +76442,9 @@ } } } + }, + "Import Data From" : { + }, "Import data" : { "extractionState" : "stale", @@ -122087,9 +122094,6 @@ }, "Received" : { - }, - "Receiving %@" : { - }, "Recent" : { "localizations" : { @@ -122744,6 +122748,9 @@ } } } + }, + "Redis Cluster serves database 0 only, so it cannot switch databases." : { + }, "Redis driver has no TLS fallback. Preferred and Required both force TLS. Use Required for Redis Cloud, Upstash, and AWS ElastiCache encrypted endpoints." : { "localizations" : { @@ -137969,9 +137976,6 @@ } } } - }, - "Sending %@" : { - }, "Sent" : { @@ -152504,6 +152508,7 @@ } }, "Table Actions" : { + "extractionState" : "stale", "localizations" : { "ko" : { "stringUnit" : { @@ -166923,9 +166928,6 @@ } } } - }, - "Throughput" : { - }, "Throughput is measured for SSH tunnels and SOCKS proxies, the transports TablePro carries the bytes for itself." : { @@ -171964,6 +171966,40 @@ } } }, + "Unknown explain variant '%@'." : { + "localizations" : { + "ko" : { + "stringUnit" : { + "state" : "translated", + "value" : "알 수 없는 EXPLAIN 변형 '%@'입니다." + } + }, + "tr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Bilinmeyen açıklama varyantı '%@'." + } + }, + "vi" : { + "stringUnit" : { + "state" : "translated", + "value" : "Biến thể EXPLAIN không xác định '%@'." + } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "未知的 EXPLAIN 变体 '%@'。" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "未知的 EXPLAIN 變體 '%@'。" + } + } + } + }, "Unlicensed" : { "localizations" : { "ko" : { diff --git a/TablePro/Views/Compare/CompareEndpointToolbarController.swift b/TablePro/Views/Compare/CompareEndpointToolbarController.swift index d7a7a04e0..70083bc53 100644 --- a/TablePro/Views/Compare/CompareEndpointToolbarController.swift +++ b/TablePro/Views/Compare/CompareEndpointToolbarController.swift @@ -101,8 +101,10 @@ internal final class CompareEndpointToolbarController: NSObject { dismiss() return } + /// Nil, because the Compare window's toolbar has no context resolver and hides nothing. guard let identifier = identifiers[side], - let anchor = ToolbarSwitcherPresenter.anchor(in: windowProvider(), identifier) else { return } + let anchor = ToolbarSwitcherPresenter.anchor(in: windowProvider(), identifier, hiddenBy: nil) + else { return } let shown = PopoverPresenter.show( relativeTo: anchor, diff --git a/TablePro/Views/Components/PopoverPresenter.swift b/TablePro/Views/Components/PopoverPresenter.swift index a74f1a356..8b2d8d5df 100644 --- a/TablePro/Views/Components/PopoverPresenter.swift +++ b/TablePro/Views/Components/PopoverPresenter.swift @@ -34,12 +34,14 @@ enum PopoverPresenter { /// present at all once the item is clipped, and a clipped item survives only as its /// `menuFormRepresentation`. /// - /// The caller must have resolved `toolbarItem` out of a visible toolbar. AppKit throws - /// `NSInvalidArgumentException` when it cannot locate the item, which Swift cannot catch, so - /// the check belongs at the call site as a precondition rather than here as error handling. - /// `show(relativeTo: NSToolbarItem)` is macOS 14, and there is no stand-in: an item whose - /// view AppKit generates reports `view` as nil, so there is nothing to anchor on below it. - /// Callers resolve their anchor through `ToolbarSwitcherPresenter.anchor`, which answers nil + /// The caller must have resolved `toolbarItem` out of `NSToolbar.items` as a top-level item. + /// AppKit throws `NSInvalidArgumentException` when it cannot locate the item, which Swift cannot + /// catch, so the check belongs at the call site as a precondition rather than here as error + /// handling. Measured on macOS 27, the one case that raised was a subitem of a group that was + /// off screen; a clipped top-level item is supported, and anchors on the clipped-items + /// indicator. `show(relativeTo: NSToolbarItem)` is macOS 14, and there is no stand-in: an item + /// whose view AppKit generates reports `view` as nil, so there is nothing to anchor on below + /// it. Callers resolve their anchor through `ToolbarSwitcherPresenter.anchor`, which answers nil /// on macOS 13 so they take their own fallback instead. @available(macOS 14.0, *) @discardableResult diff --git a/TablePro/Views/Main/Extensions/MainContentCommandActions+Switchers.swift b/TablePro/Views/Main/Extensions/MainContentCommandActions+Switchers.swift index 9045b1b93..1bae5e457 100644 --- a/TablePro/Views/Main/Extensions/MainContentCommandActions+Switchers.swift +++ b/TablePro/Views/Main/Extensions/MainContentCommandActions+Switchers.swift @@ -57,13 +57,15 @@ internal extension MainContentCommandActions { coordinator?.switcherPresenter?.dismiss() } - /// Anchored to the Database subitem, which is the capsule the user pressed. The group is two - /// capsules wide, so anchoring to it points the chooser at the seam between them; the presenter - /// falls back to the group by itself once AppKit clips it into the overflow menu. + /// Anchored to the Database item, which is the capsule the user pressed. It is a top-level + /// centred item, so it anchors on its own capsule, measured within 2pt of where the old subitem + /// anchor landed at a 1200pt window. Clipped, AppKit anchors it on the clipped-items indicator + /// itself; hidden by the context, the presenter takes the floating panel. private func presentDatabaseSwitcher(on coordinator: MainContentCoordinator, target: ContainerSwitchTarget?) { coordinator.switcherPresenter?.present( from: coordinator.contentWindow, anchoredTo: MainWindowToolbar.database, + hiddenBy: coordinator.splitViewController?.toolbarOwner?.visibility, subject: .container(target), contentSize: DatabaseSwitcherPopover.contentSize ) { dismiss in diff --git a/TablePro/Views/Main/MainContentCommandActions.swift b/TablePro/Views/Main/MainContentCommandActions.swift index 9ff1d2040..3236b6d56 100644 --- a/TablePro/Views/Main/MainContentCommandActions.swift +++ b/TablePro/Views/Main/MainContentCommandActions.swift @@ -936,7 +936,7 @@ final class MainContentCommandActions: ObservableObject { var supportsServerDashboard: Bool { guard let type = coordinator?.connection.type else { return false } - return ServerDashboardQueryProviderFactory.provider(for: type) != nil + return ServerDashboardQueryProviderFactory.supportsDashboard(for: type) } func showUsersAndRoles() { diff --git a/TablePro/Views/Sidebar/SidebarScopeControl.swift b/TablePro/Views/Sidebar/SidebarScopeControl.swift new file mode 100644 index 000000000..7d2a4fe3b --- /dev/null +++ b/TablePro/Views/Sidebar/SidebarScopeControl.swift @@ -0,0 +1,64 @@ +// +// SidebarScopeControl.swift +// TablePro +// + +import AppKit + +/// Which list the sidebar shows, as its own row above the filter field. +/// +/// It sits over the list it switches, where Xcode keeps its navigator chooser, rather than in the +/// titlebar. In the toolbar it held two permanent hit targets in a row that also has to carry the +/// connection, its container and the content commands, and it answered for a pane the window could +/// have collapsed. +/// +/// Worded segments rather than glyphs. The toolbar version drew `list.bullet` and `star`, and with +/// no description on either image VoiceOver announced them as "List" and "favorite". Measured on +/// macOS 27, a worded control publishes a radio group whose two radio buttons carry the segments' +/// own labels, "Tables" and "Favorites", so it cannot name itself wrongly. +/// +/// The words fit in every language the app ships. Measured at the sidebar's 280pt minimum, which +/// leaves the row 260pt inside its insets, the widest is Turkish at 238pt, English needs 158pt and +/// Simplified Chinese 100pt. +@MainActor +internal final class SidebarScopeControl: NSSegmentedControl { + internal static let tabs: [SidebarTab] = [.tables, .favorites] + + internal init() { + super.init(frame: .zero) + segmentCount = Self.tabs.count + trackingMode = .selectOne + segmentDistribution = .fillEqually + controlSize = .regular + for (index, tab) in Self.tabs.enumerated() { + setLabel(Self.title(for: tab), forSegment: index) + } + setAccessibilityIdentifier("sidebar-scope") + } + + @available(*, unavailable) + required init?(coder: NSCoder) { + fatalError("SidebarScopeControl does not support NSCoder init") + } + + /// Nil draws no segment selected, which is the state a window with no connection shows. + internal var selectedTab: SidebarTab? { + get { + Self.tabs.indices.contains(selectedSegment) ? Self.tabs[selectedSegment] : nil + } + set { + let index = newValue.flatMap { Self.tabs.firstIndex(of: $0) } ?? -1 + guard selectedSegment != index else { return } + selectedSegment = index + } + } + + internal static func title(for tab: SidebarTab) -> String { + switch tab { + case .tables: + String(localized: "Tables") + case .favorites: + String(localized: "Favorites") + } + } +} diff --git a/TablePro/Views/Toolbar/ToolbarSwitcherPresenter.swift b/TablePro/Views/Toolbar/ToolbarSwitcherPresenter.swift index 31cc3c151..18c35dc06 100644 --- a/TablePro/Views/Toolbar/ToolbarSwitcherPresenter.swift +++ b/TablePro/Views/Toolbar/ToolbarSwitcherPresenter.swift @@ -20,7 +20,9 @@ import SwiftUI /// Two surfaces, chosen by whether an anchor exists: /// - The item is in a visible toolbar: an `NSPopover` anchored to it, which is the macOS idiom for /// a toolbar control that reveals a chooser. A clipped item still resolves, and AppKit presents it -/// "from another appropriate affordance in the window" itself. +/// "from another appropriate affordance in the window" itself: measured on macOS 27 at a 420pt +/// window, both centred items anchored on the clipped-items indicator, a 36pt square, and neither +/// raised. /// - No anchor: the same content in the floating panel Open Quickly already uses, which belongs to /// the window rather than to the toolbar. @MainActor @@ -60,9 +62,13 @@ internal final class ToolbarSwitcherPresenter { /// `subject` is what makes "the same command" answerable. One presenter serves the connection /// chooser and the container chooser, so an identity check on presentation alone would make /// either command close the other rather than replace it. + /// + /// `hiddenBy` is the toolbar's own record of what it took out of the titlebar, forwarded to + /// `anchor(in:_:hiddenBy:)`. internal func present( from window: NSWindow?, anchoredTo identifier: NSToolbarItem.Identifier, + hiddenBy visibility: ToolbarVisibility?, subject: Subject, contentSize: NSSize, @ViewBuilder content: (_ dismiss: @escaping () -> Void) -> some View @@ -74,7 +80,7 @@ internal final class ToolbarSwitcherPresenter { } presentedSubject = subject - if let item = Self.anchor(in: window, identifier) { + if let item = Self.anchor(in: window, identifier, hiddenBy: visibility) { /// `.transient`, not `PopoverPresenter`'s `.semitransient` default: a semitransient /// popover ignores interaction outside its own window, so moving to another window or /// another app would leave the chooser floating over a window it no longer belongs to. @@ -125,14 +131,36 @@ internal final class ToolbarSwitcherPresenter { popover = nil } + /// The toolbar item a chooser presents from, or nil for the floating panel. + /// + /// It asks AppKit nothing about what is on screen, because nothing AppKit answers is safe to + /// act on. Measured on macOS 27, one visit to Customize Toolbar leaves `NSToolbar.visibleItems` + /// and `NSToolbarItem.isVisible` over-reporting for good, the item's `view` is nil for every + /// native item, and the titlebar's view hierarchy keeps a stale run of item viewers behind. So + /// the question splits in two, and neither half is a visibility reading. + /// + /// Whether the item is reachable is the app's own record: an item the resolver hid answers nil, + /// and the chooser takes the floating panel rather than a popover anchored on nothing, which + /// AppKit would place at the centre of the window. Which instance to anchor on is + /// `NSToolbar.items`, measured correct through every palette visit, down to each item's + /// identity. An item the user removed is absent from it, and answers nil the same way. + /// + /// A clipped item is neither, and needs no answer: every item here is a top-level item, and + /// AppKit anchors a clipped top-level item on the clipped-items indicator by itself. Only a + /// subitem of a group that was off screen ever raised, and there are no subitems left to anchor + /// on. + /// + /// `hiddenBy` has no default. Nil is a real answer, a toolbar with no resolver that hides + /// nothing, and a caller has to say so rather than get it by omission. + /// /// A hidden toolbar is treated as no anchor at all. `toggleToolbarShown` only flips /// `NSToolbar.isVisible` and leaves the items in place, so the item still resolves and AppKit - /// documents nothing about what anchoring to it then does. Since the failure mode of guessing - /// wrong is an `NSInvalidArgumentException` that Swift cannot catch, this takes the branch it - /// can reason about instead of the one it would have to measure. + /// documents nothing about what anchoring to it then does. That property belongs to the toolbar + /// rather than to an item, and it is measured to read correctly after a palette visit. internal static func anchor( in window: NSWindow?, - _ identifier: NSToolbarItem.Identifier + _ identifier: NSToolbarItem.Identifier, + hiddenBy visibility: ToolbarVisibility? ) -> NSToolbarItem? { /// Anchoring a popover on a toolbar item is macOS 14, and an item whose view AppKit /// generates reports `view` as nil, so there is nothing to anchor on below it. Answering @@ -140,37 +168,7 @@ internal final class ToolbarSwitcherPresenter { /// toolbar already takes. guard #available(macOS 14.0, *) else { return nil } guard let toolbar = window?.toolbar, toolbar.isVisible else { return nil } - return anchor(identifier, in: toolbar.items, visible: toolbar.visibleItems ?? []) - } - - /// The anchor for an identifier that may name a subitem of a group rather than an item the - /// toolbar carries directly. - /// - /// The connection and the container are two subitems of one centred native group, and anchoring - /// both choosers to the group put each of them on the seam between the two capsules rather than - /// under the one it belongs to. Measured on a 1200pt window: the group's midpoint is 600.0, the - /// Connection capsule's is 543.2 and the Container capsule's is 671.8, and a popover anchored to - /// the group lands at 600.0 for both. A subitem does resolve as an anchor and lands on its own - /// capsule to within a point, even though `NSToolbar.items` lists groups only and a native - /// group's subitems carry no `view`. - /// - /// It resolves only while the group is on screen. Once AppKit clips the group into the overflow - /// menu its subitems have no view and `NSPopover.show(relativeTo:)` raises - /// `NSInvalidArgumentException` ("view has no window"), which Swift cannot catch; measured, that - /// is exactly the width at which `visibleItems` stops naming the group. The group keeps working - /// there, because AppKit presents a clipped item from another affordance in the window itself, - /// so an overflowed group is the fallback rather than the floating panel. - internal static func anchor( - _ identifier: NSToolbarItem.Identifier, - in items: [NSToolbarItem], - visible: [NSToolbarItem] - ) -> NSToolbarItem? { - if let item = items.first(where: { $0.itemIdentifier == identifier }) { return item } - let groups = items.compactMap { $0 as? NSToolbarItemGroup } - guard let group = groups.first(where: { group in - group.subitems.contains { $0.itemIdentifier == identifier } - }) else { return nil } - guard visible.contains(where: { $0.itemIdentifier == group.itemIdentifier }) else { return group } - return group.subitems.first { $0.itemIdentifier == identifier } + guard visibility?.hides(identifier) != true else { return nil } + return toolbar.items.first { $0.itemIdentifier == identifier } } } diff --git a/TableProTests/Core/ServerDashboard/ServerDashboardQueryProviderFactoryTests.swift b/TableProTests/Core/ServerDashboard/ServerDashboardQueryProviderFactoryTests.swift new file mode 100644 index 000000000..8eca4803b --- /dev/null +++ b/TableProTests/Core/ServerDashboard/ServerDashboardQueryProviderFactoryTests.swift @@ -0,0 +1,40 @@ +// +// ServerDashboardQueryProviderFactoryTests.swift +// TableProTests +// + +@testable import TablePro +import Testing + +/// The support question is asked on every toolbar and menu validation pass, so it is answered +/// without building a provider. It has to give the answer building one would. +@Suite("Server dashboard provider factory") +@MainActor +struct ServerDashboardQueryProviderFactoryTests { + @Test("Support is answered without a provider, and agrees with building one for every known engine") + func supportAgreesWithTheProvider() { + let types = DatabaseType.allKnownTypes + #expect(types.count > 10, "Only \(types.count) known types; the walk would pass vacuously") + for type in types { + #expect( + ServerDashboardQueryProviderFactory.supportsDashboard(for: type) + == (ServerDashboardQueryProviderFactory.provider(for: type) != nil), + "\(type.rawValue)" + ) + } + } + + @Test("An engine no plugin has heard of has no dashboard") + func unknownEngineHasNone() { + let unknown = DatabaseType(rawValue: "com.example.not-a-database") + #expect(!ServerDashboardQueryProviderFactory.supportsDashboard(for: unknown)) + #expect(ServerDashboardQueryProviderFactory.provider(for: unknown) == nil) + } + + @Test("PostgreSQL, MySQL and SQLite each have one") + func knownEnginesHaveOne() { + for type in [DatabaseType.postgresql, .mysql, .mariadb, .sqlite, .redshift, .cockroachdb] { + #expect(ServerDashboardQueryProviderFactory.supportsDashboard(for: type), "\(type.rawValue)") + } + } +} diff --git a/TableProTests/Core/Services/Infrastructure/ConnectionActionsMenuDelegateTests.swift b/TableProTests/Core/Services/Infrastructure/ConnectionActionsMenuDelegateTests.swift new file mode 100644 index 000000000..b831a4ca3 --- /dev/null +++ b/TableProTests/Core/Services/Infrastructure/ConnectionActionsMenuDelegateTests.swift @@ -0,0 +1,204 @@ +// +// ConnectionActionsMenuDelegateTests.swift +// TableProTests +// + +import AppKit +@testable import TablePro +import Testing + +/// Stands in for the toolbar the delegate asks, so a test can move the context between two opens. +@MainActor +private final class ContextSource { + var context: ToolbarContext + + init(_ context: ToolbarContext) { + self.context = context + } +} + +/// What the Actions pull-down draws for a context. The resolver decides the entries and is pinned by +/// its own suite; this pins how they become menu items, which is where a target, a missing +/// `representedObject` or a lost chord would break the menu without the resolver noticing. +@Suite("Connection actions menu delegate") +@MainActor +struct ConnectionActionsMenuDelegateTests { + private static func context( + tabKind: TabType? = .table, + contentMode: ConnectionWorkspaceContentMode = .browse, + isConnected: Bool = true + ) -> ToolbarContext { + ToolbarContext( + tabKind: tabKind, + resultsMode: .data, + contentMode: contentMode, + pane: isConnected ? .content : .unavailable(.notConnected), + isConnected: isConnected, + hasSelectedWorkspace: true, + supportsImport: true, + supportsServerDashboard: true, + isAIEnabled: true + ) + } + + private static func makeDelegate(for context: ToolbarContext) -> ConnectionActionsMenuDelegate { + ConnectionActionsMenuDelegate(importFormats: ImportFormatMenuDelegate(), context: { context }) + } + + /// The submenus' delegates are held by the Actions delegate and `NSMenu.delegate` is weak, so a + /// case that reads them keeps the Actions delegate alive for as long as it reads, the way the + /// toolbar does for the life of the window. + private static func withItems( + for context: ToolbarContext, + keyboard: KeyboardSettings = KeyboardSettings(), + _ body: ([NSMenuItem]) throws -> Void + ) rethrows { + let delegate = makeDelegate(for: context) + try withExtendedLifetime(delegate) { + try body(delegate.items(for: context, keyboard: keyboard)) + } + } + + /// With a target, a command is validated by that object instead of the responder chain, and the + /// toolbar's own `validateMenuItem` answers true for every action it did not build. A submenu's + /// own row is AppKit's to wire, and it targets the submenu. + @Test("No command carries a target, so the window validates every one") + func entriesCarryNoTarget() { + for contentMode in ConnectionWorkspaceContentMode.allCases { + Self.withItems(for: Self.context(contentMode: contentMode)) { items in + for item in items where !item.isSeparatorItem { + if let submenu = item.submenu { + #expect(item.target === submenu, "\(item.title)") + } else { + #expect(item.target == nil, "\(item.title)") + } + } + } + } + } + + @Test("Sections are divided by separators, and nothing else is") + func sectionsAreDividedBySeparators() { + let context = Self.context() + let sections = ConnectionActionsMenuResolver.sections(context) + let items = Self.makeDelegate(for: context).items(for: context, keyboard: KeyboardSettings()) + + #expect(items.filter(\.isSeparatorItem).count == sections.count - 1) + #expect(items.count == sections.reduce(0) { $0 + $1.entries.count } + sections.count - 1) + #expect(items.first?.isSeparatorItem == false) + #expect(items.last?.isSeparatorItem == false) + } + + /// The chord comes from the user's own binding, the way the menu bar's does, so a rebind in + /// Settings reaches this menu on its next open. + @Test("An entry with a shortcut shows the user's binding") + func shortcutsFollowTheBinding() throws { + var keyboard = KeyboardSettings() + keyboard.setShortcut(.character("j", command: true, control: true), for: .addRow) + let context = Self.context() + let items = Self.makeDelegate(for: context).items(for: context, keyboard: keyboard) + let addRow = try #require(items.first { $0.action == NSSelectorFromString("addRow:") }) + + #expect(addRow.keyEquivalent == "j") + #expect(addRow.keyEquivalentModifierMask == [.command, .control]) + } + + /// AppKit ignores a key equivalent on an item that owns a submenu, so the root carries none, + /// and the leaves are filled when it opens. + @Test("The import formats and the modes open submenus their delegates fill") + func submenusHaveDelegates() throws { + try Self.withItems(for: Self.context()) { items in + let importRoot = try #require(items.first { $0.title == String(localized: "Import Data From") }) + let modeRoot = try #require(items.first { $0.title == String(localized: "Mode") }) + + for root in [importRoot, modeRoot] { + #expect(root.keyEquivalent.isEmpty) + #expect(root.submenu?.delegate != nil) + } + #expect(importRoot.submenu?.delegate is ImportFormatMenuDelegate) + #expect(modeRoot.submenu?.delegate is ContentModeMenuDelegate) + } + } + + /// The command ⇧⌘I runs is drawn as a leaf the responder chain validates, so the window dims it + /// when there is nothing to import, and it shows the user's own chord for it. As a submenu's row + /// it could show neither. + @Test("Import Data… is a leaf that reaches the window and shows its binding") + func importDataIsAPlainLeaf() throws { + var keyboard = KeyboardSettings() + keyboard.setShortcut(.character("u", command: true, control: true), for: .importData) + try Self.withItems(for: Self.context(), keyboard: keyboard) { items in + let leaf = try #require(items.first { $0.title == String(localized: "Import Data…") }) + + #expect(leaf.submenu == nil) + #expect(leaf.action == #selector(MainSplitViewController.importData(_:))) + #expect(leaf.target == nil) + #expect(leaf.keyEquivalent == "u") + #expect(leaf.keyEquivalentModifierMask == [.command, .control]) + } + } + + /// The format list's row cannot be dimmed through the responder chain, so an empty list says why + /// it is empty rather than opening as a blank sliver. + @Test("An import list with nothing to offer says so") + func emptyImportListSaysSo() throws { + let menu = NSMenu() + menu.addItem(ImportFormatMenuDelegate.item(for: ImportFormatOption(id: "stale", name: "Stale"))) + + ImportFormatMenuDelegate().menuNeedsUpdate(menu) + + let placeholder = try #require(menu.items.first) + #expect(menu.items.count == 1) + #expect(placeholder.title == String(localized: "None Available")) + #expect(placeholder.action == nil) + #expect(placeholder.isEnabled == false) + } + + /// `setContentModeFromMenu(_:)` reads the mode out of `representedObject` and does nothing + /// without it, and the window's validation reads the same value to tick the current mode. + @Test("Each mode entry names its mode, for the action and for the checkmark") + func modeEntriesNameTheirMode() { + let menu = NSMenu() + ContentModeMenuDelegate().menuNeedsUpdate(menu) + + #expect(menu.items.count == ConnectionWorkspaceContentMode.allCases.count) + for (item, mode) in zip(menu.items, ConnectionWorkspaceContentMode.allCases) { + #expect(item.title == mode.localizedTitle) + #expect(item.representedObject as? String == mode.rawValue) + #expect(item.action == #selector(MainSplitViewController.setContentModeFromMenu(_:))) + #expect(item.target == nil) + } + } + + /// The parent row already says Import Data From, so the leaf is the format alone. Under that + /// parent the sidebar's "From CSV…" would read twice. + @Test("An import format entry names its format and reaches the window") + func importFormatEntryNamesItsFormat() { + let item = ImportFormatMenuDelegate.item(for: ImportFormatOption(id: "csv", name: "CSV")) + + #expect(item.title == "CSV\u{2026}") + #expect(item.representedObject as? String == "csv") + #expect(item.action == #selector(MainSplitViewController.importDataFormat(_:))) + #expect(item.target == nil) + } + + /// Filled on open from the context the toolbar is pointed at when it opens, so a menu opened + /// once cannot go on describing a tab the window has left. + @Test("The menu is rebuilt from the current context each time it opens") + func menuFollowsTheContextOnEachOpen() { + let source = ContextSource(Self.context(tabKind: .table)) + let delegate = ConnectionActionsMenuDelegate( + importFormats: ImportFormatMenuDelegate(), + context: { source.context } + ) + let menu = NSMenu() + + delegate.menuNeedsUpdate(menu) + #expect(menu.items.contains { $0.action == NSSelectorFromString("addRow:") }) + + source.context = Self.context(tabKind: .query) + delegate.menuNeedsUpdate(menu) + #expect(!menu.items.contains { $0.action == NSSelectorFromString("addRow:") }) + #expect(menu.items.contains { $0.action == NSSelectorFromString("toggleResults:") }) + } +} diff --git a/TableProTests/Core/Services/Infrastructure/ConnectionActionsMenuResolverTests.swift b/TableProTests/Core/Services/Infrastructure/ConnectionActionsMenuResolverTests.swift index 578e0bfa8..3ba3a9d68 100644 --- a/TableProTests/Core/Services/Infrastructure/ConnectionActionsMenuResolverTests.swift +++ b/TableProTests/Core/Services/Infrastructure/ConnectionActionsMenuResolverTests.swift @@ -27,6 +27,7 @@ struct ConnectionActionsMenuResolverTests { String(localized: "Export Results…"), String(localized: "Export Tables…"), String(localized: "Import Data…"), + String(localized: "Import Data From"), String(localized: "Show DDL"), String(localized: "Copy DDL"), String(localized: "Show Query History"), @@ -88,6 +89,29 @@ struct ConnectionActionsMenuResolverTests { } } + /// The list above is only a claim until the menu bar is asked. Every title in it has to be one the + /// built menu bar draws, a submenu's own row included, or a pull-down entry could name a twin + /// that does not exist and the rule would pass on a typo. + @Test("Every twin the rule names is in the built menu bar") + @MainActor + func menuBarTitlesAreInTheMenuBar() { + var drawn: Set = [] + collectTitles(from: MainMenuBuilder.build(keyboard: KeyboardSettings()), into: &drawn) + + #expect(drawn.count > 50, "Only \(drawn.count) titles collected; the walk missed the menus") + for title in Self.menuBarTitles { + #expect(drawn.contains(title), "\(title) is named as a twin but the menu bar has no such item") + } + } + + @MainActor + private func collectTitles(from menu: NSMenu, into titles: inout Set) { + for item in menu.items where !item.isSeparatorItem { + titles.insert(item.title) + if let submenu = item.submenu { collectTitles(from: submenu, into: &titles) } + } + } + /// A section is a run drawn between two separators. Past about six entries a run stops reading /// as a group and becomes a list, which is what the pull-down exists to avoid. @Test("No section runs longer than six entries") @@ -136,13 +160,52 @@ struct ConnectionActionsMenuResolverTests { // MARK: - Capability gates - @Test("The import submenu is offered only by an engine that has one") - func importSubmenuFollowsTheDriver() { + @Test("Import is offered only by an engine that has it") + func importFollowsTheDriver() { let withImport = Self.entries(Self.context(supportsImport: true)) let without = Self.entries(Self.context(supportsImport: false)) #expect(withImport.contains { $0.submenu == .importFormats }) + #expect(withImport.contains { $0.selector == NSSelectorFromString("importData:") }) #expect(without.contains { $0.submenu == .importFormats } == false) + #expect(without.contains { $0.selector == NSSelectorFromString("importData:") } == false) + } + + /// The command ⇧⌘I runs is a leaf that says so, and the format list is a row of its own. A row + /// that owns a submenu can carry neither an action nor a chord, so folding the two into one row + /// drew a submenu with no command and no shortcut, and File > Import draws the same two rows. + @Test("Import Data… is a plain leaf beside a list of formats, the way File > Import draws them") + func importIsALeafBesideTheFormatList() throws { + let entries = Self.entries(Self.context(supportsImport: true)) + let leaf = try #require(entries.first { $0.title == String(localized: "Import Data…") }) + let list = try #require(entries.first { $0.submenu == .importFormats }) + + #expect(leaf.selector == NSSelectorFromString("importData:")) + #expect(leaf.shortcut == .importData) + #expect(leaf.submenu == nil) + #expect(list.title == String(localized: "Import Data From")) + #expect(list.title.hasSuffix("…") == false, "A submenu's row opens a menu, not a dialog, so it takes no ellipsis") + let leafIndex = try #require(entries.firstIndex(of: leaf)) + #expect(entries.indices.contains(leafIndex + 1)) + #expect(entries[leafIndex + 1] == list, "The format list sits right under the command it refines") + } + + /// A submenu's row is wired by AppKit to its submenu the moment one is assigned, and AppKit + /// ignores a key equivalent on it, so a selector or a chord declared there is a promise the menu + /// never keeps. + @Test("No submenu row declares a selector or a shortcut") + func submenuRowsDeclareNoCommand() { + for tabKind in Self.tabKinds + [nil] { + for contentMode in ConnectionWorkspaceContentMode.allCases { + for isConnected in [true, false] { + let context = Self.context(tabKind: tabKind, contentMode: contentMode, isConnected: isConnected) + for entry in Self.entries(context) where entry.submenu != nil { + #expect(entry.selector == nil, "\(entry.title)") + #expect(entry.shortcut == nil, "\(entry.title)") + } + } + } + } } @Test("Server Dashboard is offered only by an engine that has one") @@ -251,13 +314,16 @@ struct ConnectionActionsMenuResolverTests { #expect(reconnect?.selector == NSSelectorFromString("retryConnection")) } - @Test("Every entry carries a selector and a title") + @Test("Every entry is a titled command with a selector, or a titled submenu row") func everyEntryIsComplete() { for tabKind in Self.tabKinds + [nil] { for contentMode in ConnectionWorkspaceContentMode.allCases { for entry in Self.entries(Self.context(tabKind: tabKind, contentMode: contentMode)) { #expect(entry.title.isEmpty == false) - #expect(NSStringFromSelector(entry.selector).isEmpty == false) + #expect((entry.selector == nil) == (entry.submenu != nil), "\(entry.title)") + if let selector = entry.selector { + #expect(NSStringFromSelector(selector).isEmpty == false) + } } } } diff --git a/TableProTests/Core/Services/Infrastructure/ConnectionWindowChromeTests.swift b/TableProTests/Core/Services/Infrastructure/ConnectionWindowChromeTests.swift index 72615a379..438e02222 100644 --- a/TableProTests/Core/Services/Infrastructure/ConnectionWindowChromeTests.swift +++ b/TableProTests/Core/Services/Infrastructure/ConnectionWindowChromeTests.swift @@ -91,10 +91,10 @@ struct ConnectionWindowChromeTests { harness.controller.transition(to: .connecting, for: harness.selected.connectionId) let identifiers = try #require(harness.window.toolbar).items.map(\.itemIdentifier) - /// `connection` itself is a subitem of the centred group, so the group is the identifier a - /// toolbar reports. Both of these are the window's own commands and answer with no subject. - #expect(identifiers.contains(MainWindowToolbar.connectionGroup)) - #expect(identifiers.contains(MainWindowToolbar.sidebarToggle)) + /// Both of these are the window's own commands and answer with no subject: Switch + /// Connection, and AppKit's sidebar toggle, which the window validates in every phase. + #expect(identifiers.contains(MainWindowToolbar.connection)) + #expect(identifiers.contains(.toggleSidebar)) #expect(harness.controller.commandActions == nil) } @@ -116,13 +116,49 @@ struct ConnectionWindowChromeTests { #expect(!before.isEmpty) } - /// Switch Connection reaches the window itself, so it needs no subject. The toolbar's sidebar - /// item is the Tables/Favorites segmented control and does need one, however window-owned the - /// sidebar is: the tab it selects is per-connection state. - @Test("Switch Connection answers with no coordinator and the sidebar segment does not") + /// Switch Connection reaches the window itself, so it needs no subject, and the toolbar's own + /// validation says so for a window whose connection has not come up. + @Test("Switch Connection answers with no coordinator behind the toolbar") func windowScopedToolbarItemsAnswerWithoutASubject() throws { - #expect(MainWindowToolbar.isWindowScoped(MainWindowToolbar.connection)) - #expect(!MainWindowToolbar.isWindowScoped(MainWindowToolbar.sidebarToggle)) + let harness = try Harness() + defer { harness.tearDown() } + + harness.controller.transition(to: .unavailable(.notConnected), for: harness.selected.connectionId) + #expect(harness.controller.commandActions == nil) + + let owner = try #require(harness.controller.toolbarOwner) + #expect(owner.validateToolbarItem(NSToolbarItem(itemIdentifier: MainWindowToolbar.connection))) + #expect(!owner.validateToolbarItem(NSToolbarItem(itemIdentifier: MainWindowToolbar.refresh))) + } + + /// The toolbar's shape is the selected workspace's, and a workspace with no session has no + /// coordinator. A switch between two of them is a repoint from nothing to nothing, which returns + /// before it reaches the toolbar, so the outgoing connection's hidden set stayed on screen: a + /// down SQLite connection took the container capsule away from the down PostgreSQL one selected + /// after it, until that one came up. + @available(macOS 15.0, *) + @Test("The toolbar's shape follows a switch between two connections with no session") + func toolbarShapeFollowsACoordinatorlessSwitch() throws { + let harness = try Harness(selectedType: .sqlite, siblingType: .postgresql) + defer { harness.tearDown() } + + let owner = try #require(harness.controller.toolbarOwner) + #expect(harness.controller.commandActions == nil) + #expect(owner.coordinator == nil) + #expect( + owner.visibility.hides(MainWindowToolbar.database), + "A file-based engine has no container to switch, so the capsule starts hidden" + ) + + harness.controller.workspaces.select(harness.sibling.connectionId) + #expect(owner.coordinator == nil, "Neither workspace has a coordinator, which is the case under test") + #expect( + !owner.visibility.hides(MainWindowToolbar.database), + "PostgreSQL switches databases, so its capsule has to come back" + ) + + harness.controller.workspaces.select(harness.selected.connectionId) + #expect(owner.visibility.hides(MainWindowToolbar.database), "And go again on the way back") } /// The sidebar is the window's and stands in every phase, so its command answers in every @@ -245,8 +281,10 @@ struct ConnectionWindowChromeTests { let harness = try Harness() defer { harness.tearDown() } - #expect(harness.controller.switcherPresenter === harness.controller.switcherPresenter) - #expect(harness.controller.quickSwitcherPanel === harness.controller.quickSwitcherPanel) + let presenter = harness.controller.switcherPresenter + let panel = harness.controller.quickSwitcherPanel + #expect(harness.controller.switcherPresenter === presenter) + #expect(harness.controller.quickSwitcherPanel === panel) } /// The connections strip and the View menu reach a window's other connections without asking a @@ -281,14 +319,17 @@ struct ConnectionWindowChromeTests { private struct Harness { let controller: MainSplitViewController let selected: ConnectionWorkspace + let sibling: ConnectionWorkspace let window: NSWindow - private let sibling: ConnectionWorkspace private let connection: DatabaseConnection - init() throws { - connection = TestFixtures.makeConnection(name: "Selected") + init(selectedType: DatabaseType = .mysql, siblingType: DatabaseType = .mysql) throws { + connection = TestFixtures.makeConnection(name: "Selected", type: selectedType) selected = Self.makeWorkspace(connection: connection, phase: .connected) - sibling = Self.makeWorkspace(connection: TestFixtures.makeConnection(name: "Sibling"), phase: .connected) + sibling = Self.makeWorkspace( + connection: TestFixtures.makeConnection(name: "Sibling", type: siblingType), + phase: .connected + ) controller = MainSplitViewController(payload: nil, sessionState: nil, adopting: selected) controller.workspaces.insert(sibling, select: false) diff --git a/TableProTests/Core/Services/Infrastructure/ContentModeTests.swift b/TableProTests/Core/Services/Infrastructure/ContentModeTests.swift index 931c077c0..ce60c32b2 100644 --- a/TableProTests/Core/Services/Infrastructure/ContentModeTests.swift +++ b/TableProTests/Core/Services/Infrastructure/ContentModeTests.swift @@ -69,58 +69,12 @@ struct ContentModeTests { #expect(TrailingPaneSurface.resolved(.assistant, isAIEnabled: true) == .assistant) } - // MARK: - The toolbar control - - /// Measured on macOS 27: an expanded `selectOne` group publishes a radio group whose buttons - /// take their name from each image's `accessibilityDescription`, never from `labels:`. With nil - /// the sidebar control announced its SF Symbol names, "List" and "favorite". - @Test("Every toolbar segment names itself for assistive clients") - func segmentsAreNamed() { - let mode = MainWindowToolbar.makeContentModeGroup(target: nil, action: #selector(NSResponder.selectAll(_:))) - let sidebar = MainWindowToolbar.makeSidebarSegmentGroup(target: nil, action: #selector(NSResponder.selectAll(_:))) - - for group in [mode, sidebar] { - for subitem in group.subitems { - #expect(subitem.image?.accessibilityDescription?.isEmpty == false) - } - } - } - - /// The overflow menu sends an `NSMenuItem`, and reading `selectedIndex` off whatever arrived - /// meant choosing a mode from the overflow did nothing at all. - @Test("A segment action resolves its index from either sender") - func segmentIndexAcceptsBothSenders() { - let group = MainWindowToolbar.makeContentModeGroup(target: nil, action: #selector(NSResponder.selectAll(_:))) - group.selectedIndex = 1 - - let fromGroup = MainWindowToolbar.segmentIndex(from: group, group: group) - #expect(fromGroup == 1) - - let menuItem = NSMenuItem() - menuItem.tag = 0 - #expect(MainWindowToolbar.segmentIndex(from: menuItem, group: group) == 0) - - #expect(MainWindowToolbar.segmentIndex(from: nil, group: group) == 1) - } - - @Test("The mode control owns an overflow menu with one item per mode") - func menuFormHasEveryMode() throws { - let group = MainWindowToolbar.makeContentModeGroup(target: nil, action: #selector(NSResponder.selectAll(_:))) - let submenu = try #require(group.menuFormRepresentation?.submenu) - - #expect(submenu.items.count == ConnectionWorkspaceContentMode.allCases.count) - for (index, item) in submenu.items.enumerated() { - #expect(item.tag == index) - #expect(item.title == ConnectionWorkspaceContentMode.allCases[index].localizedTitle) - } - } - - /// `isNavigational` lets AppKit lift an item out of its declared slot and pin it to the leading - /// edge, which is what put the sidebar control past the sidebar divider. - @Test("The mode control stays in the slot it was given") - func modeControlIsNotNavigational() { - let group = MainWindowToolbar.makeContentModeGroup(target: nil, action: #selector(NSResponder.selectAll(_:))) - #expect(group.isNavigational == false) - #expect(group.selectionMode == NSToolbarItemGroup.SelectionMode.selectOne) + /// A mode switch is one of the three moments the titlebar may change shape, so it has to reach + /// the key the toolbar compares before it writes anything. + @Test("A mode switch changes the titlebar's visibility key") + func modeSwitchChangesTheVisibilityKey() { + let browse = ToolbarContext(tabKind: .table, contentMode: .browse, isAIEnabled: true) + let agent = ToolbarContext(tabKind: .table, contentMode: .agent, isAIEnabled: true) + #expect(browse.visibilityKey != agent.visibilityKey) } } diff --git a/TableProTests/Core/Services/Infrastructure/MenuValidationCoverageTests.swift b/TableProTests/Core/Services/Infrastructure/MenuValidationCoverageTests.swift index e88bcbe4e..311f5090e 100644 --- a/TableProTests/Core/Services/Infrastructure/MenuValidationCoverageTests.swift +++ b/TableProTests/Core/Services/Infrastructure/MenuValidationCoverageTests.swift @@ -88,6 +88,52 @@ struct MenuValidationCoverageTests { } } + /// The toolbar's Actions pull-down carries no target, so each entry reaches the window's + /// controller through the responder chain and is validated there, the way the menu bar's own + /// commands are. An entry the controller does not implement reaches nothing and AppKit draws it + /// disabled, and one it implements with no arm stays lit over a window that cannot run it. + /// Every context the resolver can be asked about is walked, and so are the leaves its two + /// submenus fill when they open. A submenu's own row carries no selector, so it adds none. + @Test("Every Actions entry reaches the window and is decided there") + func everyActionsEntryIsAnsweredAndDecided() { + let tabKinds: [TabType?] = [ + .query, .table, .createTable, .erDiagram, .serverDashboard, .usersRoles, .insights, .objectSource, nil, + ] + var selectors: Set = [ImportFormatMenuDelegate.action, ContentModeMenuDelegate.action] + for tabKind in tabKinds { + for contentMode in ConnectionWorkspaceContentMode.allCases { + for isConnected in [true, false] { + let context = ToolbarContext( + tabKind: tabKind, + resultsMode: .data, + contentMode: contentMode, + pane: isConnected ? .content : .unavailable(.notConnected), + isConnected: isConnected, + hasSelectedWorkspace: true, + supportsImport: true, + supportsServerDashboard: true, + isAIEnabled: true + ) + for section in ConnectionActionsMenuResolver.sections(context) { + selectors.formUnion(section.entries.compactMap(\.selector)) + } + } + } + } + + let unanswered = selectors + .filter { !MainSplitViewController.instancesRespond(to: $0) } + .map(NSStringFromSelector) + let undecided = selectors + .filter { !liveValidatedSelectors.contains($0) } + .filter { MainSplitViewController.resolvedEnablement($0, context: MenuValidationContext()) == nil } + .map(NSStringFromSelector) + + #expect(selectors.count > 20, "Only \(selectors.count) selectors collected; the walk missed contexts") + #expect(unanswered.isEmpty, "The window does not implement these, so AppKit draws them dead: \(unanswered)") + #expect(undecided.isEmpty, "No arm in resolvedEnablement, so these stay lit: \(undecided)") + } + /// The fall-through still has to stand for everything the window does not own, or the system's /// own items would arrive disabled. @Test("A command the window does not own is left alone") diff --git a/TableProTests/Core/Services/Infrastructure/ToolbarContextResolverTests.swift b/TableProTests/Core/Services/Infrastructure/ToolbarContextResolverTests.swift index 2e2f716b2..8c9556db4 100644 --- a/TableProTests/Core/Services/Infrastructure/ToolbarContextResolverTests.swift +++ b/TableProTests/Core/Services/Infrastructure/ToolbarContextResolverTests.swift @@ -56,9 +56,38 @@ struct ToolbarContextResolverTests { ) } + private static func hidden(_ context: ToolbarContext) -> Set { + ToolbarContextResolver.visibility(for: context.visibilityKey).hidden + } + + /// Every context the window can reach, as far as the item set is concerned. + private static var everyContext: [ToolbarContext] { + var contexts: [ToolbarContext] = [] + for tabKind in tabKinds + [nil] { + for contentMode in ConnectionWorkspaceContentMode.allCases { + for pane in panes { + for isFileBased in [true, false] { + for supportsContainerSwitching in [true, false] { + contexts.append( + context( + tabKind: tabKind, + contentMode: contentMode, + pane: pane, + isFileBased: isFileBased, + supportsContainerSwitching: supportsContainerSwitching + ) + ) + } + } + } + } + } + return contexts + } + private static func visibleCount(_ context: ToolbarContext) -> Int { - let hidden = ToolbarContextResolver.hidden(context) - return defaultHitTargets.filter { !hidden.contains($0) }.count + let hiddenSet = Self.hidden(context) + return defaultHitTargets.filter { !hiddenSet.contains($0) }.count } // MARK: - The ceiling @@ -101,28 +130,42 @@ struct ToolbarContextResolverTests { /// put it and dims. Only the default set may be taken off screen. @Test("The hidden set never reaches past the default set") func hiddenStaysInsideTheDefaultSet() { - for tabKind in Self.tabKinds + [nil] { - for contentMode in ConnectionWorkspaceContentMode.allCases { - for isFileBased in [true, false] { - for supportsContainerSwitching in [true, false] { - let hidden = ToolbarContextResolver.hidden( - Self.context( - tabKind: tabKind, - contentMode: contentMode, - isFileBased: isFileBased, - supportsContainerSwitching: supportsContainerSwitching - ) - ) - #expect(hidden.isSubset(of: ToolbarContextResolver.hideableIdentifiers)) - } - } - } + for context in Self.everyContext { + #expect(Self.hidden(context).isSubset(of: ToolbarContextResolver.hideableIdentifiers)) + } + } + + /// The other half of the same rule, stated against the list a user actually drags from. With + /// the intersection inside the resolver this holds for any context a later change invents, + /// not only for the three items the contexts name today. + @Test("An item only the palette offers is never hidden") + func paletteOnlyItemsAreNeverHidden() { + let paletteOnly = Set(MainWindowToolbar.allowedItemIdentifiers) + .subtracting(MainWindowToolbar.defaultItemIdentifiers) + #expect(!paletteOnly.isEmpty) + for context in Self.everyContext { + #expect(Self.hidden(context).isDisjoint(with: paletteOnly)) + } + } + + /// Spaces and tracking separators are in the hideable set, because it is the default list and + /// filtering them out by prefix would take the two pane toggles with them. A context has no + /// reason to name one, and hiding a tracking separator would unhook the titlebar from a pane. + @Test("No context hides a space or a tracking separator") + func spacesAreNeverHidden() { + var spaces: Set = [.flexibleSpace, .space, .sidebarTrackingSeparator] + if #available(macOS 14.0, *) { + spaces.insert(.inspectorTrackingSeparator) + } + for context in Self.everyContext { + #expect(Self.hidden(context).isDisjoint(with: spaces)) } } /// The window's own identity, the pull-down that carries everything displaced, the control that /// says whether a keystroke can reach a live table, and the two pane toggles. None of these has - /// a context in which it means nothing. + /// a context in which it means nothing, and the connection capsule is also what Switch + /// Connection presents from. @Test("The permanent controls are never hidden") func permanentControlsAreNeverHidden() { let permanent: Set = [ @@ -132,33 +175,8 @@ struct ToolbarContextResolverTests { MainWindowToolbar.safeMode, MainWindowToolbar.inspector, ] - for tabKind in Self.tabKinds + [nil] { - for contentMode in ConnectionWorkspaceContentMode.allCases { - for pane in Self.panes { - let hidden = ToolbarContextResolver.hidden( - Self.context(tabKind: tabKind, contentMode: contentMode, pane: pane) - ) - #expect(hidden.isDisjoint(with: permanent)) - } - } - } - } - - /// Measured on macOS 27: hiding both subitems makes the group vanish while `group.isHidden` - /// stays false, and a popover anchored on it then opens at the window's centre. - @Test("The centred group never loses both of its capsules") - func centredGroupKeepsACapsule() { - for tabKind in Self.tabKinds + [nil] { - for contentMode in ConnectionWorkspaceContentMode.allCases { - for isFileBased in [true, false] { - let hidden = ToolbarContextResolver.hidden( - Self.context(tabKind: tabKind, contentMode: contentMode, isFileBased: isFileBased) - ) - let both = hidden.contains(MainWindowToolbar.connection) - && hidden.contains(MainWindowToolbar.database) - #expect(both == false) - } - } + for context in Self.everyContext { + #expect(Self.hidden(context).isDisjoint(with: permanent)) } } @@ -166,7 +184,7 @@ struct ToolbarContextResolverTests { /// `isHidden` is written only from the slow-moving subset, so the item set can change on a tab /// switch, a mode switch or a connection switch and on nothing else. A keystroke in a cell - /// editor costs one struct comparison and moves nothing. + /// editor builds the key, compares it and moves nothing. @Test("Visibility ignores everything transient") func visibilityIgnoresTransientState() { let quiet = ToolbarContext( @@ -186,7 +204,6 @@ struct ToolbarContextResolverTests { pane: .connecting, isConnected: false, hasSelectedWorkspace: true, - isTrailingPaneOpen: true, canToggleTrailingPane: false, pendingChange: .data, hasDataPendingChanges: true, @@ -195,19 +212,77 @@ struct ToolbarContextResolverTests { canRestorePreviousValues: true, canNavigateBack: true, canNavigateForward: true, - supportsContainerSwitching: true, - hasAgentSession: true + supportsContainerSwitching: true ) #expect(quiet.visibilityKey == busy.visibilityKey) - #expect(ToolbarContextResolver.hidden(quiet) == ToolbarContextResolver.hidden(busy)) + #expect(Self.hidden(quiet) == Self.hidden(busy)) + } + + /// The whole context and the key the toolbar builds on its own have to carry the same eight + /// facts, or a context built for enablement would disagree with the shape it is drawn over. + @Test("A context built over a key carries that key back") + func contextOverAKeyRoundTrips() { + let key = ToolbarContext.VisibilityKey( + tabKind: .createTable, + resultsMode: .structure, + contentMode: .agent, + isFileBased: true, + supportsContainerSwitching: false, + supportsImport: true, + supportsServerDashboard: true, + isAIEnabled: true + ) + let context = ToolbarContext( + key: key, + pane: .content, + isConnected: true, + hasSelectedWorkspace: true, + canToggleTrailingPane: true, + pendingChange: .createTable, + hasDataPendingChanges: false, + blocksAllWrites: false, + canAddRow: false, + canRestorePreviousValues: false, + canNavigateBack: false, + canNavigateForward: false + ) + + #expect(context.visibilityKey == key) + #expect(context.pendingChange == .createTable) + } + + // MARK: - The commit verb + + /// The verb comes from the tab kind, never from what is staged, so an edit that makes a + /// definition valid or invalid cannot relabel the control and reflow a labelled titlebar. + @Test("The commit verb follows the tab kind", arguments: tabKinds + [nil]) + func commitVerbFollowsTheTabKind(tabKind: TabType?) { + let expected: String + switch tabKind { + case .createTable: + expected = String(localized: "Create Table") + case .usersRoles: + expected = String(localized: "Apply Changes") + default: + expected = String(localized: "Save Changes") + } + #expect(ToolbarContextResolver.commitVerb(for: tabKind) == expected) + } + + /// Three verbs for three different commits. Two kinds sharing one would have the control offer + /// to save a definition that is about to be created. + @Test("The three commit verbs are distinct") + func commitVerbsAreDistinct() { + let verbs = Set([TabType.table, .createTable, .usersRoles].map { ToolbarContextResolver.commitVerb(for: $0) }) + #expect(verbs.count == 3) } // MARK: - Per-kind sets @Test("An unsaved definition has nothing to reload") func createTableHidesRefresh() { - let hidden = ToolbarContextResolver.hidden(Self.context(tabKind: .createTable)) + let hidden = Self.hidden(Self.context(tabKind: .createTable)) #expect(hidden.contains(MainWindowToolbar.refresh)) #expect(hidden.contains(MainWindowToolbar.saveChanges) == false) } @@ -217,7 +292,7 @@ struct ToolbarContextResolverTests { arguments: [TabType.erDiagram, .serverDashboard, .insights, .objectSource] ) func readOnlyKindsHideSaveChanges(tabKind: TabType) { - let hidden = ToolbarContextResolver.hidden(Self.context(tabKind: tabKind)) + let hidden = Self.hidden(Self.context(tabKind: tabKind)) #expect(hidden.contains(MainWindowToolbar.saveChanges)) #expect(hidden.contains(MainWindowToolbar.refresh) == false) } @@ -226,43 +301,43 @@ struct ToolbarContextResolverTests { TabType.query, .table, .usersRoles, ]) func editableKindsKeepBoth(tabKind: TabType) { - let hidden = ToolbarContextResolver.hidden(Self.context(tabKind: tabKind)) + let hidden = Self.hidden(Self.context(tabKind: tabKind)) #expect(hidden.contains(MainWindowToolbar.saveChanges) == false) #expect(hidden.contains(MainWindowToolbar.refresh) == false) } @Test("Agent mode has no grid to reload and nothing mounted to commit") func agentModeHidesBothContentCommands() { - let hidden = ToolbarContextResolver.hidden(Self.context(contentMode: .agent)) + let hidden = Self.hidden(Self.context(contentMode: .agent)) #expect(hidden.contains(MainWindowToolbar.refresh)) #expect(hidden.contains(MainWindowToolbar.saveChanges)) } @Test("A window with no selected tab keeps the full set") func noSelectedTabKeepsEverything() { - #expect(ToolbarContextResolver.hidden(Self.context(tabKind: nil)).isEmpty) + #expect(Self.hidden(Self.context(tabKind: nil)).isEmpty) } @Test("The results mode never moves an item", arguments: ResultsViewMode.allCases) func resultsModeNeverMovesAnything(mode: ResultsViewMode) { #expect( - ToolbarContextResolver.hidden(Self.context(resultsMode: mode)) - == ToolbarContextResolver.hidden(Self.context(resultsMode: .data)) + Self.hidden(Self.context(resultsMode: mode)) + == Self.hidden(Self.context(resultsMode: .data)) ) } @Test("The container capsule goes when the engine has nothing to switch to") func containerCapsuleVisibility() { #expect( - ToolbarContextResolver.hidden(Self.context(isFileBased: false, supportsContainerSwitching: true)) + Self.hidden(Self.context(isFileBased: false, supportsContainerSwitching: true)) .contains(MainWindowToolbar.database) == false ) #expect( - ToolbarContextResolver.hidden(Self.context(isFileBased: true, supportsContainerSwitching: true)) + Self.hidden(Self.context(isFileBased: true, supportsContainerSwitching: true)) .contains(MainWindowToolbar.database) ) #expect( - ToolbarContextResolver.hidden(Self.context(isFileBased: false, supportsContainerSwitching: false)) + Self.hidden(Self.context(isFileBased: false, supportsContainerSwitching: false)) .contains(MainWindowToolbar.database) ) } @@ -333,7 +408,6 @@ struct ToolbarContextResolverTests { pane: .unavailable(.notConnected), isConnected: false, hasSelectedWorkspace: true, - isTrailingPaneOpen: true, canToggleTrailingPane: true ) #expect(ToolbarContextResolver.isEnabled(MainWindowToolbar.inspector, context: context)) diff --git a/TableProTests/Core/Transport/TransportRateLabelTests.swift b/TableProTests/Core/Transport/TransportRateLabelTests.swift deleted file mode 100644 index 9cd21387e..000000000 --- a/TableProTests/Core/Transport/TransportRateLabelTests.swift +++ /dev/null @@ -1,102 +0,0 @@ -// -// TransportRateLabelTests.swift -// TableProTests -// - -import AppKit -import Foundation -@testable import TablePro -import Testing - -@Suite("TransportRateLabel") -struct TransportRateLabelTests { - @Test("An idle transport reads zero rather than going blank") - func idleReadsZero() { - let text = TransportRateLabel.text(for: .zero) - - #expect(text.contains("0")) - #expect(text.hasSuffix("kB/s")) - } - - @Test("No reading yet reads the same as idle") - func absentRateReadsAsIdle() { - #expect(TransportRateLabel.text(for: nil) == TransportRateLabel.text(for: .zero)) - } - - /// `.byteCount` spells the smallest unit out in full: measured, `allowedUnits: [.kb]` still - /// returns "512 bytes" for 512 and "Zero kB" for 0. Both are four characters wider than the - /// figures around them, which is why the label is assembled rather than formatted. - @Test("A rate below a kilobyte never spells out bytes") - func subKilobyteNeverSpellsBytes() { - let text = TransportRateLabel.text(for: TransportRate(receivedPerSecond: 512, sentPerSecond: 0)) - - #expect(!text.lowercased().contains("byte")) - #expect(text.hasSuffix("kB/s")) - } - - @Test("Units step up at a thousand") - func unitsStepUp() { - let kilo = TransportRateLabel.text(for: TransportRate(receivedPerSecond: 145_408, sentPerSecond: 0)) - let mega = TransportRateLabel.text(for: TransportRate(receivedPerSecond: 4_700_000, sentPerSecond: 0)) - let giga = TransportRateLabel.text(for: TransportRate(receivedPerSecond: 2_400_000_000, sentPerSecond: 0)) - - #expect(kilo.hasSuffix("kB/s")) - #expect(mega.hasSuffix("MB/s")) - #expect(giga.hasSuffix("GB/s")) - } - - @Test("The busier direction picks the arrow") - func arrowFollowsTheBusierDirection() { - let down = TransportRateLabel.text(for: TransportRate(receivedPerSecond: 145_408, sentPerSecond: 12)) - let up = TransportRateLabel.text(for: TransportRate(receivedPerSecond: 12, sentPerSecond: 145_408)) - - #expect(down.hasPrefix("\u{2193}")) - #expect(up.hasPrefix("\u{2191}")) - } - - @Test("An arrow is never what VoiceOver is given to read") - func accessibilityValueNamesTheDirection() { - let down = TransportRateLabel.accessibilityValue(for: TransportRate(receivedPerSecond: 145_408, sentPerSecond: 0)) - let up = TransportRateLabel.accessibilityValue(for: TransportRate(receivedPerSecond: 0, sentPerSecond: 145_408)) - - #expect(!down.contains("\u{2193}")) - #expect(!up.contains("\u{2191}")) - #expect(down != up) - } - - /// The field measures itself against these once, so a format that can produce something wider - /// than all of them would clip its own text. - @Test("No rate renders wider than the widest candidate the field is sized from") - func noRateOutgrowsTheField() { - let font = NSFont.monospacedDigitSystemFont(ofSize: NSFont.smallSystemFontSize, weight: .regular) - let budget = TransportRateLabel.widestCandidates - .map { ($0 as NSString).size(withAttributes: [.font: font]).width } - .max() ?? 0 - - let rates: [Double] = [0, 1, 512, 1_000, 1_500, 9_900, 64_000, 145_408, 512_000, 999_000, - 1_200_000, 4_700_000, 12_500_000, 88_000_000, 999_000_000, 2_400_000_000] - - for value in rates { - let text = TransportRateLabel.text(for: TransportRate(receivedPerSecond: value, sentPerSecond: 0)) - let width = (text as NSString).size(withAttributes: [.font: font]).width - #expect(width <= budget, "\(value) B/s rendered \"\(text)\" at \(width)pt, past the \(budget)pt budget") - } - } - - /// Monospaced digits are what make the drawn text hold still inside a fixed field: the figure - /// changes, the glyph advances do not. - @Test("Every figure of the same shape draws to the same width") - func figuresOfOneShapeDrawAlike() { - let font = NSFont.monospacedDigitSystemFont(ofSize: NSFont.smallSystemFontSize, weight: .regular) - let threeDigitKilobytes: [Double] = [111_000, 145_408, 512_000, 999_000] - - let widths = threeDigitKilobytes.map { value -> CGFloat in - let text = TransportRateLabel.text(for: TransportRate(receivedPerSecond: value, sentPerSecond: 0)) - return (text as NSString).size(withAttributes: [.font: font]).width - } - - let widest = widths.max() ?? 0 - let narrowest = widths.min() ?? 0 - #expect(widest - narrowest < 0.01, "Monospaced digits must not vary: spread was \(widest - narrowest)pt") - } -} diff --git a/TableProTests/Models/PendingChangeKindTests.swift b/TableProTests/Models/PendingChangeKindTests.swift index 67476858a..1da484b3f 100644 --- a/TableProTests/Models/PendingChangeKindTests.swift +++ b/TableProTests/Models/PendingChangeKindTests.swift @@ -167,13 +167,4 @@ struct PendingChangeKindTests { ) == nil ) } - - @Test("Each kind names the verb its tab commits with") - func commitTitles() { - #expect(PendingChangeKind.data.commitTitle == PendingChangeKind.structure.commitTitle) - #expect(PendingChangeKind.data.commitTitle == PendingChangeKind.file.commitTitle) - #expect(PendingChangeKind.createTable.commitTitle != PendingChangeKind.data.commitTitle) - #expect(PendingChangeKind.principals.commitTitle != PendingChangeKind.data.commitTitle) - #expect(PendingChangeKind.principals.commitTitle != PendingChangeKind.createTable.commitTitle) - } } diff --git a/TableProTests/Services/MainWindowToolbarLayoutTests.swift b/TableProTests/Services/MainWindowToolbarLayoutTests.swift index 0ecaa7641..24ea370d6 100644 --- a/TableProTests/Services/MainWindowToolbarLayoutTests.swift +++ b/TableProTests/Services/MainWindowToolbarLayoutTests.swift @@ -11,26 +11,28 @@ import Testing @MainActor struct MainWindowToolbarLayoutTests { - @Test("Sidebar toggle is ordered into the sidebar's titlebar strip") - func sidebarToggleOrderedBeforeTrackingSeparator() throws { + /// Items ahead of `.sidebarTrackingSeparator` lay out in the sidebar's own titlebar strip and + /// follow its divider. The sidebar toggle is the only thing that belongs there: the list chooser + /// that used to share the strip now sits at the top of the sidebar, over the list it switches. + @Test("AppKit's sidebar toggle is alone in the sidebar's titlebar strip") + func sidebarToggleIsAloneBeforeTheTrackingSeparator() throws { let identifiers = MainWindowToolbar.defaultItemIdentifiers - let toggleIndex = try #require(identifiers.firstIndex(of: MainWindowToolbar.sidebarToggle)) let separatorIndex = try #require(identifiers.firstIndex(of: .sidebarTrackingSeparator)) - #expect(toggleIndex < separatorIndex) + #expect(Array(identifiers[..