Skip to content

Commit

Permalink
Maintain zoom when moving focus (#11046)
Browse files Browse the repository at this point in the history
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
Make it so you can navigate pane focus without unzooming.

<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes #7215
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) 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

<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
- Slight refactor to bring the MRU pane logic into the `NavigateDirection` function
- The actual zoom behavior was not a problem, the only issue is that because most of the panes weren't in the UI tree I had to disable using the actual sizes. There is nothing wrong with that, since the synthetic sizing is required anyways, but I'm curious what other peoples' thoughts are.

<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed

![output](https://user-images.githubusercontent.com/6185249/130901911-91676da2-db40-412d-b726-61a3f559ae17.gif)
  • Loading branch information
Rosefield authored Sep 2, 2021
1 parent a0670cb commit 13bc71d
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 115 deletions.
124 changes: 47 additions & 77 deletions src/cascadia/TerminalApp/Pane.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,23 +223,35 @@ bool Pane::ResizePane(const ResizeDirection& direction)
// Arguments:
// - sourcePane: the pane to navigate from
// - direction: which direction to go in
// - mruPanes: the list of most recently used panes, in order
// Return Value:
// - The result of navigating from source according to direction, which may be
// nullptr (i.e. no pane was found in that direction).
std::shared_ptr<Pane> Pane::NavigateDirection(const std::shared_ptr<Pane> sourcePane, const FocusDirection& direction)
std::shared_ptr<Pane> Pane::NavigateDirection(const std::shared_ptr<Pane> sourcePane, const FocusDirection& direction, const std::vector<uint32_t>& mruPanes)
{
// Can't navigate anywhere if we are a leaf
if (_IsLeaf())
{
return nullptr;
}

// If the MRU previous pane is requested we can't move; the tab handles MRU
if (direction == FocusDirection::None || direction == FocusDirection::Previous)
if (direction == FocusDirection::None)
{
return nullptr;
}

// Previous movement relies on the last used panes
if (direction == FocusDirection::Previous)
{
// If there is actually a previous pane.
if (mruPanes.size() > 1)
{
// This could return nullptr if the id is not actually in the tree.
return FindPane(mruPanes.at(1));
}
return nullptr;
}

// Check if we in-order traversal is requested
if (direction == FocusDirection::NextInOrder)
{
Expand Down Expand Up @@ -277,11 +289,11 @@ std::shared_ptr<Pane> Pane::NavigateDirection(const std::shared_ptr<Pane> source
// and its neighbor must necessarily be contained within the same child.
if (!DirectionMatchesSplit(direction, _splitState))
{
if (auto p = _firstChild->NavigateDirection(sourcePane, direction))
if (const auto p = _firstChild->NavigateDirection(sourcePane, direction, mruPanes))
{
return p;
}
return _secondChild->NavigateDirection(sourcePane, direction);
return _secondChild->NavigateDirection(sourcePane, direction, mruPanes);
}

// Since the direction is the same as our split, it is possible that we must
Expand Down Expand Up @@ -541,16 +553,14 @@ bool Pane::SwapPanes(std::shared_ptr<Pane> first, std::shared_ptr<Pane> second)
}

// Method Description:
// - Given two panes, test whether the `direction` side of first is adjacent to second.
// - Given two panes' offsets, test whether the `direction` side of first is adjacent to second.
// Arguments:
// - first: The reference pane.
// - second: the pane to test adjacency with.
// - firstOffset: The offset for the reference pane
// - secondOffset: the offset to test adjacency with.
// - direction: The direction to search in from the reference pane.
// Return Value:
// - true if the two panes are adjacent.
bool Pane::_IsAdjacent(const std::shared_ptr<Pane> first,
const PanePoint firstOffset,
const std::shared_ptr<Pane> second,
bool Pane::_IsAdjacent(const PanePoint firstOffset,
const PanePoint secondOffset,
const FocusDirection& direction) const
{
Expand All @@ -560,25 +570,11 @@ bool Pane::_IsAdjacent(const std::shared_ptr<Pane> first,
return abs(left - right) < 1e-4F;
};

auto getXMax = [](PanePoint offset, std::shared_ptr<Pane> pane) {
// If we are past startup panes should have real dimensions
if (pane->GetRootElement().ActualWidth() > 0)
{
return offset.x + gsl::narrow_cast<float>(pane->GetRootElement().ActualWidth());
}

// If we have simulated dimensions we rely on the calculated scale
auto getXMax = [](PanePoint offset) {
return offset.x + offset.scaleX;
};

auto getYMax = [](PanePoint offset, std::shared_ptr<Pane> pane) {
// If we are past startup panes should have real dimensions
if (pane->GetRootElement().ActualHeight() > 0)
{
return offset.y + gsl::narrow_cast<float>(pane->GetRootElement().ActualHeight());
}

// If we have simulated dimensions we rely on the calculated scale
auto getYMax = [](PanePoint offset) {
return offset.y + offset.scaleY;
};

Expand All @@ -588,8 +584,8 @@ bool Pane::_IsAdjacent(const std::shared_ptr<Pane> first,
// corner of the first element is within the second element's height
if (direction == FocusDirection::Left)
{
auto sharesBorders = floatEqual(firstOffset.x, getXMax(secondOffset, second));
auto withinHeight = (firstOffset.y >= secondOffset.y) && (firstOffset.y < getYMax(secondOffset, second));
const auto sharesBorders = floatEqual(firstOffset.x, getXMax(secondOffset));
const auto withinHeight = (firstOffset.y >= secondOffset.y) && (firstOffset.y < getYMax(secondOffset));

return sharesBorders && withinHeight;
}
Expand All @@ -598,8 +594,8 @@ bool Pane::_IsAdjacent(const std::shared_ptr<Pane> first,
// corner of the first element is within the second element's height
else if (direction == FocusDirection::Right)
{
auto sharesBorders = floatEqual(getXMax(firstOffset, first), secondOffset.x);
auto withinHeight = (firstOffset.y >= secondOffset.y) && (firstOffset.y < getYMax(secondOffset, second));
const auto sharesBorders = floatEqual(getXMax(firstOffset), secondOffset.x);
const auto withinHeight = (firstOffset.y >= secondOffset.y) && (firstOffset.y < getYMax(secondOffset));

return sharesBorders && withinHeight;
}
Expand All @@ -608,8 +604,8 @@ bool Pane::_IsAdjacent(const std::shared_ptr<Pane> first,
// corner of the first element is within the second element's width
else if (direction == FocusDirection::Up)
{
auto sharesBorders = floatEqual(firstOffset.y, getYMax(secondOffset, second));
auto withinWidth = (firstOffset.x >= secondOffset.x) && (firstOffset.x < getXMax(secondOffset, second));
const auto sharesBorders = floatEqual(firstOffset.y, getYMax(secondOffset));
const auto withinWidth = (firstOffset.x >= secondOffset.x) && (firstOffset.x < getXMax(secondOffset));

return sharesBorders && withinWidth;
}
Expand All @@ -618,8 +614,8 @@ bool Pane::_IsAdjacent(const std::shared_ptr<Pane> first,
// corner of the first element is within the second element's width
else if (direction == FocusDirection::Down)
{
auto sharesBorders = floatEqual(getYMax(firstOffset, first), secondOffset.y);
auto withinWidth = (firstOffset.x >= secondOffset.x) && (firstOffset.x < getXMax(secondOffset, second));
const auto sharesBorders = floatEqual(getYMax(firstOffset), secondOffset.y);
const auto withinWidth = (firstOffset.x >= secondOffset.x) && (firstOffset.x < getXMax(secondOffset));

return sharesBorders && withinWidth;
}
Expand All @@ -640,51 +636,25 @@ std::pair<Pane::PanePoint, Pane::PanePoint> Pane::_GetOffsetsForPane(const PaneP
auto firstOffset = parentOffset;
auto secondOffset = parentOffset;

// When panes are initialized they don't have dimensions yet.
if (_firstChild->GetRootElement().ActualHeight() > 0)
// Make up fake dimensions using an exponential layout. This is useful
// since we might need to navigate when there are panes not attached to
// the ui tree, such as initialization, command running, and zoom.
// Basically create the tree layout on the fly by partitioning [0,1].
// This could run into issues if the tree depth is >127 (or other
// degenerate splits) as a float's mantissa only has so many bits of
// precision.

if (_splitState == SplitState::Horizontal)
{
// The second child has an offset depending on the split
if (_splitState == SplitState::Horizontal)
{
const auto diff = gsl::narrow_cast<float>(_firstChild->GetRootElement().ActualHeight());
secondOffset.y += diff;
// However, if a command is run in an existing window that opens multiple new panes
// the parent will have a size (triggering this) and then the children will go
// to the other branch.
firstOffset.scaleY = diff;
secondOffset.scaleY = parentOffset.scaleY - diff;
}
else
{
const auto diff = gsl::narrow_cast<float>(_firstChild->GetRootElement().ActualWidth());
secondOffset.x += diff;
firstOffset.scaleX = diff;
secondOffset.scaleX = parentOffset.scaleX - diff;
}
secondOffset.y += (1 - _desiredSplitPosition) * parentOffset.scaleY;
firstOffset.scaleY *= _desiredSplitPosition;
secondOffset.scaleY *= (1 - _desiredSplitPosition);
}
else
{
// Since we don't have real dimensions make up fake ones using an
// exponential layout. Basically create the tree layout on the fly by
// partitioning [0,1].
// This could run into issues if the tree depth is >127 (or other
// degenerate splits) as a float's mantissa only has so many bits of
// precision.

// In theory this could always be used, but there might be edge cases
// where using the actual sizing information provides a better result.
if (_splitState == SplitState::Horizontal)
{
secondOffset.y += (1 - _desiredSplitPosition) * parentOffset.scaleY;
firstOffset.scaleY *= _desiredSplitPosition;
secondOffset.scaleY *= (1 - _desiredSplitPosition);
}
else
{
secondOffset.x += (1 - _desiredSplitPosition) * parentOffset.scaleX;
firstOffset.scaleX *= _desiredSplitPosition;
secondOffset.scaleX *= (1 - _desiredSplitPosition);
}
secondOffset.x += (1 - _desiredSplitPosition) * parentOffset.scaleX;
firstOffset.scaleX *= _desiredSplitPosition;
secondOffset.scaleX *= (1 - _desiredSplitPosition);
}

return { firstOffset, secondOffset };
Expand Down Expand Up @@ -721,7 +691,7 @@ Pane::PaneNeighborSearch Pane::_FindNeighborForPane(const FocusDirection& direct
// If we are a leaf node test if we adjacent to the focus node
if (_IsLeaf())
{
if (_IsAdjacent(searchResult.source, searchResult.sourceOffset, shared_from_this(), offset, direction))
if (_IsAdjacent(searchResult.sourceOffset, offset, direction))
{
searchResult.neighbor = shared_from_this();
}
Expand Down
6 changes: 4 additions & 2 deletions src/cascadia/TerminalApp/Pane.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@ class Pane : public std::enable_shared_from_this<Pane>
void ResizeContent(const winrt::Windows::Foundation::Size& newSize);
void Relayout();
bool ResizePane(const winrt::Microsoft::Terminal::Settings::Model::ResizeDirection& direction);
std::shared_ptr<Pane> NavigateDirection(const std::shared_ptr<Pane> sourcePane, const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction);
std::shared_ptr<Pane> NavigateDirection(const std::shared_ptr<Pane> sourcePane,
const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction,
const std::vector<uint32_t>& mruPanes);
bool SwapPanes(std::shared_ptr<Pane> first, std::shared_ptr<Pane> second);

std::shared_ptr<Pane> NextPane(const std::shared_ptr<Pane> pane);
Expand Down Expand Up @@ -201,7 +203,7 @@ class Pane : public std::enable_shared_from_this<Pane>

std::shared_ptr<Pane> _FindParentOfPane(const std::shared_ptr<Pane> pane);
std::pair<PanePoint, PanePoint> _GetOffsetsForPane(const PanePoint parentOffset) const;
bool _IsAdjacent(const std::shared_ptr<Pane> first, const PanePoint firstOffset, const std::shared_ptr<Pane> second, const PanePoint secondOffset, const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction) const;
bool _IsAdjacent(const PanePoint firstOffset, const PanePoint secondOffset, const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction) const;
PaneNeighborSearch _FindNeighborForPane(const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction,
PaneNeighborSearch searchResult,
const bool focusIsSecondSide,
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1164,7 +1164,6 @@ namespace winrt::TerminalApp::implementation
{
if (const auto terminalTab{ _GetFocusedTabImpl() })
{
_UnZoomIfNeeded();
return terminalTab->NavigateFocus(direction);
}
return false;
Expand Down
65 changes: 30 additions & 35 deletions src/cascadia/TerminalApp/TerminalTab.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -648,26 +648,21 @@ namespace winrt::TerminalApp::implementation
// to the terminal when no other panes are present (GH#6219)
bool TerminalTab::NavigateFocus(const FocusDirection& direction)
{
if (direction == FocusDirection::Previous)
{
if (_mruPanes.size() < 2)
{
return false;
}
// To get to the previous pane, get the id of the previous pane and focus to that
return _rootPane->FocusPane(_mruPanes.at(1));
}
else
// NOTE: This _must_ be called on the root pane, so that it can propagate
// throughout the entire tree.
if (const auto newFocus = _rootPane->NavigateDirection(_activePane, direction, _mruPanes))
{
// NOTE: This _must_ be called on the root pane, so that it can propagate
// throughout the entire tree.
if (auto newFocus = _rootPane->NavigateDirection(_activePane, direction))
const auto res = _rootPane->FocusPane(newFocus);

if (_zoomedPane)
{
return _rootPane->FocusPane(newFocus);
UpdateZoom(newFocus);
}

return false;
return res;
}

return false;
}

// Method Description:
Expand All @@ -680,27 +675,11 @@ namespace winrt::TerminalApp::implementation
// - true if two panes were swapped.
bool TerminalTab::SwapPane(const FocusDirection& direction)
{
if (direction == FocusDirection::Previous)
{
if (_mruPanes.size() < 2)
{
return false;
}
if (auto lastPane = _rootPane->FindPane(_mruPanes.at(1)))
{
return _rootPane->SwapPanes(_activePane, lastPane);
}
}
else
// NOTE: This _must_ be called on the root pane, so that it can propagate
// throughout the entire tree.
if (auto neighbor = _rootPane->NavigateDirection(_activePane, direction, _mruPanes))
{
// NOTE: This _must_ be called on the root pane, so that it can propagate
// throughout the entire tree.
if (auto neighbor = _rootPane->NavigateDirection(_activePane, direction))
{
return _rootPane->SwapPanes(_activePane, neighbor);
}

return false;
return _rootPane->SwapPanes(_activePane, neighbor);
}

return false;
Expand Down Expand Up @@ -1506,6 +1485,22 @@ namespace winrt::TerminalApp::implementation
return _rootPane->PreCalculateCanSplit(_activePane, splitType, splitSize, availableSpace).value_or(false);
}

// Method Description:
// - Updates the zoomed pane when the focus changes
// Arguments:
// - newFocus: the new pane to be zoomed
// Return Value:
// - <none>
void TerminalTab::UpdateZoom(std::shared_ptr<Pane> newFocus)
{
// clear the existing content so the old zoomed pane can be added back to the root tree
Content(nullptr);
_rootPane->Restore(_zoomedPane);
_zoomedPane = newFocus;
_rootPane->Maximize(_zoomedPane);
Content(_zoomedPane->GetRootElement());
}

// Method Description:
// - Toggle our zoom state.
// * If we're not zoomed, then zoom the active pane, making it take the
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/TerminalTab.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ namespace winrt::TerminalApp::implementation
void ResetRuntimeTabColor();
void ActivateColorPicker();

void UpdateZoom(std::shared_ptr<Pane> newFocus);
void ToggleZoom();
bool IsZoomed();
void EnterZoom();
Expand Down

0 comments on commit 13bc71d

Please sign in to comment.