diff --git a/TablePro/Core/Services/Infrastructure/TabWindowController.swift b/TablePro/Core/Services/Infrastructure/TabWindowController.swift index 0cd6935c13..105263efa6 100644 --- a/TablePro/Core/Services/Infrastructure/TabWindowController.swift +++ b/TablePro/Core/Services/Infrastructure/TabWindowController.swift @@ -96,10 +96,14 @@ internal final class TabWindowController: NSWindowController, NSWindowDelegate { /// `adopting` carries a connection that is moving here from another window, whole. Everything /// the user has in it lives on that object, so the window takes it rather than building a /// second one around the same connection. + /// + /// `pinnedWindowSize` is the screenshot size, passed in so a test can build the real window at + /// one without the UI test sandbox. internal init( payload: EditorTabPayload, sessionState: SessionStateFactory.SessionState? = nil, autoConnect: Bool = false, + pinnedWindowSize: CGSize? = ScreenshotEnvironment.windowSize, adopting workspace: ConnectionWorkspace? = nil ) { self.payload = payload @@ -123,18 +127,9 @@ internal final class TabWindowController: NSWindowController, NSWindowDelegate { super.init(window: window) window.isReleasedWhenClosed = false + Self.placeInitialFrame(of: window, pinnedSize: pinnedWindowSize) window.delegate = self - if !window.setFrameUsingName(Self.frameAutosaveName) { - let visibleSize = (window.screen ?? NSScreen.main)?.visibleFrame.size - ?? NSSize(width: 1_440, height: 900) - window.setContentSize(NSSize( - width: min(1_200, visibleSize.width), - height: min(800, visibleSize.height) - )) - window.center() - } - Self.lifecycleLogger.info( "[open] TabWindowController.init payloadId=\(payload.id, privacy: .public) connId=\(payload.connectionId, privacy: .public) controllerId=\(self.controllerId, privacy: .public) eagerToolbar=\(sessionState != nil)" ) @@ -145,6 +140,36 @@ internal final class TabWindowController: NSWindowController, NSWindowDelegate { fatalError("TabWindowController does not support NSCoder init") } + /// Every frame the window starts with is settled here, before it is shown or laid out. A resize + /// issued from inside the window's own layout pass lands under `_layoutSubtreeWithOldSize:`, + /// which has already captured the old size and resizes the content view by the difference a + /// second time. Measured on macOS 27 with a 1200x800 window pinned to 1512x861 from a SwiftUI + /// `viewDidMoveToWindow`, which is where the screenshot pin used to run: the split view came out + /// 1824x922 at y -61, so the sidebar's top row sat under the titlebar and the inspector divider + /// stood 312pt past the window's edge, taking the toolbar's trailing items into the overflow + /// menu. Activating the window afterwards did not repair it. + /// + /// Placed before the controller becomes the window's delegate, so the starting frame is not + /// filed under `frameAutosaveName` as if the user had chosen it. A pinned size is not theirs, and + /// a unit test building this window would otherwise write over the real saved frame, which is + /// not namespaced outside the UI test sandbox. The frame is still saved on close. + private static func placeInitialFrame(of window: NSWindow, pinnedSize: CGSize?) { + let visibleFrame = (window.screen ?? NSScreen.main)?.visibleFrame + if let pinnedSize { + let pinned = visibleFrame.map { ScreenshotEnvironment.pinnedFrame(size: pinnedSize, in: $0) } + ?? NSRect(origin: window.frame.origin, size: pinnedSize) + window.setFrame(pinned, display: false) + return + } + guard !window.setFrameUsingName(frameAutosaveName) else { return } + let visibleSize = visibleFrame?.size ?? NSSize(width: 1_440, height: 900) + window.setContentSize(NSSize( + width: min(1_200, visibleSize.width), + height: min(800, visibleSize.height) + )) + window.center() + } + /// The one place an editor window's chrome is configured, so a test can hold the whole shape. internal static func makeEditorWindow() -> NSWindow { let window = EditorWindow( diff --git a/TablePro/Core/Testing/ScreenshotEnvironment.swift b/TablePro/Core/Testing/ScreenshotEnvironment.swift index 38f9296221..0faf199ab0 100644 --- a/TablePro/Core/Testing/ScreenshotEnvironment.swift +++ b/TablePro/Core/Testing/ScreenshotEnvironment.swift @@ -53,17 +53,17 @@ internal enum ScreenshotEnvironment { /// Centred on the visible frame rather than placed at a fixed origin. `screencapture` reads the /// window's own bounds, so a window hanging off the screen edge comes back clipped by the /// display instead of failing. - @MainActor - internal static func pinWindowSize(_ window: NSWindow) { - guard let size = windowSize else { return } - guard let screen = window.screen ?? NSScreen.main else { return } - - let visible = screen.visibleFrame - let origin = NSPoint( - x: visible.midX - size.width / 2, - y: visible.midY - size.height / 2 + /// + /// Only a frame. Where it is applied is the caller's decision, and there is one right answer: + /// see `TabWindowController.placeInitialFrame(of:pinnedSize:)`. + internal static func pinnedFrame(size: CGSize, in visibleFrame: NSRect) -> NSRect { + NSRect( + origin: NSPoint( + x: visibleFrame.midX - size.width / 2, + y: visibleFrame.midY - size.height / 2 + ), + size: size ) - window.setFrame(NSRect(origin: origin, size: size), display: true) } private static func sandboxedValue(of variable: String) -> String? { diff --git a/TablePro/Views/Main/Extensions/MainContentView+Setup.swift b/TablePro/Views/Main/Extensions/MainContentView+Setup.swift index 7054c21180..e258902781 100644 --- a/TablePro/Views/Main/Extensions/MainContentView+Setup.swift +++ b/TablePro/Views/Main/Extensions/MainContentView+Setup.swift @@ -279,7 +279,6 @@ extension MainContentView { splitVC.pointToolbar(at: coordinator) } - ScreenshotEnvironment.pinWindowSize(window) MainContentView.lifecycleLogger.info( "[open] configureWindow done windowId=\(windowId, privacy: .public) isPreview=\(isPreview) elapsedMs=\(Int(Date().timeIntervalSince(start) * 1_000))" ) diff --git a/TableProTests/Core/Testing/ScreenshotEnvironmentTests.swift b/TableProTests/Core/Testing/ScreenshotEnvironmentTests.swift index ce39f720b7..4b3e59b9ab 100644 --- a/TableProTests/Core/Testing/ScreenshotEnvironmentTests.swift +++ b/TableProTests/Core/Testing/ScreenshotEnvironmentTests.swift @@ -3,23 +3,24 @@ // TableProTests // +import CoreGraphics import Foundation @testable import TablePro import Testing -@Suite("ScreenshotEnvironment frame parsing") +@Suite("ScreenshotEnvironment frame") struct ScreenshotEnvironmentTests { @Test("Reads the size the marketing shots are cut to") func readsWidthAndHeight() throws { let size = try #require(ScreenshotEnvironment.size(from: "1512x861")) - #expect(size.width == 1512) + #expect(size.width == 1_512) #expect(size.height == 861) } @Test("Accepts an upper case separator") func acceptsUpperCaseSeparator() throws { let size = try #require(ScreenshotEnvironment.size(from: "1512X861")) - #expect(size.width == 1512) + #expect(size.width == 1_512) } /// Every one of these has to come back nil rather than a default. A default would open the @@ -32,4 +33,15 @@ struct ScreenshotEnvironmentTests { func refusesMalformedInput(_ raw: String) { #expect(ScreenshotEnvironment.size(from: raw) == nil) } + + @Test("The pinned frame is centred on the screen's visible frame") + func pinnedFrameIsCentred() { + let visible = NSRect(x: 0, y: 25, width: 2_560, height: 1_410) + + let frame = ScreenshotEnvironment.pinnedFrame(size: CGSize(width: 1_512, height: 861), in: visible) + + #expect(frame.size == CGSize(width: 1_512, height: 861)) + #expect(frame.midX == visible.midX) + #expect(frame.midY == visible.midY) + } } diff --git a/TableProTests/Services/EditorWindowInitialFrameTests.swift b/TableProTests/Services/EditorWindowInitialFrameTests.swift new file mode 100644 index 0000000000..eff3f7a30a --- /dev/null +++ b/TableProTests/Services/EditorWindowInitialFrameTests.swift @@ -0,0 +1,112 @@ +// +// EditorWindowInitialFrameTests.swift +// TableProTests +// + +import AppKit +import SwiftUI +import Testing + +@testable import TablePro + +/// A connection window opened in the background under a pinned screenshot size came up with its +/// sidebar's top row under the titlebar and its toolbar's trailing items in the overflow menu, and +/// stayed that way. The pin resized the window from a `WindowAccessor` callback, which runs inside +/// the window's layout pass. +@Suite("Editor window initial frame", .serialized) +@MainActor +struct EditorWindowInitialFrameTests { + private let pinnedSize = CGSize(width: 1_000, height: 700) + + /// Built through the initializer every connection window comes from, so moving the pin anywhere + /// after construction fails here: the window then starts at its content's own size. + @Test("A connection window starts at its pinned size, before it is ever shown") + func connectionWindowIsBuiltAtItsPinnedSize() throws { + try withConnectionWindow { window in + #expect(!window.isVisible) + #expect(window.frame.size == pinnedSize) + } + } + + /// The window's content view is the split view every pane hangs from, so it has to span exactly + /// the window: the sidebar's top inset, the inspector divider and the toolbar sections that + /// track both are all measured against it. + @Test("A connection window's split view spans the pinned window once it is laid out") + func splitViewSpansThePinnedWindow() throws { + try withConnectionWindow { window in + window.layoutIfNeeded() + + #expect(window.contentView?.frame.size == pinnedSize) + #expect(window.contentView?.frame.origin == .zero) + } + } + + /// Why the pin cannot live anywhere a view reports its window from. The connection's content + /// arrives in a pane that is already on the window, the way `refreshPanes` hands it over, and + /// SwiftUI mounts its `WindowAccessor` while it renders inside the window's layout pass. A + /// resize from there is applied to the content view twice. If this starts failing, AppKit has + /// changed and the reasoning in `TabWindowController.placeInitialFrame(of:pinnedSize:)` should + /// be measured again. + @Test("A resize from a WindowAccessor callback leaves the content view out of step with the window") + func resizingFromALayoutCallbackOvershootsTheContent() { + let window = TabWindowController.makeEditorWindow() + window.isReleasedWhenClosed = false + defer { window.close() } + let detail = Self.installSplit(in: window) + window.layoutIfNeeded() + let initialSize = window.frame.size + let target = pinnedSize + + detail.rootView = AnyView(Color.clear.background(WindowAccessor { accessed in + accessed.setFrame(NSRect(origin: accessed.frame.origin, size: target), display: true) + })) + window.layoutIfNeeded() + + #expect(window.frame.size == target) + #expect(window.contentView?.frame.width == target.width + (target.width - initialSize.width)) + #expect(window.contentView?.frame.height == target.height + (target.height - initialSize.height)) + } + + // MARK: - Helpers + + /// No session and a workspace handed in whole, so nothing reaches the connection store. The + /// window is never shown or closed: closing runs the controller's own teardown, which saves the + /// frame into the real defaults and cancels connects. + private func withConnectionWindow(_ body: (NSWindow) throws -> Void) throws { + let connection = TestFixtures.makeConnection(name: "Pinned frame") + let workspace = ConnectionWorkspace( + connectionId: connection.id, + payload: nil, + autoConnect: false, + payloadConnection: connection, + session: nil, + sessionState: nil, + trailingPaneState: nil, + phase: .connecting + ) + let controller = TabWindowController( + payload: EditorTabPayload(connectionId: connection.id), + pinnedWindowSize: pinnedSize, + adopting: workspace + ) + let window = try #require(controller.window) + defer { + window.delegate = nil + window.contentViewController = nil + workspace.teardown() + } + try body(window) + } + + private static func installSplit(in window: NSWindow) -> NSHostingController { + let split = NSSplitViewController() + let sidebar = NSViewController() + sidebar.view = NSView() + split.addSplitViewItem(NSSplitViewItem(sidebarWithViewController: sidebar)) + let detail = NSHostingController(rootView: AnyView(Color.clear)) + detail.sizingOptions = [] + split.addSplitViewItem(NSSplitViewItem(viewController: detail)) + window.contentViewController = split + return detail + } +}