Skip to content

Fix moving selection past scroll area - #13318

Merged
4 commits merged into
mainfrom
dev/cazamor/kbd-sln/bugfix-movePastBottom
Jun 22, 2022
Merged

4 commits merged into
mainfrom
dev/cazamor/kbd-sln/bugfix-movePastBottom

Conversation

@carlos-zamora

@carlos-zamora Carlos Zamora (carlos-zamora) commented Jun 17, 2022 •

Copy link
Copy Markdown

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.

Dustin L. Howett (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 (carlos-zamora) added the Needs-Second It's a PR that needs another sign-off label Jun 22, 2022

Copy link
Copy Markdown
Member

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 Dustin L. Howett (DHowett) added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jun 22, 2022
@ghost

Copy link
Copy Markdown

Hello Dustin L. Howett (@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
Copy Markdown

🎉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