From 2065fa7b768ab96305c08003270a8d176fecfeb0 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) ## Summary of the Pull Request 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. ## References #8238 ## PR Checklist * [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. ## Detailed Description of the Pull Request / Additional comments 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. ## Validation Steps Performed ![immagine](https://user-images.githubusercontent.com/1140981/115059601-0dbc2480-9ee7-11eb-9889-d9ef8e6e7613.png) --- 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 4bba499074a..97f6c7f9275 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->_CloseRequestedHandlers(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->_CloseRequestedHandlers(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 4db5426c22a..6ae3093d346 100644 --- a/src/cascadia/TerminalApp/TabBase.h +++ b/src/cascadia/TerminalApp/TabBase.h @@ -52,7 +52,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 fb101afd043..b681bb3a76d 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->_CloseRequestedHandlers(nullptr, nullptr); - } - }); - closeTabMenuItem.Text(RS_(L"TabClose")); - closeTabMenuItem.Icon(closeSymbol); - // "Color..." Controls::MenuFlyoutItem chooseColorMenuItem; Controls::FontIcon colorPickSymbol; @@ -870,8 +855,7 @@ namespace winrt::TerminalApp::implementation newTabFlyout.Items().Append(renameTabMenuItem); newTabFlyout.Items().Append(duplicateTabMenuItem); 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 9887e785358..f7b97253ae7 100644 --- a/src/cascadia/TerminalApp/TerminalTab.h +++ b/src/cascadia/TerminalApp/TerminalTab.h @@ -100,8 +100,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{};