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

Graphing Calculator - Add Shortcuts to zoom in/out #882

Merged
merged 1 commit into from Dec 19, 2019
Merged

Graphing Calculator - Add Shortcuts to zoom in/out #882

merged 1 commit into from Dec 19, 2019

Conversation

rudyhuyn
Copy link
Contributor

Description of the changes:

  • Support Ctrl + -/+ (in addition to the existing Ctrl + Numpad -/+)

⚠ Important change:

  • We use the new RS3+ UWP api to manage shortcuts instead of our own implementation. The UWP being more powerful and supporting multi-shortcuts. We should plan to migrate all other shortcuts in the future
  • The current shortcut was localized, not this implementation, but I don't think it's something we need.

Also:

  • simplify KeyboardShortcutManager
  • fix a bug in KeyboardShortcutManager (s_VirtualKeyControlInverseChordsForButtons were used instead of s_VirtualKeyInverseChordsForButtons)

How changes were validated:

  • Manually, English and French keyboard layout.

joseartrivera
joseartrivera reviewed Dec 18, 2019
View reviewed changes
src/Calculator/Views/GraphingCalculator/GraphingCalculator.xaml.cpp
// OemMinus and OemAdd aren't declared in the VirtualKey enum, we can't add this accelerator XAML-side
auto virtualKey = ref new KeyboardAccelerator();
virtualKey->Key = (VirtualKey)187; //OemMinus key
virtualKey->Modifiers = VirtualKeyModifiers::Control;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the need for Ctrl instead of just +/- directly? I think it might be more intuitive and discoverable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Specs :D

I suppose to have the same behavior than Maps.

cc @grochocki

src/CalcViewModel/Common/KeyboardShortcutManager.cpp Show resolved Hide resolved
@joseartrivera joseartrivera merged commit 38da8d7 into microsoft:feature/GraphingCalculator Dec 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joseartrivera joseartrivera joseartrivera approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@rudyhuyn @joseartrivera