Skip to content

Commit

Permalink
Fix moving selection past scroll area (#13318)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request
Introduced in #10824, this fixes a bug where you could use keyboard selection to move below the scroll area. Instead, we now clamp movement to the mutable viewport (aka the scrollable area). Specifically, we clamp to the corners (i.e. 0,0 or bottom right cell of scroll area).

## Validation Steps Performed
✅ (no output) try to move past bottom of viewport
✅ (with output, at bottom of scroll area) try to move past viewport
✅ (with output, NOT at bottom of scroll area) try to move past viewport
✅ try to move past top of viewport
  • Loading branch information
carlos-zamora authored Jun 22, 2022
1 parent 24a53d4 commit 72a4e93
Showing 1 changed file with 7 additions and 4 deletions.
11 changes: 7 additions & 4 deletions src/cascadia/TerminalCore/TerminalSelection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ void Terminal::UpdateSelection(SelectionDirection direction, SelectionExpansion
// signifying that the other endpoint is the one we want to move.
auto targetPos{ MovingEnd() ? _selection->end : _selection->start };

// 2. Perform the movement
// 2.A) Perform the movement
switch (mode)
{
case SelectionExpansion::Char:
Expand All @@ -393,6 +393,9 @@ void Terminal::UpdateSelection(SelectionDirection direction, SelectionExpansion
break;
}

// 2.B) Clamp the movement to the mutable viewport
targetPos = std::min(targetPos, _GetMutableViewport().BottomRightInclusive());

// 3. Actually modify the selection
if (_markMode && !mods.IsShiftPressed())
{
Expand All @@ -410,17 +413,17 @@ void Terminal::UpdateSelection(SelectionDirection direction, SelectionExpansion
}

// 4. Scroll (if necessary)
if (const auto viewport = _GetVisibleViewport(); !viewport.IsInBounds(targetPos))
if (const auto visibleViewport = _GetVisibleViewport(); !visibleViewport.IsInBounds(targetPos))
{
if (const auto amtAboveView = viewport.Top() - targetPos.Y; amtAboveView > 0)
if (const auto amtAboveView = visibleViewport.Top() - targetPos.Y; amtAboveView > 0)
{
// anchor is above visible viewport, scroll by that amount
_scrollOffset += amtAboveView;
}
else
{
// anchor is below visible viewport, scroll by that amount
const auto amtBelowView = targetPos.Y - viewport.BottomInclusive();
const auto amtBelowView = targetPos.Y - visibleViewport.BottomInclusive();
_scrollOffset -= amtBelowView;
}
_NotifyScrollEvent();
Expand Down

0 comments on commit 72a4e93

Please sign in to comment.