diff --git a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp index 4b98b1a9552..12ebf60c941 100644 --- a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp +++ b/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) { InitializeJsonReader(); @@ -76,6 +80,7 @@ namespace TerminalAppLocalTests private: void _initializeTerminalPage(winrt::com_ptr& page, winrt::com_ptr& initialSettings); + winrt::com_ptr _commonSetup(); }; void TabTests::EnsureTestsActivate() @@ -517,4 +522,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: + // - + // Return Value: + // - The initialized TerminalPage, ready to use. + winrt::com_ptr 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 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 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); + } } diff --git a/src/cascadia/TerminalApp/ActionArgs.h b/src/cascadia/TerminalApp/ActionArgs.h index 3f7e829cd04..648acf2d5c9 100644 --- a/src/cascadia/TerminalApp/ActionArgs.h +++ b/src/cascadia/TerminalApp/ActionArgs.h @@ -214,6 +214,9 @@ namespace winrt::TerminalApp::implementation struct MoveFocusArgs : public MoveFocusArgsT { MoveFocusArgs() = default; + MoveFocusArgs(TerminalApp::Direction direction) : + _Direction{ direction } {}; + GETSET_PROPERTY(TerminalApp::Direction, Direction, TerminalApp::Direction::None); static constexpr std::string_view DirectionKey{ "direction" }; @@ -308,6 +311,11 @@ namespace winrt::TerminalApp::implementation struct SplitPaneArgs : public SplitPaneArgsT { SplitPaneArgs() = default; + SplitPaneArgs(winrt::TerminalApp::SplitState style, const winrt::TerminalApp::NewTerminalArgs& terminalArgs) : + _SplitStyle{ style }, + _TerminalArgs{ terminalArgs } {}; + SplitPaneArgs(SplitType splitMode) : + _SplitMode{ splitMode } {}; GETSET_PROPERTY(winrt::TerminalApp::SplitState, SplitStyle, winrt::TerminalApp::SplitState::Automatic); GETSET_PROPERTY(winrt::TerminalApp::NewTerminalArgs, TerminalArgs, nullptr); GETSET_PROPERTY(winrt::TerminalApp::SplitType, SplitMode, winrt::TerminalApp::SplitType::Manual); @@ -553,4 +561,6 @@ namespace winrt::TerminalApp::factory_implementation { BASIC_FACTORY(ActionEventArgs); BASIC_FACTORY(NewTerminalArgs); + BASIC_FACTORY(MoveFocusArgs); + BASIC_FACTORY(SplitPaneArgs); } diff --git a/src/cascadia/TerminalApp/ActionArgs.idl b/src/cascadia/TerminalApp/ActionArgs.idl index 51ecf282941..a45ab1553f5 100644 --- a/src/cascadia/TerminalApp/ActionArgs.idl +++ b/src/cascadia/TerminalApp/ActionArgs.idl @@ -87,6 +87,7 @@ namespace TerminalApp [default_interface] runtimeclass MoveFocusArgs : IActionArgs { + MoveFocusArgs(Direction direction); Direction Direction { get; }; }; @@ -102,6 +103,9 @@ namespace TerminalApp [default_interface] runtimeclass SplitPaneArgs : IActionArgs { + SplitPaneArgs(SplitState split, NewTerminalArgs terminalArgs); + SplitPaneArgs(SplitType splitMode); + SplitState SplitStyle { get; }; NewTerminalArgs TerminalArgs { get; }; SplitType SplitMode { get; }; diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp index 6d2e765c3a1..50f2a1304a4 100644 --- a/src/cascadia/TerminalApp/AppActionHandlers.cpp +++ b/src/cascadia/TerminalApp/AppActionHandlers.cpp @@ -132,10 +132,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); } diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index c58de58b59d..4465f0b38fe 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -657,6 +657,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(); diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index 43f29440745..105aea91be4 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -32,6 +32,7 @@ namespace winrt::TerminalApp::implementation }); _activePane = _rootPane; + Content(_rootPane->GetRootElement()); _MakeTabViewItem(); } @@ -58,24 +59,6 @@ namespace winrt::TerminalApp::implementation _RecalculateAndApplyTabColor(); } - // Method Description: - // - Get the root UIElement of this Tab's root pane. - // Arguments: - // - - // 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 @@ -502,6 +485,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: @@ -1012,6 +1011,7 @@ namespace winrt::TerminalApp::implementation _rootPane->Maximize(_zoomedPane); // Update the tab header to show the magnifying glass _UpdateTabHeader(); + Content(_zoomedPane->GetRootElement()); } void Tab::ExitZoom() { @@ -1019,6 +1019,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/Tab.h b/src/cascadia/TerminalApp/Tab.h index 2cc39b23375..aab1cd51382 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 GetFocusedProfile() const noexcept; @@ -77,6 +76,8 @@ namespace winrt::TerminalApp::implementation OBSERVABLE_GETSET_PROPERTY(winrt::hstring, Title, _PropertyChangedHandlers); OBSERVABLE_GETSET_PROPERTY(winrt::Windows::UI::Xaml::Controls::IconSource, IconSource, _PropertyChangedHandlers, nullptr); + OBSERVABLE_GETSET_PROPERTY(winrt::Windows::UI::Xaml::UIElement, Content, _PropertyChangedHandlers, nullptr); + private: std::shared_ptr _rootPane{ nullptr }; std::shared_ptr _activePane{ nullptr }; diff --git a/src/cascadia/TerminalApp/Tab.idl b/src/cascadia/TerminalApp/Tab.idl index 5aba64605fc..f491f18aa70 100644 --- a/src/cascadia/TerminalApp/Tab.idl +++ b/src/cascadia/TerminalApp/Tab.idl @@ -7,5 +7,6 @@ namespace TerminalApp { String Title { get; }; Windows.UI.Xaml.Controls.IconSource IconSource { get; }; + Windows.UI.Xaml.UIElement Content { get; }; } } diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 86e9e4e7a24..32ed7e54b36 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -1163,6 +1163,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); + } + } } }); @@ -1275,9 +1285,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()); } } @@ -1964,7 +1975,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