Skip to content

Commit 674c918

Browse files
mpela81DHowett
authored andcommitted
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 2065fa7) (cherry picked from commit c4e02e7)
1 parent 4cee91f commit 674c918

File tree

4 files changed

+30
-46
lines changed

4 files changed

+30
-46
lines changed

src/cascadia/TerminalApp/TabBase.cpp

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -45,35 +45,19 @@ namespace winrt::TerminalApp::implementation
4545
{
4646
auto weakThis{ get_weak() };
4747

48-
// Close
49-
Controls::MenuFlyoutItem closeTabMenuItem;
50-
Controls::FontIcon closeSymbol;
51-
closeSymbol.FontFamily(Media::FontFamily{ L"Segoe MDL2 Assets" });
52-
closeSymbol.Glyph(L"\xE711");
53-
54-
closeTabMenuItem.Click([weakThis](auto&&, auto&&) {
55-
if (auto tab{ weakThis.get() })
56-
{
57-
tab->_ClosedHandlers(nullptr, nullptr);
58-
}
59-
});
60-
closeTabMenuItem.Text(RS_(L"TabClose"));
61-
closeTabMenuItem.Icon(closeSymbol);
62-
6348
// Build the menu
6449
Controls::MenuFlyout newTabFlyout;
65-
newTabFlyout.Items().Append(_CreateCloseSubMenu());
66-
newTabFlyout.Items().Append(closeTabMenuItem);
50+
_AppendCloseMenuItems(newTabFlyout);
6751
TabViewItem().ContextFlyout(newTabFlyout);
6852
}
6953

7054
// Method Description:
71-
// - Creates a sub-menu containing menu items to close multiple tabs
55+
// - Append the close menu items to the context menu flyout
7256
// Arguments:
73-
// - <none>
57+
// - flyout - the menu flyout to which the close items must be appended
7458
// Return Value:
75-
// - the created MenuFlyoutSubItem
76-
Controls::MenuFlyoutSubItem TabBase::_CreateCloseSubMenu()
59+
// - <none>
60+
void TabBase::_AppendCloseMenuItems(winrt::Windows::UI::Xaml::Controls::MenuFlyout flyout)
7761
{
7862
auto weakThis{ get_weak() };
7963

@@ -95,12 +79,30 @@ namespace winrt::TerminalApp::implementation
9579
});
9680
_closeOtherTabsMenuItem.Text(RS_(L"TabCloseOther"));
9781

98-
Controls::MenuFlyoutSubItem closeSubMenu;
99-
closeSubMenu.Text(RS_(L"TabCloseSubMenu"));
100-
closeSubMenu.Items().Append(_closeTabsAfterMenuItem);
101-
closeSubMenu.Items().Append(_closeOtherTabsMenuItem);
82+
// Close
83+
Controls::MenuFlyoutItem closeTabMenuItem;
84+
Controls::FontIcon closeSymbol;
85+
closeSymbol.FontFamily(Media::FontFamily{ L"Segoe MDL2 Assets" });
86+
closeSymbol.Glyph(L"\xE711");
87+
88+
closeTabMenuItem.Click([weakThis](auto&&, auto&&) {
89+
if (auto tab{ weakThis.get() })
90+
{
91+
tab->_ClosedHandlers(nullptr, nullptr);
92+
}
93+
});
94+
closeTabMenuItem.Text(RS_(L"TabClose"));
95+
closeTabMenuItem.Icon(closeSymbol);
10296

103-
return closeSubMenu;
97+
// GH#8238 append the close menu items to the flyout itself until crash in XAML is fixed
98+
//Controls::MenuFlyoutSubItem closeSubMenu;
99+
//closeSubMenu.Text(RS_(L"TabCloseSubMenu"));
100+
//closeSubMenu.Items().Append(_closeTabsAfterMenuItem);
101+
//closeSubMenu.Items().Append(_closeOtherTabsMenuItem);
102+
//flyout.Items().Append(closeSubMenu);
103+
flyout.Items().Append(_closeTabsAfterMenuItem);
104+
flyout.Items().Append(_closeOtherTabsMenuItem);
105+
flyout.Items().Append(closeTabMenuItem);
104106
}
105107

106108
// Method Description:

src/cascadia/TerminalApp/TabBase.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ namespace winrt::TerminalApp::implementation
5151
virtual void _CreateContextMenu();
5252
virtual winrt::hstring _CreateToolTipTitle();
5353

54-
winrt::Windows::UI::Xaml::Controls::MenuFlyoutSubItem _CreateCloseSubMenu();
54+
void _AppendCloseMenuItems(winrt::Windows::UI::Xaml::Controls::MenuFlyout flyout);
5555
void _EnableCloseMenuItems();
5656
void _CloseTabsAfter();
5757
void _CloseOtherTabs();

src/cascadia/TerminalApp/TerminalTab.cpp

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -784,21 +784,6 @@ namespace winrt::TerminalApp::implementation
784784
{
785785
auto weakThis{ get_weak() };
786786

787-
// Close
788-
Controls::MenuFlyoutItem closeTabMenuItem;
789-
Controls::FontIcon closeSymbol;
790-
closeSymbol.FontFamily(Media::FontFamily{ L"Segoe MDL2 Assets" });
791-
closeSymbol.Glyph(L"\xE711");
792-
793-
closeTabMenuItem.Click([weakThis](auto&&, auto&&) {
794-
if (auto tab{ weakThis.get() })
795-
{
796-
tab->_ClosedHandlers(nullptr, nullptr);
797-
}
798-
});
799-
closeTabMenuItem.Text(RS_(L"TabClose"));
800-
closeTabMenuItem.Icon(closeSymbol);
801-
802787
// "Color..."
803788
Controls::MenuFlyoutItem chooseColorMenuItem;
804789
Controls::FontIcon colorPickSymbol;
@@ -852,8 +837,7 @@ namespace winrt::TerminalApp::implementation
852837
newTabFlyout.Items().Append(chooseColorMenuItem);
853838
newTabFlyout.Items().Append(renameTabMenuItem);
854839
newTabFlyout.Items().Append(menuSeparator);
855-
newTabFlyout.Items().Append(_CreateCloseSubMenu());
856-
newTabFlyout.Items().Append(closeTabMenuItem);
840+
_AppendCloseMenuItems(newTabFlyout);
857841
TabViewItem().ContextFlyout(newTabFlyout);
858842
}
859843

src/cascadia/TerminalApp/TerminalTab.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,6 @@ namespace winrt::TerminalApp::implementation
9999
winrt::TerminalApp::ColorPickupFlyout _tabColorPickup{};
100100
std::optional<winrt::Windows::UI::Color> _themeTabColor{};
101101
std::optional<winrt::Windows::UI::Color> _runtimeTabColor{};
102-
winrt::Windows::UI::Xaml::Controls::MenuFlyoutItem _closeOtherTabsMenuItem{};
103-
winrt::Windows::UI::Xaml::Controls::MenuFlyoutItem _closeTabsAfterMenuItem{};
104102
winrt::TerminalApp::TabHeaderControl _headerControl{};
105103
winrt::TerminalApp::TerminalTabStatus _tabStatus{};
106104

0 commit comments

Comments
 (0)