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.

Sign up for GitHub

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

Jump to bottom

KeyChord should just use the existing WinRT types instead of defining our own #877

Closed
zadjii-msft opened this issue May 17, 2019 · 1 comment · Fixed by #10603
Closed

KeyChord should just use the existing WinRT types instead of defining our own #877

zadjii-msft opened this issue May 17, 2019 · 1 comment · Fixed by #10603
Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal Backlog

Comments

@zadjii-msft
Copy link
Member

Was looking at this in conjunction with resolving suggestions for #805. The XAML framework uses Windows::System::VirtualKey and Windows::System::VirtualKeyModifiers...is there a reason that Terminal should create their own types here that are now maintaining modifiers and enum values for something that already exists? Retrieving the keybindings with a KeyChord that has it's own values is proving to be challenging using them anywhere in the UI due to different types and not being able to cast them accordingly well.

Originally posted by @timheuer in #748 (comment)
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label May 17, 2019
@zadjii-msft zadjii-msft mentioned this issue May 17, 2019
Merged
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label May 17, 2019
@DHowett-MSFT DHowett-MSFT added Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Help Wanted We encourage anyone to jump in on these. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels May 20, 2019
@miniksa miniksa added Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. and removed Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. labels May 20, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label May 20, 2019
@timheuer timheuer mentioned this issue May 21, 2019
Merged
5 tasks
@zadjii-msft zadjii-msft added this to the Terminal Backlog milestone Jun 19, 2019
@mimvdb mimvdb mentioned this issue Jul 9, 2021
Merged
@ghost ghost added the In-PR This issue has a related PR label Jul 9, 2021
@ghost ghost closed this as completed in #10603 Jul 12, 2021
@ghost ghost removed the In-PR This issue has a related PR label Jul 12, 2021
ghost pushed a commit that referenced this issue Jul 12, 2021
@mimvdb
Use WinRT VirtualKeyModifiers instead of a custom enum (#10603)
17e68a0 
Replaces `KeyModifiers` with the pretty much equivalent
`VirtualKeyModifiers` enum in winrt.

After doing this I noticed #10593 which changes the KeyChords a lot, but
it seems these PRs are still compatible

The issue also mentions replacing Vkey with
`Windows::System::VirtualKey`, but I chose not to because that enum only
includes a subset of the keys terminal supports here (no VK_OEM_* keys)

## Validation Steps Performed
Changed key bind in config, and confirmed it still works after
restarting terminal

Closes #877
@ghost ghost added the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label Jul 12, 2021
@ghost
Copy link

ghost commented Jul 14, 2021

🎉This issue was addressed in #10603, which has now been successfully released as Windows Terminal Preview v1.10.1933.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Terminal Backlog
Development

Successfully merging a pull request may close this issue.

Use WinRT VirtualKeyModifiers instead of a custom enum mimvdb/terminal
3 participants
@DHowett-MSFT @miniksa @zadjii-msft