Fix exiting a zoomed pane

src/cascadia/LocalTests_TerminalApp/TabTests.cpp

@@ -62,6 +62,10 @@ namespace TerminalAppLocalTests
         TEST_METHOD(TryDuplicateBadTab);
         TEST_METHOD(TryDuplicateBadPane);
 
+        TEST_METHOD(TryZoomPane);
+        TEST_METHOD(MoveFocusFromZoomedPane);
+        TEST_METHOD(CloseZoomedPane);
+
         TEST_CLASS_SETUP(ClassSetup)
         {
             return true;
@@ -75,6 +79,7 @@ namespace TerminalAppLocalTests
     private:
         void _initializeTerminalPage(winrt::com_ptr<winrt::TerminalApp::implementation::TerminalPage>& page,
                                      CascadiaSettings initialSettings);
+        winrt::com_ptr<winrt::TerminalApp::implementation::TerminalPage> _commonSetup();
     };
 
     void TabTests::EnsureTestsActivate()
@@ -496,4 +501,192 @@ namespace TerminalAppLocalTests
         });
     }
 
+    // Method Description:
+    // - This is a helper method for setting up a TerminalPage with some common
+    //   settings, and creating the first tab.
+    // Arguments:
+    // - <none>
+    // Return Value:
+    // - The initialized TerminalPage, ready to use.
+    winrt::com_ptr<winrt::TerminalApp::implementation::TerminalPage> TabTests::_commonSetup()
+    {
+        const std::string settingsJson0{ R"(
+        {
+            "defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}",
+            "profiles": [
+                {
+                    "name" : "profile0",
+                    "guid": "{6239a42c-1111-49a3-80bd-e8fdd045185c}",
+                    "historySize": 1
+                },
+                {
+                    "name" : "profile1",
+                    "guid": "{6239a42c-2222-49a3-80bd-e8fdd045185c}",
+                    "historySize": 2
+                }
+            ]
+        })" };
+
+        CascadiaSettings settings0{ til::u8u16(settingsJson0) };
+        VERIFY_IS_NOT_NULL(settings0);
+
+        const auto guid1 = Microsoft::Console::Utils::GuidFromString(L"{6239a42c-1111-49a3-80bd-e8fdd045185c}");
+        const auto guid2 = Microsoft::Console::Utils::GuidFromString(L"{6239a42c-2222-49a3-80bd-e8fdd045185c}");
+
+        // This is super wacky, but we can't just initialize the
+        // com_ptr<impl::TerminalPage> in the lambda and assign it back out of
+        // the lambda. We'll crash trying to get a weak_ref to the TerminalPage
+        // during TerminalPage::Create() below.
+        //
+        // Instead, create the winrt object, then get a com_ptr to the
+        // implementation _from_ the winrt object. This seems to work, even if
+        // it's weird.
+        winrt::com_ptr<winrt::TerminalApp::implementation::TerminalPage> page{ nullptr };
+        _initializeTerminalPage(page, settings0);
+
+        auto result = RunOnUIThread([&page]() {
+            VERIFY_ARE_EQUAL(1u, page->_tabs.Size());
+        });
+        VERIFY_SUCCEEDED(result);
+
+        return page;
+    }
+
+    void TabTests::TryZoomPane()
+    {
+        auto page = _commonSetup();
+
+        Log::Comment(L"Create a second pane");
+        auto result = RunOnUIThread([&page]() {
+            SplitPaneArgs args{ SplitType::Duplicate };
+            ActionEventArgs eventArgs{ args };
+            // eventArgs.Args(args);
+            page->_HandleSplitPane(nullptr, eventArgs);
+            auto firstTab = page->_GetStrongTabImpl(0);
+
+            VERIFY_ARE_EQUAL(2, firstTab->GetLeafPaneCount());
+            VERIFY_IS_FALSE(firstTab->IsZoomed());
+        });
+        VERIFY_SUCCEEDED(result);
+
+        Log::Comment(L"Zoom in on the pane");
+        result = RunOnUIThread([&page]() {
+            ActionEventArgs eventArgs{};
+            page->_HandleTogglePaneZoom(nullptr, eventArgs);
+            auto firstTab = page->_GetStrongTabImpl(0);
+            VERIFY_ARE_EQUAL(2, firstTab->GetLeafPaneCount());
+            VERIFY_IS_TRUE(firstTab->IsZoomed());
+        });
+        VERIFY_SUCCEEDED(result);
+
+        Log::Comment(L"Zoom out of the pane");
+        result = RunOnUIThread([&page]() {
+            ActionEventArgs eventArgs{};
+            page->_HandleTogglePaneZoom(nullptr, eventArgs);
+            auto firstTab = page->_GetStrongTabImpl(0);
+            VERIFY_ARE_EQUAL(2, firstTab->GetLeafPaneCount());
+            VERIFY_IS_FALSE(firstTab->IsZoomed());
+        });
+        VERIFY_SUCCEEDED(result);
+    }
+
+    void TabTests::MoveFocusFromZoomedPane()
+    {
+        auto page = _commonSetup();
+
+        Log::Comment(L"Create a second pane");
+        auto result = RunOnUIThread([&page]() {
+            // Set up action
+            SplitPaneArgs args{ SplitType::Duplicate };
+            ActionEventArgs eventArgs{ args };
+            page->_HandleSplitPane(nullptr, eventArgs);
+            auto firstTab = page->_GetStrongTabImpl(0);
+
+            VERIFY_ARE_EQUAL(2, firstTab->GetLeafPaneCount());
+            VERIFY_IS_FALSE(firstTab->IsZoomed());
+        });
+        VERIFY_SUCCEEDED(result);
+
+        Log::Comment(L"Zoom in on the pane");
+        result = RunOnUIThread([&page]() {
+            // Set up action
+            ActionEventArgs eventArgs{};
+
+            page->_HandleTogglePaneZoom(nullptr, eventArgs);
+
+            auto firstTab = page->_GetStrongTabImpl(0);
+            VERIFY_ARE_EQUAL(2, firstTab->GetLeafPaneCount());
+            VERIFY_IS_TRUE(firstTab->IsZoomed());
+        });
+        VERIFY_SUCCEEDED(result);
+
+        Log::Comment(L"Move focus. This will cause us to un-zoom.");
+        result = RunOnUIThread([&page]() {
+            // Set up action
+            MoveFocusArgs args{ Direction::Left };
+            ActionEventArgs eventArgs{ args };
+
+            page->_HandleMoveFocus(nullptr, eventArgs);
+
+            auto firstTab = page->_GetStrongTabImpl(0);
+            VERIFY_ARE_EQUAL(2, firstTab->GetLeafPaneCount());
+            VERIFY_IS_FALSE(firstTab->IsZoomed());
+        });
+        VERIFY_SUCCEEDED(result);
+    }
+
+    void TabTests::CloseZoomedPane()
+    {
+        auto page = _commonSetup();
+
+        Log::Comment(L"Create a second pane");
+        auto result = RunOnUIThread([&page]() {
+            // Set up action
+            SplitPaneArgs args{ SplitType::Duplicate };
+            ActionEventArgs eventArgs{ args };
+            page->_HandleSplitPane(nullptr, eventArgs);
+            auto firstTab = page->_GetStrongTabImpl(0);
+
+            VERIFY_ARE_EQUAL(2, firstTab->GetLeafPaneCount());
+            VERIFY_IS_FALSE(firstTab->IsZoomed());
+        });
+        VERIFY_SUCCEEDED(result);
+
+        Log::Comment(L"Zoom in on the pane");
+        result = RunOnUIThread([&page]() {
+            // Set up action
+            ActionEventArgs eventArgs{};
+
+            page->_HandleTogglePaneZoom(nullptr, eventArgs);
+
+            auto firstTab = page->_GetStrongTabImpl(0);
+            VERIFY_ARE_EQUAL(2, firstTab->GetLeafPaneCount());
+            VERIFY_IS_TRUE(firstTab->IsZoomed());
+        });
+        VERIFY_SUCCEEDED(result);
+
+        Log::Comment(L"Close Pane. This should cause us to un-zoom, and remove the second pane from the tree");
+        result = RunOnUIThread([&page]() {
+            // Set up action
+            ActionEventArgs eventArgs{};
+
+            page->_HandleClosePane(nullptr, eventArgs);
+
+            auto firstTab = page->_GetStrongTabImpl(0);
+            VERIFY_IS_FALSE(firstTab->IsZoomed());
+        });
+        VERIFY_SUCCEEDED(result);
+
+        // Introduce a slight delay to let the events finish propagating
+        Sleep(250);
+
+        Log::Comment(L"Check to ensure there's only one pane left.");
+
+        result = RunOnUIThread([&page]() {
+            auto firstTab = page->_GetStrongTabImpl(0);
+            VERIFY_ARE_EQUAL(1, firstTab->GetLeafPaneCount());
+            VERIFY_IS_FALSE(firstTab->IsZoomed());
+        });
+        VERIFY_SUCCEEDED(result);
+    }
 }

src/cascadia/TerminalApp/AppActionHandlers.cpp

@@ -133,10 +133,9 @@ namespace winrt::TerminalApp::implementation
             // be removed before it's re-added in Pane::Restore
             _tabContent.Children().Clear();
 
+            // Togging the zoom on the tab will cause the tab to inform us of
+            // the new root Content for this tab.
             activeTab->ToggleZoom();
-
-            // Update the selected tab, to trigger us to re-add the tab's GetRootElement to the UI tree
-            _UpdatedSelectedTab(_tabView.SelectedIndex());
         }
         args.Handled(true);
     }

src/cascadia/TerminalApp/Pane.cpp

@@ -702,6 +702,16 @@ void Pane::_CloseChild(const bool closeFirst)
         if (_lastActive)
         {
             _control.Focus(FocusState::Programmatic);
+
+            // See GH#7252
+            // Manually fire off the GotFocus event. Typically, this is done
+            // automatically when the control gets focused. However, if we're
+            // `exit`ing a zoomed pane, then the other sibling isn't in the UI
+            // tree currently. So the above call to Focus won't actually focus
+            // the control. Because Tab is relying on GotFocus to know who the
+            // active pane in the tree is, without this call, _no one_ will be
+            // the active pane any longer.
+            _GotFocusHandlers(shared_from_this());
         }
 
         _UpdateBorders();
@@ -813,11 +823,13 @@ winrt::fire_and_forget Pane::_CloseChildRoutine(const bool closeFirst)
         const auto animationsEnabledInOS = uiSettings.AnimationsEnabled();
         const auto animationsEnabledInApp = Media::Animation::Timeline::AllowDependentAnimations();
 
+        // GH#7252: If either child is zoomed, just skip the animation. It won't work.
+        const bool eitherChildZoomed = pane->_firstChild->_zoomed || pane->_secondChild->_zoomed;
         // If animations are disabled, just skip this and go straight to
         // _CloseChild. Curiously, the pane opening animation doesn't need this,
         // and will skip straight to Completed when animations are disabled, but
         // this one doesn't seem to.
-        if (!animationsEnabledInOS || !animationsEnabledInApp)
+        if (!animationsEnabledInOS || !animationsEnabledInApp || eitherChildZoomed)
         {
             pane->_CloseChild(closeFirst);
             co_return;

src/cascadia/TerminalApp/Tab.cpp

@@ -33,6 +33,7 @@ namespace winrt::TerminalApp::implementation
         });
 
         _activePane = _rootPane;
+        Content(_rootPane->GetRootElement());
 
         _MakeTabViewItem();
         _MakeSwitchToTabCommand();
@@ -61,24 +62,6 @@ namespace winrt::TerminalApp::implementation
         _RecalculateAndApplyTabColor();
     }
 
-    // Method Description:
-    // - Get the root UIElement of this Tab's root pane.
-    // Arguments:
-    // - <none>
-    // Return Value:
-    // - The UIElement acting as root of the Tab's root pane.
-    UIElement Tab::GetRootElement()
-    {
-        if (_zoomedPane)
-        {
-            return _zoomedPane->GetRootElement();
-        }
-        else
-        {
-            return _rootPane->GetRootElement();
-        }
-    }
-
     // Method Description:
     // - Returns nullptr if no children of this tab were the last control to be
     //   focused, or the TermControl that _was_ the last control to be focused (if
@@ -509,6 +492,22 @@ namespace winrt::TerminalApp::implementation
                 tab->_RecalculateAndApplyTabColor();
             }
         });
+
+        // Add a Closed event handler to the Pane. If the pane closes out from
+        // underneath us, and it's zoomed, we want to be able to make sure to
+        // update our state accordingly to un-zoom that pane. See GH#7252.
+        pane->Closed([weakThis](auto&& /*s*/, auto && /*e*/) -> winrt::fire_and_forget {
+            if (auto tab{ weakThis.get() })
+            {
+                if (tab->_zoomedPane)
+                {
+                    co_await winrt::resume_foreground(tab->Content().Dispatcher());
+
+                    tab->Content(tab->_rootPane->GetRootElement());
+                    tab->ExitZoom();
+                }
+            }
+        });
     }
 
     // Method Description:
@@ -1070,13 +1069,15 @@ namespace winrt::TerminalApp::implementation
         _rootPane->Maximize(_zoomedPane);
         // Update the tab header to show the magnifying glass
         _UpdateTabHeader();
+        Content(_zoomedPane->GetRootElement());
     }
     void Tab::ExitZoom()
     {
         _rootPane->Restore(_zoomedPane);
         _zoomedPane = nullptr;
         // Update the tab header to hide the magnifying glass
         _UpdateTabHeader();
+        Content(_rootPane->GetRootElement());
     }
 
     bool Tab::IsZoomed()

src/cascadia/TerminalApp/Tab.h

@@ -24,7 +24,6 @@ namespace winrt::TerminalApp::implementation
         void Initialize(const winrt::Microsoft::Terminal::TerminalControl::TermControl& control);
 
         winrt::Microsoft::UI::Xaml::Controls::TabViewItem GetTabViewItem();
-        winrt::Windows::UI::Xaml::UIElement GetRootElement();
         winrt::Microsoft::Terminal::TerminalControl::TermControl GetActiveTerminalControl() const;
         std::optional<GUID> GetFocusedProfile() const noexcept;
 
@@ -88,6 +87,8 @@ namespace winrt::TerminalApp::implementation
         // The TabViewNumTabs is the number of Tab objects in TerminalPage's _tabs vector.
         OBSERVABLE_GETSET_PROPERTY(uint32_t, TabViewNumTabs, _PropertyChangedHandlers, 0);
 
+        OBSERVABLE_GETSET_PROPERTY(winrt::Windows::UI::Xaml::UIElement, Content, _PropertyChangedHandlers, nullptr);
+
     private:
         std::shared_ptr<Pane> _rootPane{ nullptr };
         std::shared_ptr<Pane> _activePane{ nullptr };

src/cascadia/TerminalApp/Tab.idl

@@ -11,6 +11,8 @@ namespace TerminalApp
         Microsoft.Terminal.Settings.Model.Command SwitchToTabCommand { get; };
         UInt32 TabViewIndex { get; };
 
+        Windows.UI.Xaml.UIElement Content { get; };
+
         void SetDispatch(ShortcutActionDispatch dispatch);
     }
 }

src/cascadia/TerminalApp/TerminalPage.cpp

@@ -1140,6 +1140,16 @@ namespace winrt::TerminalApp::implementation
                 {
                     page->_UpdateTitle(*tab);
                 }
+                else if (args.PropertyName() == L"Content")
+                {
+                    if (tab == page->_GetFocusedTab())
+                    {
+                        page->_tabContent.Children().Clear();
+                        page->_tabContent.Children().Append(tab->Content());
+
+                        tab->SetFocused(true);
+                    }
+                }
             }
         });
 
@@ -1252,9 +1262,10 @@ namespace winrt::TerminalApp::implementation
             // Remove the content from the tab first, so Pane::UnZoom can
             // re-attach the content to the tree w/in the pane
             _tabContent.Children().Clear();
+            // In ExitZoom, we'll change the Tab's Content(), triggering the
+            // content changed event, which will re-attach the tab's new content
+            // root to the tree.
             activeTab->ExitZoom();
-            // Re-attach the tab's content to the UI tree.
-            _tabContent.Children().Append(activeTab->GetRootElement());
         }
     }
 
@@ -1945,7 +1956,7 @@ namespace winrt::TerminalApp::implementation
                 auto tab{ _GetStrongTabImpl(index) };
 
                 _tabContent.Children().Clear();
-                _tabContent.Children().Append(tab->GetRootElement());
+                _tabContent.Children().Append(tab->Content());
 
                 // GH#7409: If the tab switcher is open, then we _don't_ want to
                 // automatically focus the new tab here. The tab switcher wants