Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Connections strip not scrolling to the entry you switch to.
- Background connections stuck on the session preparation screen after connecting. (#2545)
- Blank sidebar, grid and inspector on a connection opened into a window that was already on screen.
- Connections strip hidden, with Switch Connection and the Show Next and Previous Connection items disabled, whenever the connection on screen was not connected.
- Choosing a connection from the connection list re-fronting its window without switching to it.
- Grid cells left at the old column positions until the next click, after a resize, an auto-fit, a reorder, hiding a column, or a row-number width change. (#2449, #2446)
- The row-number column draggable out of first place, which walked it to the far right on the next refresh.
- A time entered into a date cell discarded when the stored value carried no time.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@ internal enum ConnectionWindowPane: Equatable {
case empty
}

/// What the window's one sidebar item holds. `railOnly` is the state that exists because the
/// workspace rail and the object browser share that item and answer to different owners.
internal enum SidebarChromeMode: Equatable {
case revealed
case railOnly
case hidden

internal var showsObjectBrowser: Bool { self == .revealed }
}

internal enum ConnectionWindowPaneResolver {
internal static func pane(
phase: ConnectionWindowPhase,
Expand All @@ -33,8 +43,8 @@ internal enum ConnectionWindowPaneResolver {
}
}

/// A sidebar and an inspector with nothing to put in them are not chrome, they are two empty
/// columns that promise a session the window does not have yet.
/// An object browser and an inspector with nothing to put in them are not chrome, they are two
/// empty columns that promise a session the window does not have yet.
internal static func hidesChrome(for pane: ConnectionWindowPane) -> Bool {
switch pane {
case .content:
Expand All @@ -44,6 +54,21 @@ internal enum ConnectionWindowPaneResolver {
}
}

/// How much of the window's sidebar survives the pane it is standing next to.
///
/// The rule above is right about the object browser and wrong about the workspace rail, which
/// lists every connection the window hosts and belongs to the window rather than to any one of
/// them. They share a split item because AppKit grants full-height sidebar layout to exactly one
/// leading sidebar, so collapsing for an empty object browser took the switcher with it and left
/// the window's other connections with no way in.
internal static func sidebarChromeMode(
for pane: ConnectionWindowPane,
hasRail: Bool
) -> SidebarChromeMode {
guard hidesChrome(for: pane) else { return .revealed }
return hasRail ? .railOnly : .hidden
}

/// The tab strip's band is a list of tabs, so it appears only when there is a list worth
/// showing: content behind it, and more than one tab in it. A window with a single tab keeps
/// the chrome it always had, which is what the system does too.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,9 +229,15 @@ extension MainSplitViewController: NSMenuItemValidation {
}
}

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

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

@objc func showNextWorkspace(_ sender: Any?) {
commandActions?.showNextWorkspace()
activateWorkspace(offsetBy: 1)
}

@objc func setResultView(_ sender: Any?) {
Expand Down
139 changes: 110 additions & 29 deletions TablePro/Core/Services/Infrastructure/MainSplitViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,6 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi
var tabStripObservationIsArmed = false
var tabStripObservedManager: ObjectIdentifier?

private var chromeState: ChromeState = .unapplied

// MARK: - Panel Layout State

/// One name for the window's split view, because one `NSSplitView` can only carry one.
Expand Down Expand Up @@ -253,6 +251,10 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi
navigationSidebar.railController.host = self
navigationSidebar.railController.onLayoutChange = { [weak self] _ in
self?.navigationSidebar.applyRailWidth(animated: false)
/// The row size is a setting, so the rail's own width changes under a sidebar already
/// narrowed to it. Reapplying the clamp is what moves both thicknesses onto the new
/// allowance rather than clipping the rail against the old one.
self?.reapplySidebarClampIfNarrowed()
self?.recomputeWindowMinSize()
}
sidebarSplitItem = NSSplitViewItem(sidebarWithViewController: navigationSidebar)
Expand Down Expand Up @@ -935,6 +937,11 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi
navigationSidebar.setRailVisible(visible, animated: view.window != nil) { [weak self] in
self?.recomputeWindowMinSize()
}
/// The rail appearing or going is a chrome change of its own, and it happens without the
/// selected connection's phase moving: a sibling opening or closing is enough. Without this
/// a window whose connection is down keeps a fully collapsed sidebar when the rail arrives,
/// or an empty clamped column after it leaves.
applyPaneChrome()
}

func activateWorkspace(offsetBy offset: Int) {
Expand All @@ -943,8 +950,25 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi

// MARK: - Sidebar

/// Whether the object browser is off screen, which is the question every caller is really
/// asking: the toolbar's segment, the Show/Hide Sidebar title and the reveal actions. A sidebar
/// narrowed to the workspace rail is an open split item with no object browser in it, so the
/// item's own flag is not the answer on its own.
var isSidebarCollapsed: Bool {
sidebarSplitItem?.isCollapsed ?? true
guard sidebarChromeMode.showsObjectBrowser else { return true }
return sidebarSplitItem?.isCollapsed ?? true
}

var isSidebarUserCollapsible: Bool {
sidebarSplitItem?.canCollapse ?? false
}

var sidebarThicknessRange: (min: CGFloat, max: CGFloat) {
(sidebarSplitItem?.minimumThickness ?? 0, sidebarSplitItem?.maximumThickness ?? 0)
}

var railAllowance: CGFloat {
navigationSidebar?.railAllowance ?? 0
}

/// Every collapse route reaches AppKit's own `toggleSidebar(_:)`: the View menu sends the
Expand All @@ -957,13 +981,15 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi
}

func focusSidebarSearch() {
guard sidebarChromeMode.showsObjectBrowser else { return }
if sidebarSplitItem?.isCollapsed == true {
sidebarSplitItem?.animator().isCollapsed = false
}
navigationSidebar.objectBrowser.focusSearchField()
}

func presentDatabaseFilter() {
guard sidebarChromeMode.showsObjectBrowser else { return }
guard let connectionId = currentSession?.connection.id else { return }
if sidebarSplitItem?.isCollapsed == true {
sidebarSplitItem?.isCollapsed = false
Expand All @@ -981,7 +1007,11 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi
state.databaseFilterSelected = []
}

/// Refused while the sidebar is narrowed to the workspace rail. The item is open, so the
/// collapse branch below would read it as showing and collapse it, taking the rail and every
/// route to the window's other connections with it.
func setSidebarTab(_ tab: SidebarTab) {
guard sidebarChromeMode.showsObjectBrowser else { return }
guard let connectionId = currentSession?.connection.id else { return }
let sidebarState = SharedSidebarState.forConnection(connectionId)

Expand Down Expand Up @@ -1062,8 +1092,12 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi
)
}

/// Inert while the sidebar is clamped to the rail. Seven other call sites reach
/// `recomputeWindowMinSize`, and any of them writing the object browser's minimum back over the
/// clamp would leave a minimum above the maximum.
private func applySidebarMinimumThickness() {
guard let sidebarSplitItem else { return }
guard appliedSidebarMode ?? .revealed == .revealed else { return }
let resolved = Self.resolveSidebarMinimumThickness(
railAllowance: navigationSidebar?.railAllowance ?? 0
)
Expand Down Expand Up @@ -1108,52 +1142,99 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi

// MARK: - Pane Chrome

private enum ChromeState {
case unapplied
case hidden
case revealed
}

private var userPaneLayout: ChromePaneLayout?

/// What this window's sidebar is currently showing, and `nil` before the first application.
/// The resolver decides the mode; this records which one has been put on screen.
private var appliedSidebarMode: SidebarChromeMode?

internal var sidebarChromeMode: SidebarChromeMode {
ConnectionWindowPaneResolver.sidebarChromeMode(
for: currentPane,
hasRail: navigationSidebar?.isRailVisible ?? false
)
}

/// A split item's collapse state is written into the autosave record, which is how the
/// inspector remembers being hidden. Collapsing the sidebar for a phase the user did not
/// choose would persist that as their layout and lose the width they set, so autosaving is
/// switched off for the whole span the chrome is hidden and switched back on to restore it.
/// switched off for the whole span the chrome is not revealed and switched back on to restore
/// it. The same is true of a clamp, which writes its narrow width into the record the way a
/// collapse writes the collapsed flag.
func applyPaneChrome() {
if ConnectionWindowPaneResolver.hidesChrome(for: currentPane) {
hideWindowChrome()
} else {
revealWindowChrome()
}
applySidebarChromeMode(sidebarChromeMode)
applyTabStripVisibility()
toolbarOwner?.managedToolbar.validateVisibleItems()
recomputeWindowMinSize()
}

private func hideWindowChrome() {
guard chromeState != .hidden else { return }
chromeState = .hidden
private func applySidebarChromeMode(_ mode: SidebarChromeMode) {
guard appliedSidebarMode != mode else { return }
let previous = appliedSidebarMode
appliedSidebarMode = mode

guard mode != .revealed else {
revealWindowChrome()
return
}

/// Captured on the way out of `revealed` and never again, because the geometry a
/// `railOnly` to `hidden` step would see is the clamp, not the width the user chose.
if previous == nil || previous == .revealed {
resignFirstResponderInsideChrome()
splitView.autosaveName = nil
userPaneLayout = ChromePaneLayout(
isSidebarCollapsed: sidebarSplitItem.isCollapsed,
isInspectorCollapsed: inspectorSplitItem.isCollapsed
)
}

resignFirstResponderInsideChrome()
splitView.autosaveName = nil
userPaneLayout = ChromePaneLayout(
isSidebarCollapsed: sidebarSplitItem.isCollapsed,
isInspectorCollapsed: inspectorSplitItem.isCollapsed
)
sidebarSplitItem.isCollapsed = true
inspectorSplitItem.isCollapsed = true
switch mode {
case .railOnly:
sidebarSplitItem.isCollapsed = false
clampSidebarToRail()
case .hidden:
releaseSidebarClamp()
sidebarSplitItem.isCollapsed = true
case .revealed:
break
}
view.window?.recalculateKeyViewLoop()
}

/// Autosaving is off while the chrome is hidden, so the record still holds what the user had.
/// AppKit will not re-apply it though: assigning an autosave name to a split view that has
/// Narrowed rather than collapsed, so the rail stays on screen while the object browser it
/// shares a split item with goes. Measured: clamping and later releasing returns the item to
/// the width the user set, but a `setPosition` while the clamp holds discards it, which is why
/// nothing else may write the sidebar's thickness for the span.
private func clampSidebarToRail() {
let allowance = navigationSidebar?.railAllowance ?? 0
sidebarSplitItem.minimumThickness = allowance
sidebarSplitItem.maximumThickness = allowance
/// A clamp is not a lock. AppKit still collapses a collapsible item on a divider
/// double-click or a drag to the edge, which no menu or toolbar validation sees, and the
/// mode is already applied so nothing would open it again.
sidebarSplitItem.canCollapse = false
}

internal func reapplySidebarClampIfNarrowed() {
guard appliedSidebarMode == .railOnly else { return }
clampSidebarToRail()
}

private func releaseSidebarClamp() {
sidebarSplitItem.canCollapse = true
sidebarSplitItem.maximumThickness = Self.sidebarMaxThickness
applySidebarMinimumThickness()
}

/// Autosaving is off while the chrome is not revealed, so the record still holds what the user
/// had. AppKit will not re-apply it though: assigning an autosave name to a split view that has
/// already laid out restores nothing. The state captured on the way in is therefore what gives
/// the panes back. Forcing the sidebar open here instead reopened a sidebar the user had
/// deliberately hidden, every time a connection dropped and came back.
private func revealWindowChrome() {
guard chromeState != .revealed else { return }
chromeState = .revealed
releaseSidebarClamp()

/// Only a reveal that follows a hide has something to put back. A first reveal is a window
/// opening on a live connection, where the panes are already where the user's autosaved
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ extension MainWindowToolbar: NSToolbarItemValidation {
let supportsServerDashboard: Bool
let canNavigateBack: Bool
let canNavigateForward: Bool
/// Separate from `connected` because a connection that is still dialing counts as alive
/// while its sidebar is narrowed to the workspace rail, and a segment that toggles an
/// object browser the window is not showing has nothing to toggle. Defaulted, because a
/// context built for a connected pane is describing a window that has one.
var showsObjectBrowser = true
}

/// Listed exhaustively so a new state has to choose a side instead of inheriting "alive".
Expand All @@ -46,8 +51,10 @@ extension MainWindowToolbar: NSToolbarItemValidation {
return true
case Self.database:
return context.connected && !context.fileBased && context.supportsContainerSwitching
case Self.refresh, Self.quickSwitcher, Self.newTab, Self.exportTables, Self.sidebarToggle:
case Self.refresh, Self.quickSwitcher, Self.newTab, Self.exportTables:
return context.connected
case Self.sidebarToggle:
return context.connected && context.showsObjectBrowser
case Self.addRow:
return context.connected && context.canAddRow
case Self.restorePreviousValues:
Expand Down Expand Up @@ -86,10 +93,16 @@ extension MainWindowToolbar: NSToolbarItemValidation {
supportsImport: PluginManager.shared.supportsImport(for: state.databaseType),
supportsServerDashboard: coordinator?.commandActions?.supportsServerDashboard ?? false,
canNavigateBack: coordinator?.canNavigateBack ?? false,
canNavigateForward: coordinator?.canNavigateForward ?? false
canNavigateForward: coordinator?.canNavigateForward ?? false,
showsObjectBrowser: coordinator?.splitViewController?.sidebarChromeMode.showsObjectBrowser ?? false
)
}

/// No subject disables the whole toolbar, Switch Connection included. Every item here needs the
/// coordinator that presents it, so enabling one without a subject would leave a live-looking
/// button that does nothing. The routes to a window's other connections that survive a
/// connection going down are the connections strip and the View menu, neither of which asks a
/// coordinator anything.
func validateToolbarItem(_ item: NSToolbarItem) -> Bool {
guard let context = validationContext() else { return false }
return Self.isEnabled(itemIdentifier: item.itemIdentifier, context: context)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,17 @@ internal final class SidebarContainerViewController: NSViewController {
view.addSubview(hostingView)
searchField.nextKeyView = hostingView

/// The insets are a margin, not an invariant, so they yield rather than break when the
/// window narrows the sidebar to the workspace rail and leaves this view no width at all.
let searchLeading = searchField.leadingAnchor.constraint(equalTo: view.leadingAnchor, constant: 10)
let searchTrailing = searchField.trailingAnchor.constraint(equalTo: view.trailingAnchor, constant: -10)
searchLeading.priority = .defaultHigh
searchTrailing.priority = .defaultHigh

NSLayoutConstraint.activate([
searchField.topAnchor.constraint(equalTo: view.safeAreaLayoutGuide.topAnchor, constant: 5),
searchField.leadingAnchor.constraint(equalTo: view.leadingAnchor, constant: 10),
searchField.trailingAnchor.constraint(equalTo: view.trailingAnchor, constant: -10),
searchLeading,
searchTrailing,

hostingView.topAnchor.constraint(equalTo: searchField.bottomAnchor, constant: 5),
hostingView.leadingAnchor.constraint(equalTo: view.leadingAnchor),
Expand Down
Loading
Loading