From 13bc71de3c9a1885c63dc5cb6916a5b35e62342f Mon Sep 17 00:00:00 2001 From: Schuyler Rosefield Date: Thu, 2 Sep 2021 10:36:17 -0400 Subject: [PATCH] Maintain zoom when moving focus (#11046) ## Summary of the Pull Request Make it so you can navigate pane focus without unzooming. ## 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 ## 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. ## Validation Steps Performed ![output](https://user-images.githubusercontent.com/6185249/130901911-91676da2-db40-412d-b726-61a3f559ae17.gif) --- src/cascadia/TerminalApp/Pane.cpp | 124 ++++++++-------------- src/cascadia/TerminalApp/Pane.h | 6 +- src/cascadia/TerminalApp/TerminalPage.cpp | 1 - src/cascadia/TerminalApp/TerminalTab.cpp | 65 ++++++------ src/cascadia/TerminalApp/TerminalTab.h | 1 + 5 files changed, 82 insertions(+), 115 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 28646b11b..814869c4d 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -223,10 +223,11 @@ 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::NavigateDirection(const std::shared_ptr sourcePane, const FocusDirection& direction) +std::shared_ptr Pane::NavigateDirection(const std::shared_ptr sourcePane, const FocusDirection& direction, const std::vector& mruPanes) { // Can't navigate anywhere if we are a leaf if (_IsLeaf()) @@ -234,12 +235,23 @@ std::shared_ptr Pane::NavigateDirection(const std::shared_ptr source 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) { @@ -277,11 +289,11 @@ std::shared_ptr Pane::NavigateDirection(const std::shared_ptr 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 @@ -541,16 +553,14 @@ bool Pane::SwapPanes(std::shared_ptr first, std::shared_ptr 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 first, - const PanePoint firstOffset, - const std::shared_ptr second, +bool Pane::_IsAdjacent(const PanePoint firstOffset, const PanePoint secondOffset, const FocusDirection& direction) const { @@ -560,25 +570,11 @@ bool Pane::_IsAdjacent(const std::shared_ptr first, return abs(left - right) < 1e-4F; }; - auto getXMax = [](PanePoint offset, std::shared_ptr pane) { - // If we are past startup panes should have real dimensions - if (pane->GetRootElement().ActualWidth() > 0) - { - return offset.x + gsl::narrow_cast(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) { - // If we are past startup panes should have real dimensions - if (pane->GetRootElement().ActualHeight() > 0) - { - return offset.y + gsl::narrow_cast(pane->GetRootElement().ActualHeight()); - } - - // If we have simulated dimensions we rely on the calculated scale + auto getYMax = [](PanePoint offset) { return offset.y + offset.scaleY; }; @@ -588,8 +584,8 @@ bool Pane::_IsAdjacent(const std::shared_ptr 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; } @@ -598,8 +594,8 @@ bool Pane::_IsAdjacent(const std::shared_ptr 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; } @@ -608,8 +604,8 @@ bool Pane::_IsAdjacent(const std::shared_ptr 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; } @@ -618,8 +614,8 @@ bool Pane::_IsAdjacent(const std::shared_ptr 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; } @@ -640,51 +636,25 @@ std::pair 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(_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(_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 }; @@ -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(); } diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index 2b9a49c0e..77a891b26 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -75,7 +75,9 @@ public: void ResizeContent(const winrt::Windows::Foundation::Size& newSize); void Relayout(); bool ResizePane(const winrt::Microsoft::Terminal::Settings::Model::ResizeDirection& direction); - std::shared_ptr NavigateDirection(const std::shared_ptr sourcePane, const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction); + std::shared_ptr NavigateDirection(const std::shared_ptr sourcePane, + const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction, + const std::vector& mruPanes); bool SwapPanes(std::shared_ptr first, std::shared_ptr second); std::shared_ptr NextPane(const std::shared_ptr pane); @@ -201,7 +203,7 @@ private: std::shared_ptr _FindParentOfPane(const std::shared_ptr pane); std::pair _GetOffsetsForPane(const PanePoint parentOffset) const; - bool _IsAdjacent(const std::shared_ptr first, const PanePoint firstOffset, const std::shared_ptr 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, diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 8ae486548..3e11a9eab 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -1164,7 +1164,6 @@ namespace winrt::TerminalApp::implementation { if (const auto terminalTab{ _GetFocusedTabImpl() }) { - _UnZoomIfNeeded(); return terminalTab->NavigateFocus(direction); } return false; diff --git a/src/cascadia/TerminalApp/TerminalTab.cpp b/src/cascadia/TerminalApp/TerminalTab.cpp index a3a2a1651..99865cbbb 100644 --- a/src/cascadia/TerminalApp/TerminalTab.cpp +++ b/src/cascadia/TerminalApp/TerminalTab.cpp @@ -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) + // 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)) { - if (_mruPanes.size() < 2) + const auto res = _rootPane->FocusPane(newFocus); + + if (_zoomedPane) { - 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 (auto newFocus = _rootPane->NavigateDirection(_activePane, direction)) - { - return _rootPane->FocusPane(newFocus); + UpdateZoom(newFocus); } - return false; + return res; } + + return false; } // Method Description: @@ -680,27 +675,11 @@ namespace winrt::TerminalApp::implementation // - true if two panes were swapped. bool TerminalTab::SwapPane(const FocusDirection& direction) { - if (direction == FocusDirection::Previous) + // 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)) { - 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)) - { - return _rootPane->SwapPanes(_activePane, neighbor); - } - - return false; + return _rootPane->SwapPanes(_activePane, neighbor); } return false; @@ -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: + // - + void TerminalTab::UpdateZoom(std::shared_ptr 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 diff --git a/src/cascadia/TerminalApp/TerminalTab.h b/src/cascadia/TerminalApp/TerminalTab.h index b1824d98e..2ca51f1c5 100644 --- a/src/cascadia/TerminalApp/TerminalTab.h +++ b/src/cascadia/TerminalApp/TerminalTab.h @@ -79,6 +79,7 @@ namespace winrt::TerminalApp::implementation void ResetRuntimeTabColor(); void ActivateColorPicker(); + void UpdateZoom(std::shared_ptr newFocus); void ToggleZoom(); bool IsZoomed(); void EnterZoom();