Skip to content

Do not dismiss selection if the Windows keys is pressed as a key-combination#9163

Merged
6 commits merged intomicrosoft:mainfrom
imaginary-person:win-button-do-not-dismiss-selection
Feb 17, 2021
Merged

Do not dismiss selection if the Windows keys is pressed as a key-combination#9163
6 commits merged intomicrosoft:mainfrom
imaginary-person:win-button-do-not-dismiss-selection

Conversation

@imaginary-person
Copy link
Contributor

@imaginary-person imaginary-person commented Feb 14, 2021

Aims to fix #8791.

Summary of the Pull Request

Prior to this PR, if the Windows key was pressed as a part of a key combination, then selection was being dismissed. For example, when a user pressed Windows + Shift + S keys to invoke the Capture & Annotate tool.
This PR adds an exception for not clearing selection when either of the two Windows keys are pressed as part of a key combination.
It was tested manually by trying to reproduce the issue.

PR Checklist

Validation Steps Performed

  1. Build Terminal.
  2. Write anything & make a selection.
  3. Press Windows+ Shift + S keys.
  4. The Capture & Annotate tool appears but the selection made in step 2 isn't dismissed (doesn't disappear).

Get changes from main repo
microsoft#8791. Don't dismiss selection if Windows key was also pressed.
Remove trailing whitespace
@ghost ghost added Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels Feb 14, 2021
Clear whitespace in empty line
Revise code for better abstraction & readability
@imaginary-person imaginary-person changed the title Win button do not dismiss selection Do not dismiss selection if the Windows keys is pressed as a key-combination Feb 14, 2021
Copy link
Contributor

@Don-Vito Don-Vito left a comment

Choose a reason for hiding this comment

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

Not an expert, but besides a nit looks good to me! 😊

Probably, not as a part of this PR, we should also prevent the winkey combinations from scrolling (inside Terminal::SendKeyEvent)

Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

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

Thanks!

@carlos-zamora carlos-zamora added AutoMerge Marked for automatic merge by the bot when requirements are met zPreview-Service-Consider labels Feb 17, 2021
@ghost
Copy link

ghost commented Feb 17, 2021

Hello @carlos-zamora!

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 7d37ba2 into microsoft:main Feb 17, 2021
DHowett pushed a commit that referenced this pull request Feb 23, 2021
…ination (#9163)

Aims to fix #8791.

<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
Prior to this PR, if the Windows key was pressed as a part of a key combination, then selection was being dismissed. For example,  when a user pressed `Windows` + `Shift` + `S` keys to invoke the _Capture & Annotate_ tool.
This PR adds an exception for not clearing selection when either of the two Windows keys are pressed as part of a key combination.
It was tested manually by trying to reproduce the issue.

<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [ ] Closes #8791
* [x ] CLA signed.
* [x ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #8791

<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
1. Build Terminal.
2. Write anything & make a selection.
3. Press `Windows`+ `Shift` + `S` keys.
4. The _Capture & Annotate_ tool appears but the selection made in step 2 isn't dismissed (doesn't disappear).

(cherry picked from commit 7d37ba2)
@ghost
Copy link

ghost commented Mar 1, 2021

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

Handy links:

@ghost ghost mentioned this pull request Mar 1, 2021
@ghost
Copy link

ghost commented Mar 1, 2021

🎉Windows Terminal Preview v1.7.572.0 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

Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Win+Shift+S dismisses the selection

6 participants