From 674c9182d7a6eb43513d4e6fbbffa9b411a5203e Mon Sep 17 00:00:00 2001 From: MPela <1140981+mpela81@users.noreply.github.com> Date: Wed, 21 Apr 2021 12:53:19 +0200 Subject: [PATCH] Add Close menu items to the context menu flyout (#9859) Add the "Close other tabs"/"Close tabs to the right" menu items straight to the tab context menu to work around #8238. We can't add them into a dedicated sub-menu until the upstream crash is fixed. * [X] Closes #8238 * [X] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [ ] Tests added/passed * [ ] Documentation updated * [ ] Schema updated. * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Moved the creation of the close menu items to a single function. Once the originating crash is fixed, the sub-menu can be restored by just replacing a few lines of code. ![immagine](https://user-images.githubusercontent.com/1140981/115059601-0dbc2480-9ee7-11eb-9889-d9ef8e6e7613.png) (cherry picked from commit 2065fa7b768ab96305c08003270a8d176fecfeb0) (cherry picked from commit c4e02e7274018105b8b7de483b60c8288c4459fd) --- src/cascadia/TerminalApp/TabBase.cpp | 54 ++++++++++++------------ src/cascadia/TerminalApp/TabBase.h | 2 +- src/cascadia/TerminalApp/TerminalTab.cpp | 18 +------- src/cascadia/TerminalApp/TerminalTab.h | 2 - 4 files changed, 30 insertions(+), 46 deletions(-) diff --git a/src/cascadia/TerminalApp/TabBase.cpp b/src/cascadia/TerminalApp/TabBase.cpp index ab083149877..311a5a6e0b4 100644 --- a/src/cascadia/TerminalApp/TabBase.cpp +++ b/src/cascadia/TerminalApp/TabBase.cpp @@ -45,35 +45,19 @@ namespace winrt::TerminalApp::implementation { auto weakThis{ get_weak() }; - // Close - Controls::MenuFlyoutItem closeTabMenuItem; - Controls::FontIcon closeSymbol; - closeSymbol.FontFamily(Media::FontFamily{ L"Segoe MDL2 Assets" }); - closeSymbol.Glyph(L"\xE711"); - - closeTabMenuItem.Click([weakThis](auto&&, auto&&) { - if (auto tab{ weakThis.get() }) - { - tab->_ClosedHandlers(nullptr, nullptr); - } - }); - closeTabMenuItem.Text(RS_(L"TabClose")); - closeTabMenuItem.Icon(closeSymbol); - // Build the menu Controls::MenuFlyout newTabFlyout; - newTabFlyout.Items().Append(_CreateCloseSubMenu()); - newTabFlyout.Items().Append(closeTabMenuItem); + _AppendCloseMenuItems(newTabFlyout); TabViewItem().ContextFlyout(newTabFlyout); } // Method Description: - // - Creates a sub-menu containing menu items to close multiple tabs + // - Append the close menu items to the context menu flyout // Arguments: - // - + // - flyout - the menu flyout to which the close items must be appended // Return Value: - // - the created MenuFlyoutSubItem - Controls::MenuFlyoutSubItem TabBase::_CreateCloseSubMenu() + // - + void TabBase::_AppendCloseMenuItems(winrt::Windows::UI::Xaml::Controls::MenuFlyout flyout) { auto weakThis{ get_weak() }; @@ -95,12 +79,30 @@ namespace winrt::TerminalApp::implementation }); _closeOtherTabsMenuItem.Text(RS_(L"TabCloseOther")); - Controls::MenuFlyoutSubItem closeSubMenu; - closeSubMenu.Text(RS_(L"TabCloseSubMenu")); - closeSubMenu.Items().Append(_closeTabsAfterMenuItem); - closeSubMenu.Items().Append(_closeOtherTabsMenuItem); + // Close + Controls::MenuFlyoutItem closeTabMenuItem; + Controls::FontIcon closeSymbol; + closeSymbol.FontFamily(Media::FontFamily{ L"Segoe MDL2 Assets" }); + closeSymbol.Glyph(L"\xE711"); + + closeTabMenuItem.Click([weakThis](auto&&, auto&&) { + if (auto tab{ weakThis.get() }) + { + tab->_ClosedHandlers(nullptr, nullptr); + } + }); + closeTabMenuItem.Text(RS_(L"TabClose")); + closeTabMenuItem.Icon(closeSymbol); - return closeSubMenu; + // GH#8238 append the close menu items to the flyout itself until crash in XAML is fixed + //Controls::MenuFlyoutSubItem closeSubMenu; + //closeSubMenu.Text(RS_(L"TabCloseSubMenu")); + //closeSubMenu.Items().Append(_closeTabsAfterMenuItem); + //closeSubMenu.Items().Append(_closeOtherTabsMenuItem); + //flyout.Items().Append(closeSubMenu); + flyout.Items().Append(_closeTabsAfterMenuItem); + flyout.Items().Append(_closeOtherTabsMenuItem); + flyout.Items().Append(closeTabMenuItem); } // Method Description: diff --git a/src/cascadia/TerminalApp/TabBase.h b/src/cascadia/TerminalApp/TabBase.h index e47227fd37d..d5bdcb34fa2 100644 --- a/src/cascadia/TerminalApp/TabBase.h +++ b/src/cascadia/TerminalApp/TabBase.h @@ -51,7 +51,7 @@ namespace winrt::TerminalApp::implementation virtual void _CreateContextMenu(); virtual winrt::hstring _CreateToolTipTitle(); - winrt::Windows::UI::Xaml::Controls::MenuFlyoutSubItem _CreateCloseSubMenu(); + void _AppendCloseMenuItems(winrt::Windows::UI::Xaml::Controls::MenuFlyout flyout); void _EnableCloseMenuItems(); void _CloseTabsAfter(); void _CloseOtherTabs(); diff --git a/src/cascadia/TerminalApp/TerminalTab.cpp b/src/cascadia/TerminalApp/TerminalTab.cpp index 86319ad2cd9..4c7b7dcd469 100644 --- a/src/cascadia/TerminalApp/TerminalTab.cpp +++ b/src/cascadia/TerminalApp/TerminalTab.cpp @@ -784,21 +784,6 @@ namespace winrt::TerminalApp::implementation { auto weakThis{ get_weak() }; - // Close - Controls::MenuFlyoutItem closeTabMenuItem; - Controls::FontIcon closeSymbol; - closeSymbol.FontFamily(Media::FontFamily{ L"Segoe MDL2 Assets" }); - closeSymbol.Glyph(L"\xE711"); - - closeTabMenuItem.Click([weakThis](auto&&, auto&&) { - if (auto tab{ weakThis.get() }) - { - tab->_ClosedHandlers(nullptr, nullptr); - } - }); - closeTabMenuItem.Text(RS_(L"TabClose")); - closeTabMenuItem.Icon(closeSymbol); - // "Color..." Controls::MenuFlyoutItem chooseColorMenuItem; Controls::FontIcon colorPickSymbol; @@ -852,8 +837,7 @@ namespace winrt::TerminalApp::implementation newTabFlyout.Items().Append(chooseColorMenuItem); newTabFlyout.Items().Append(renameTabMenuItem); newTabFlyout.Items().Append(menuSeparator); - newTabFlyout.Items().Append(_CreateCloseSubMenu()); - newTabFlyout.Items().Append(closeTabMenuItem); + _AppendCloseMenuItems(newTabFlyout); TabViewItem().ContextFlyout(newTabFlyout); } diff --git a/src/cascadia/TerminalApp/TerminalTab.h b/src/cascadia/TerminalApp/TerminalTab.h index f8fb560ee60..dbe36e9957e 100644 --- a/src/cascadia/TerminalApp/TerminalTab.h +++ b/src/cascadia/TerminalApp/TerminalTab.h @@ -99,8 +99,6 @@ namespace winrt::TerminalApp::implementation winrt::TerminalApp::ColorPickupFlyout _tabColorPickup{}; std::optional _themeTabColor{}; std::optional _runtimeTabColor{}; - winrt::Windows::UI::Xaml::Controls::MenuFlyoutItem _closeOtherTabsMenuItem{}; - winrt::Windows::UI::Xaml::Controls::MenuFlyoutItem _closeTabsAfterMenuItem{}; winrt::TerminalApp::TabHeaderControl _headerControl{}; winrt::TerminalApp::TerminalTabStatus _tabStatus{};