Skip to content
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.

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

Add Split Tab option to tab context menu #10832

Merged
16 commits merged into from
Aug 5, 2021
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
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
13 changes: 8 additions & 5 deletions src/cascadia/TerminalApp/Resources/en-US/Resources.resw
Original file line number Diff line number Diff line change
Expand Up @@ -226,15 +226,15 @@
<value>Warnings were found while parsing your keybindings:</value>
</data>
<data name="TooManyKeysForChord" xml:space="preserve">
<value>&#x2022; Found a keybinding with too many strings for the "keys" array. There should only be one string value in the "keys" array.</value>
<comment>{Locked="\"keys\"","&#x2022;"} This glyph is a bullet, used in a bulleted list.</comment>
<value>&amp;#x2022; Found a keybinding with too many strings for the "keys" array. There should only be one string value in the "keys" array.</value>
cinnamon-msft marked this conversation as resolved.
Show resolved Hide resolved
cinnamon-msft marked this conversation as resolved.
Show resolved Hide resolved
<comment>{Locked="\"keys\"","&amp;#x2022;"} This glyph is a bullet, used in a bulleted list.</comment>
</data>
<data name="FailedToParseSubCommands" xml:space="preserve">
<value>&#x2022; Failed to parse all subcommands of nested command.</value>
<value>&amp;#x2022; Failed to parse all subcommands of nested command.</value>
</data>
<data name="MissingRequiredParameter" xml:space="preserve">
<value>&#x2022; Found a keybinding that was missing a required parameter value. This keybinding will be ignored.</value>
<comment>{Locked="&#x2022;"} This glyph is a bullet, used in a bulleted list.</comment>
<value>&amp;#x2022; Found a keybinding that was missing a required parameter value. This keybinding will be ignored.</value>
<comment>{Locked="&amp;#x2022;"} This glyph is a bullet, used in a bulleted list.</comment>
</data>
<data name="LegacyGlobalsProperty" xml:space="preserve">
<value>The "globals" property is deprecated - your settings might need updating. </value>
Expand Down Expand Up @@ -662,6 +662,9 @@
<data name="DropPathTabRun.Text" xml:space="preserve">
<value>Open a new tab in given starting directory</value>
</data>
<data name="SplitTabText" xml:space="preserve">
<value>Split Tab</value>
</data>
<data name="DropPathTabNewWindow.Text" xml:space="preserve">
<value>Open a new window with given starting directory</value>
</data>
Expand Down
24 changes: 24 additions & 0 deletions src/cascadia/TerminalApp/TabManagement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,16 @@ namespace winrt::TerminalApp::implementation
}
});

newTabImpl->SplitTabRequested([weakTab, weakThis{ get_weak() }]() {
auto page{ weakThis.get() };
auto tab{ weakTab.get() };

if (page && tab)
{
page->_SplitTab(*tab);
}
});

auto tabViewItem = newTabImpl->TabViewItem();
_tabView.TabItems().Append(tabViewItem);

Expand Down Expand Up @@ -357,6 +367,20 @@ namespace winrt::TerminalApp::implementation
CATCH_LOG();
}

// Method Description:
// - Sets the specified tab as the focused tab and splits its active pane
// Arguments:
// - tab: tab to split
void TerminalPage::_SplitTab(TerminalTab& tab)
{
try
{
_SetFocusedTab(tab);
_SplitPane(tab, SplitState::Automatic, SplitType::Duplicate);
Copy link
Member

Choose a reason for hiding this comment

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

Okay so this is a nit, and will probably spur a bigger discussion, but this is technically different than what alt+clicking on the new tab button does today. That opens the default profile, it doesn't duplicate the current profile. I think it's probably fine to make the alt+click duplicate as well.

@team We cool with changing the alt+click on new tab button behavior? (it can be a follow-up)

Copy link
Member

Choose a reason for hiding this comment

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

I don't follow why alt+clicking the new tab button should duplicate the current profile. The new tab button has always been "do X with the default profile", so I don't get why we should switch that to target the current profile.

IIRC there was discussion about the new tab button completely switching over to targeting the current profile. If we make that transition though, I'm ok with any interaction with the new tab button then operating on the current profile. (I think Leonard wanted it to run duplicateTab instead of openTab by default).

Copy link
Member

Choose a reason for hiding this comment

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

I suppose I'm more wondering if this is a consistency issue. Is it weird that the context menu duplicates but the button uses the default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assumed people would want to duplicate their profile while splitting if they used the tab context menu. That way they have the option of either duplicating with the context menu or getting their default profile by using the new tab button.

Copy link
Member

Choose a reason for hiding this comment

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

I can buy that

}
CATCH_LOG();
}

// Method Description:
// - Removes the tab (both TerminalControl and XAML) after prompting for approval
// Arguments:
Expand Down
39 changes: 33 additions & 6 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1246,6 +1246,33 @@ namespace winrt::TerminalApp::implementation
return;
}

_SplitPane(*focusedTab, splitType, splitMode, splitSize, newTerminalArgs);
}

// Method Description:
// - Split the focused pane of the given tab, either horizontally or vertically, and place the
// given TermControl into the newly created pane.
// - If splitType == SplitState::None, this method does nothing.
// Arguments:
// - tab: The tab that is going to be split.
// - splitType: one value from the TerminalApp::SplitState enum, indicating how the
// new pane should be split from its parent.
// - splitMode: value from TerminalApp::SplitType enum, indicating the profile to be used in the newly split pane.
// - newTerminalArgs: An object that may contain a blob of parameters to
// control which profile is created and with possible other
// configurations. See CascadiaSettings::BuildSettings for more details.
void TerminalPage::_SplitPane(TerminalTab& tab,
const SplitState splitType,
const SplitType splitMode,
const float splitSize,
const NewTerminalArgs& newTerminalArgs)
{
// Do nothing if we're requesting no split.
if (splitType == SplitState::None)
{
return;
}

try
{
TerminalSettingsCreateResult controlSettings{ nullptr };
Expand All @@ -1254,12 +1281,12 @@ namespace winrt::TerminalApp::implementation

if (splitMode == SplitType::Duplicate)
{
std::optional<GUID> current_guid = focusedTab->GetFocusedProfile();
std::optional<GUID> current_guid = tab.GetFocusedProfile();
if (current_guid)
{
profileFound = true;
controlSettings = TerminalSettings::CreateWithProfileByID(_settings, current_guid.value(), *_bindings);
const auto workingDirectory = focusedTab->GetActiveTerminalControl().WorkingDirectory();
const auto workingDirectory = tab.GetActiveTerminalControl().WorkingDirectory();
const auto validWorkingDirectory = !workingDirectory.empty();
if (validWorkingDirectory)
{
Expand Down Expand Up @@ -1295,10 +1322,10 @@ namespace winrt::TerminalApp::implementation
auto realSplitType = splitType;
if (realSplitType == SplitState::Automatic)
{
realSplitType = focusedTab->PreCalculateAutoSplit(availableSpace);
realSplitType = tab.PreCalculateAutoSplit(availableSpace);
}

const auto canSplit = focusedTab->PreCalculateCanSplit(realSplitType, splitSize, availableSpace);
const auto canSplit = tab.PreCalculateCanSplit(realSplitType, splitSize, availableSpace);
if (!canSplit)
{
return;
Expand All @@ -1307,11 +1334,11 @@ namespace winrt::TerminalApp::implementation
auto newControl = _InitControl(controlSettings, controlConnection);

// Hookup our event handlers to the new terminal
_RegisterTerminalEvents(newControl, *focusedTab);
_RegisterTerminalEvents(newControl, tab);

_UnZoomIfNeeded();

focusedTab->SplitPane(realSplitType, splitSize, realGuid, newControl);
tab.SplitPane(realSplitType, splitSize, realGuid, newControl);
}
CATCH_LOG();
}
Expand Down
7 changes: 7 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,8 @@ namespace winrt::TerminalApp::implementation
void _DuplicateFocusedTab();
void _DuplicateTab(const TerminalTab& tab);

void _SplitTab(TerminalTab& tab);

winrt::Windows::Foundation::IAsyncAction _HandleCloseTabRequested(winrt::TerminalApp::TabBase tab);
void _CloseTabAtIndex(uint32_t index);
void _RemoveTab(const winrt::TerminalApp::TabBase& tab);
Expand Down Expand Up @@ -254,6 +256,11 @@ namespace winrt::TerminalApp::implementation
const Microsoft::Terminal::Settings::Model::SplitType splitMode = Microsoft::Terminal::Settings::Model::SplitType::Manual,
const float splitSize = 0.5f,
const Microsoft::Terminal::Settings::Model::NewTerminalArgs& newTerminalArgs = nullptr);
void _SplitPane(TerminalTab& tab,
const Microsoft::Terminal::Settings::Model::SplitState splitType,
const Microsoft::Terminal::Settings::Model::SplitType splitMode = Microsoft::Terminal::Settings::Model::SplitType::Manual,
const float splitSize = 0.5f,
const Microsoft::Terminal::Settings::Model::NewTerminalArgs& newTerminalArgs = nullptr);
void _ResizePane(const Microsoft::Terminal::Settings::Model::ResizeDirection& direction);
void _ToggleSplitOrientation();

Expand Down
19 changes: 19 additions & 0 deletions src/cascadia/TerminalApp/TerminalTab.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -936,12 +936,30 @@ namespace winrt::TerminalApp::implementation
duplicateTabMenuItem.Icon(duplicateTabSymbol);
}

Controls::MenuFlyoutItem splitTabMenuItem;
{
// "Split Tab"
Controls::FontIcon splitTabSymbol;
splitTabSymbol.FontFamily(Media::FontFamily{ L"Segoe MDL2 Assets" });
splitTabSymbol.Glyph(L"\xF246"); // ViewDashboard

splitTabMenuItem.Click([weakThis](auto&&, auto&&) {
if (auto tab{ weakThis.get() })
{
tab->_SplitTabRequestedHandlers();
}
});
splitTabMenuItem.Text(RS_(L"SplitTabText"));
splitTabMenuItem.Icon(splitTabSymbol);
}

// Build the menu
Controls::MenuFlyout contextMenuFlyout;
Controls::MenuFlyoutSeparator menuSeparator;
contextMenuFlyout.Items().Append(chooseColorMenuItem);
contextMenuFlyout.Items().Append(renameTabMenuItem);
contextMenuFlyout.Items().Append(duplicateTabMenuItem);
contextMenuFlyout.Items().Append(splitTabMenuItem);
contextMenuFlyout.Items().Append(menuSeparator);

// GH#5750 - When the context menu is dismissed with ESC, toss the focus
Expand Down Expand Up @@ -1315,4 +1333,5 @@ namespace winrt::TerminalApp::implementation
DEFINE_EVENT(TerminalTab, ColorCleared, _colorCleared, winrt::delegate<>);
DEFINE_EVENT(TerminalTab, TabRaiseVisualBell, _TabRaiseVisualBellHandlers, winrt::delegate<>);
DEFINE_EVENT(TerminalTab, DuplicateRequested, _DuplicateRequestedHandlers, winrt::delegate<>);
DEFINE_EVENT(TerminalTab, SplitTabRequested, _SplitTabRequestedHandlers, winrt::delegate<>);
}
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/TerminalTab.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ namespace winrt::TerminalApp::implementation
DECLARE_EVENT(ColorCleared, _colorCleared, winrt::delegate<>);
DECLARE_EVENT(TabRaiseVisualBell, _TabRaiseVisualBellHandlers, winrt::delegate<>);
DECLARE_EVENT(DuplicateRequested, _DuplicateRequestedHandlers, winrt::delegate<>);
DECLARE_EVENT(SplitTabRequested, _SplitTabRequestedHandlers, winrt::delegate<>);
TYPED_EVENT(TaskbarProgressChanged, IInspectable, IInspectable);

private:
Expand Down