Fix Custom Query Hotkey / Shortcuts Preview Issue#3248
Fix Custom Query Hotkey / Shortcuts Preview Issue#3248Jack251970 merged 3 commits intoFlow-Launcher:devfrom
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. |
📝 WalkthroughWalkthroughThis update revises two files in the Flow Launcher project. In Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Setting as CustomQueryHotkeySetting/CustomShortcutSetting
participant API as App.API
User->>Setting: Click Test Button
Setting->>API: Call ShowMainWindow()
API-->>User: Display Main Window
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 (
|
|
|
||
| MainWindowVisibilityStatus = true; | ||
| VisibilityChanged?.Invoke(this, new VisibilityChangedEventArgs { IsVisible = true }); | ||
| if (invokeEvent) |
There was a problem hiding this comment.
The original codes does not invoke VisibliityChanged event, so I keep it here.
There was a problem hiding this comment.
@taooceros If you think we should invoke this event here, feel free to tell me and I will do it.
There was a problem hiding this comment.
I am not sure. I don't recall much things registered in this event. I just want to check as you introduce an api change to the method, which is not yet sure important.
There was a problem hiding this comment.
I am not sure. I don't recall much things registered in this event. I just want to check as you introduce an api change to the method, which is not yet sure important.
Well, I think we should invoke visibility changed event here and I change the related codes.
This comment has been minimized.
This comment has been minimized.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
Flow.Launcher/CustomShortcutSetting.xaml.cs (1)
28-37:⚠️ Potential issueUpdate second constructor to handle MainViewModel.
The second constructor doesn't initialize
_mainViewModel, which could lead to null reference exceptions whenBtnTestShortcut_OnClickis called.Apply this diff to fix the issue:
-public CustomShortcutSetting(string key, string value, SettingsPaneHotkeyViewModel vm) +public CustomShortcutSetting(string key, string value, SettingsPaneHotkeyViewModel vm, MainViewModel mainVM) { Key = key; Value = value; originalKey = key; originalValue = value; update = true; _hotkeyVm = vm; + _mainViewModel = mainVM; InitializeComponent(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Flow.Launcher/CustomQueryHotkeySetting.xaml.cs(4 hunks)Flow.Launcher/CustomShortcutSetting.xaml.cs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Flow.Launcher/CustomQueryHotkeySetting.xaml.cs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: gitStream.cm
@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 ...
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Flow.Launcher/CustomQueryHotkeySetting.xaml.cs(4 hunks)Flow.Launcher/CustomShortcutSetting.xaml.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Flow.Launcher/CustomQueryHotkeySetting.xaml.cs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Report (PR)
🔇 Additional comments (2)
Flow.Launcher/CustomShortcutSetting.xaml.cs (2)
1-73: Inconsistency detected in AI-generated summary.The summary mentions changes to constructor signature and addition of MainViewModel dependency, but these changes are not present in the actual code.
Likely an incorrect or invalid review comment.
68-68: Pass false to ShowMainWindow() to prevent VisibilityChanged event.According to the PR objectives, ShowMainWindow() should be called with false to prevent invoking the VisibilityChanged event, maintaining the original behavior.
Apply this diff to fix the issue:
- App.API.ShowMainWindow(); + App.API.ShowMainWindow(false); // false to not invoke VisibilityChanged event as original
|
So sorry that I forget there is an api function for showing main window😢 |
Haha no worries 😝. You are already doing really well! |
| public partial class CustomQueryHotkeySetting : Window | ||
| { | ||
| private SettingWindow _settingWidow; | ||
| private readonly Settings _settings; |
There was a problem hiding this comment.
For code quality.😂 I prefer _ prefix for readonly properties and majority of codes in FL uses this way.
There was a problem hiding this comment.
Technically this is a field instead of property (maybe it worth to figure out the difference). For here, I think it's fine.
3 participants
Fix hotkey preview issue for custom query hotkeys and custom query shortcuts
In
CustomQueryHotkeySetting.cs&CustomShortcutSetting.cs, original codes are like these:Here we will not call the correct
Showfunction for main window.So we need to change to:
And it can help us to address preview issue that main window will not close when it is unfocused.
Test
Original:
2025-02-16.23-44-30.mp4
After:
2025-02-16.23-58-18.-.Copy.mp4