diff --git a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp index e623c9cf055..d32a41aee94 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) { return true; @@ -75,6 +79,7 @@ namespace TerminalAppLocalTests private: void _initializeTerminalPage(winrt::com_ptr& page, CascadiaSettings initialSettings); + winrt::com_ptr _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: + // - + // 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/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp index 64e2a63985c..9a998115460 100644 --- a/src/cascadia/TerminalApp/AppActionHandlers.cpp +++ b/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); } diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 96f99a6d644..8641793606d 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/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; diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index 21c8b17d39f..2f9cc705838 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(); @@ -61,24 +62,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 @@ -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,6 +1069,7 @@ namespace winrt::TerminalApp::implementation _rootPane->Maximize(_zoomedPane); // Update the tab header to show the magnifying glass _UpdateTabHeader(); + Content(_zoomedPane->GetRootElement()); } void Tab::ExitZoom() { @@ -1077,6 +1077,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 4dae6bc8744..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 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 _rootPane{ nullptr }; std::shared_ptr _activePane{ nullptr }; diff --git a/src/cascadia/TerminalApp/Tab.idl b/src/cascadia/TerminalApp/Tab.idl index db9746c99ee..7e88c587816 100644 --- a/src/cascadia/TerminalApp/Tab.idl +++ b/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); } } diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 988664bdc69..08935dfc8f1 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/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 diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.h b/src/cascadia/TerminalSettingsModel/ActionArgs.h index b53ea55b548..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() = default; + MoveFocusArgs(Model::Direction direction) : + _Direction{ direction } {}; + GETSET_PROPERTY(Model::Direction, Direction, Direction::None); static constexpr std::string_view DirectionKey{ "direction" }; @@ -370,6 +373,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); @@ -673,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 6d460484dd6..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; }; }; @@ -109,7 +110,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; };