macOS: prep the tab bar when system appearance changes

macos/Sources/Features/Terminal/Window Styles/TitlebarTabsTahoeTerminalWindow.swift

@@ -9,20 +9,10 @@ class TitlebarTabsTahoeTerminalWindow: TransparentTitlebarTerminalWindow, NSTool
     /// The view model for SwiftUI views
     private var viewModel = ViewModel()
 
-    override func awakeFromNib() {
+    deinit {
-        super.awakeFromNib()
+        tabBarObserver = nil
-
-        // We must hide the title since we're going to be moving tabs into
-        // the titlebar which have their own title.
-        titleVisibility = .hidden
-
-        // Create a toolbar
-        let toolbar = NSToolbar(identifier: "TerminalToolbar")
-        toolbar.delegate = self
-        toolbar.centeredItemIdentifiers.insert(.title)
-        self.toolbar = toolbar
-        toolbarStyle = .unifiedCompact
     }
+
     // MARK: NSWindow
 
     override var title: String {
@@ -43,11 +33,27 @@ class TitlebarTabsTahoeTerminalWindow: TransparentTitlebarTerminalWindow, NSTool
         }
     }
 
+    override func awakeFromNib() {
+        super.awakeFromNib()
+
+        // We must hide the title since we're going to be moving tabs into
+        // the titlebar which have their own title.
+        titleVisibility = .hidden
+
+        // Create a toolbar
+        let toolbar = NSToolbar(identifier: "TerminalToolbar")
+        toolbar.delegate = self
+        toolbar.centeredItemIdentifiers.insert(.title)
+        self.toolbar = toolbar
+        toolbarStyle = .unifiedCompact
+    }
+
     override func becomeMain() {
         super.becomeMain()
-        DispatchQueue.main.asyncAfter(deadline: .now() + .seconds(2)) {
+
-            self.contentView?.printViewHierarchy()
+        // Check if we have a tab bar and set it up if we have to. See the comment
-        }
+        // on this function to learn why we need to check this here.
+        setupTabBar()
     }
 
     // This is called by macOS for native tabbing in order to add the tab bar. We hook into
@@ -66,18 +72,68 @@ class TitlebarTabsTahoeTerminalWindow: TransparentTitlebarTerminalWindow, NSTool
 
         super.addTitlebarAccessoryViewController(childViewController)
 
-        // View model updates must happen on their own ticks
-        DispatchQueue.main.async {
-            self.viewModel.hasTabBar = true
-        }
-
         // Setup the tab bar to go into the titlebar.
         DispatchQueue.main.async {
             // HACK: wait a tick before doing anything, to avoid edge cases during startup... :/
             // If we don't do this then on launch windows with restored state with tabs will end
             // up with messed up tab bars that don't show all tabs.
-            let accessoryView = childViewController.view
+            self.setupTabBar()
-            guard let clipView = accessoryView.firstSuperview(withClassName: "NSTitlebarAccessoryClipView") else { return }
+        }
+    }
+
+    override func removeTitlebarAccessoryViewController(at index: Int) {
+        guard let childViewController = titlebarAccessoryViewControllers[safe: index],
+                isTabBar(childViewController) else {
+            super.removeTitlebarAccessoryViewController(at: index)
+            return
+        }
+
+        super.removeTitlebarAccessoryViewController(at: index)
+
+        removeTabBar()
+    }
+
+    // MARK: Tab Bar Setup
+
+    private var tabBarObserver: NSObjectProtocol? {
+        didSet {
+            // When we change this we want to clear our old observer
+            guard let oldValue else { return }
+            NotificationCenter.default.removeObserver(oldValue)
+        }
+    }
+
+    /// Take the NSTabBar that is on the window and convert it into titlebar tabs.
+    ///
+    /// Let me explain more background on what is happening here. When a tab bar is created, only the
+    /// main window actually has an NSTabBar. When an NSWindow in the tab group gains main, AppKit
+    /// creates/moves (unsure which) the NSTabBar for it and shows it. When it loses main, the tab bar
+    /// is removed from the view hierarchy.
+    ///
+    /// We can't detect this via `addTitlebarAccessoryViewController` because AppKit
+    /// _always_ creates an accessory view controller for every window in the tab group, but puts a
+    /// zero-sized NSView into it (that the tab bar is then attached to later).
+    ///
+    /// The best way I've found to detect this is to search for and setup the tab bar anytime the
+    /// window gains focus. There are probably edge cases to check but to resolve all this I made
+    /// this function which is idempotent to call.
+    ///
+    /// There are more scenarios to look out for and they're documented within the method.
+    func setupTabBar() {
+        // We only want to setup the observer once
+        guard tabBarObserver == nil else { return }
+
+        // Find our tab bar. If it doesn't exist we don't do anything.
+        guard let tabBar = contentView?.rootView.firstDescendant(withClassName: "NSTabBar") else { return }
+
+        // View model updates must happen on their own ticks.
+        DispatchQueue.main.async {
+            self.viewModel.hasTabBar = true
+        }
+
+        // Find our clip view
+        guard let clipView = tabBar.firstSuperview(withClassName: "NSTitlebarAccessoryClipView") else { return }
+        guard let accessoryView = clipView.subviews[safe: 0] else { return }
         guard let titlebarView = clipView.firstSuperview(withClassName: "NSTitlebarView") else { return }
         guard let toolbarView = titlebarView.firstDescendant(withClassName: "NSToolbarView") else { return }
 
@@ -94,37 +150,57 @@ class TitlebarTabsTahoeTerminalWindow: TransparentTitlebarTerminalWindow, NSTool
         // Constrain the accessory clip view (the parent of the accessory view
         // usually that clips the children) to the container view.
         clipView.translatesAutoresizingMaskIntoConstraints = false
-            clipView.leftAnchor.constraint(equalTo: container.leftAnchor, constant: leftPadding).isActive = true
-            clipView.rightAnchor.constraint(equalTo: container.rightAnchor).isActive = true
-            clipView.topAnchor.constraint(equalTo: container.topAnchor, constant: 2).isActive = true
-            clipView.heightAnchor.constraint(equalTo: container.heightAnchor).isActive = true
-            clipView.needsLayout = true
-
-            // Constrain the actual accessory view (the tab bar) to the clip view
-            // so it takes up the full space.
         accessoryView.translatesAutoresizingMaskIntoConstraints = false
-            accessoryView.leftAnchor.constraint(equalTo: clipView.leftAnchor).isActive = true
+
-            accessoryView.rightAnchor.constraint(equalTo: clipView.rightAnchor).isActive = true
+        // Setup all our constraints
-            accessoryView.topAnchor.constraint(equalTo: clipView.topAnchor).isActive = true
+        NSLayoutConstraint.activate([
-            accessoryView.heightAnchor.constraint(equalTo: clipView.heightAnchor).isActive = true
+            clipView.leftAnchor.constraint(equalTo: container.leftAnchor, constant: leftPadding),
+            clipView.rightAnchor.constraint(equalTo: container.rightAnchor),
+            clipView.topAnchor.constraint(equalTo: container.topAnchor, constant: 2),
+            clipView.heightAnchor.constraint(equalTo: container.heightAnchor),
+            accessoryView.leftAnchor.constraint(equalTo: clipView.leftAnchor),
+            accessoryView.rightAnchor.constraint(equalTo: clipView.rightAnchor),
+            accessoryView.topAnchor.constraint(equalTo: clipView.topAnchor),
+            accessoryView.heightAnchor.constraint(equalTo: clipView.heightAnchor),
+        ])
+
+        clipView.needsLayout = true
         accessoryView.needsLayout = true
+
+        // We need to setup an observer for the NSTabBar frame. When we change system
+        // appearance, the tab bar temporarily becomes width/height 0 and breaks all our
+        // constraints and AppKit responds by nuking the whole tab bar cause it doesn't
+        // know what to do with it. We need to detect this before bad things happen.
+        tabBar.postsFrameChangedNotifications = true
+        tabBarObserver = NotificationCenter.default.addObserver(
+            forName: NSView.frameDidChangeNotification,
+            object: tabBar,
+            queue: .main
+        ) { [weak self] _ in
+            guard let self else { return }
+
+            // Check if either width or height is zero
+            guard tabBar.frame.size.width == 0 || tabBar.frame.size.height == 0 else { return }
+
+            // Remove the observer so we can call setup again.
+            self.tabBarObserver = nil
+
+            // Wait a tick to let the new tab bars appear and then set them up.
+            DispatchQueue.main.async {
+                self.setupTabBar()
+            }
         }
     }
 
-    override func removeTitlebarAccessoryViewController(at index: Int) {
+    func removeTabBar() {
-        guard let childViewController = titlebarAccessoryViewControllers[safe: index],
-                isTabBar(childViewController) else {
-            super.removeTitlebarAccessoryViewController(at: index)
-            return
-        }
-
-        super.removeTitlebarAccessoryViewController(at: index)
-
         // View model needs to be updated on another tick because it
         // triggers view updates.
         DispatchQueue.main.async {
             self.viewModel.hasTabBar = false
         }
+
+        // Clear our observations
+        self.tabBarObserver = nil
     }
 
     // MARK: NSToolbarDelegate