Skip to content

Commit

Permalink
Support for navigating panes by MRU (#8183)
Browse files Browse the repository at this point in the history 
Adds a "move to previous pane" and "move to next pane" keybinding, which
navigates to the last/first focused pane

We assign pane IDs on creation and maintain a vector of active pane IDs
in MRU order. Navigating panes by MRU then requires specifying which
pane ID we want to focus. 

From our offline discussion (thanks @zadjii-msft for the concise
description):

> For the record, the full spec I'm imagining is:
> 
> { command": { "action": "focus(Next|Prev)Pane", "order": "inOrder"|"mru", "useSwitcher": true|false } },
> 
> and order defaults to mru, and useSwitcher will default to true, when
> there is a switcher. So 
> 
> { command": { "action": "focusNextPane" } },
> { command": { "action": "focusNextPane", "order": "mru" } },
> 
> these are the same action. (but right now we don't support the order
> param)
>  
> Then there'll be another PR for "focusPane(target=id)"
> 
> Then a third PR for "focus(Next|Prev)Pane(order=inOrder)"

> for the record, I prefer this approach over the "one action to rule
> them all" version with both target and order/direction as params,
> because I don't like the confusion of what happens if there's both
> target and order/direction provided. 

References #1000 
Closes #2871
  • Loading branch information
@PankajBhojwani
PankajBhojwani authored Dec 11, 2020
1 parent eb2be37 commit 04309a2
Show file tree
Hide file tree
Showing 15 changed files with 210 additions and 79 deletions.
33 changes: 19 additions & 14 deletions doc/cascadia/profiles.schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,17 @@
],
"type": "string"
},
"Direction": {
"FocusDirection": {
"enum": [
"left",
"right",
"up",
"down",
"previous"
],
"type": "string"
},
"ResizeDirection": {
"enum": [
"left",
"right",
Expand Down Expand Up @@ -298,9 +308,9 @@
"properties": {
"action": { "type": "string", "pattern": "moveFocus" },
"direction": {
"$ref": "#/definitions/Direction",
"$ref": "#/definitions/FocusDirection",
"default": "left",
"description": "The direction to move focus in, between panes"
"description": "The direction to move focus in, between panes. Direction can be 'previous' to move to the most recently used pane."
}
}
}
Expand All @@ -315,9 +325,9 @@
"properties": {
"action": { "type": "string", "pattern": "resizePane" },
"direction": {
"$ref": "#/definitions/Direction",
"$ref": "#/definitions/ResizeDirection",
"default": "left",
"description": "The direction to move the pane separator in"
"description": "The direction to move the pane separator in."
}
}
}
Expand All @@ -332,10 +342,7 @@
},
{
"properties": {
"action": {
"type": "string",
"pattern": "sendInput"
},
"action": { "type": "string", "pattern": "sendInput" },
"input": {
"type": "string",
"default": "",
Expand Down Expand Up @@ -375,10 +382,7 @@
},
{
"properties": {
"action": {
"type": "string",
"pattern": "openSettings"
},
"action": { "type": "string", "pattern": "openSettings" },
"target": {
"type": "string",
"default": "settingsFile",
Expand Down Expand Up @@ -834,7 +838,8 @@
"desktopWallpaper"
]
}
]
],
"type": [ "string", "null" ]
},
"backgroundImageAlignment": {
"default": "center",
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/LocalTests_TerminalApp/TabTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,7 @@ namespace TerminalAppLocalTests
Log::Comment(L"Move focus. This will cause us to un-zoom.");
result = RunOnUIThread([&page]() {
// Set up action
MoveFocusArgs args{ Direction::Left };
MoveFocusArgs args{ FocusDirection::Left };
ActionEventArgs eventArgs{ args };

page->_HandleMoveFocus(nullptr, eventArgs);
Expand Down
8 changes: 4 additions & 4 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,14 +228,14 @@ namespace winrt::TerminalApp::implementation
{
if (const auto& realArgs = args.ActionArgs().try_as<ResizePaneArgs>())
{
if (realArgs.Direction() == Direction::None)
if (realArgs.ResizeDirection() == ResizeDirection::None)
{
// Do nothing
args.Handled(false);
}
else
{
_ResizePane(realArgs.Direction());
_ResizePane(realArgs.ResizeDirection());
args.Handled(true);
}
}
Expand All @@ -246,14 +246,14 @@ namespace winrt::TerminalApp::implementation
{
if (const auto& realArgs = args.ActionArgs().try_as<MoveFocusArgs>())
{
if (realArgs.Direction() == Direction::None)
if (realArgs.FocusDirection() == FocusDirection::None)
{
// Do nothing
args.Handled(false);
}
else
{
_MoveFocus(realArgs.Direction());
_MoveFocus(realArgs.FocusDirection());
args.Handled(true);
}
}
Expand Down
59 changes: 52 additions & 7 deletions src/cascadia/TerminalApp/Pane.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,15 +130,15 @@ void Pane::Relayout()
// decreasing the size of our first child.
// Return Value:
// - false if we couldn't resize this pane in the given direction, else true.
bool Pane::_Resize(const Direction& direction)
bool Pane::_Resize(const ResizeDirection& direction)
{
if (!DirectionMatchesSplit(direction, _splitState))
{
return false;
}

float amount = .05f;
if (direction == Direction::Right || direction == Direction::Down)
if (direction == ResizeDirection::Right || direction == ResizeDirection::Down)
{
amount = -amount;
}
Expand Down Expand Up @@ -171,7 +171,7 @@ bool Pane::_Resize(const Direction& direction)
// - direction: The direction to move the separator in.
// Return Value:
// - true if we or a child handled this resize request.
bool Pane::ResizePane(const Direction& direction)
bool Pane::ResizePane(const ResizeDirection& direction)
{
// If we're a leaf, do nothing. We can't possibly have a descendant with a
// separator the correct direction.
Expand Down Expand Up @@ -223,14 +223,14 @@ bool Pane::ResizePane(const Direction& direction)
// - direction: The direction to move the focus in.
// Return Value:
// - true if we handled this focus move request.
bool Pane::_NavigateFocus(const Direction& direction)
bool Pane::_NavigateFocus(const FocusDirection& direction)
{
if (!DirectionMatchesSplit(direction, _splitState))
{
return false;
}

const bool focusSecond = (direction == Direction::Right) || (direction == Direction::Down);
const bool focusSecond = (direction == FocusDirection::Right) || (direction == FocusDirection::Down);

const auto newlyFocusedChild = focusSecond ? _secondChild : _firstChild;

Expand Down Expand Up @@ -261,7 +261,7 @@ bool Pane::_NavigateFocus(const Direction& direction)
// - direction: The direction to move the focus in.
// Return Value:
// - true if we or a child handled this focus move request.
bool Pane::NavigateFocus(const Direction& direction)
bool Pane::NavigateFocus(const FocusDirection& direction)
{
// If we're a leaf, do nothing. We can't possibly have a descendant with a
// separator the correct direction.
Expand Down Expand Up @@ -655,9 +655,10 @@ void Pane::_CloseChild(const bool closeFirst)
// inherited from us, so the flag will be set for both children.
_borders = _firstChild->_borders & _secondChild->_borders;

// take the control and profile of the pane that _wasn't_ closed.
// take the control, profile and id of the pane that _wasn't_ closed.
_control = remainingChild->_control;
_profile = remainingChild->_profile;
_id = remainingChild->Id();

// Add our new event handler before revoking the old one.
_connectionStateChangedToken = _control.ConnectionStateChanged({ this, &Pane::_ControlConnectionStateChangedHandler });
Expand Down Expand Up @@ -1493,6 +1494,9 @@ std::pair<std::shared_ptr<Pane>, std::shared_ptr<Pane>> Pane::_Split(SplitState

_SetupEntranceAnimation();

// Clear out our ID, only leaves should have IDs
_id = {};

return { _firstChild, _secondChild };
}

Expand Down Expand Up @@ -1566,6 +1570,47 @@ void Pane::Restore(std::shared_ptr<Pane> zoomedPane)
}
}

// Method Description:
// - Retrieves the ID of this pane
// - NOTE: The caller should make sure that this pane is a leaf,
// otherwise the ID value will not make sense (leaves have IDs, parents do not)
// Return Value:
// - The ID of this pane
uint16_t Pane::Id() noexcept
{
return _id.value();
}

// Method Description:
// - Sets this pane's ID
// - Panes are given IDs upon creation by TerminalTab
// Arguments:
// - The number to set this pane's ID to
void Pane::Id(uint16_t id) noexcept
{
_id = id;
}

// Method Description:
// - Recursive function that focuses a pane with the given ID
// Arguments:
// - The ID of the pane we want to focus
void Pane::FocusPane(const uint16_t id)
{
if (_IsLeaf() && id == _id)
{
_control.Focus(FocusState::Programmatic);
}
else
{
if (_firstChild && _secondChild)
{
_firstChild->FocusPane(id);
_secondChild->FocusPane(id);
}
}
}

// Method Description:
// - Gets the size in pixels of each of our children, given the full size they
// should fill. Since these children own their own separators (borders), this
Expand Down
28 changes: 17 additions & 11 deletions src/cascadia/TerminalApp/Pane.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ class Pane : public std::enable_shared_from_this<Pane>
const GUID& profile);
void ResizeContent(const winrt::Windows::Foundation::Size& newSize);
void Relayout();
bool ResizePane(const winrt::Microsoft::Terminal::Settings::Model::Direction& direction);
bool NavigateFocus(const winrt::Microsoft::Terminal::Settings::Model::Direction& direction);
bool ResizePane(const winrt::Microsoft::Terminal::Settings::Model::ResizeDirection& direction);
bool NavigateFocus(const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction);

bool CanSplit(winrt::Microsoft::Terminal::Settings::Model::SplitState splitType);
std::pair<std::shared_ptr<Pane>, std::shared_ptr<Pane>> Split(winrt::Microsoft::Terminal::Settings::Model::SplitState splitType,
Expand All @@ -75,6 +75,10 @@ class Pane : public std::enable_shared_from_this<Pane>
void Maximize(std::shared_ptr<Pane> zoomedPane);
void Restore(std::shared_ptr<Pane> zoomedPane);

uint16_t Id() noexcept;
void Id(uint16_t id) noexcept;
void FocusPane(const uint16_t id);

WINRT_CALLBACK(Closed, winrt::Windows::Foundation::EventHandler<winrt::Windows::Foundation::IInspectable>);
DECLARE_EVENT(GotFocus, _GotFocusHandlers, winrt::delegate<std::shared_ptr<Pane>>);
DECLARE_EVENT(PaneRaiseVisualBell, _PaneRaiseVisualBellHandlers, winrt::delegate<std::shared_ptr<Pane>>);
Expand All @@ -95,6 +99,8 @@ class Pane : public std::enable_shared_from_this<Pane>
winrt::Microsoft::Terminal::Settings::Model::SplitState _splitState{ winrt::Microsoft::Terminal::Settings::Model::SplitState::None };
float _desiredSplitPosition;

std::optional<uint16_t> _id;

bool _lastActive{ false };
std::optional<GUID> _profile{ std::nullopt };
winrt::event_token _connectionStateChangedToken{ 0 };
Expand Down Expand Up @@ -124,8 +130,8 @@ class Pane : public std::enable_shared_from_this<Pane>
void _SetupEntranceAnimation();
void _UpdateBorders();

bool _Resize(const winrt::Microsoft::Terminal::Settings::Model::Direction& direction);
bool _NavigateFocus(const winrt::Microsoft::Terminal::Settings::Model::Direction& direction);
bool _Resize(const winrt::Microsoft::Terminal::Settings::Model::ResizeDirection& direction);
bool _NavigateFocus(const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction);

void _CloseChild(const bool closeFirst);
winrt::fire_and_forget _CloseChildRoutine(const bool closeFirst);
Expand Down Expand Up @@ -156,15 +162,15 @@ class Pane : public std::enable_shared_from_this<Pane>
// - This is used for pane resizing (which will need a pane separator
// that's perpendicular to the direction to be able to move the separator
// in that direction).
// - Additionally, it will be used for moving focus between panes, which
// again happens _across_ a separator.
// - Also used for moving focus between panes, which again happens _across_ a separator.
// Arguments:
// - direction: The Direction to compare
// - splitType: The winrt::TerminalApp::SplitState to compare
// Return Value:
// - true iff the direction is perpendicular to the splitType. False for
// winrt::TerminalApp::SplitState::None.
static constexpr bool DirectionMatchesSplit(const winrt::Microsoft::Terminal::Settings::Model::Direction& direction,
template<typename T>
static constexpr bool DirectionMatchesSplit(const T& direction,
const winrt::Microsoft::Terminal::Settings::Model::SplitState& splitType)
{
if (splitType == winrt::Microsoft::Terminal::Settings::Model::SplitState::None)
Expand All @@ -173,13 +179,13 @@ class Pane : public std::enable_shared_from_this<Pane>
}
else if (splitType == winrt::Microsoft::Terminal::Settings::Model::SplitState::Horizontal)
{
return direction == winrt::Microsoft::Terminal::Settings::Model::Direction::Up ||
direction == winrt::Microsoft::Terminal::Settings::Model::Direction::Down;
return direction == T::Up ||
direction == T::Down;
}
else if (splitType == winrt::Microsoft::Terminal::Settings::Model::SplitState::Vertical)
{
return direction == winrt::Microsoft::Terminal::Settings::Model::Direction::Left ||
direction == winrt::Microsoft::Terminal::Settings::Model::Direction::Right;
return direction == T::Left ||
direction == T::Right;
}
return false;
}
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalApp/TerminalPage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1395,7 +1395,7 @@ namespace winrt::TerminalApp::implementation
// - direction: The direction to move the focus in.
// Return Value:
// - <none>
void TerminalPage::_MoveFocus(const Direction& direction)
void TerminalPage::_MoveFocus(const FocusDirection& direction)
{
if (auto index{ _GetFocusedTabIndex() })
{
Expand Down Expand Up @@ -1669,7 +1669,7 @@ namespace winrt::TerminalApp::implementation
// - direction: The direction to move the separator in.
// Return Value:
// - <none>
void TerminalPage::_ResizePane(const Direction& direction)
void TerminalPage::_ResizePane(const ResizeDirection& direction)
{
if (auto index{ _GetFocusedTabIndex() })
{
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalApp/TerminalPage.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ namespace winrt::TerminalApp::implementation

void _SelectNextTab(const bool bMoveRight);
bool _SelectTab(const uint32_t tabIndex);
void _MoveFocus(const Microsoft::Terminal::Settings::Model::Direction& direction);
void _MoveFocus(const Microsoft::Terminal::Settings::Model::FocusDirection& direction);

winrt::Microsoft::Terminal::TerminalControl::TermControl _GetActiveControl();
std::optional<uint32_t> _GetFocusedTabIndex() const noexcept;
Expand All @@ -191,7 +191,7 @@ namespace winrt::TerminalApp::implementation
// MSFT:20641986: Add keybindings for New Window
void _Scroll(ScrollDirection scrollDirection, const Windows::Foundation::IReference<uint32_t>& rowsToScroll);
void _SplitPane(const Microsoft::Terminal::Settings::Model::SplitState splitType, const Microsoft::Terminal::Settings::Model::SplitType splitMode = Microsoft::Terminal::Settings::Model::SplitType::Manual, const Microsoft::Terminal::Settings::Model::NewTerminalArgs& newTerminalArgs = nullptr);
void _ResizePane(const Microsoft::Terminal::Settings::Model::Direction& direction);
void _ResizePane(const Microsoft::Terminal::Settings::Model::ResizeDirection& direction);
void _ScrollPage(ScrollDirection scrollDirection);
void _ScrollToBufferEdge(ScrollDirection scrollDirection);
void _SetAcceleratorForMenuItem(Windows::UI::Xaml::Controls::MenuFlyoutItem& menuItem, const winrt::Microsoft::Terminal::TerminalControl::KeyChord& keyChord);
Expand Down
Loading

0 comments on commit 04309a2

Please sign in to comment.