Skip to content

Commit

Permalink
Raise ShortcutActions with the sender (tab, control) context (#15773)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
zadjii-msft authored Aug 24, 2023
1 parent e10b7e4 commit 30eb9ee
Show file tree
Hide file tree
Showing 12 changed files with 248 additions and 272 deletions.
10 changes: 5 additions & 5 deletions src/cascadia/LocalTests_TerminalApp/TabTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ namespace TerminalAppLocalTests

Log::Comment(NoThrowString().Format(L"Duplicate the first pane"));
result = RunOnUIThread([&page]() {
page->_SplitPane(SplitDirection::Automatic, 0.5f, page->_MakePane(nullptr, page->_GetFocusedTab(), nullptr));
page->_SplitPane(nullptr, SplitDirection::Automatic, 0.5f, page->_MakePane(nullptr, page->_GetFocusedTab(), nullptr));

VERIFY_ARE_EQUAL(1u, page->_tabs.Size());
auto tab = page->_GetTerminalTabImpl(page->_tabs.GetAt(0));
Expand All @@ -535,7 +535,7 @@ namespace TerminalAppLocalTests

Log::Comment(NoThrowString().Format(L"Duplicate the pane, and don't crash"));
result = RunOnUIThread([&page]() {
page->_SplitPane(SplitDirection::Automatic, 0.5f, page->_MakePane(nullptr, page->_GetFocusedTab(), nullptr));
page->_SplitPane(nullptr, SplitDirection::Automatic, 0.5f, page->_MakePane(nullptr, page->_GetFocusedTab(), nullptr));

VERIFY_ARE_EQUAL(1u, page->_tabs.Size());
auto tab = page->_GetTerminalTabImpl(page->_tabs.GetAt(0));
Expand Down Expand Up @@ -857,7 +857,7 @@ namespace TerminalAppLocalTests
// | 1 | 2 |
// | | |
// -------------------
page->_SplitPane(SplitDirection::Right, 0.5f, page->_MakePane(nullptr, page->_GetFocusedTab(), nullptr));
page->_SplitPane(nullptr, SplitDirection::Right, 0.5f, page->_MakePane(nullptr, page->_GetFocusedTab(), nullptr));
secondId = tab->_activePane->Id().value();
});
Sleep(250);
Expand All @@ -875,7 +875,7 @@ namespace TerminalAppLocalTests
// | 3 | |
// | | |
// -------------------
page->_SplitPane(SplitDirection::Down, 0.5f, page->_MakePane(nullptr, page->_GetFocusedTab(), nullptr));
page->_SplitPane(nullptr, SplitDirection::Down, 0.5f, page->_MakePane(nullptr, page->_GetFocusedTab(), nullptr));
auto tab = page->_GetTerminalTabImpl(page->_tabs.GetAt(0));
// Split again to make the 3rd tab
thirdId = tab->_activePane->Id().value();
Expand All @@ -895,7 +895,7 @@ namespace TerminalAppLocalTests
// | 3 | 4 |
// | | |
// -------------------
page->_SplitPane(SplitDirection::Down, 0.5f, page->_MakePane(nullptr, page->_GetFocusedTab(), nullptr));
page->_SplitPane(nullptr, SplitDirection::Down, 0.5f, page->_MakePane(nullptr, page->_GetFocusedTab(), nullptr));
auto tab = page->_GetTerminalTabImpl(page->_tabs.GetAt(0));
fourthId = tab->_activePane->Id().value();
});
Expand Down
Loading

0 comments on commit 30eb9ee

Please sign in to comment.