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.

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

Fix moving selection past scroll area #13318

Merged
4 commits merged into from
Jun 22, 2022
Merged

Conversation

carlos-zamora
Copy link
Member

@carlos-zamora carlos-zamora commented Jun 17, 2022

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

@carlos-zamora

This comment was marked as resolved.

DHowett pushed a commit that referenced this pull request Jun 22, 2022
## 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
@carlos-zamora carlos-zamora added the Needs-Second It's a PR that needs another sign-off label Jun 22, 2022
@ghost ghost requested review from PankajBhojwani and lhecker June 22, 2022 16:41
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

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

i love how much easier our lives are with these smart helpers

@DHowett DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jun 22, 2022
@ghost
Copy link

ghost commented Jun 22, 2022

Hello @DHowett!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 72a4e93 into main Jun 22, 2022
@ghost ghost deleted the dev/cazamor/kbd-sln/bugfix-movePastBottom branch June 22, 2022 23:16
@ghost
Copy link

ghost commented Jul 6, 2022

🎉Windows Terminal Preview v1.15.186 has been released which incorporates this pull request.:tada:

Handy links:

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AutoMerge Marked for automatic merge by the bot when requirements are met Needs-Second It's a PR that needs another sign-off
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants