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

Clean up Pane #2494

Merged
merged 4 commits into from
Aug 28, 2019
Merged

Clean up Pane #2494

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
138fd3f
Merge pane splitting methods
richardszalay Aug 20, 2019
76ff685
Rename Tab::(Can)AddSplit to (Can)SplitPane to align with Pane methods
richardszalay Aug 21, 2019
50d7e09
Remove unused local variable in Pane::_CanSplit
richardszalay Aug 21, 2019
a09a591
Remove redundant 'else' branches in Pane
richardszalay Aug 21, 2019
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
123 changes: 59 additions & 64 deletions src/cascadia/TerminalApp/Pane.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,26 +164,26 @@ bool Pane::ResizePane(const Direction& direction)
{
return _Resize(direction);
}
else

// If neither of our children were the focused leaf, then recurse into
// our children and see if they can handle the resize.
// For each child, if it has a focused descendant, try having that child
// handle the resize.
// If the child wasn't able to handle the resize, it's possible that
// there were no descendants with a separator the correct direction. If
// our separator _is_ the correct direction, then we should be the pane
// to resize. Otherwise, just return false, as we couldn't handle it
// either.
if ((!_firstChild->_IsLeaf()) && _firstChild->_HasFocusedChild())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pattern ("find the non-leaf's child, if it has one") appears a lot. Is there a way to pull it out into a helper? I know that we do slightly different things with each one, but the pattern looks the same:

if(!<x>.leaf && <x>.child) {
    make <x> do <A>, or do <_A> ourselves
}

perhaps we can:

auto x = _findFirstChildPaneMatchingThoseCriteriaOrNot();
return x.has_value() && x->A() || _A();

{
// If neither of our children were the focused leaf, then recurse into
// our children and see if they can handle the resize.
// For each child, if it has a focused descendant, try having that child
// handle the resize.
// If the child wasn't able to handle the resize, it's possible that
// there were no descendants with a separator the correct direction. If
// our separator _is_ the correct direction, then we should be the pane
// to resize. Otherwise, just return false, as we couldn't handle it
// either.
if ((!_firstChild->_IsLeaf()) && _firstChild->_HasFocusedChild())
{
return _firstChild->ResizePane(direction) || _Resize(direction);
}
else if ((!_secondChild->_IsLeaf()) && _secondChild->_HasFocusedChild())
{
return _secondChild->ResizePane(direction) || _Resize(direction);
}
return _firstChild->ResizePane(direction) || _Resize(direction);
}

if ((!_secondChild->_IsLeaf()) && _secondChild->_HasFocusedChild())
{
return _secondChild->ResizePane(direction) || _Resize(direction);
}

return false;
}

Expand Down Expand Up @@ -253,26 +253,26 @@ bool Pane::NavigateFocus(const Direction& direction)
{
return _NavigateFocus(direction);
}
else

// If neither of our children were the focused leaf, then recurse into
// our children and see if they can handle the focus move.
// For each child, if it has a focused descendant, try having that child
// handle the focus move.
// If the child wasn't able to handle the focus move, it's possible that
// there were no descendants with a separator the correct direction. If
// our separator _is_ the correct direction, then we should be the pane
// to move focus into our other child. Otherwise, just return false, as
// we couldn't handle it either.
if ((!_firstChild->_IsLeaf()) && _firstChild->_HasFocusedChild())
{
// If neither of our children were the focused leaf, then recurse into
// our children and see if they can handle the focus move.
// For each child, if it has a focused descendant, try having that child
// handle the focus move.
// If the child wasn't able to handle the focus move, it's possible that
// there were no descendants with a separator the correct direction. If
// our separator _is_ the correct direction, then we should be the pane
// to move focus into our other child. Otherwise, just return false, as
// we couldn't handle it either.
if ((!_firstChild->_IsLeaf()) && _firstChild->_HasFocusedChild())
{
return _firstChild->NavigateFocus(direction) || _NavigateFocus(direction);
}
else if ((!_secondChild->_IsLeaf()) && _secondChild->_HasFocusedChild())
{
return _secondChild->NavigateFocus(direction) || _NavigateFocus(direction);
}
return _firstChild->NavigateFocus(direction) || _NavigateFocus(direction);
}

if ((!_secondChild->_IsLeaf()) && _secondChild->_HasFocusedChild())
{
return _secondChild->NavigateFocus(direction) || _NavigateFocus(direction);
}

return false;
}

Expand Down Expand Up @@ -348,15 +348,13 @@ std::shared_ptr<Pane> Pane::GetFocusedPane()
{
return _lastFocused ? shared_from_this() : nullptr;
}
else

auto firstFocused = _firstChild->GetFocusedPane();
if (firstFocused != nullptr)
{
auto firstFocused = _firstChild->GetFocusedPane();
if (firstFocused != nullptr)
{
return firstFocused;
}
return _secondChild->GetFocusedPane();
return firstFocused;
}
return _secondChild->GetFocusedPane();
}

// Method Description:
Expand Down Expand Up @@ -785,21 +783,22 @@ void Pane::_ApplySplitDefinitions()
// - True if the pane can be split. False otherwise.
bool Pane::CanSplit(SplitState splitType)
{
if (!_IsLeaf())
if (_IsLeaf())
{
if (_firstChild->_HasFocusedChild())
{
return _firstChild->CanSplit(splitType);
}
else if (_secondChild->_HasFocusedChild())
{
return _secondChild->CanSplit(splitType);
}
return _CanSplit(splitType);
}

return false;
if (_firstChild->_HasFocusedChild())
{
return _firstChild->CanSplit(splitType);
}

if (_secondChild->_HasFocusedChild())
{
return _secondChild->CanSplit(splitType);
}

return _CanSplit(splitType);
return false;
}

// Method Description:
Expand Down Expand Up @@ -839,8 +838,6 @@ void Pane::Split(SplitState splitType, const GUID& profile, const TermControl& c
// - True if the pane can be split. False otherwise.
bool Pane::_CanSplit(SplitState splitType)
{
const bool changeWidth = _splitState == SplitState::Vertical;

const Size actualSize{ gsl::narrow_cast<float>(_root.ActualWidth()),
gsl::narrow_cast<float>(_root.ActualHeight()) };

Expand Down Expand Up @@ -953,14 +950,12 @@ Size Pane::_GetMinSize() const
{
return _control.MinimumSize();
}
else
{
const auto firstSize = _firstChild->_GetMinSize();
const auto secondSize = _secondChild->_GetMinSize();
const auto newWidth = firstSize.Width + secondSize.Width + (_splitState == SplitState::Vertical ? PaneSeparatorSize : 0);
const auto newHeight = firstSize.Height + secondSize.Height + (_splitState == SplitState::Horizontal ? PaneSeparatorSize : 0);
return { newWidth, newHeight };
}

const auto firstSize = _firstChild->_GetMinSize();
const auto secondSize = _secondChild->_GetMinSize();
const auto newWidth = firstSize.Width + secondSize.Width + (_splitState == SplitState::Vertical ? PaneSeparatorSize : 0);
const auto newHeight = firstSize.Height + secondSize.Height + (_splitState == SplitState::Horizontal ? PaneSeparatorSize : 0);
return { newWidth, newHeight };
}

DEFINE_EVENT(Pane, Closed, _closedHandlers, ConnectionClosedEventArgs);