Skip to content

Tab menu: show "restart" + maintain items' enable state #19972

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
carlos-zamora wants to merge 1 commit into
base: main
Choose a base branch
from
+30 −19
Open

Tab menu: show "restart" + maintain items' enable state #19972

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 21 additions & 12 deletions src/cascadia/TerminalApp/Tab.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ namespace winrt::TerminalApp::implementation
_activePane = nullptr;

_closePaneMenuItem.Visibility(WUX::Visibility::Collapsed);
_restartConnectionMenuItem.Visibility(WUX::Visibility::Collapsed);

auto firstId = _nextPaneId;

Expand Down Expand Up @@ -86,6 +85,7 @@ namespace winrt::TerminalApp::implementation

_MakeTabViewItem();
_CreateContextMenu();
_UpdateMenuItemEnablement();

_headerControl.TabStatus(_tabStatus);

Expand Down Expand Up @@ -1271,13 +1271,6 @@ namespace winrt::TerminalApp::implementation

_tabStatus.IsConnectionClosed(isClosed);
}

if (_activePane)
{
_restartConnectionMenuItem.Visibility(_activePane->IsConnectionClosed() ?
WUX::Visibility::Visible :
WUX::Visibility::Collapsed);
}
}

void Tab::_RestartActivePaneConnection()
Expand Down Expand Up @@ -1348,6 +1341,22 @@ namespace winrt::TerminalApp::implementation
}
});
}

_UpdateMenuItemEnablement();
Copy link
Member

@DHowett DHowett Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: MenuItemStates may be clearer and more future-proof

}

void Tab::_UpdateMenuItemEnablement()
{
// Terminal-specific menu items
const auto content = _activePane ? _activePane->GetContent() : nullptr;
const auto isTerm = content && content.try_as<winrt::TerminalApp::TerminalPaneContent>() != nullptr;
_duplicateTabMenuItem.IsEnabled(isTerm);
_exportTabMenuItem.IsEnabled(isTerm);
_findMenuItem.IsEnabled(isTerm);
_restartConnectionMenuItem.IsEnabled(isTerm);

// Snippets Pane can technically be split
_splitTabMenuItem.IsEnabled(isTerm || (content && content.try_as<winrt::TerminalApp::SnippetsPaneContent>() != nullptr));
Comment on lines +1358 to +1359
Copy link
Member Author

@carlos-zamora carlos-zamora Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Frankly, snippets pane should be a side-car like terminal chat

}

// Method Description:
Expand Down Expand Up @@ -1652,7 +1661,7 @@ namespace winrt::TerminalApp::implementation
Automation::AutomationProperties::SetHelpText(renameTabMenuItem, renameTabToolTip);
}

Controls::MenuFlyoutItem duplicateTabMenuItem;
Controls::MenuFlyoutItem duplicateTabMenuItem = _duplicateTabMenuItem;
{
// "Duplicate tab"
Controls::FontIcon duplicateTabSymbol;
Expand All @@ -1669,7 +1678,7 @@ namespace winrt::TerminalApp::implementation
Automation::AutomationProperties::SetHelpText(duplicateTabMenuItem, duplicateTabToolTip);
}

Controls::MenuFlyoutItem splitTabMenuItem;
Controls::MenuFlyoutItem splitTabMenuItem = _splitTabMenuItem;
Copy link
Member

@DHowett DHowett Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't do this; just use _splitTabMenuItem directly for the duration.

Copy link
Member

@DHowett DHowett Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

either that or create them here as locals and then std::move them into their _underscore versions

Copy link
Member

@DHowett DHowett Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doubly-alternatively: you could consider just having a std::vector<MenuFlyoutItem> for the menu items you specifically want to enable when a TermControl is focused. you can then just enumerate them

{
// "Split tab"
Controls::FontIcon splitTabSymbol;
Expand Down Expand Up @@ -1698,7 +1707,7 @@ namespace winrt::TerminalApp::implementation
Automation::AutomationProperties::SetHelpText(closePaneMenuItem, closePaneToolTip);
}

Controls::MenuFlyoutItem exportTabMenuItem;
Controls::MenuFlyoutItem exportTabMenuItem = _exportTabMenuItem;
{
// "Export tab"
Controls::FontIcon exportTabSymbol;
Expand All @@ -1715,7 +1724,7 @@ namespace winrt::TerminalApp::implementation
Automation::AutomationProperties::SetHelpText(exportTabMenuItem, exportTabToolTip);
}

Controls::MenuFlyoutItem findMenuItem;
Controls::MenuFlyoutItem findMenuItem = _findMenuItem;
{
// "Find"
Controls::FontIcon findSymbol;
Expand Down
16 changes: 9 additions & 7 deletions src/cascadia/TerminalApp/Tab.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,17 @@ namespace winrt::TerminalApp::implementation
static constexpr double HeaderRenameBoxWidthTitleLength{ std::numeric_limits<double>::infinity() };

winrt::Windows::UI::Xaml::FocusState _focusState{ winrt::Windows::UI::Xaml::FocusState::Unfocused };
winrt::Windows::UI::Xaml::Controls::MenuFlyoutItem _closeOtherTabsMenuItem{};
winrt::Windows::UI::Xaml::Controls::MenuFlyoutItem _closeTabsAfterMenuItem{};
winrt::Windows::UI::Xaml::Controls::MenuFlyoutItem _duplicateTabMenuItem{};
winrt::Windows::UI::Xaml::Controls::MenuFlyoutItem _splitTabMenuItem{};
winrt::Windows::UI::Xaml::Controls::MenuFlyoutItem _moveToNewWindowMenuItem{};
winrt::Windows::UI::Xaml::Controls::MenuFlyoutItem _moveRightMenuItem{};
winrt::Windows::UI::Xaml::Controls::MenuFlyoutItem _moveLeftMenuItem{};
winrt::Windows::UI::Xaml::Controls::MenuFlyoutItem _exportTabMenuItem{};
winrt::Windows::UI::Xaml::Controls::MenuFlyoutItem _findMenuItem{};
winrt::Windows::UI::Xaml::Controls::MenuFlyoutItem _restartConnectionMenuItem{};
winrt::Windows::UI::Xaml::Controls::MenuFlyoutItem _closeOtherTabsMenuItem{};
winrt::Windows::UI::Xaml::Controls::MenuFlyoutItem _closeTabsAfterMenuItem{};
winrt::Windows::UI::Xaml::Controls::MenuFlyoutItem _closePaneMenuItem{};
winrt::TerminalApp::ShortcutActionDispatch _dispatch;
Microsoft::Terminal::Settings::Model::IActionMapView _actionMap{ nullptr };
winrt::hstring _keyChord{};
Expand All @@ -159,9 +165,6 @@ namespace winrt::TerminalApp::implementation
std::shared_ptr<Pane> _activePane{ nullptr };
std::shared_ptr<Pane> _zoomedPane{ nullptr };

Windows::UI::Xaml::Controls::MenuFlyoutItem _closePaneMenuItem;
Windows::UI::Xaml::Controls::MenuFlyoutItem _restartConnectionMenuItem;

winrt::Microsoft::Terminal::Settings::Model::IconStyle _lastIconStyle;
winrt::hstring _lastIconPath{};
std::optional<winrt::Windows::UI::Color> _runtimeTabColor{};
Expand Down Expand Up @@ -220,6 +223,7 @@ namespace winrt::TerminalApp::implementation
void _AttachEventHandlersToPane(std::shared_ptr<Pane> pane);

void _UpdateActivePane(std::shared_ptr<Pane> pane);
void _UpdateMenuItemEnablement();

winrt::hstring _GetActiveTitle() const;

Expand All @@ -230,8 +234,6 @@ namespace winrt::TerminalApp::implementation
void _UpdateConnectionClosedState();
void _RestartActivePaneConnection();

void _DuplicateTab();

winrt::Windows::UI::Xaml::Media::Brush _BackgroundBrush();

void _MakeTabViewItem();
Expand Down
Loading