-
-
Notifications
You must be signed in to change notification settings - Fork 344
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
Fix custom query hotkey settings issue #3354
Conversation
This comment has been minimized.
This comment has been minimized.
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughThe pull request updates the hotkey handling in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Flow.Launcher/HotkeyControl.xaml.cs (2)
145-145
: Error message has grammatical error.The error message "Hotkey type not setted" contains a grammatical error. The correct form would be "Hotkey type not set" or "Hotkey type not specified".
- _ => throw new System.NotImplementedException("Hotkey type not setted") + _ => throw new System.NotImplementedException("Hotkey type not set")
200-200
: Same grammatical error in setter's error message.The error message "Hotkey type not setted" contains the same grammatical error as in the getter. The correct form would be "Hotkey type not set" or "Hotkey type not specified".
- throw new System.NotImplementedException("Hotkey type not setted"); + throw new System.NotImplementedException("Hotkey type not set");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/HotkeyControl.xaml.cs
(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
🔇 Additional comments (4)
Flow.Launcher/HotkeyControl.xaml.cs (4)
87-87
: Good change to default TypeProperty value.Changing the default initialization from
HotkeyType.Hotkey
toHotkeyType.None
aligns with the new enum value and properly supports custom hotkeys from dialog results.
98-99
: Appropriate addition ofNone
enum value.Adding the
None
enum value with a descriptive comment clarifies its purpose for getting hotkeys from dialog results. This addition is necessary for handling custom hotkeys that aren't part of the settings.
121-121
: Good implementation for custom hotkey storage.The addition of the private
hotkey
field and its usage in the getter whenType
isHotkeyType.None
properly separates custom hotkeys from settings-based hotkeys.Also applies to: 128-130
152-156
: Good implementation for custom hotkey assignment.The addition of the
HotkeyType.None
case in the setter properly handles custom hotkeys by storing the value in the privatehotkey
field.
This comment has been minimized.
This comment has been minimized.
do you mind briefly introduce what's the issue? |
As you can see, the new version of And previously, the default value of |
Now I added new |
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 ...
|
🥷 Code experts: Yusyuriv, onesounds Jack251970 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: To learn more about /:\ gitStream - Visit our Docs |
Add new
enum
type to handle logic for custom query hotkey setting dialogAs you can see, the new version of
HotkeyControl
usesType
to specific theHotkey
getter and setter.And previously, the default value of
Type
isHotkey
so theHotkeyControl
inCustomQueryHotkeySettings
will fallback tocase HotkeyType.Hotkey:
, resulting the change of the search hotkey.So we need to add a new
enum
type so thatHotkeyControl
will cache the hotkey value and return it as result which is needed forCustomQueryHotkeySettings
(it uses )Follow on with #3310.
Test