Support the use of Windows key & four-combination hotkey #3157
Conversation
This comment has been minimized.
This comment has been minimized.
|
🥷 Code experts: Yusyuriv, onesounds Yusyuriv has most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame: To learn more about /:\ gitStream - Visit our Docs |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
jjw24
changed the title
jjw24
changed the title
jjw24
changed the title
jjw24
changed the title
📝 WalkthroughWalkthroughThe pull request introduces a comprehensive overhaul of the hotkey management system in Flow Launcher. The changes involve transitioning from the NHotkey library to ChefKeys, refactoring the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (12)
Flow.Launcher/Helper/HotKeyMapper.cs (1)
65-68: Exposing ChefKeys registration checks.
TheCanRegisterHotkeymethod correctly delegates toChefKeysManager. Consider surfacing reason codes if future debug is needed.Flow.Launcher/HotkeyControl.xaml.cs (3)
77-79: Commented-out code.
These lines are now replaced by the approach in lines 80-82. If not needed, remove them to reduce confusion.
113-125: IsWPFHotkeyControl dependency property.
This property neatly segregates WPF hotkey handling from ChefKeys-only contexts. Good architectural choice for controlling flow based on environment.
159-159: Commented-out SetHotkey.
Remove this if no longer intended to avoid confusion for future maintainers.Flow.Launcher/HotkeyControlDialog.xaml.cs (1)
45-50: Added isWPFHotkeyControl parameter.
Constructor now fully distinguishes WPF from ChefKeys usage, preventing conflicts in key handling.Flow.Launcher.Infrastructure/Hotkey/HotkeyModel.cs (1)
80-88: Removed older constructor.
Ensures no confusion between ChefKeys usage and WPF usage.Flow.Launcher/CustomQueryHotkeySetting.xaml.cs (1)
50-50: Maintain consistency when updating hotkeys.
After reassigning the hotkey, confirm that any references toupdateCustomHotkey.Hotkey(such as in logs or other internal structures) are updated to avoid stale data.Flow.Launcher/Resources/Pages/WelcomePage2.xaml (2)
116-116: Provide short help text for the default hotkey update.
Switching from “Alt+Space” to “LeftAlt+Space” adds clarity but might surprise existing users. Consider adding a short on-screen note or a release note describing this change.
120-120: Confirm that the window title label is localized if needed.
Using a dynamic resource for the window title is excellent for localization, but verify that translations exist for all preferred locales, so the label isn’t shown in English-only.Flow.Launcher.Infrastructure/UserSettings/Settings.cs (2)
291-305: Ensure all relevant hotkeys are included or safely removed.These lines add hotkeys to the
RegisteredHotkeyscollection only if they are not empty. Verify that any newly added custom hotkeys are tested for collision or duplication with other shortcuts in the system or externally managed by ChefKeys.Do you want me to generate a script to detect collisions between these hotkeys and those commented out below, or any known system shortcuts?
337-364: Reassess commented-out hotkeys.A significant block of preexisting hotkeys is now commented out. If they are legacy shortcuts no longer supported, consider removing them altogether to keep the codebase clean. If they are placeholders awaiting future reactivation, add a comment clarifying their intended usage to help maintain code clarity.
Flow.Launcher/Flow.Launcher.csproj (1)
86-86: Validate ChefKeys package stability and compatibility.Introducing the
ChefKeyspackage is a core dependency change. Confirm that version0.1.1is stable for production and that there are no known issues that could impact the hotkey functionality. If necessary, pin or range-limit the version to prevent inadvertent upgrades in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
Flow.Launcher.Infrastructure/Hotkey/HotkeyModel.cs(5 hunks)Flow.Launcher.Infrastructure/KeyConstant.cs(1 hunks)Flow.Launcher.Infrastructure/UserSettings/Settings.cs(3 hunks)Flow.Launcher/CustomQueryHotkeySetting.xaml(1 hunks)Flow.Launcher/CustomQueryHotkeySetting.xaml.cs(2 hunks)Flow.Launcher/Flow.Launcher.csproj(1 hunks)Flow.Launcher/Helper/HotKeyMapper.cs(3 hunks)Flow.Launcher/HotkeyControl.xaml.cs(7 hunks)Flow.Launcher/HotkeyControlDialog.xaml.cs(7 hunks)Flow.Launcher/Languages/en.xaml(0 hunks)Flow.Launcher/Resources/Pages/WelcomePage2.xaml(1 hunks)Flow.Launcher/Resources/Pages/WelcomePage2.xaml.cs(1 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneHotkeyViewModel.cs(2 hunks)Flow.Launcher/SettingPages/Views/SettingsPaneHotkey.xaml(1 hunks)
💤 Files with no reviewable changes (1)
- Flow.Launcher/Languages/en.xaml
🔇 Additional comments (65)
Flow.Launcher/Helper/HotKeyMapper.cs (8)
7-8: ChefKeys integration.
The addition of ChefKeys imports aligns with the new hotkey management system.
16-16: Blank line addition.
No functional impact.
22-24: Starting the ChefKeysManager early.
Registering the default hotkey and calling Start() here is logical. Just ensure _settings.Hotkey is valid on startup to avoid runtime errors.
28-28: ToggleHotkey method.
Logic appears correct: ignoring hotkeys when _mainViewModel.ShouldIgnoreHotkeys() is true and then toggling the launcher otherwise.
34-36: Registering hotkeys with collision checks.
Invoking ChefKeysManager.RegisterHotkey(...) directly is concise, but consider verifying collisions or handling potential exceptions.
39-42: Conditional unregistration.
Good safeguard for empty hotkey strings.
55-55: Registering custom plugin hotkey.
Well-structured callback to handle custom queries.
70-72: CheckHotkeyAvailability and CheckHotkeyValid.
Straightforward pass-through to ChefKeysManager.
Flow.Launcher/HotkeyControl.xaml.cs (11)
4-4: Added System.Globalization import.
Confirm usage. If unused, consider removing to keep dependencies lean.
16-17: Introducing a dialog field.
Storing the HotkeyControlDialog instance suggests repeated dialog usage.
80-82: Hotkey model creation.
Reading the hotkey string into a HotkeyModel and setting UI displays is clear and more maintainable than inline string parsing.
134-136: CheckHotkeyValid pass-through.
Delegating validity checks to HotKeyMapper.CheckHotkeyValid. Straightforward approach.
137-138: CheckWPFHotkeyAvailability method.
Calls ValidateForWpf. Keeps WPF-specific checks neatly encapsulated.
144-144: Defining CurrentHotkey.
Defaulting to an empty HotkeyModel on initialization ensures the remainder of the class can rely on a stable reference.
154-156: Instantiating HotkeyControlDialog.
Passing IsWPFHotkeyControl helps unify the logic for WPF vs. ChefKeys. Good to see ShowAsync usage for non-blocking UI.
172-191: SetHotkey logic.
• Checks for null or empty raw hotkey.
• Validates availability before assigning.
• Unregisters the old hotkey if WPF-based.
Overall, a clean approach to updating the hotkey while avoiding duplicates.
204-206: Delete flow.
Unregistering then resetting display is a sound pattern, preventing leftover registrations.
Line range hint 213-217: Handling empty or null hotkey.
Displays "none" if there's no key present—good for user clarity.
Line range hint 223-227: Availability check logic.
Selects the correct approach based on IsWPFHotkeyControl. Good handling for multi-environment hotkeys.
Flow.Launcher/HotkeyControlDialog.xaml.cs (18)
10-11: ChefKeys import.
Ensures the dialog can manipulate ChefKeysManager. Check whether all references to old hotkey libraries are safely removed or replaced.
23-31: New hotkey fields and flags.
CurrentHotkey and HotkeyToUpdate separation clarifies the difference between existing vs. pending changes. isWPFHotkeyControl further refines behavior.
52-53: ChefKeys Start Menu blocking.
Setting ChefKeysManager.StartMenuEnableBlocking = true indicates you’re blocking Windows key from opening Start. Verify no system-level side effects.
60-64: Synchronizing current and updated hotkey.
Assigning HotkeyToUpdate from CurrentHotkey ensures the dialog can revert or discard changes accurately.
66-68: Preloading display.
SetKeysToDisplay(CurrentHotkey) and resetting clearKeysOnFirstType to true is a clean approach for the initial dialog state.
71-72: Blocking Start Menu.
Confirm no user frustration arises from forcibly blocking the Start Menu while the dialog is open.
77-79: Reset action.
Directly overwriting HotkeyToUpdate with the default hotkey. Good for a full revert.
84-92: Delete action.
Clearing everything and returning the “none” display is consistent with a removed hotkey.
97-98: Disable blocking before cancel.
Correct to re-enable normal Windows key usage on cancel.
105-106: Disable blocking before save.
Similar application of re-enabling the Windows key once the user commits or discards changes.
114-116: Constructing ResultValue.
Combining new and old hotkeys with a colon format is a neat way to pass them back to the caller.
127-132: Handling first typed key.
Clearing keys for the first input is an intuitive approach—prevents leftover data from previous sessions.
134-135: Ignore StartMenuSimulatedKey.
Preventing a duplicated input from the simulated Windows key ensures cleaner user experience.
137-143: ChefKeys usage vs. WPF usage.
Splitting logic for isWPFHotkeyControl ensures that multi-key combos for ChefKeys do not conflict with the single-key WPF approach.
157-172: KeysToDisplay handling.
Properly clearing UI states and falling back to “none” if empty. The code consistently updates the list for user feedback.
Line range hint 183-207: Detecting registered hotkeys.
Displays a helpful message and toggles button availability if the key is already occupied. Great user feedback approach.
223-227: Combined WPF & ChefKeys validation.
Uses the appropriate check method, then toggles the UI depending on success or failure. Nicely integrated.
242-246: Distinguishing check methods.
Splitting availability checks for standard hotkeys vs. WPF gestures clarifies code flow.
Flow.Launcher.Infrastructure/Hotkey/HotkeyModel.cs (17)
6-6: Optional plugin reference.
Flow.Launcher.Plugin might be used for Key enumeration or extended checks. Ensure no unintentional coupling.
17-19: New string properties.
HotkeyRaw and PreviousHotkey facilitate incremental or specialized changes.
56-61: SetHotkeyFromString method.
Separating parsing from the constructor is cleaner and more extensible.
64-73: WPF-specific hotkey assignment.
Accurately sets modifiers and re-derives HotkeyRaw. Good for bridging both ChefKeys and WPF usage.
75-78: String-based constructor.
Calls SetHotkeyFromString(hotkey). Straight to the point.
89-94: AddString for ChefKeys combos.
Allows incremental building of multi-key combos. Good design for repeated input flows.
95-96: MaxKeysReached check.
Prevents overly complex combos that might confuse users or exceed system constraints.
97-105: Clear method.
Resets the struct to a blank state, including HotkeyRaw and PreviousHotkey.
116-121: Handling LeftAlt/RightAlt.
Expanding to specific alt keys ensures granular control over hotkey usage.
124-129: Handling LeftShift/RightShift.
Similarly helps differentiate shift keys.
132-137: Handling LWin and RWin.
Matches PR objective to allow left or right Windows key usage independently.
140-145: Handling LeftCtrl/RightCtrl.
Completes the suite of side-specific modifiers.
170-174: ToChefKeysString.
Joins the enumerated display keys for consistent ChefKeys usage.
177-178: DisplayKeysRaw.
Returns the raw hotkey tokens to support dynamic UI updates.
179-207: EnumerateDisplayKeys.
Smart approach to unify how keys are displayed. Conditionals for hidden Key.LWin, etc., keep logic clean for different contexts.
209-225: Key string conversion for ChefKeys.
Mapping generic “Ctrl/Alt/Shift/Win” to side-specific forms is a good design choice.
230-234: ValidateForWpf.
Ensures that partial modifiers like LeftShift or RightShift are not used as the primary key, which avoids invalid gestures.
Flow.Launcher.Infrastructure/KeyConstant.cs (2)
6-7: Introducing LeftAlt constant.
Adds clarity by differentiating left vs. generic alt references.
10-10: File end blank line.
No issue identified.
Flow.Launcher/Resources/Pages/WelcomePage2.xaml.cs (1)
30-30: Validate hotkey uniqueness and potential conflicts.
While switching from SetHotkey to RegisterHotkey, ensure that HotkeyRaw is guaranteed to be unique or conflict-free in the rest of the system. If another component registers the same hotkey, that might cause unexpected behavior.
Flow.Launcher/CustomQueryHotkeySetting.xaml.cs (2)
40-40: Confirm validation of user-supplied hotkey strings.
Using HotkeyControl.CurrentHotkey.HotkeyRaw directly may bypass validation if HotkeyRaw is not checked. Ensure that invalid or duplicate hotkeys are handled gracefully.
52-52: Cleanly handle unregistered hotkeys.
Unregistering the old hotkey is a good practice. Ensure that any side effects, like disposal events or callbacks, are also addressed to prevent memory leaks or leftover event handlers.
Flow.Launcher/SettingPages/ViewModels/SettingsPaneHotkeyViewModel.cs (2)
36-36: Confirm thorough testing for custom toggling logic.
While using RegisterHotkey for toggle functionality can simplify code, double-check that toggling works correctly if the user rebinds the hotkey or uses multiple toggle hotkeys.
60-60: Prevent orphaned references when deleting hotkeys.
Unregistering the custom hotkey is essential, just confirm that the UI or internal references to the removed hotkey are also cleared so there's no inconsistent state left behind.
Flow.Launcher/Resources/Pages/WelcomePage2.xaml (1)
121-121: Ensure correct tool handling of IsWPFHotkeyControl.
Setting this property to False means the WPF input pipeline won't treat this hotkey control in the default WPF hotkey manner. Make sure the new approach is tested across different OS versions if possible.
Flow.Launcher/CustomQueryHotkeySetting.xaml (1)
110-111: Confirm consistency of IsWPFHotkeyControl usage across the codebase.
Marking IsWPFHotkeyControl="False" signals that this hotkey control uses the new ChefKeys approach rather than the WPF-based hotkey mechanism. This is appropriate if the rest of the newly introduced hotkey handling indeed covers all of this control’s needs. Verify that no logic relying on the old WPF hotkey handling is inadvertently bypassed.
Flow.Launcher.Infrastructure/UserSettings/Settings.cs (1)
18-18: Double-check conflicts with OS-level shortcuts.
Switching the default app hotkey to LeftAlt + Space might conflict with the Windows system menu shortcut. Ensure that this override is intentional and will not hamper standard user workflows.
Flow.Launcher/SettingPages/Views/SettingsPaneHotkey.xaml (1)
36-41: Beware of potential Alt+Space conflicts outside WPF.
The new default hotkey "LeftAlt+Space" here matches the updated setting from Settings.cs. Confirm that this override does not intercept the normal behavior of Alt+Space (typically the system menu) unless that is the intended functionality. Also, setting IsWPFHotkeyControl="False" ensures ChefKeys handles it rather than typical WPF hotkey logic, so keep usage consistent throughout the solution.
jjw24
changed the title
This comment has been minimized.
This comment has been minimized.
|
I found a few issues:
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
Changes:
Support for query window & Custom Query Hotkeys to:
Context:
Tested:
Close #662, #2709, #1358,