[1.14] Fix moving selection past scroll area #13353
Conversation
| 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(); |
There was a problem hiding this comment.
These changes appear unnecessary right?
There was a problem hiding this comment.
Eh, they're more helpful on main than here. I just wanted to create more clarity to distinguish between the visible viewport and mutable viewport. I'll still keep it at least to keep things as consistent as possible between these two branches.
|
Interesting! If you merge a code suggestion, approved reviews are dismissed. @lhecker you're gonna have to look over this again. Sorry! |
That's a branch policy that applies to all |
A port of commit 0c5a1e9 from #13358. This fixes an oversight from #13353. That ended up not fixing when trying to move past the top boundary. This is also a better fix overall. By moving the fix to `_MoveByChar`, we have consistency between all the `_MoveByX` functions in that they automatically clamp to be within the scroll area.
|
🎉 Handy links: |
Summary of the Pull Request
1.14 port of #13318
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 only clamp the y-coordinate to make the experience similar to that of mouse selection.
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