Add Split Tab option to tab context menu #10832

cinnamon-msft · 2021-07-30T17:45:55Z

Summary of the Pull Request

Adds the Split Tab option to the tab context menu.
Clicking this option will auto split the active pane of the tab into a duplicate pane.
Clicking on an unfocused tab and splitting it will bring that tab into focus and split its active pane.

We could make this a flyout from the context menu to let people choose horizontal/vertical split in the future if it's requested.

I'm also wondering if this should be called Split Pane instead of Split Tab?

References

#1912

PR Checklist

Closes Split Pane via a context menu #5025
CLA signed. If not, go over here and sign the CLA
Tests added/passed
Documentation updated. If checked, please file a pull request on our docs repo and link it here: #xxx
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. Issue number where discussion took place: #xxx

Detailed Description of the Pull Request / Additional comments

Split.Tab.mp4

Validation Steps Performed

src/cascadia/TerminalApp/TerminalPage.cpp

src/cascadia/TerminalApp/TabManagement.cpp

…om/microsoft/terminal into cinnamon/fhl/splitpane-contextmenu

carlos-zamora

Just the one comment. Thanks for doing this!

src/cascadia/TerminalApp/TerminalPage.cpp

zadjii-msft

I'm gonna approve, but I'm also going to merge in carlos's suggestion

src/cascadia/TerminalApp/Resources/en-US/Resources.resw

zadjii-msft · 2021-08-04T14:11:52Z

src/cascadia/TerminalApp/TabManagement.cpp

+        try
+        {
+            _SetFocusedTab(tab);
+            _SplitPane(tab, SplitState::Automatic, SplitType::Duplicate);


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)

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).

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?

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.

I can buy that

src/cascadia/TerminalApp/TerminalPage.cpp

Co-authored-by: Carlos Zamora <carlos.zamora@microsoft.com>

…om/microsoft/terminal into cinnamon/fhl/splitpane-contextmenu

src/cascadia/TerminalApp/Resources/en-US/Resources.resw

ghost · 2021-08-05T13:20:34Z

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

ghost · 2021-08-31T17:14:27Z

🎉Windows Terminal Preview v1.11.2421.0 has been released which incorporates this pull request.:tada:

Handy links:

This PR's goal is to allow something like a `Tab` to raise a ShortcutAction, by saying "this action should be performed on ME". We've had a whole category of these issues in the past: * #15734 * #15760 * #13579 * #13942 * #13942 * Heck even dating back to #10832 So, this tries to remove a bit of that footgun. This probably isn't the _final_ form of what this refactor might look like, but the code is certainly better than before. Basically, there's a few bits: * `ShortcutActionDispatch.DoAction` now takes a `sender`, which can be _anything_. * Most actions that use a "Get the focused _thing_ then do something to it" are changed to "If there was a sender, let's use that - otherwise, we'll use the focused _thing_". * TerminalTab was largely refactored to use this, instead of making requests to the `TerminalPage` to just do a thing to it. I've got a few TODO!s left, but wanted to get initial feedback. * [x] `TerminalPage::_HandleTogglePaneZoom` * [x] `TerminalPage::_HandleFocusPane` * [x] `TerminalPage::_MoveTab` Closes #15734

This PR's goal is to allow something like a `Tab` to raise a ShortcutAction, by saying "this action should be performed on ME". We've had a whole category of these issues in the past: * #15734 * #15760 * #13579 * #13942 * #13942 * Heck even dating back to #10832 So, this tries to remove a bit of that footgun. This probably isn't the _final_ form of what this refactor might look like, but the code is certainly better than before. Basically, there's a few bits: * `ShortcutActionDispatch.DoAction` now takes a `sender`, which can be _anything_. * Most actions that use a "Get the focused _thing_ then do something to it" are changed to "If there was a sender, let's use that - otherwise, we'll use the focused _thing_". * TerminalTab was largely refactored to use this, instead of making requests to the `TerminalPage` to just do a thing to it. I've got a few TODO!s left, but wanted to get initial feedback. * [x] `TerminalPage::_HandleTogglePaneZoom` * [x] `TerminalPage::_HandleFocusPane` * [x] `TerminalPage::_MoveTab` Closes #15734 (cherry picked from commit 30eb9ee) Service-Card-Id: 90438493 Service-Version: 1.18

cinnamon-msft added 2 commits July 30, 2021 10:04

this works, probably needs to be cleaned up

aeab433

code cleanup

65162e7

ghost added Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal. labels Jul 30, 2021

cinnamon-msft added 2 commits July 30, 2021 10:55

code formatting

7fe2e18

remove unnecessary function call

592d238

carlos-zamora requested changes Aug 2, 2021

View reviewed changes

src/cascadia/TerminalApp/TerminalPage.cpp Outdated Show resolved Hide resolved

src/cascadia/TerminalApp/TabManagement.cpp Outdated Show resolved Hide resolved

ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 2, 2021

remove duplicate code

e9d5073

ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 3, 2021

Merge branch 'main' into cinnamon/fhl/splitpane-contextmenu

688616f

cinnamon-msft requested a review from carlos-zamora August 3, 2021 17:05

cinnamon-msft added 3 commits August 3, 2021 10:11

code formatting

e96d152

Merge branch 'cinnamon/fhl/splitpane-contextmenu' of https://github.c…

71ff006

…om/microsoft/terminal into cinnamon/fhl/splitpane-contextmenu

remove more stuff

13a5036

carlos-zamora approved these changes Aug 3, 2021

View reviewed changes

src/cascadia/TerminalApp/TerminalPage.cpp Outdated Show resolved Hide resolved

zadjii-msft approved these changes Aug 4, 2021

View reviewed changes

zadjii-msft and others added 5 commits August 4, 2021 09:18

Update src/cascadia/TerminalApp/TerminalPage.cpp

31b3b81

Co-authored-by: Carlos Zamora <carlos.zamora@microsoft.com>

fix syntax

8262f15

Merge branch 'cinnamon/fhl/splitpane-contextmenu' of https://github.c…

a2b23b2

…om/microsoft/terminal into cinnamon/fhl/splitpane-contextmenu

fix resw formatting

f1c3b99

my vs doesn't like bullet points

ba71960

cinnamon-msft commented Aug 4, 2021

View reviewed changes

src/cascadia/TerminalApp/Resources/en-US/Resources.resw Outdated Show resolved Hide resolved

Update src/cascadia/TerminalApp/Resources/en-US/Resources.resw

0b51400

cinnamon-msft commented Aug 4, 2021

View reviewed changes

src/cascadia/TerminalApp/Resources/en-US/Resources.resw Outdated Show resolved Hide resolved

more fixes

3cdc194

zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Aug 5, 2021

ghost merged commit 0b4839d into main Aug 5, 2021

ghost deleted the cinnamon/fhl/splitpane-contextmenu branch August 5, 2021 13:46

ghost mentioned this pull request Aug 31, 2021

Split Pane via a context menu #5025

Closed

zadjii-msft mentioned this pull request Jul 27, 2023

Raise ShortcutActions with the sender (tab, control) context #15773

Merged

3 tasks

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Split Tab option to tab context menu #10832

Add Split Tab option to tab context menu #10832

cinnamon-msft commented Jul 30, 2021 •

edited

Loading

carlos-zamora left a comment

zadjii-msft left a comment

zadjii-msft Aug 4, 2021

carlos-zamora Aug 4, 2021

zadjii-msft Aug 4, 2021

cinnamon-msft Aug 4, 2021

zadjii-msft Aug 4, 2021

ghost commented Aug 5, 2021

ghost commented Aug 31, 2021

Add Split Tab option to tab context menu #10832

Add Split Tab option to tab context menu #10832

Conversation

cinnamon-msft commented Jul 30, 2021 • edited Loading

Summary of the Pull Request

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

carlos-zamora left a comment

Choose a reason for hiding this comment

zadjii-msft left a comment

Choose a reason for hiding this comment

zadjii-msft Aug 4, 2021

Choose a reason for hiding this comment

carlos-zamora Aug 4, 2021

Choose a reason for hiding this comment

zadjii-msft Aug 4, 2021

Choose a reason for hiding this comment

cinnamon-msft Aug 4, 2021

Choose a reason for hiding this comment

zadjii-msft Aug 4, 2021

Choose a reason for hiding this comment

ghost commented Aug 5, 2021

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

ghost commented Aug 31, 2021

cinnamon-msft commented Jul 30, 2021 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.