Tab menu: show "restart" + maintain items' enable state#19972
Open
carlos-zamora wants to merge 1 commit intomainfrom
Open
Tab menu: show "restart" + maintain items' enable state#19972carlos-zamora wants to merge 1 commit intomainfrom
carlos-zamora wants to merge 1 commit intomainfrom
Conversation
carlos-zamora
commented
Mar 13, 2026
Comment on lines
+1358
to
+1359
| // Snippets Pane can technically be split | ||
| _splitTabMenuItem.IsEnabled(isTerm || (content && content.try_as<winrt::TerminalApp::SnippetsPaneContent>() != nullptr)); |
Member
Author
There was a problem hiding this comment.
Frankly, snippets pane should be a side-car like terminal chat
DHowett
requested changes
Mar 17, 2026
| }); | ||
| } | ||
|
|
||
| _UpdateMenuItemEnablement(); |
Member
There was a problem hiding this comment.
nit: MenuItemStates may be clearer and more future-proof
| } | ||
|
|
||
| Controls::MenuFlyoutItem splitTabMenuItem; | ||
| Controls::MenuFlyoutItem splitTabMenuItem = _splitTabMenuItem; |
Member
There was a problem hiding this comment.
don't do this; just use _splitTabMenuItem directly for the duration.
| } | ||
|
|
||
| Controls::MenuFlyoutItem splitTabMenuItem; | ||
| Controls::MenuFlyoutItem splitTabMenuItem = _splitTabMenuItem; |
Member
There was a problem hiding this comment.
either that or create them here as locals and then std::move them into their _underscore versions
| } | ||
|
|
||
| Controls::MenuFlyoutItem splitTabMenuItem; | ||
| Controls::MenuFlyoutItem splitTabMenuItem = _splitTabMenuItem; |
Member
There was a problem hiding this comment.
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of the Pull Request
Changes how/when we display the "restart connection" item in the tab's context menu. Now, we always display it, but it's disabled if we're not in a terminal tab.
Applies similar enable/disable logic to the rest of the menu items. Previous to this, they just wouldn't do anything (which is fair, they didn't make any sense).
Validation Steps Performed
Check tab's menu in following scenarios:
✅ terminal pane
✅ settings tab
✅ snippets pane
Closes #18891