-
-
Notifications
You must be signed in to change notification settings - Fork 285
fix(inspector): prevent window overflow on inspector pane toggle #1463
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
6984de3
e24ea5b
77923ac
a9fae7a
4e69ac6
4436989
5f5f4dc
799e231
3539993
58cd110
5aa4388
45075ac
96dfe45
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -212,6 +212,7 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi | |
| inspectorSplitItem.canCollapse = true | ||
| inspectorSplitItem.minimumThickness = 270 | ||
| inspectorSplitItem.maximumThickness = 400 | ||
| inspectorSplitItem.collapseBehavior = .preferResizingSplitViewWithFixedSiblings | ||
| addSplitViewItem(inspectorSplitItem) | ||
|
|
||
| if currentSession?.driver == nil { | ||
|
|
@@ -225,17 +226,21 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi | |
| inspectorSplitItem.isCollapsed = !inspectorPresented | ||
| } | ||
|
|
||
| override func splitViewDidResizeSubviews(_ notification: Notification) { | ||
| super.splitViewDidResizeSubviews(notification) | ||
| recomputeWindowMinSize() | ||
| } | ||
|
|
||
| private func materializeInspectorIfNeeded() { | ||
| guard !hasMaterializedInspector, let inspectorHosting else { return } | ||
| hasMaterializedInspector = true | ||
| inspectorHosting.rootView = AnyView(buildInspectorView()) | ||
| } | ||
|
|
||
| private func setCollapsed(_ isCollapsed: Bool, for splitItem: NSSplitViewItem?) { | ||
| guard let splitItem, splitItem.isCollapsed != isCollapsed else { return } | ||
| if view.window?.isVisible == true { | ||
| splitItem.animator().isCollapsed = isCollapsed | ||
| } else { | ||
| splitItem.isCollapsed = isCollapsed | ||
| } | ||
| } | ||
|
|
||
| override func viewWillAppear() { | ||
| super.viewWillAppear() | ||
| guard let window = view.window else { return } | ||
|
|
@@ -257,7 +262,6 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi | |
| } | ||
|
|
||
| installObservers() | ||
| recomputeWindowMinSize() | ||
| window.recalculateKeyViewLoop() | ||
| } | ||
|
|
||
|
|
@@ -324,11 +328,7 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi | |
| sessionState = nil | ||
| currentSession = nil | ||
| sidebarContainer.updateSidebarState(nil, windowState: nil) | ||
| if view.window?.isVisible == true { | ||
| sidebarSplitItem.animator().isCollapsed = true | ||
| } else { | ||
| sidebarSplitItem.isCollapsed = true | ||
| } | ||
| setCollapsed(true, for: sidebarSplitItem) | ||
| } | ||
| return | ||
| } | ||
|
|
@@ -356,11 +356,7 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi | |
| } | ||
|
|
||
| let collapseSidebar = newSession.driver == nil | ||
| if view.window?.isVisible == true { | ||
| sidebarSplitItem.animator().isCollapsed = collapseSidebar | ||
| } else { | ||
| sidebarSplitItem.isCollapsed = collapseSidebar | ||
| } | ||
| setCollapsed(collapseSidebar, for: sidebarSplitItem) | ||
| rebuildPanes() | ||
| } | ||
|
|
||
|
|
@@ -526,15 +522,13 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi | |
|
|
||
| func showInspector() { | ||
| materializeInspectorIfNeeded() | ||
| inspectorSplitItem?.animator().isCollapsed = false | ||
| setCollapsed(false, for: inspectorSplitItem) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the sidebar is visible, opening the inspector requires about Useful? React with 👍 / 👎. |
||
| UserDefaults.standard.set(true, forKey: Self.inspectorPresentedKey) | ||
| recomputeWindowMinSize() | ||
| } | ||
|
|
||
| func hideInspector() { | ||
| inspectorSplitItem?.animator().isCollapsed = true | ||
| setCollapsed(true, for: inspectorSplitItem) | ||
| UserDefaults.standard.set(false, forKey: Self.inspectorPresentedKey) | ||
| recomputeWindowMinSize() | ||
| } | ||
|
|
||
| @objc override func toggleInspector(_ sender: Any?) { | ||
|
|
@@ -560,58 +554,14 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi | |
|
|
||
| if sidebarSplitItem?.isCollapsed == true { | ||
| sidebarState.selectedSidebarTab = tab | ||
| sidebarSplitItem?.animator().isCollapsed = false | ||
| setCollapsed(false, for: sidebarSplitItem) | ||
| } else if sidebarState.selectedSidebarTab == tab { | ||
| sidebarSplitItem?.animator().isCollapsed = true | ||
| setCollapsed(true, for: sidebarSplitItem) | ||
| } else { | ||
| sidebarState.selectedSidebarTab = tab | ||
| } | ||
| } | ||
|
|
||
| // MARK: - Dynamic Window Minimum Size | ||
|
|
||
| private static let baseWindowMinWidth: CGFloat = 720 | ||
| private static let baseWindowMinHeight: CGFloat = 480 | ||
|
|
||
| private func recomputeWindowMinSize() { | ||
| guard let window = view.window else { return } | ||
| let sidebarVisible = !(sidebarSplitItem?.isCollapsed ?? true) | ||
| let inspectorVisible = !(inspectorSplitItem?.isCollapsed ?? true) | ||
|
|
||
| let detailMin: CGFloat = detailSplitItem?.minimumThickness ?? 400 | ||
| let sidebarMin: CGFloat = sidebarSplitItem?.minimumThickness ?? 280 | ||
| let inspectorMin: CGFloat = inspectorSplitItem?.minimumThickness ?? 270 | ||
| let dividerThickness = splitView.dividerThickness | ||
|
|
||
| var width: CGFloat = detailMin | ||
| if sidebarVisible { | ||
| width += sidebarMin + dividerThickness | ||
| } | ||
| if inspectorVisible { | ||
| width += inspectorMin + dividerThickness | ||
| } | ||
|
|
||
| let resolvedWidth = max(Self.baseWindowMinWidth, width) | ||
| let newMinSize = NSSize(width: resolvedWidth, height: Self.baseWindowMinHeight) | ||
|
|
||
| guard window.minSize != newMinSize else { return } | ||
| window.minSize = newMinSize | ||
|
|
||
| var frame = window.frame | ||
| var resized = false | ||
| if frame.size.width < resolvedWidth { | ||
| frame.size.width = resolvedWidth | ||
| resized = true | ||
| } | ||
| if frame.size.height < Self.baseWindowMinHeight { | ||
| frame.size.height = Self.baseWindowMinHeight | ||
| resized = true | ||
| } | ||
| if resized { | ||
| window.setFrame(frame, display: true, animate: false) | ||
| } | ||
| } | ||
|
|
||
| // MARK: - Constants | ||
|
|
||
| private static let inspectorPresentedKey = "com.TablePro.rightPanel.isPresented" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -130,6 +130,13 @@ enum PasswordSourceResolver { | |
| stdoutPipe.fileHandleForReading.readabilityHandler = nil | ||
| stderrPipe.fileHandleForReading.readabilityHandler = nil | ||
|
|
||
| if let remainingStdout = try? stdoutPipe.fileHandleForReading.readToEnd(), !remainingStdout.isEmpty { | ||
| stdoutCollector.append(remainingStdout) | ||
| } | ||
| if let remainingStderr = try? stderrPipe.fileHandleForReading.readToEnd(), !remainingStderr.isEmpty { | ||
| stderrCollector.append(remainingStderr) | ||
|
Comment on lines
+133
to
+137
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If a password command exits after starting a background process that inherits stdout or stderr (for example a shell helper that daemonizes), Useful? React with 👍 / 👎. |
||
| } | ||
|
|
||
| if stdoutCollector.overflowed { | ||
| throw ResolutionError.outputTooLarge | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only helps when AppKit performs an uncollapse transition; it does not cover the startup path where
inspectorPresentedis restored andviewDidLoadsetsinspectorSplitItem.isCollapsed = falsebefore the window is visible. I checkedTabWindowController: it restores and savesMainEditorWindowframes, so a previously saved narrow frame can reopen with the inspector already visible and no collapse animation to trigger this behavior, leaving the same squeezed/overflowing panes until the user toggles the inspector again. Please also handle the initially visible inspector after the window/frame is restored.Useful? React with 👍 / 👎.