Skip to content

Commit

Permalink
Add Close menu items to the context menu flyout (#9859)
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
mpela81 authored and DHowett committed May 14, 2021
1 parent 4cee91f commit 674c918
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 46 deletions.
54 changes: 28 additions & 26 deletions src/cascadia/TerminalApp/TabBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
// - <none>
// - flyout - the menu flyout to which the close items must be appended
// Return Value:
// - the created MenuFlyoutSubItem
Controls::MenuFlyoutSubItem TabBase::_CreateCloseSubMenu()
// - <none>
void TabBase::_AppendCloseMenuItems(winrt::Windows::UI::Xaml::Controls::MenuFlyout flyout)
{
auto weakThis{ get_weak() };

Expand All @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/TabBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
18 changes: 1 addition & 17 deletions src/cascadia/TerminalApp/TerminalTab.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down
2 changes: 0 additions & 2 deletions src/cascadia/TerminalApp/TerminalTab.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,6 @@ namespace winrt::TerminalApp::implementation
winrt::TerminalApp::ColorPickupFlyout _tabColorPickup{};
std::optional<winrt::Windows::UI::Color> _themeTabColor{};
std::optional<winrt::Windows::UI::Color> _runtimeTabColor{};
winrt::Windows::UI::Xaml::Controls::MenuFlyoutItem _closeOtherTabsMenuItem{};
winrt::Windows::UI::Xaml::Controls::MenuFlyoutItem _closeTabsAfterMenuItem{};
winrt::TerminalApp::TabHeaderControl _headerControl{};
winrt::TerminalApp::TerminalTabStatus _tabStatus{};

Expand Down

0 comments on commit 674c918

Please sign in to comment.