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

wpf: only dismiss selection for real chars, not modifiers #5388

Merged
merged 1 commit into from
Apr 17, 2020

Conversation

DHowett-MSFT
Copy link
Contributor

Selection would act up when you were using shift to ignore VT mouse
mode: we would get hundreds of WM_KEYDOWN for VK_SHIFT and dismiss the
selection every time.

I took the opportunity to move the actual responsibility for key event
dispatch into HwndTerminal. In the future, I'd like to make more of the
TerminalXxx calls just call impl methods on HwndTerminal.

PR Checklist

  • Closes #xxx
  • CLA
  • Tests added/passed
  • Requires documentation to be updated
  • Core Contributor Discussion

Validation Steps Performed

Selection now works in VT mouse mode.

Selection would act up when you were using shift to ignore VT mouse
mode: we would get hundreds of WM_KEYDOWN for VK_SHIFT and dismiss the
selection every time.

I took the opportunity to move the actual responsibility for key event
dispatch into HwndTerminal. In the future, I'd like to make more of the
TerminalXxx calls just call impl methods on HwndTerminal.
@DHowett-MSFT DHowett-MSFT requested a review from ZoeyR April 17, 2020 07:20
@DHowett-MSFT
Copy link
Contributor Author

It's easier to review them side-by-side instead of in the diff view.

@zadjii-msft
Copy link
Member

@msftbot make sure @ZoeyR signs off on this

@ghost ghost added the AutoMerge Marked for automatic merge by the bot when requirements are met label Apr 17, 2020
@ghost
Copy link

ghost commented Apr 17, 2020

Hello @zadjii-msft!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

  • I'll only merge this pull request if it's approved by @ZoeyR

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

@zadjii-msft zadjii-msft added Area-WPFControl Things related to the WPF version of the TermControl and removed AutoMerge Marked for automatic merge by the bot when requirements are met labels Apr 17, 2020
@DHowett-MSFT DHowett-MSFT merged commit 5740e19 into master Apr 17, 2020
@DHowett-MSFT DHowett-MSFT deleted the dev/duhowett/wpf/holy_shift branch April 17, 2020 18:28
@ghost
Copy link

ghost commented Apr 22, 2020

🎉Windows Terminal Preview v0.11.1121.0 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-WPFControl Things related to the WPF version of the TermControl
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants