Add option to show taskbar when Flow Launcher is opend #4177
Conversation
Co-authored-by: VictoriousRaptor <[email protected]>
…howing Co-authored-by: VictoriousRaptor <[email protected]>
Co-authored-by: VictoriousRaptor <[email protected]>
Co-authored-by: VictoriousRaptor <[email protected]>
Co-authored-by: VictoriousRaptor <[email protected]>
|
🥷 Code experts: Jack251970 Jack251970, jjw24 have 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: ✨ Comment |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughAdds a user setting and UI toggle to temporarily show/hide the Windows taskbar when Flow Launcher opens: new native MonitorFromWindow declaration, Win32Helper Show/Hide taskbar APIs, Settings property and UI binding, localization keys, and MainViewModel coordination to show/hide per-instance. Changes
Sequence DiagramsequenceDiagram
participant User
participant MainViewModel
participant Settings
participant Win32Helper
participant NativeAPI
User->>MainViewModel: Open (Show)
MainViewModel->>Settings: read ShowTaskbarWhenInvoked
alt enabled & not already shown
MainViewModel->>Win32Helper: ShowTaskbar()
Win32Helper->>NativeAPI: FindWindow("Shell_TrayWnd"/"Shell_SecondaryTrayWnd")
NativeAPI-->>Win32Helper: hwnd
Win32Helper->>NativeAPI: MonitorFromWindow(hwnd)
NativeAPI-->>Win32Helper: monitor
Win32Helper->>NativeAPI: PostMessage(TrayBarFlag 0x05D1, WPARAM=1, monitor)
Win32Helper-->>MainViewModel: returned (set flag)
end
User->>MainViewModel: Close (Hide)
alt taskbar was shown by Flow
MainViewModel->>Win32Helper: HideTaskbar()
Win32Helper->>NativeAPI: FindWindow(...)
NativeAPI-->>Win32Helper: hwnd
Win32Helper->>NativeAPI: PostMessage(TrayBarFlag 0x05D1, WPARAM=0)
Win32Helper-->>MainViewModel: returned (clear flag)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@Jack251970 do you have dual monitor setup to help test this? |
pr is not ready as the potential dual monitor issue and the issue addressed by code rabbit so I disable the auto merge now. |
|
@VictoriousRaptor Feel free to ping me if you need help with testing under dual monitor setup |
Yeah please test it if you're free. |
|
@VictoriousRaptor No matter on which monitor I open Flow, it is always the taskbar on the main monitor opened. |
|
@Jack251970 Hi Jack, I've just tried to implement showing or hiding taskbar on Flow's monitor only. Please check it if you're free. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Flow.Launcher/ViewModel/MainViewModel.cs (1)
1007-1017: Minor formatting inconsistency.The
setaccessor has inconsistent indentation compared to thegetaccessor.🔎 Suggested formatting fix
private bool isOnPrimaryScreen = false; public bool OnPrimaryScreen -{ +{ get => isOnPrimaryScreen; - set - { - isOnPrimaryScreen = value; - OnPropertyChanged(); - } + set + { + isOnPrimaryScreen = value; + OnPropertyChanged(); + } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Flow.Launcher.Infrastructure/Win32Helper.csFlow.Launcher/ViewModel/MainViewModel.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- Flow.Launcher.Infrastructure/Win32Helper.cs
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-16T10:48:30.573Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher/App.xaml.cs:252-275
Timestamp: 2025-10-16T10:48:30.573Z
Learning: In Flow Launcher's App.xaml.cs, the plugin initialization block (lines 252-275) that includes PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, _mainVM.QueryResults(), and API.SaveAppAllSettings() does not require additional Task.Run wrappers or Dispatcher.InvokeAsync calls according to maintainer Jack251970, as the threading model is already safe as designed.
Applied to files:
Flow.Launcher/ViewModel/MainViewModel.cs
📚 Learning: 2025-06-08T14:12:21.348Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3672
File: Flow.Launcher/MainWindow.xaml.cs:244-247
Timestamp: 2025-06-08T14:12:21.348Z
Learning: In Flow.Launcher, App.NotifyIcon is created before MainWindow creation, so null checks for App.NotifyIcon are not necessary when accessing it from MainWindow code.
Applied to files:
Flow.Launcher/ViewModel/MainViewModel.cs
📚 Learning: 2025-06-08T14:12:12.842Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3672
File: Flow.Launcher/MainWindow.xaml.cs:318-318
Timestamp: 2025-06-08T14:12:12.842Z
Learning: In Flow.Launcher, the App.NotifyIcon static property is initialized in the App class before MainWindow creation, so null checks are not needed when accessing App.NotifyIcon in MainWindow lifecycle methods.
Applied to files:
Flow.Launcher/ViewModel/MainViewModel.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (2)
Flow.Launcher/ViewModel/MainViewModel.cs (2)
67-68: LGTM!Good addition of the tracking flag to ensure balanced
ShowTaskbar/HideTaskbarcalls, as suggested in the previous review.
2113-2118: LGTM!The Show() implementation correctly guards against duplicate calls using the tracking flag.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Flow.Launcher/ViewModel/MainViewModel.cs (1)
1007-1016: Naming convention inconsistency: apply underscore prefix to private field.The backing field
isOnPrimaryScreendoesn't follow the underscore prefix convention used by all other private fields in this class (_lastQuery,_taskbarShownByFlow,_isDialogJump, etc.). Consider renaming to_isOnPrimaryScreen.- private bool isOnPrimaryScreen = false; + private bool _isOnPrimaryScreen = false; public bool OnPrimaryScreen { - get => isOnPrimaryScreen; + get => _isOnPrimaryScreen; set { - isOnPrimaryScreen = value; + _isOnPrimaryScreen = value; OnPropertyChanged(); } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/ViewModel/MainViewModel.cs
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-16T10:48:30.573Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher/App.xaml.cs:252-275
Timestamp: 2025-10-16T10:48:30.573Z
Learning: In Flow Launcher's App.xaml.cs, the plugin initialization block (lines 252-275) that includes PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, _mainVM.QueryResults(), and API.SaveAppAllSettings() does not require additional Task.Run wrappers or Dispatcher.InvokeAsync calls according to maintainer Jack251970, as the threading model is already safe as designed.
Applied to files:
Flow.Launcher/ViewModel/MainViewModel.cs
📚 Learning: 2025-06-08T14:12:21.348Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3672
File: Flow.Launcher/MainWindow.xaml.cs:244-247
Timestamp: 2025-06-08T14:12:21.348Z
Learning: In Flow.Launcher, App.NotifyIcon is created before MainWindow creation, so null checks for App.NotifyIcon are not necessary when accessing it from MainWindow code.
Applied to files:
Flow.Launcher/ViewModel/MainViewModel.cs
📚 Learning: 2025-06-08T14:12:12.842Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3672
File: Flow.Launcher/MainWindow.xaml.cs:318-318
Timestamp: 2025-06-08T14:12:12.842Z
Learning: In Flow.Launcher, the App.NotifyIcon static property is initialized in the App class before MainWindow creation, so null checks are not needed when accessing App.NotifyIcon in MainWindow lifecycle methods.
Applied to files:
Flow.Launcher/ViewModel/MainViewModel.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (1)
Flow.Launcher/ViewModel/MainViewModel.cs (1)
67-67: LGTM! Tracking flag correctly addresses the balance concern.The
_taskbarShownByFlowflag ensures thatShowTaskbar()andHideTaskbar()calls remain balanced regardless of when the setting is toggled, preventing the taskbar from being left in an unexpected state.
Here you are: Non-primary screen: |
|
@Jack251970 I guess it's about when we detect Flow's screen. Maybe main window is moved after we detected. Anyways I wanna make it simple. Is showing all taskbars acceptable? Or what's Windows' solution, when you press WIN key on a monitor? Are all taskbars shown?
|
No matter which screen I click on, the taskbar of the main screen is always displayed |
|
I think we do not need to consider the task bar in the non-primary screen (At least Microsoft did not consider this) |
|
@VictoriousRaptor Hi, I fixed the primary window detection. But it still cannot work though. The task bar in the non-primary window cannot be shown and hidden either |
|
Logs seems to be correct now: |
|
I just cannot get it work for non-primary task bar.... |
it's possible because we're currently using an undocumented way. I think working on primary is good engouh though, considering Microsoft uses the primary too. I've read an article saying that primary taskbar is not on primary screen somtimes so making it perfect would be more complicated. Just in case you're interested to explore how to make it work, you can check the first commit of this pr and set the taskbar to always on top mode. |
|
Well, let us revert to the original version which just operates the task bar in the primary screen |
|
And I have removed the third todo |
Moved taskbar show logic to after keyboard layout switch in Show().Ensures consistent taskbar state when invoking or hiding the app.
VictoriousRaptor
force-pushed
the
copilot/add-show-taskbar-option
branch
from
362ac27 to
2c00b31
Compare
I did a |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/ViewModel/MainViewModel.cs
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher/App.xaml.cs:252-275
Timestamp: 2025-10-16T10:48:30.573Z
Learning: In Flow Launcher's App.xaml.cs, the plugin initialization block (lines 252-275) that includes PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, _mainVM.QueryResults(), and API.SaveAppAllSettings() does not require additional Task.Run wrappers or Dispatcher.InvokeAsync calls according to maintainer Jack251970, as the threading model is already safe as designed.
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3672
File: Flow.Launcher/MainWindow.xaml.cs:318-318
Timestamp: 2025-06-08T14:12:12.842Z
Learning: In Flow.Launcher, the App.NotifyIcon static property is initialized in the App class before MainWindow creation, so null checks are not needed when accessing App.NotifyIcon in MainWindow lifecycle methods.
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3672
File: Flow.Launcher/MainWindow.xaml.cs:244-247
Timestamp: 2025-06-08T14:12:21.348Z
Learning: In Flow.Launcher, App.NotifyIcon is created before MainWindow creation, so null checks for App.NotifyIcon are not necessary when accessing it from MainWindow code.
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Flow.Launcher/HotkeyControlDialog.xaml:6-6
Timestamp: 2025-09-05T11:56:27.267Z
Learning: In Flow.Launcher's migration to iNKORE.UI.WPF.Modern UI framework, dialog resource keys like PopuBGColor, PopupButtonAreaBGColor, PopupButtonAreaBorderColor, and PopupTextColor are provided by the iNKORE.UI.WPF.Modern framework itself, not defined locally in the codebase theme files.
Learnt from: Yusyuriv
Repo: Flow-Launcher/Flow.Launcher PR: 3057
File: Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs:0-0
Timestamp: 2024-11-03T07:40:11.014Z
Learning: In Flow Launcher, when using Windows Forms dialogs (e.g., in `JsonRPCPluginSettings.cs`), path validation is enabled by default in `OpenFileDialog` and `FolderBrowserDialog`, preventing users from selecting invalid paths, but it's possible to opt out of this validation on individual dialogs.
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher/App.xaml.cs:246-262
Timestamp: 2025-07-21T09:19:49.684Z
Learning: In Flow Launcher's App.xaml.cs, the asynchronous plugin initialization task (containing AbstractPluginEnvironment.PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, and AutoPluginUpdates) does not require additional try-catch error handling according to maintainer Jack251970, as these operations are designed to handle exceptions internally.
📚 Learning: 2025-10-16T10:48:30.573Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher/App.xaml.cs:252-275
Timestamp: 2025-10-16T10:48:30.573Z
Learning: In Flow Launcher's App.xaml.cs, the plugin initialization block (lines 252-275) that includes PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, _mainVM.QueryResults(), and API.SaveAppAllSettings() does not require additional Task.Run wrappers or Dispatcher.InvokeAsync calls according to maintainer Jack251970, as the threading model is already safe as designed.
Applied to files:
Flow.Launcher/ViewModel/MainViewModel.cs
📚 Learning: 2025-06-08T14:12:21.348Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3672
File: Flow.Launcher/MainWindow.xaml.cs:244-247
Timestamp: 2025-06-08T14:12:21.348Z
Learning: In Flow.Launcher, App.NotifyIcon is created before MainWindow creation, so null checks for App.NotifyIcon are not necessary when accessing it from MainWindow code.
Applied to files:
Flow.Launcher/ViewModel/MainViewModel.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
Flow.Launcher/ViewModel/MainViewModel.cs (3)
67-67: LGTM: Tracking flag correctly implemented.The
_taskbarShownByFlowfield properly tracks whether Flow has shown the taskbar, ensuring balanced show/hide operations even if the user toggles the setting while Flow is open. This addresses previous review feedback.
2150-2156: Tracking flag correctly used, but inherits monitor detection issue.The condition correctly checks both the setting and the tracking flag before showing the taskbar, ensuring balanced show/hide calls (addressing previous review feedback ✅). However, this code still uses
isOnPrimaryScreenwithout detecting which monitor Flow is actually on—see the critical issue at lines 1007-1016.
2225-2230: Tracking flag correctly used, but inherits monitor detection issue.The condition correctly relies solely on
_taskbarShownByFlowwithout checking the setting value, ensuring balanced hide operations (addressing previous review feedback ✅). However, this code still usesisOnPrimaryScreenwithout detecting which monitor Flow is on—see the critical issue at lines 1007-1016.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/ViewModel/MainViewModel.cs
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3672
File: Flow.Launcher/MainWindow.xaml.cs:244-247
Timestamp: 2025-06-08T14:12:21.348Z
Learning: In Flow.Launcher, App.NotifyIcon is created before MainWindow creation, so null checks for App.NotifyIcon are not necessary when accessing it from MainWindow code.
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3672
File: Flow.Launcher/MainWindow.xaml.cs:318-318
Timestamp: 2025-06-08T14:12:12.842Z
Learning: In Flow.Launcher, the App.NotifyIcon static property is initialized in the App class before MainWindow creation, so null checks are not needed when accessing App.NotifyIcon in MainWindow lifecycle methods.
📚 Learning: 2025-10-16T10:48:30.573Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher/App.xaml.cs:252-275
Timestamp: 2025-10-16T10:48:30.573Z
Learning: In Flow Launcher's App.xaml.cs, the plugin initialization block (lines 252-275) that includes PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, _mainVM.QueryResults(), and API.SaveAppAllSettings() does not require additional Task.Run wrappers or Dispatcher.InvokeAsync calls according to maintainer Jack251970, as the threading model is already safe as designed.
Applied to files:
Flow.Launcher/ViewModel/MainViewModel.cs
📚 Learning: 2025-06-08T14:12:21.348Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3672
File: Flow.Launcher/MainWindow.xaml.cs:244-247
Timestamp: 2025-06-08T14:12:21.348Z
Learning: In Flow.Launcher, App.NotifyIcon is created before MainWindow creation, so null checks for App.NotifyIcon are not necessary when accessing it from MainWindow code.
Applied to files:
Flow.Launcher/ViewModel/MainViewModel.cs
📚 Learning: 2025-06-08T14:12:12.842Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3672
File: Flow.Launcher/MainWindow.xaml.cs:318-318
Timestamp: 2025-06-08T14:12:12.842Z
Learning: In Flow.Launcher, the App.NotifyIcon static property is initialized in the App class before MainWindow creation, so null checks are not needed when accessing App.NotifyIcon in MainWindow lifecycle methods.
Applied to files:
Flow.Launcher/ViewModel/MainViewModel.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
Flow.Launcher/ViewModel/MainViewModel.cs (2)
67-67: LGTM! Tracking flag correctly addresses past review feedback.The
_taskbarShownByFlowfield properly tracks whether Flow showed the taskbar, ensuring balanced show/hide operations regardless of when the setting is toggled. This addresses the concern raised in previous reviews about unbalanced calls.
2214-2219: Critical: Undefined variable causes compilation error.
isOnPrimaryScreenis used at line 2217 but is never declared or initialized. This code will not compile.Based on the PR discussion and past review iterations, monitor detection remains unresolved. The simplest fix is to call
HideTaskbar()without parameters, matching theShowTaskbar()call at line 2143 and using the defaultprimaryTaskbar = truevalue.🔎 Proposed fix
// Hide the taskbar if the setting is enabled if (_taskbarShownByFlow) { - Win32Helper.HideTaskbar(isOnPrimaryScreen); + Win32Helper.HideTaskbar(); _taskbarShownByFlow = false; }Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Flow.Launcher/ViewModel/MainViewModel.cs (2)
2140-2145: Logic is correct; consider optional exception handling.The implementation correctly checks both the setting and the tracking flag before showing the taskbar, preventing redundant Win32 calls. The simplified primary-taskbar-only approach aligns with the PR discussion outcome.
Optional: Add exception handling for robustness
While
FindWindowExandPostMessageare stable Win32 APIs, wrapping the call in a try-catch could prevent unexpected failures from affecting the launcher's visibility:// Show the taskbar if the setting is enabled if (Settings.ShowTaskbarWhenInvoked && !_taskbarShownByFlow) { - Win32Helper.ShowTaskbar(); - _taskbarShownByFlow = true; + try + { + Win32Helper.ShowTaskbar(); + _taskbarShownByFlow = true; + } + catch (Exception ex) + { + App.API.LogException(ClassName, "Failed to show taskbar", ex); + } }
2214-2219: Correctly implements balanced hide logic.The condition properly relies solely on
_taskbarShownByFlow, ensuring the taskbar is restored even if the user toggled the setting while Flow was open. This directly addresses the critical issue flagged in previous reviews.Optional: Add exception handling for robustness
Similar to the Show() method, consider wrapping the Win32 call:
// Hide the taskbar if the setting is enabled if (_taskbarShownByFlow) { - Win32Helper.HideTaskbar(); - _taskbarShownByFlow = false; + try + { + Win32Helper.HideTaskbar(); + _taskbarShownByFlow = false; + } + catch (Exception ex) + { + App.API.LogException(ClassName, "Failed to hide taskbar", ex); + _taskbarShownByFlow = false; // Reset flag even on failure + } }Note: Resetting the flag even on failure prevents the state from getting stuck in an inconsistent state.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/ViewModel/MainViewModel.cs
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3672
File: Flow.Launcher/MainWindow.xaml.cs:318-318
Timestamp: 2025-06-08T14:12:12.842Z
Learning: In Flow.Launcher, the App.NotifyIcon static property is initialized in the App class before MainWindow creation, so null checks are not needed when accessing App.NotifyIcon in MainWindow lifecycle methods.
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher/App.xaml.cs:252-275
Timestamp: 2025-10-16T10:48:30.573Z
Learning: In Flow Launcher's App.xaml.cs, the plugin initialization block (lines 252-275) that includes PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, _mainVM.QueryResults(), and API.SaveAppAllSettings() does not require additional Task.Run wrappers or Dispatcher.InvokeAsync calls according to maintainer Jack251970, as the threading model is already safe as designed.
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3672
File: Flow.Launcher/MainWindow.xaml.cs:244-247
Timestamp: 2025-06-08T14:12:21.348Z
Learning: In Flow.Launcher, App.NotifyIcon is created before MainWindow creation, so null checks for App.NotifyIcon are not necessary when accessing it from MainWindow code.
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Flow.Launcher/HotkeyControlDialog.xaml:6-6
Timestamp: 2025-09-05T11:56:27.267Z
Learning: In Flow.Launcher's migration to iNKORE.UI.WPF.Modern UI framework, dialog resource keys like PopuBGColor, PopupButtonAreaBGColor, PopupButtonAreaBorderColor, and PopupTextColor are provided by the iNKORE.UI.WPF.Modern framework itself, not defined locally in the codebase theme files.
📚 Learning: 2025-10-16T10:48:30.573Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher/App.xaml.cs:252-275
Timestamp: 2025-10-16T10:48:30.573Z
Learning: In Flow Launcher's App.xaml.cs, the plugin initialization block (lines 252-275) that includes PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, _mainVM.QueryResults(), and API.SaveAppAllSettings() does not require additional Task.Run wrappers or Dispatcher.InvokeAsync calls according to maintainer Jack251970, as the threading model is already safe as designed.
Applied to files:
Flow.Launcher/ViewModel/MainViewModel.cs
📚 Learning: 2025-06-08T14:12:21.348Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3672
File: Flow.Launcher/MainWindow.xaml.cs:244-247
Timestamp: 2025-06-08T14:12:21.348Z
Learning: In Flow.Launcher, App.NotifyIcon is created before MainWindow creation, so null checks for App.NotifyIcon are not necessary when accessing it from MainWindow code.
Applied to files:
Flow.Launcher/ViewModel/MainViewModel.cs
🧬 Code graph analysis (1)
Flow.Launcher/ViewModel/MainViewModel.cs (2)
Flow.Launcher.Infrastructure/UserSettings/Settings.cs (1)
Settings(16-632)Flow.Launcher.Infrastructure/Win32Helper.cs (3)
Win32Helper(33-1047)ShowTaskbar(1023-1033)HideTaskbar(1035-1044)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
Flow.Launcher/ViewModel/MainViewModel.cs (1)
67-67: LGTM: Tracking flag correctly addresses previous review concerns.The
_taskbarShownByFlowflag properly ensures balanced show/hide calls regardless of when the user toggles the setting, which was a critical issue flagged in previous reviews.
|
Thanks for your contribution! |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Flow.Launcher.Infrastructure/Win32Helper.cs (2)
1022-1022: Theunsafekeyword appears unnecessary.The method accesses
mon.Valuewhich is a property access, not a pointer operation. The underlying CsWin32-generated code handles pointer operations internally.🔎 Proposed fix
-public static unsafe void ShowTaskbar() +public static void ShowTaskbar()
1024-1043: Consider extracting common taskbar window finding logic.Both methods duplicate the taskbar window lookup and the
TrayBarFlagconstant. Extracting these would improve maintainability.🔎 Proposed refactor
+private const uint TrayBarFlag = 0x05D1; + +private static HWND FindTaskbarWindow() +{ + return PInvoke.FindWindowEx(HWND.Null, HWND.Null, "Shell_TrayWnd", null); +} + public static void ShowTaskbar() { // Find the taskbar window - var taskbarHwnd = PInvoke.FindWindowEx(HWND.Null, HWND.Null, "Shell_TrayWnd", null); + var taskbarHwnd = FindTaskbarWindow(); if (taskbarHwnd == HWND.Null) return; // Magic from https://github.com/Oliviaophia/SmartTaskbar - const uint TrayBarFlag = 0x05D1; var mon = PInvoke.MonitorFromWindow(taskbarHwnd, Windows.Win32.Graphics.Gdi.MONITOR_FROM_FLAGS.MONITOR_DEFAULTTONEAREST); PInvoke.PostMessage(taskbarHwnd, TrayBarFlag, new WPARAM(1), new LPARAM((nint)mon.Value)); } public static void HideTaskbar() { // Find the taskbar window - var taskbarHwnd = PInvoke.FindWindowEx(HWND.Null, HWND.Null, "Shell_TrayWnd", null); + var taskbarHwnd = FindTaskbarWindow(); if (taskbarHwnd == HWND.Null) return; // Magic from https://github.com/Oliviaophia/SmartTaskbar - const uint TrayBarFlag = 0x05D1; PInvoke.PostMessage(taskbarHwnd, TrayBarFlag, new WPARAM(0), IntPtr.Zero); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher.Infrastructure/Win32Helper.cs
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3672
File: Flow.Launcher/MainWindow.xaml.cs:318-318
Timestamp: 2025-06-08T14:12:12.842Z
Learning: In Flow.Launcher, the App.NotifyIcon static property is initialized in the App class before MainWindow creation, so null checks are not needed when accessing App.NotifyIcon in MainWindow lifecycle methods.
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher/App.xaml.cs:252-275
Timestamp: 2025-10-16T10:48:30.573Z
Learning: In Flow Launcher's App.xaml.cs, the plugin initialization block (lines 252-275) that includes PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, _mainVM.QueryResults(), and API.SaveAppAllSettings() does not require additional Task.Run wrappers or Dispatcher.InvokeAsync calls according to maintainer Jack251970, as the threading model is already safe as designed.
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3672
File: Flow.Launcher/MainWindow.xaml.cs:244-247
Timestamp: 2025-06-08T14:12:21.348Z
Learning: In Flow.Launcher, App.NotifyIcon is created before MainWindow creation, so null checks for App.NotifyIcon are not necessary when accessing it from MainWindow code.
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Flow.Launcher/HotkeyControlDialog.xaml:6-6
Timestamp: 2025-09-05T11:56:27.267Z
Learning: In Flow.Launcher's migration to iNKORE.UI.WPF.Modern UI framework, dialog resource keys like PopuBGColor, PopupButtonAreaBGColor, PopupButtonAreaBorderColor, and PopupTextColor are provided by the iNKORE.UI.WPF.Modern framework itself, not defined locally in the codebase theme files.
📚 Learning: 2025-06-08T14:12:21.348Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3672
File: Flow.Launcher/MainWindow.xaml.cs:244-247
Timestamp: 2025-06-08T14:12:21.348Z
Learning: In Flow.Launcher, App.NotifyIcon is created before MainWindow creation, so null checks for App.NotifyIcon are not necessary when accessing it from MainWindow code.
Applied to files:
Flow.Launcher.Infrastructure/Win32Helper.cs
🧬 Code graph analysis (1)
Flow.Launcher.Infrastructure/Win32Helper.cs (2)
Flow.Launcher.Infrastructure/PInvokeExtensions.cs (1)
PInvoke(7-45)Flow.Launcher.Infrastructure/DialogJump/Models/WindowsExplorer.cs (1)
HWND(115-120)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (1)
Flow.Launcher.Infrastructure/Win32Helper.cs (1)
1020-1045: Implementation correctly targets the primary taskbar.The logic finds
Shell_TrayWnd(the primary taskbar) and usesMonitorFromWindowto get its monitor handle. This aligns with the PR objective to operate only on the primary taskbar after dual-monitor toggling proved unreliable.Note: As indicated in the PR discussion, this implementation does not support showing/hiding taskbars on non-primary monitors in multi-monitor setups. The taskbar shown/hidden will always be the primary monitor's taskbar.
There was a problem hiding this comment.
Pull request overview
This PR adds a new user setting to temporarily show the Windows taskbar when Flow Launcher is opened, which is particularly useful when the taskbar is configured to auto-hide. When the setting is enabled, Flow will show the taskbar upon opening and hide it again when closed.
Key Changes:
- Added
ShowTaskbarWhenInvokedsetting with a default value offalse - Implemented taskbar show/hide functionality using Windows messages (message 0x05D1) to control taskbar visibility
- Added UI controls and localization for the new setting in the General settings pane
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Flow.Launcher/ViewModel/MainViewModel.cs | Added _taskbarShownByFlow flag and integrated taskbar show/hide calls in the Show() and Hide() methods |
| Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml | Added UI toggle switch for the new taskbar setting |
| Flow.Launcher/Languages/en.xaml | Added English localization strings for the setting name and tooltip |
| Flow.Launcher.Infrastructure/Win32Helper.cs | Implemented ShowTaskbar() and HideTaskbar() methods using Win32 API calls |
| Flow.Launcher.Infrastructure/UserSettings/Settings.cs | Added ShowTaskbarWhenInvoked boolean property |
| Flow.Launcher.Infrastructure/NativeMethods.txt | Added MonitorFromWindow to the list of P/Invoke methods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…r#4177) * Add option to show taskbar when Flow Launcher is invoked Co-authored-by: VictoriousRaptor <[email protected]> * Fix: Use ABM_ACTIVATE instead of ABM_SETSTATE for temporary taskbar showing Co-authored-by: VictoriousRaptor <[email protected]> * Remove unnecessary unsafe keyword from ShowTaskbar method Co-authored-by: VictoriousRaptor <[email protected]> * Fix missing closing braces in Win32Helper.cs Co-authored-by: VictoriousRaptor <[email protected]> * Change wording from 'invoked' to 'opened' in localization strings Co-authored-by: VictoriousRaptor <[email protected]> * Show/hide tasking when showing/hiding Flow * Remove unused APPBARDATA * Guard HideTaskbar() with state so show/hide stay balanced * Improve taskbar visibility management in MainViewModel Moved taskbar show logic to after keyboard layout switch in Show().Ensures consistent taskbar state when invoking or hiding the app. * Clean code * Clean code * Remove blank line --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: Jack Ye <[email protected]> Co-authored-by: Jack251970 <[email protected]>
Show taskbar when Flow is opened
Tested