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.

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

Jump to bottom

Move Pane to Tab (GH7075) #10780

Merged
24 commits merged into from
Aug 12, 2021
Merged

Move Pane to Tab (GH7075) #10780

Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
0fb4b38
first very-much-not-working pass. Need to figure out how to manage al…
Rosefield Jul 20, 2021
4adfdee
GH7075 Move Pane to Tab
Rosefield Jul 24, 2021
e33b7aa
Remove control event handlers when pane is detached. Make sure event …
Rosefield Jul 25, 2021
64346fb
Appease the spelling bot
Rosefield Jul 25, 2021
d66d02e
Flesh out comments on new methods. run code formatter
Rosefield Jul 25, 2021
b8e95e3
Fix typos
Rosefield Jul 25, 2021
3bb24b2
Anticipate some code review refactoring. Make sure that we clean up e…
Rosefield Jul 29, 2021
4077160
Fix duplicated comment. Only say we handled an action if we actually …
Rosefield Jul 29, 2021
a7d7aec
box the event token so that we can pass a reference to it to the even…
Rosefield Aug 1, 2021
e7a9d21
Merge remote-tracking branch 'origin/main' into feature/gh7075-move-p…
Rosefield Aug 1, 2021
eea166c
Merge remote-tracking branch 'origin/main' into feature/gh7075-move-p…
Rosefield Aug 2, 2021
abe6a2b
Fix bug where border is lost when child is closed. Make it explicit w…
Rosefield Aug 2, 2021
530f658
spelling
Rosefield Aug 2, 2021
d1ed59c
Add action to keybindings in schema
Rosefield Aug 4, 2021
23b1ccf
Make separate DetachRoot function so that logic can be used separatel…
Rosefield Aug 4, 2021
ae1a84a
fix comment
Rosefield Aug 5, 2021
ef5089f
dont crash if we try to move a pane to a non-terminal tab
Rosefield Aug 6, 2021
2188f6b
Merge remote-tracking branch 'origin/main' into feature/gh7075-move-p…
Rosefield Aug 7, 2021
c6c9cc1
Rename move-pane -> swap-pane, move-pane-to-tab -> move-pane
Rosefield Aug 10, 2021
012747a
Merge remote-tracking branch 'origin/main' into feature/gh7075-move-p…
Rosefield Aug 10, 2021
4d9e571
formatting
Rosefield Aug 10, 2021
e91c69c
update tests to be swap-pane as well
Rosefield Aug 10, 2021
61a3cae
Make things that can be const, const
Rosefield Aug 12, 2021
1e048e2
Fix crash introduced with last merge; dont try to ask the root pane f…
Rosefield Aug 12, 2021
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
1 change: 1 addition & 0 deletions .github/actions/spelling/allow/apis.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ localtime
lround
LSHIFT
memicmp
mptt
mov
msappx
MULTIPLEUSE
Expand Down
25 changes: 22 additions & 3 deletions doc/cascadia/profiles.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@
"identifyWindows",
"moveFocus",
"movePane",
"swapPane",
"moveTab",
"newTab",
"newWindow",
Expand Down Expand Up @@ -493,6 +494,23 @@
"type": "integer",
"default": 0,
"description": "Which tab to switch to, with the first being 0"
}
}
}
],
"required": [ "index" ]
},
"MovePaneAction": {
"description": "Arguments corresponding to a Move Pane Action",
"allOf": [
{ "$ref": "#/definitions/ShortcutAction" },
{
"properties": {
"action": { "type": "string", "pattern": "movePane" },
"index": {
"type": "integer",
"default": 0,
"description": "Which tab to move the pane to, with the first being 0"
}
}
}
Expand All @@ -516,13 +534,13 @@
],
"required": [ "direction" ]
},
"MovePaneAction": {
"description": "Arguments corresponding to a Move Pane Action",
"SwapPaneAction": {
"description": "Arguments corresponding to a Swap Pane Action",
"allOf": [
{ "$ref": "#/definitions/ShortcutAction" },
{
"properties": {
"action": { "type": "string", "pattern": "movePane" },
"action": { "type": "string", "pattern": "swapPane" },
"direction": {
"$ref": "#/definitions/FocusDirection",
"default": "left",
Expand Down Expand Up @@ -972,6 +990,7 @@
{ "$ref": "#/definitions/SwitchToTabAction" },
{ "$ref": "#/definitions/MoveFocusAction" },
{ "$ref": "#/definitions/MovePaneAction" },
{ "$ref": "#/definitions/SwapPaneAction" },
{ "$ref": "#/definitions/ResizePaneAction" },
{ "$ref": "#/definitions/SendInputAction" },
{ "$ref": "#/definitions/SplitPaneAction" },
Expand Down
35 changes: 15 additions & 20 deletions src/cascadia/LocalTests_TerminalApp/CommandlineTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ namespace TerminalAppLocalTests
TEST_METHOD(ParseComboCommandlineIntoArgs);
TEST_METHOD(ParseFocusTabArgs);
TEST_METHOD(ParseMoveFocusArgs);
TEST_METHOD(ParseMovePaneArgs);
TEST_METHOD(ParseSwapPaneArgs);
TEST_METHOD(ParseArgumentsWithParsingTerminators);
TEST_METHOD(ParseFocusPaneArgs);

Expand Down Expand Up @@ -1208,14 +1208,9 @@ namespace TerminalAppLocalTests
}
}

void CommandlineTest::ParseMovePaneArgs()
void CommandlineTest::ParseSwapPaneArgs()
{
BEGIN_TEST_METHOD_PROPERTIES()
TEST_METHOD_PROPERTY(L"Data:useShortForm", L"{false, true}")
END_TEST_METHOD_PROPERTIES()

INIT_TEST_PROPERTY(bool, useShortForm, L"If true, use `mp` instead of `move-pane`");
const wchar_t* subcommand = useShortForm ? L"mp" : L"move-pane";
const wchar_t* subcommand = L"swap-pane";

{
AppCommandlineArgs appArgs{};
Expand All @@ -1236,9 +1231,9 @@ namespace TerminalAppLocalTests
VERIFY_ARE_EQUAL(ShortcutAction::NewTab, appArgs._startupActions.at(0).Action());

auto actionAndArgs = appArgs._startupActions.at(1);
VERIFY_ARE_EQUAL(ShortcutAction::MovePane, actionAndArgs.Action());
VERIFY_ARE_EQUAL(ShortcutAction::SwapPane, actionAndArgs.Action());
VERIFY_IS_NOT_NULL(actionAndArgs.Args());
auto myArgs = actionAndArgs.Args().try_as<MovePaneArgs>();
auto myArgs = actionAndArgs.Args().try_as<SwapPaneArgs>();
VERIFY_IS_NOT_NULL(myArgs);
VERIFY_ARE_EQUAL(FocusDirection::Left, myArgs.Direction());
}
Expand All @@ -1253,9 +1248,9 @@ namespace TerminalAppLocalTests
VERIFY_ARE_EQUAL(ShortcutAction::NewTab, appArgs._startupActions.at(0).Action());

auto actionAndArgs = appArgs._startupActions.at(1);
VERIFY_ARE_EQUAL(ShortcutAction::MovePane, actionAndArgs.Action());
VERIFY_ARE_EQUAL(ShortcutAction::SwapPane, actionAndArgs.Action());
VERIFY_IS_NOT_NULL(actionAndArgs.Args());
auto myArgs = actionAndArgs.Args().try_as<MovePaneArgs>();
auto myArgs = actionAndArgs.Args().try_as<SwapPaneArgs>();
VERIFY_IS_NOT_NULL(myArgs);
VERIFY_ARE_EQUAL(FocusDirection::Right, myArgs.Direction());
}
Expand All @@ -1270,9 +1265,9 @@ namespace TerminalAppLocalTests
VERIFY_ARE_EQUAL(ShortcutAction::NewTab, appArgs._startupActions.at(0).Action());

auto actionAndArgs = appArgs._startupActions.at(1);
VERIFY_ARE_EQUAL(ShortcutAction::MovePane, actionAndArgs.Action());
VERIFY_ARE_EQUAL(ShortcutAction::SwapPane, actionAndArgs.Action());
VERIFY_IS_NOT_NULL(actionAndArgs.Args());
auto myArgs = actionAndArgs.Args().try_as<MovePaneArgs>();
auto myArgs = actionAndArgs.Args().try_as<SwapPaneArgs>();
VERIFY_IS_NOT_NULL(myArgs);
VERIFY_ARE_EQUAL(FocusDirection::Up, myArgs.Direction());
}
Expand All @@ -1287,9 +1282,9 @@ namespace TerminalAppLocalTests
VERIFY_ARE_EQUAL(ShortcutAction::NewTab, appArgs._startupActions.at(0).Action());

auto actionAndArgs = appArgs._startupActions.at(1);
VERIFY_ARE_EQUAL(ShortcutAction::MovePane, actionAndArgs.Action());
VERIFY_ARE_EQUAL(ShortcutAction::SwapPane, actionAndArgs.Action());
VERIFY_IS_NOT_NULL(actionAndArgs.Args());
auto myArgs = actionAndArgs.Args().try_as<MovePaneArgs>();
auto myArgs = actionAndArgs.Args().try_as<SwapPaneArgs>();
VERIFY_IS_NOT_NULL(myArgs);
VERIFY_ARE_EQUAL(FocusDirection::Down, myArgs.Direction());
}
Expand All @@ -1311,16 +1306,16 @@ namespace TerminalAppLocalTests
VERIFY_ARE_EQUAL(ShortcutAction::NewTab, appArgs._startupActions.at(0).Action());

auto actionAndArgs = appArgs._startupActions.at(1);
VERIFY_ARE_EQUAL(ShortcutAction::MovePane, actionAndArgs.Action());
VERIFY_ARE_EQUAL(ShortcutAction::SwapPane, actionAndArgs.Action());
VERIFY_IS_NOT_NULL(actionAndArgs.Args());
auto myArgs = actionAndArgs.Args().try_as<MovePaneArgs>();
auto myArgs = actionAndArgs.Args().try_as<SwapPaneArgs>();
VERIFY_IS_NOT_NULL(myArgs);
VERIFY_ARE_EQUAL(FocusDirection::Left, myArgs.Direction());

actionAndArgs = appArgs._startupActions.at(2);
VERIFY_ARE_EQUAL(ShortcutAction::MovePane, actionAndArgs.Action());
VERIFY_ARE_EQUAL(ShortcutAction::SwapPane, actionAndArgs.Action());
VERIFY_IS_NOT_NULL(actionAndArgs.Args());
myArgs = actionAndArgs.Args().try_as<MovePaneArgs>();
myArgs = actionAndArgs.Args().try_as<SwapPaneArgs>();
VERIFY_IS_NOT_NULL(myArgs);
VERIFY_ARE_EQUAL(FocusDirection::Right, myArgs.Direction());
}
Expand Down
20 changes: 10 additions & 10 deletions src/cascadia/LocalTests_TerminalApp/TabTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ namespace TerminalAppLocalTests
TEST_METHOD(MoveFocusFromZoomedPane);
TEST_METHOD(CloseZoomedPane);

TEST_METHOD(MovePanes);
TEST_METHOD(SwapPanes);

TEST_METHOD(NextMRUTab);
TEST_METHOD(VerifyCommandPaletteTabSwitcherOrder);
Expand Down Expand Up @@ -821,7 +821,7 @@ namespace TerminalAppLocalTests
VERIFY_SUCCEEDED(result);
}

void TabTests::MovePanes()
void TabTests::SwapPanes()
{
auto page = _commonSetup();

Expand Down Expand Up @@ -914,10 +914,10 @@ namespace TerminalAppLocalTests
// -------------------
TestOnUIThread([&]() {
// Set up action
MovePaneArgs args{ FocusDirection::Left };
SwapPaneArgs args{ FocusDirection::Left };
ActionEventArgs eventArgs{ args };

page->_HandleMovePane(nullptr, eventArgs);
page->_HandleSwapPane(nullptr, eventArgs);
});

Sleep(250);
Expand Down Expand Up @@ -945,10 +945,10 @@ namespace TerminalAppLocalTests
// -------------------
TestOnUIThread([&]() {
// Set up action
MovePaneArgs args{ FocusDirection::Up };
SwapPaneArgs args{ FocusDirection::Up };
ActionEventArgs eventArgs{ args };

page->_HandleMovePane(nullptr, eventArgs);
page->_HandleSwapPane(nullptr, eventArgs);
});

Sleep(250);
Expand Down Expand Up @@ -976,10 +976,10 @@ namespace TerminalAppLocalTests
// -------------------
TestOnUIThread([&]() {
// Set up action
MovePaneArgs args{ FocusDirection::Right };
SwapPaneArgs args{ FocusDirection::Right };
ActionEventArgs eventArgs{ args };

page->_HandleMovePane(nullptr, eventArgs);
page->_HandleSwapPane(nullptr, eventArgs);
});

Sleep(250);
Expand Down Expand Up @@ -1007,10 +1007,10 @@ namespace TerminalAppLocalTests
// -------------------
TestOnUIThread([&]() {
// Set up action
MovePaneArgs args{ FocusDirection::Down };
SwapPaneArgs args{ FocusDirection::Down };
ActionEventArgs eventArgs{ args };

page->_HandleMovePane(nullptr, eventArgs);
page->_HandleSwapPane(nullptr, eventArgs);
});

Sleep(250);
Expand Down
20 changes: 17 additions & 3 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,20 @@ namespace winrt::TerminalApp::implementation
}
}

void TerminalPage::_HandleMovePane(const IInspectable& /*sender*/,
const ActionEventArgs& args)
{
if (args == nullptr)
{
args.Handled(false);
}
else if (const auto& realArgs = args.ActionArgs().try_as<MovePaneArgs>())
{
auto moved = _MovePane(realArgs.TabIndex());
args.Handled(moved);
}
}

void TerminalPage::_HandleSplitPane(const IInspectable& /*sender*/,
const ActionEventArgs& args)
{
Expand Down Expand Up @@ -323,10 +337,10 @@ namespace winrt::TerminalApp::implementation
}
}

void TerminalPage::_HandleMovePane(const IInspectable& /*sender*/,
void TerminalPage::_HandleSwapPane(const IInspectable& /*sender*/,
const ActionEventArgs& args)
{
if (const auto& realArgs = args.ActionArgs().try_as<MovePaneArgs>())
if (const auto& realArgs = args.ActionArgs().try_as<SwapPaneArgs>())
{
if (realArgs.Direction() == FocusDirection::None)
{
Expand All @@ -335,7 +349,7 @@ namespace winrt::TerminalApp::implementation
}
else
{
_MovePane(realArgs.Direction());
_SwapPane(realArgs.Direction());
args.Handled(true);
}
}
Expand Down
Loading