From 4a67a5ab7d1072f02de370af1ced433ad455807d Mon Sep 17 00:00:00 2001
From: Mike Griese <migrie@microsoft.com>
Date: Tue, 13 Oct 2020 05:44:58 -0500
Subject: [PATCH 1/8] This nearly works, I just need to correctly identify the
 activated pane

---
 src/cascadia/TerminalApp/Pane.cpp         |  9 ++++++
 src/cascadia/TerminalApp/Tab.cpp          | 35 +++++++++++++++++++++++
 src/cascadia/TerminalApp/Tab.h            |  2 ++
 src/cascadia/TerminalApp/Tab.idl          |  2 ++
 src/cascadia/TerminalApp/TerminalPage.cpp | 10 +++++++
 5 files changed, 58 insertions(+)

diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp
index c58de58b59d..d1bf48ed796 100644
--- a/src/cascadia/TerminalApp/Pane.cpp
+++ b/src/cascadia/TerminalApp/Pane.cpp
@@ -327,6 +327,15 @@ void Pane::_ControlConnectionStateChangedHandler(const TermControl& /*sender*/,
         if ((mode == winrt::TerminalApp::CloseOnExitMode::Always) ||
             (mode == winrt::TerminalApp::CloseOnExitMode::Graceful && newConnectionState == ConnectionState::Closed))
         {
+            // We need to call Restore on the root of the tree, or at least our
+            // parent, to get us re-added ot their content. However, we also
+            // want to close our content here too, and remove it from the UI
+            // tree.
+
+            // if (_zoomed)
+            // {
+            //     Restore(shared_from_this());
+            // };
             _ClosedHandlers(nullptr, nullptr);
         }
     }
diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp
index f7d33efd754..7d8461f0ba8 100644
--- a/src/cascadia/TerminalApp/Tab.cpp
+++ b/src/cascadia/TerminalApp/Tab.cpp
@@ -511,6 +511,41 @@ namespace winrt::TerminalApp::implementation
                 tab->_RecalculateAndApplyTabColor();
             }
         });
+
+        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->GetRootElement().Dispatcher());
+                    tab->ExitZoom();
+                    tab->Content(tab->GetRootElement());
+                    // if (tab->_focused)
+                    // {
+                    //     tab->_Focus();
+                    // }
+                    // tab->_UpdateActivePane(tab->_rootPane->GetActivePane());
+                }
+
+                // OKAY I see what's happening here the ActivePaneChanged
+                // Handler in TerminalPage doesn't re-attach the tab content to
+                // the tree, it just updates hte title of the window.
+                //
+                // So when the pane is `exit`ed, the pane's control is removed
+                // and re-attached to the parent grid, which _isn't in the XAML
+                // tree_. And no one can go tell the TerminalPage that it needs
+                // to re set up the tab content again.
+                //
+                // The Page _manually_ does this in a few places, when various
+                // pane actions are about to take place, it'll unzoom. It would
+                // be way easier if the Tab could just manage the content of the
+                // page.
+                //
+                // Or if the Tab just had a Content that was observable, that
+                // when that changed, the page would auto readjust. That does
+                // sound like a LOT of work though.
+            }
+        });
     }
 
     // Method Description:
diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h
index 3b52488f5d0..63503d24a5b 100644
--- a/src/cascadia/TerminalApp/Tab.h
+++ b/src/cascadia/TerminalApp/Tab.h
@@ -84,6 +84,8 @@ namespace winrt::TerminalApp::implementation
         // This is needed since Tab is going to be managing its own SwitchToTab command.
         OBSERVABLE_GETSET_PROPERTY(uint32_t, TabViewIndex, _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 };
diff --git a/src/cascadia/TerminalApp/Tab.idl b/src/cascadia/TerminalApp/Tab.idl
index ceb52a55f8f..959486f0776 100644
--- a/src/cascadia/TerminalApp/Tab.idl
+++ b/src/cascadia/TerminalApp/Tab.idl
@@ -11,5 +11,7 @@ namespace TerminalApp
         Windows.UI.Xaml.Controls.IconSource IconSource { get; };
         Command SwitchToTabCommand { get; };
         UInt32 TabViewIndex { get; };
+
+        Windows.UI.Xaml.UIElement Content { get; };
     }
 }
diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
index 8dc7142a3f0..be04d94c7cd 100644
--- a/src/cascadia/TerminalApp/TerminalPage.cpp
+++ b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -1160,6 +1160,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);
+                    }
+                }
             }
         });
 

From 1e9d1d6364be806e61db2a81ca752582104fd640 Mon Sep 17 00:00:00 2001
From: Mike Griese <migrie@microsoft.com>
Date: Mon, 19 Oct 2020 10:53:27 -0500
Subject: [PATCH 2/8] This at least has the focus land in the right place after
 the tab closes, but it doesn't play nicely with the animation.

Also, there's a bit on L714 in Pane.cpp from the previous commit that should be a part of this commit, not the merge -___-
---
 src/cascadia/TerminalApp/Pane.cpp | 6 ++++--
 src/cascadia/TerminalApp/Tab.cpp  | 3 ++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp
index 71d8f169aac..adc4b56c01c 100644
--- a/src/cascadia/TerminalApp/Pane.cpp
+++ b/src/cascadia/TerminalApp/Pane.cpp
@@ -1555,8 +1555,10 @@ void Pane::Restore(std::shared_ptr<Pane> zoomedPane)
             // When we're un-zooming the pane, we'll need to re-add it to our UI
             // tree where it originally belonged. easy way: just re-add both.
             _root.Children().Clear();
-            _root.Children().Append(_firstChild->GetRootElement());
-            _root.Children().Append(_secondChild->GetRootElement());
+            auto firstRoot = _firstChild->GetRootElement();
+            auto secondRoot = _secondChild->GetRootElement();
+            _root.Children().Append(firstRoot);
+            _root.Children().Append(secondRoot);
         }
 
         // Always recurse into both children. If the (un)zoomed pane was one of
diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp
index e686359383c..3f1a960ca8b 100644
--- a/src/cascadia/TerminalApp/Tab.cpp
+++ b/src/cascadia/TerminalApp/Tab.cpp
@@ -517,8 +517,9 @@ namespace winrt::TerminalApp::implementation
                 if (tab->_zoomedPane)
                 {
                     co_await winrt::resume_foreground(tab->GetRootElement().Dispatcher());
+                    tab->Content(tab->_rootPane->GetRootElement());
                     tab->ExitZoom();
-                    tab->Content(tab->GetRootElement());
+                    // tab->_zoomedPane = nullptr;
                     // if (tab->_focused)
                     // {
                     //     tab->_Focus();

From 49e247a76358f2ce667937c2e4b9daec83fe738b Mon Sep 17 00:00:00 2001
From: Mike Griese <migrie@microsoft.com>
Date: Mon, 19 Oct 2020 11:38:28 -0500
Subject: [PATCH 3/8] H O O P L A

---
 src/cascadia/TerminalApp/Pane.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp
index adc4b56c01c..a242923d57a 100644
--- a/src/cascadia/TerminalApp/Pane.cpp
+++ b/src/cascadia/TerminalApp/Pane.cpp
@@ -823,11 +823,12 @@ winrt::fire_and_forget Pane::_CloseChildRoutine(const bool closeFirst)
         const auto animationsEnabledInOS = uiSettings.AnimationsEnabled();
         const auto animationsEnabledInApp = Media::Animation::Timeline::AllowDependentAnimations();
 
+        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;

From 042ec669eec5817a75950ef7f673f9b0cb56cad1 Mon Sep 17 00:00:00 2001
From: Mike Griese <migrie@microsoft.com>
Date: Mon, 19 Oct 2020 13:09:23 -0500
Subject: [PATCH 4/8] Trying to clean this up

  Closing a pane that's zoomed doesn't work, so I thought it might be fun to add a unit test. This is a test of _creating_ a zoomed pane, not closing the zoomed one, but we'll get there.
---
 .../LocalTests_TerminalApp/TabTests.cpp       | 65 +++++++++++++++++++
 .../TerminalApp/AppActionHandlers.cpp         |  3 +-
 src/cascadia/TerminalApp/Pane.cpp             |  6 +-
 src/cascadia/TerminalApp/Tab.cpp              |  3 +
 src/cascadia/TerminalApp/TerminalPage.cpp     |  4 +-
 .../TerminalSettingsModel/ActionArgs.h        |  2 +
 .../TerminalSettingsModel/ActionArgs.idl      |  4 +-
 7 files changed, 78 insertions(+), 9 deletions(-)

diff --git a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp
index e623c9cf055..33bca0eec99 100644
--- a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp
+++ b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp
@@ -62,6 +62,8 @@ namespace TerminalAppLocalTests
         TEST_METHOD(TryDuplicateBadTab);
         TEST_METHOD(TryDuplicateBadPane);
 
+        TEST_METHOD(TryZoomPane);
+
         TEST_CLASS_SETUP(ClassSetup)
         {
             return true;
@@ -496,4 +498,67 @@ namespace TerminalAppLocalTests
         });
     }
 
+    void TabTests::TryZoomPane()
+    {
+        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}");
+
+        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);
+
+        // Log::Comment(L"Duplicate the first tab");
+        // result = RunOnUIThread([&page]() {
+        //     page->_DuplicateTabViewItem();
+        //     VERIFY_ARE_EQUAL(2u, page->_tabs.Size());
+        // });
+        // VERIFY_SUCCEEDED(result);
+
+        Log::Comment(L"Create a second pane");
+        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);
+    }
 }
diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp
index 64e2a63985c..f83c408b53e 100644
--- a/src/cascadia/TerminalApp/AppActionHandlers.cpp
+++ b/src/cascadia/TerminalApp/AppActionHandlers.cpp
@@ -132,11 +132,10 @@ namespace winrt::TerminalApp::implementation
             // a pane is zoomed, then it's currently in the UI tree, and should
             // be removed before it's re-added in Pane::Restore
             _tabContent.Children().Clear();
-
             activeTab->ToggleZoom();
 
             // Update the selected tab, to trigger us to re-add the tab's GetRootElement to the UI tree
-            _UpdatedSelectedTab(_tabView.SelectedIndex());
+            // _UpdatedSelectedTab(_tabView.SelectedIndex());
         }
         args.Handled(true);
     }
diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp
index a242923d57a..3f651e04ba4 100644
--- a/src/cascadia/TerminalApp/Pane.cpp
+++ b/src/cascadia/TerminalApp/Pane.cpp
@@ -1556,10 +1556,8 @@ void Pane::Restore(std::shared_ptr<Pane> zoomedPane)
             // When we're un-zooming the pane, we'll need to re-add it to our UI
             // tree where it originally belonged. easy way: just re-add both.
             _root.Children().Clear();
-            auto firstRoot = _firstChild->GetRootElement();
-            auto secondRoot = _secondChild->GetRootElement();
-            _root.Children().Append(firstRoot);
-            _root.Children().Append(secondRoot);
+            _root.Children().Append(_firstChild->GetRootElement());
+            _root.Children().Append(_secondChild->GetRootElement());
         }
 
         // Always recurse into both children. If the (un)zoomed pane was one of
diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp
index 3f1a960ca8b..130202ca107 100644
--- a/src/cascadia/TerminalApp/Tab.cpp
+++ b/src/cascadia/TerminalApp/Tab.cpp
@@ -33,6 +33,7 @@ namespace winrt::TerminalApp::implementation
         });
 
         _activePane = _rootPane;
+        Content(_rootPane->GetRootElement());
 
         _MakeTabViewItem();
         _MakeSwitchToTabCommand();
@@ -1107,6 +1108,7 @@ namespace winrt::TerminalApp::implementation
         _rootPane->Maximize(_zoomedPane);
         // Update the tab header to show the magnifying glass
         _UpdateTabHeader();
+        Content(_zoomedPane->GetRootElement());
     }
     void Tab::ExitZoom()
     {
@@ -1114,6 +1116,7 @@ namespace winrt::TerminalApp::implementation
         _zoomedPane = nullptr;
         // Update the tab header to hide the magnifying glass
         _UpdateTabHeader();
+        Content(_rootPane->GetRootElement());
     }
 
     bool Tab::IsZoomed()
diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
index a15a17ef23e..066edabd8de 100644
--- a/src/cascadia/TerminalApp/TerminalPage.cpp
+++ b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -1264,7 +1264,7 @@ namespace winrt::TerminalApp::implementation
             _tabContent.Children().Clear();
             activeTab->ExitZoom();
             // Re-attach the tab's content to the UI tree.
-            _tabContent.Children().Append(activeTab->GetRootElement());
+            _tabContent.Children().Append(activeTab->Content());
         }
     }
 
@@ -1955,7 +1955,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
diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.h b/src/cascadia/TerminalSettingsModel/ActionArgs.h
index b53ea55b548..60eb53b3407 100644
--- a/src/cascadia/TerminalSettingsModel/ActionArgs.h
+++ b/src/cascadia/TerminalSettingsModel/ActionArgs.h
@@ -370,6 +370,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         SplitPaneArgs(SplitState style, const Model::NewTerminalArgs& terminalArgs) :
             _SplitStyle{ style },
             _TerminalArgs{ terminalArgs } {};
+        SplitPaneArgs(SplitType splitMode) :
+            _SplitMode{ splitMode } {};
         GETSET_PROPERTY(SplitState, SplitStyle, SplitState::Automatic);
         GETSET_PROPERTY(Model::NewTerminalArgs, TerminalArgs, nullptr);
         GETSET_PROPERTY(SplitType, SplitMode, SplitType::Manual);
diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.idl b/src/cascadia/TerminalSettingsModel/ActionArgs.idl
index 6d460484dd6..7f3490798dc 100644
--- a/src/cascadia/TerminalSettingsModel/ActionArgs.idl
+++ b/src/cascadia/TerminalSettingsModel/ActionArgs.idl
@@ -109,7 +109,9 @@ namespace Microsoft.Terminal.Settings.Model
 
     [default_interface] runtimeclass SplitPaneArgs : IActionArgs
     {
-        SplitPaneArgs(SplitState style, NewTerminalArgs terminalArgs);
+        SplitPaneArgs(SplitState split, NewTerminalArgs terminalArgs);
+        SplitPaneArgs(SplitType splitMode);
+
         SplitState SplitStyle { get; };
         NewTerminalArgs TerminalArgs { get; };
         SplitType SplitMode { get; };

From 5cf17707b2b7e2fc9a806c30444d9f9f989961fc Mon Sep 17 00:00:00 2001
From: Mike Griese <migrie@microsoft.com>
Date: Mon, 19 Oct 2020 21:29:19 -0500
Subject: [PATCH 5/8] It might just be the middle of the night, but I think
 this works

---
 .../LocalTests_TerminalApp/TabTests.cpp       | 121 +++++++++++++++++-
 src/cascadia/TerminalApp/TerminalPage.cpp     |   2 +-
 .../TerminalSettingsModel/ActionArgs.h        |   4 +
 .../TerminalSettingsModel/ActionArgs.idl      |   1 +
 4 files changed, 125 insertions(+), 3 deletions(-)

diff --git a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp
index 33bca0eec99..26b99640fd3 100644
--- a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp
+++ b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp
@@ -63,6 +63,8 @@ namespace TerminalAppLocalTests
         TEST_METHOD(TryDuplicateBadPane);
 
         TEST_METHOD(TryZoomPane);
+        TEST_METHOD(MoveFocusFromZoomedPane);
+        TEST_METHOD(CloseZoomedPane);
 
         TEST_CLASS_SETUP(ClassSetup)
         {
@@ -77,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()
@@ -498,7 +501,7 @@ namespace TerminalAppLocalTests
         });
     }
 
-    void TabTests::TryZoomPane()
+    winrt::com_ptr<winrt::TerminalApp::implementation::TerminalPage> TabTests::_commonSetup()
     {
         const std::string settingsJson0{ R"(
         {
@@ -523,6 +526,14 @@ namespace TerminalAppLocalTests
         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);
 
@@ -531,6 +542,11 @@ namespace TerminalAppLocalTests
         });
         VERIFY_SUCCEEDED(result);
 
+        return page;
+    }
+
+    void TabTests::TryZoomPane()
+    {
         // Log::Comment(L"Duplicate the first tab");
         // result = RunOnUIThread([&page]() {
         //     page->_DuplicateTabViewItem();
@@ -538,8 +554,10 @@ namespace TerminalAppLocalTests
         // });
         // VERIFY_SUCCEEDED(result);
 
+        auto page = _commonSetup();
+
         Log::Comment(L"Create a second pane");
-        result = RunOnUIThread([&page]() {
+        auto result = RunOnUIThread([&page]() {
             SplitPaneArgs args{ SplitType::Duplicate };
             ActionEventArgs eventArgs{ args };
             // eventArgs.Args(args);
@@ -560,5 +578,104 @@ namespace TerminalAppLocalTests
             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 unzoom.");
+        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 unzoom, 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_ARE_EQUAL(1, firstTab->GetLeafPaneCount());
+            VERIFY_IS_TRUE(firstTab->IsZoomed());
+        });
+        VERIFY_SUCCEEDED(result);
     }
 }
diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
index 066edabd8de..ab9079adb8e 100644
--- a/src/cascadia/TerminalApp/TerminalPage.cpp
+++ b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -1264,7 +1264,7 @@ namespace winrt::TerminalApp::implementation
             _tabContent.Children().Clear();
             activeTab->ExitZoom();
             // Re-attach the tab's content to the UI tree.
-            _tabContent.Children().Append(activeTab->Content());
+            // _tabContent.Children().Append(activeTab->Content());
         }
     }
 
diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.h b/src/cascadia/TerminalSettingsModel/ActionArgs.h
index 60eb53b3407..c70e438373e 100644
--- a/src/cascadia/TerminalSettingsModel/ActionArgs.h
+++ b/src/cascadia/TerminalSettingsModel/ActionArgs.h
@@ -255,6 +255,9 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
     struct MoveFocusArgs : public MoveFocusArgsT<MoveFocusArgs>
     {
         MoveFocusArgs() = default;
+        MoveFocusArgs(Model::Direction direction) :
+            _Direction{ direction } {};
+
         GETSET_PROPERTY(Model::Direction, Direction, Direction::None);
 
         static constexpr std::string_view DirectionKey{ "direction" };
@@ -675,6 +678,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::factory_implementation
     BASIC_FACTORY(SwitchToTabArgs);
     BASIC_FACTORY(NewTerminalArgs);
     BASIC_FACTORY(NewTabArgs);
+    BASIC_FACTORY(MoveFocusArgs);
     BASIC_FACTORY(SplitPaneArgs);
     BASIC_FACTORY(ExecuteCommandlineArgs);
     BASIC_FACTORY(CloseOtherTabsArgs);
diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.idl b/src/cascadia/TerminalSettingsModel/ActionArgs.idl
index 7f3490798dc..7a4d7eb0bf6 100644
--- a/src/cascadia/TerminalSettingsModel/ActionArgs.idl
+++ b/src/cascadia/TerminalSettingsModel/ActionArgs.idl
@@ -94,6 +94,7 @@ namespace Microsoft.Terminal.Settings.Model
 
     [default_interface] runtimeclass MoveFocusArgs : IActionArgs
     {
+        MoveFocusArgs(Direction direction);
         Direction Direction { get; };
     };
 

From 187354915cef54766da1ded9ecf42852084c52d3 Mon Sep 17 00:00:00 2001
From: Mike Griese <migrie@microsoft.com>
Date: Mon, 19 Oct 2020 22:06:49 -0500
Subject: [PATCH 6/8] cleanup for review

---
 .../LocalTests_TerminalApp/TabTests.cpp       | 27 +++++++++++++------
 src/cascadia/TerminalApp/Tab.cpp              | 20 +-------------
 src/cascadia/TerminalApp/Tab.h                |  1 -
 src/cascadia/TerminalApp/TerminalPage.cpp     |  5 ++--
 4 files changed, 23 insertions(+), 30 deletions(-)

diff --git a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp
index 26b99640fd3..1f57735da53 100644
--- a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp
+++ b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp
@@ -501,6 +501,13 @@ 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 initalized TerminalPage, ready to use.
     winrt::com_ptr<winrt::TerminalApp::implementation::TerminalPage> TabTests::_commonSetup()
     {
         const std::string settingsJson0{ R"(
@@ -547,13 +554,6 @@ namespace TerminalAppLocalTests
 
     void TabTests::TryZoomPane()
     {
-        // Log::Comment(L"Duplicate the first tab");
-        // result = RunOnUIThread([&page]() {
-        //     page->_DuplicateTabViewItem();
-        //     VERIFY_ARE_EQUAL(2u, page->_tabs.Size());
-        // });
-        // VERIFY_SUCCEEDED(result);
-
         auto page = _commonSetup();
 
         Log::Comment(L"Create a second pane");
@@ -672,9 +672,20 @@ namespace TerminalAppLocalTests
 
             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 propogating
+        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_TRUE(firstTab->IsZoomed());
+            VERIFY_IS_FALSE(firstTab->IsZoomed());
         });
         VERIFY_SUCCEEDED(result);
     }
diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp
index 130202ca107..f2a1486acf1 100644
--- a/src/cascadia/TerminalApp/Tab.cpp
+++ b/src/cascadia/TerminalApp/Tab.cpp
@@ -61,24 +61,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
@@ -517,7 +499,7 @@ namespace winrt::TerminalApp::implementation
             {
                 if (tab->_zoomedPane)
                 {
-                    co_await winrt::resume_foreground(tab->GetRootElement().Dispatcher());
+                    co_await winrt::resume_foreground(tab->Content().Dispatcher());
                     tab->Content(tab->_rootPane->GetRootElement());
                     tab->ExitZoom();
                     // tab->_zoomedPane = nullptr;
diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h
index c802ad7e23b..adfa4307b7b 100644
--- a/src/cascadia/TerminalApp/Tab.h
+++ b/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;
 
diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
index ab9079adb8e..08935dfc8f1 100644
--- a/src/cascadia/TerminalApp/TerminalPage.cpp
+++ b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -1262,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->Content());
         }
     }
 

From b032c1bda6108ce935bbede204a38e72f8b1cf8d Mon Sep 17 00:00:00 2001
From: Mike Griese <migrie@microsoft.com>
Date: Mon, 19 Oct 2020 22:17:37 -0500
Subject: [PATCH 7/8] Cleanup for review 2: 2clean4you

---
 .../TerminalApp/AppActionHandlers.cpp         |  6 ++--
 src/cascadia/TerminalApp/Pane.cpp             | 19 +++++++------
 src/cascadia/TerminalApp/Tab.cpp              | 28 +++----------------
 3 files changed, 17 insertions(+), 36 deletions(-)

diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp
index f83c408b53e..9a998115460 100644
--- a/src/cascadia/TerminalApp/AppActionHandlers.cpp
+++ b/src/cascadia/TerminalApp/AppActionHandlers.cpp
@@ -132,10 +132,10 @@ namespace winrt::TerminalApp::implementation
             // a pane is zoomed, then it's currently in the UI tree, and should
             // be removed before it's re-added in Pane::Restore
             _tabContent.Children().Clear();
-            activeTab->ToggleZoom();
 
-            // Update the selected tab, to trigger us to re-add the tab's GetRootElement to the UI tree
-            // _UpdatedSelectedTab(_tabView.SelectedIndex());
+            // Togging the zoom on the tab will cause the tab to inform us of
+            // the new root Content for this tab.
+            activeTab->ToggleZoom();
         }
         args.Handled(true);
     }
diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp
index 3f651e04ba4..8641793606d 100644
--- a/src/cascadia/TerminalApp/Pane.cpp
+++ b/src/cascadia/TerminalApp/Pane.cpp
@@ -344,15 +344,6 @@ void Pane::_ControlConnectionStateChangedHandler(const TermControl& /*sender*/,
         if ((mode == CloseOnExitMode::Always) ||
             (mode == CloseOnExitMode::Graceful && newConnectionState == ConnectionState::Closed))
         {
-            // We need to call Restore on the root of the tree, or at least our
-            // parent, to get us re-added ot their content. However, we also
-            // want to close our content here too, and remove it from the UI
-            // tree.
-
-            // if (_zoomed)
-            // {
-            //     Restore(shared_from_this());
-            // };
             Close();
         }
     }
@@ -711,6 +702,15 @@ 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());
         }
 
@@ -823,6 +823,7 @@ 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,
diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp
index 374d0499335..2f9cc705838 100644
--- a/src/cascadia/TerminalApp/Tab.cpp
+++ b/src/cascadia/TerminalApp/Tab.cpp
@@ -493,39 +493,19 @@ namespace winrt::TerminalApp::implementation
             }
         });
 
+        // 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();
-                    // tab->_zoomedPane = nullptr;
-                    // if (tab->_focused)
-                    // {
-                    //     tab->_Focus();
-                    // }
-                    // tab->_UpdateActivePane(tab->_rootPane->GetActivePane());
                 }
-
-                // OKAY I see what's happening here the ActivePaneChanged
-                // Handler in TerminalPage doesn't re-attach the tab content to
-                // the tree, it just updates hte title of the window.
-                //
-                // So when the pane is `exit`ed, the pane's control is removed
-                // and re-attached to the parent grid, which _isn't in the XAML
-                // tree_. And no one can go tell the TerminalPage that it needs
-                // to re set up the tab content again.
-                //
-                // The Page _manually_ does this in a few places, when various
-                // pane actions are about to take place, it'll unzoom. It would
-                // be way easier if the Tab could just manage the content of the
-                // page.
-                //
-                // Or if the Tab just had a Content that was observable, that
-                // when that changed, the page would auto readjust. That does
-                // sound like a LOT of work though.
             }
         });
     }

From cafec41c802fd8b75a045fc5ab6233b94f9e8df7 Mon Sep 17 00:00:00 2001
From: Mike Griese <migrie@microsoft.com>
Date: Mon, 19 Oct 2020 22:26:37 -0500
Subject: [PATCH 8/8] a couple spellcheck nits

---
 src/cascadia/LocalTests_TerminalApp/TabTests.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp
index 1f57735da53..d32a41aee94 100644
--- a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp
+++ b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp
@@ -507,7 +507,7 @@ namespace TerminalAppLocalTests
     // Arguments:
     // - <none>
     // Return Value:
-    // - The initalized TerminalPage, ready to use.
+    // - The initialized TerminalPage, ready to use.
     winrt::com_ptr<winrt::TerminalApp::implementation::TerminalPage> TabTests::_commonSetup()
     {
         const std::string settingsJson0{ R"(
@@ -620,7 +620,7 @@ namespace TerminalAppLocalTests
         });
         VERIFY_SUCCEEDED(result);
 
-        Log::Comment(L"Move focus. This will cause us to unzoom.");
+        Log::Comment(L"Move focus. This will cause us to un-zoom.");
         result = RunOnUIThread([&page]() {
             // Set up action
             MoveFocusArgs args{ Direction::Left };
@@ -665,7 +665,7 @@ namespace TerminalAppLocalTests
         });
         VERIFY_SUCCEEDED(result);
 
-        Log::Comment(L"Close Pane. This should cause us to unzoom, and remove the second pane from the tree");
+        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{};
@@ -677,7 +677,7 @@ namespace TerminalAppLocalTests
         });
         VERIFY_SUCCEEDED(result);
 
-        // Introduce a slight delay to let the events finish propogating
+        // Introduce a slight delay to let the events finish propagating
         Sleep(250);
 
         Log::Comment(L"Check to ensure there's only one pane left.");