Fix Possible Window Popup During Startup & Fix Store Plugin List Refresh Issue #3472
Conversation
|
🚨 Warning: Approaching Monthly Automation Limit Monthly PRs automated: 245/250 Your organization has used over 90% of its monthly quota for gitStream automations. Once the quota is reached, new pull requests for this month will not trigger gitStream automations and will be marked as “Skipped”. To avoid interruptions, consider optimizing your automation usage or upgrading your plan by contacting LinearB. |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue with a potential window popup during startup when both logon task and registry methods are enabled, and it also resolves a plugin store list refresh issue by returning null as a failure flag.
- Refactors auto-startup logic by replacing direct enable/disable calls with a side-effect method (CheckIsEnabled).
- Removes unused logger imports from helper files.
- Adjusts asynchronous plugin fetch logic to use null as a flag for failure.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs | Switches to new AutoStartup API calls for startup configuration. |
| Flow.Launcher/Helper/HotKeyMapper.cs | Removes unused logger import to clean up dependencies. |
| Flow.Launcher/Helper/AutoStartup.cs | Refactors startup check to a void method that performs enable/disable. |
| Flow.Launcher/App.xaml.cs | Updates auto-startup logic to use the new CheckIsEnabled method. |
| Flow.Launcher.Core/ExternalPlugins/CommunityPluginStore.cs | Modifies task result handling in plugin fetching logic. |
| Flow.Launcher.Core/ExternalPlugins/CommunityPluginSource.cs | Returns null on failure to refresh plugin source as intended. |
Comments suppressed due to low confidence (2)
Flow.Launcher/Helper/AutoStartup.cs:19
- [nitpick] The method name 'CheckIsEnabled' suggests a boolean check, but it now performs enabling/disabling actions. Consider renaming it to better reflect its side-effect behavior, for example 'EnsureAutoStartupState'.
public static void CheckIsEnabled(bool useLogonTaskForStartup)
Flow.Launcher.Core/ExternalPlugins/CommunityPluginSource.cs:73
- Ensure that all callers of FetchAsync are updated to handle a null return value, and add unit tests to verify proper handling of the plugin source failure scenario.
return null;
@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: no user but you matched threshold 10 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: 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. |
Jack251970
added
Dev branch only
|
Tested and I think it is good to go. |
📝 WalkthroughWalkthroughThis set of changes refactors the application's auto-startup logic by consolidating the enabling and disabling mechanisms for starting the application on system startup. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsPaneGeneralViewModel
participant AutoStartup
participant SystemStartup
User->>SettingsPaneGeneralViewModel: Enable/disable "Start on system startup"
SettingsPaneGeneralViewModel->>AutoStartup: ChangeToViaLogonTask()/ChangeToViaRegistry()/DisableViaLogonTaskAndRegistry()
AutoStartup->>SystemStartup: Enable/disable via logon task or registry as needed
sequenceDiagram
participant App
participant AutoStartup
App->>AutoStartup: CheckIsEnabled(useLogonTaskForStartup)
AutoStartup->>AutoStartup: Check and enforce correct startup mode (logon task or registry)
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
💤 Files with no reviewable changes (1)
🧰 Additional context used🧬 Code Graph Analysis (1)Flow.Launcher/App.xaml.cs (2)
⏰ Context from checks skipped due to timeout of 90000ms (6)
🔇 Additional comments (10)
✨ 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. 🪧 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 (
|
2 participants
Fix Possible Window Popup During Startup
If FL enabled both logon task and registry,
Hide Flow Launcher on startupwill not work since the later one will trigger main window show event. So we need to check both because if both of them are enabled.Follow on with #3218.
Fix Store Plugin List Refresh Issue
Use null as flag to return value so that we can handle plugin source issue.
Follow on with #3460.