Add modifier key support for global hotkey configuration - #92
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds modifier-key support (Ctrl/Shift/Alt) to the app’s configurable global hotkeys, including UI capture flows and settings persistence, so users can bind combinations like Ctrl+Shift+R instead of single keys.
Changes:
- Introduces
HotkeyModifiersand persists hotkey modifiers inUserSettings,UserSettingsService, andSettingsViewModel. - Updates Settings UI to capture/display modifier combinations and routes key capture through
IGlobalHotkeyService. - Extends global hotkey hook logic to match configured modifier combinations and updates/extends tests accordingly.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Pointframe/ViewModels/SettingsViewModel.cs | Adds modifier properties, display-name building for combinations, and persists modifiers on save/reset/defaults. |
| Pointframe/SettingsWindow.xaml.cs | Injects IGlobalHotkeyService, starts/stops OS-level capture mode, updates live modifier display, and applies captured vk+modifiers to the VM. |
| Pointframe/SettingsWindow.xaml | Updates copy and recording UI to show live modifier state and wires PreviewKeyUp for display updates; updates command binding name. |
| Pointframe/Services/UserSettingsService.cs | Ensures settings cloning includes the new modifier properties so Update() doesn’t drop them. |
| Pointframe/Services/IGlobalHotkeyService.cs | Makes the interface public and adds Begin/End key-capture mode API. |
| Pointframe/Services/GlobalHotkeyService.cs | Implements key-capture mode and updates hotkey matching to use stored modifier requirements. |
| Pointframe/NativeMethods.cs | Adds additional VK constants used by modifier/capture logic. |
| Pointframe/Models/UserSettings.cs | Adds persisted modifier fields and updates default record hotkey to Ctrl+Shift+R. |
| Pointframe/Models/HotkeyModifiers.cs | New [Flags] enum for Ctrl/Shift/Alt modifiers. |
| Pointframe.Tests/ViewModels/SettingsViewModelTests.cs | Updates/extends tests to cover modifier persistence and display updates. |
| Pointframe.Tests/SettingsWindowTests.cs | Updates tests for new callback-based capture path and DI constructor change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (wParam == (IntPtr)NativeMethods.WM_KEYDOWN) | ||
| { | ||
| var kb = Marshal.PtrToStructure<NativeMethods.KBDLLHOOKSTRUCT>(lParam); |
There was a problem hiding this comment.
Alt-based hotkeys (and Alt during key capture) may not work because low-level keyboard hooks receive WM_SYSKEYDOWN for Alt+key combinations, but this code only processes WM_KEYDOWN. Consider handling WM_SYSKEYDOWN (and possibly WM_SYSKEYUP for completeness) in both capture mode and normal hotkey matching, and add the needed constants in NativeMethods.
| /// <summary> | ||
| /// Starts key-capture mode. Non-modifier key presses are blocked at the OS level | ||
| /// and routed to <paramref name="onKeyPressed"/> instead of firing snip/record events. | ||
| /// Modifier keys (Ctrl/Shift/Alt/Win) pass through so WPF can update the live display. |
There was a problem hiding this comment.
The XML docs say modifier keys include Win and that they pass through so WPF can update the live display, but the current UI/model only tracks Ctrl/Shift/Alt (Win isn’t included in HotkeyModifiers or displayed in SettingsWindow). Either add Win support end-to-end or update the documentation to match the actual supported modifiers.
| /// Modifier keys (Ctrl/Shift/Alt/Win) pass through so WPF can update the live display. | |
| /// Modifier keys (Ctrl/Shift/Alt) pass through so WPF can update the live display. |
| [Fact] | ||
| public void HotkeyCapture_PreviewKeyDown_StoresNewHotkey() | ||
| public void OnCaptureHotkeyKeyPressed_StoresNewHotkey() | ||
| { | ||
| StaTestHelper.Run(() => | ||
| { | ||
| var window = CreateWindow(out var viewModel); | ||
| viewModel.IsRecordingHotkey = true; | ||
| var args = CreateKeyArgs(Key.A); | ||
|
|
||
| InvokePrivateHandler(window, "HotkeyCapture_PreviewKeyDown", window, args); | ||
| InvokeCallback(window, "OnCaptureHotkeyKeyPressed", (uint)KeyInterop.VirtualKeyFromKey(Key.A), HotkeyModifiers.None); | ||
|
|
||
| Assert.True(args.Handled); | ||
| Assert.Equal((uint)KeyInterop.VirtualKeyFromKey(Key.A), viewModel.RegionCaptureHotkey); | ||
| Assert.False(viewModel.IsRecordingHotkey); | ||
| }); |
There was a problem hiding this comment.
These tests cover that a captured key is stored, but don’t verify the new modifier behavior introduced by the callback signature. Consider asserting that RegionCaptureHotkeyModifiers is updated when a non-None modifier set is passed, and adding a similar test for OnRecordHotkeyKeyPressed to ensure record hotkey modifiers are persisted to the view model.
…ests - Handle WM_SYSKEYDOWN alongside WM_KEYDOWN so Alt+key combinations are correctly captured and matched in the low-level keyboard hook - Add WM_SYSKEYDOWN constant to NativeMethods - Fix XML doc on IGlobalHotkeyService.BeginKeyCaptureMode to remove incorrect mention of Win key (only Ctrl/Shift/Alt are supported) - Add tests for modifier persistence: OnCaptureHotkeyKeyPressed and OnRecordHotkeyKeyPressed now assert that non-None modifier sets are saved to the view model Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Tokenless uploads to Codecov are no longer accepted. The upload was silently failing with 'Token required - not valid tokenless upload' while CI still reported success due to fail_ci_if_error: false. Requires adding CODECOV_TOKEN to repository secrets (Settings > Secrets and variables > Actions). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
- Add GlobalHotkeyServiceTests covering BeginKeyCaptureMode,
EndKeyCaptureMode, Dispose, ModifiersMatch, and IsModifierVk
- Add SettingsWindowTests for record hotkey panel visibility,
Escape cancellation, and PreviewKeyDown/Up handlers for both
capture and record hotkey flows
- Add SettingsViewModelTests covering BuildHotkeyDisplayName with
vk==0 ('Not set'), Alt modifier, and ResetCurrentSection for
both Capture and Recording sections
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
No description provided.