[FEATURE] Plugin settings context menu option#4381
Merged
Jack251970 merged 12 commits intoFlow-Launcher:devfrom Apr 4, 2026
Merged
[FEATURE] Plugin settings context menu option#4381Jack251970 merged 12 commits intoFlow-Launcher:devfrom
Jack251970 merged 12 commits intoFlow-Launcher:devfrom
Conversation
DavidGBrett
changed the title
DavidGBrett
force-pushed
the
plugin-settings-context-menu-option
branch
2 times, most recently
from
7791256 to
01f890a
Compare
|
🥷 Code experts: Jack251970 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:
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. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an "Open Plugin Settings" flow: new PluginSettingsWindow UI, Public API method to open it (with UI-thread marshaling), localized strings, a context-menu entry to launch it from plugin results, and a window visibility helper/refactor. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CM as Context Menu
participant MVM as MainViewModel
participant API as PublicAPIInstance
participant UI as PluginSettingsWindow
participant Store as Settings Store
User->>CM: Right-click plugin result
CM->>MVM: Select "Plugin Settings"
MVM->>API: OpenPluginSettingsWindow(pluginId)
API->>API: Dispatcher.Invoke (switch to UI thread)
API->>UI: Find existing or create PluginSettingsWindow(pluginId)
API->>UI: WindowVisibilityHelper.ShowOrActivate(window)
UI->>Store: Load plugin settings & metadata
UI->>User: Render settings UI
User->>UI: Modify settings, then close
UI->>Store: Save settings
Store->>API: API.SavePluginSettings()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Flow.Launcher/PublicAPIInstance.cs`:
- Around line 150-165: Change OpenPluginSettingsDialog to return bool and report
success/failure: update its signature from void to bool, use
Application.Current.Dispatcher.Invoke<TResult> to return true when
PluginSettingsWindow(pluginId) is successfully constructed and shown, and return
false from the catch path after calling LogException(ClassName, $"Failed to open
plugin settings window for plugin id '{pluginId}'", e) and
ShowMsgError(Localize.pluginSettingsWindowOpenFailed()); keep the existing
try/catch and UI calls (PluginSettingsWindow, window.Show()) but ensure the
method returns the boolean result so callers can decide whether to hide the
launcher.
In `@Flow.Launcher/ViewModel/MainViewModel.cs`:
- Around line 1824-1844: The helper ContextMenuPluginSettings currently calls
PluginManager.GetPluginForId(id) and dereferences .Metadata which can be null
and is unnecessary because the Result already carries the plugin directory;
remove the extra lookup and null-risk by deleting the
PluginManager.GetPluginForId(id) call and the metadata variable and use
result.PluginDirectory directly when building the menu (keep using
result.PluginID for id and result.OriginQuery as-is).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 06d798a3-5a9f-4130-9e48-b82ebcffa889
📒 Files selected for processing (6)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.csFlow.Launcher/Languages/en.xamlFlow.Launcher/PluginSettingsWindow.xamlFlow.Launcher/PluginSettingsWindow.xaml.csFlow.Launcher/PublicAPIInstance.csFlow.Launcher/ViewModel/MainViewModel.cs
DavidGBrett
force-pushed
the
plugin-settings-context-menu-option
branch
from
7a3daf5 to
a72e9a1
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Flow.Launcher/PluginSettingsWindow.xaml.cs">
<violation number="1" location="Flow.Launcher/PluginSettingsWindow.xaml.cs:44">
P2: `Title` is set before `InitializeComponent()`, but the XAML defines `Title="Plugin Settings"`, so `InitializeComponent()` will overwrite the localized/plugin-specific title. Set the title after `InitializeComponent()` to keep the localized value.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
DavidGBrett
force-pushed
the
plugin-settings-context-menu-option
branch
from
a72e9a1 to
bb5f1eb
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Flow.Launcher/PluginSettingsWindow.xaml (1)
1-5: Add explicit namespace prefix forWindowChrome.Per Microsoft documentation,
WindowChromefromSystem.Windows.Shellrequires an explicit namespace declaration. This pattern is currently used throughout the codebase without theshellnamespace prefix and appears to function; however, adding the prefix aligns with official WPF guidance and ensures proper type resolution.🧩 Suggested XAML fix
<Window x:Class="Flow.Launcher.PluginSettingsWindow" xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:ui="http://schemas.inkore.net/lib/ui/wpf/modern" + xmlns:shell="http://schemas.microsoft.com/winfx/2006/xaml/presentation/shell" Title="Plugin Settings" @@ - <WindowChrome.WindowChrome> - <WindowChrome CaptionHeight="32" ResizeBorderThickness="{x:Static SystemParameters.WindowResizeBorderThickness}" /> - </WindowChrome.WindowChrome> + <shell:WindowChrome.WindowChrome> + <shell:WindowChrome CaptionHeight="32" ResizeBorderThickness="{x:Static SystemParameters.WindowResizeBorderThickness}" /> + </shell:WindowChrome.WindowChrome>Note: This pattern appears throughout the codebase in other XAML files without the namespace prefix; consider a consistent fix across all affected files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Flow.Launcher/PluginSettingsWindow.xaml` around lines 1 - 5, Add an explicit XML namespace for System.Windows.Shell and update usages of WindowChrome to use that prefix: declare a xmlns:shell="clr-namespace:System.Windows.Shell;assembly=PresentationFramework" at the top of the XAML (next to existing xmlns entries) and change any direct <WindowChrome ...> elements to <shell:WindowChrome ...> (and any corresponding attribute/type references) so the WindowChrome type is resolved via the shell namespace; apply the same pattern for other XAML files that reference WindowChrome.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Flow.Launcher/PluginSettingsWindow.xaml.cs`:
- Around line 24-43: The window is binding directly to live shared instances
returned by PluginManager.GetPluginForId and
_settings.PluginSettings.GetPluginSettings, causing concurrent editor windows to
mutate the same objects and produce stale UI; change the constructor to create a
deep/shallow copy of the plugin metadata and settings (e.g., clone the object
returned by GetPluginForId and GetPluginSettings) and assign those copies into
the new PluginViewModel (PluginPair, PluginSettingsObject) so the window edits
an isolated instance, then on OK/close validate and merge/commit the changes
back into the shared Settings/PluginManager (or use a dedicated CommitChanges
method) to persist edits; alternatively enforce a single-editor-per-plugin by
checking for an existing open editor for pluginId before creating the window.
---
Nitpick comments:
In `@Flow.Launcher/PluginSettingsWindow.xaml`:
- Around line 1-5: Add an explicit XML namespace for System.Windows.Shell and
update usages of WindowChrome to use that prefix: declare a
xmlns:shell="clr-namespace:System.Windows.Shell;assembly=PresentationFramework"
at the top of the XAML (next to existing xmlns entries) and change any direct
<WindowChrome ...> elements to <shell:WindowChrome ...> (and any corresponding
attribute/type references) so the WindowChrome type is resolved via the shell
namespace; apply the same pattern for other XAML files that reference
WindowChrome.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 636393bb-3a5b-4b8c-9d52-e5ab290c44f1
📒 Files selected for processing (6)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.csFlow.Launcher/Languages/en.xamlFlow.Launcher/PluginSettingsWindow.xamlFlow.Launcher/PluginSettingsWindow.xaml.csFlow.Launcher/PublicAPIInstance.csFlow.Launcher/ViewModel/MainViewModel.cs
✅ Files skipped from review due to trivial changes (2)
- Flow.Launcher/ViewModel/MainViewModel.cs
- Flow.Launcher/Languages/en.xaml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
Flow.Launcher/PluginSettingsWindow.xaml.cs (2)
22-24: Minor: Inconsistent brace placement.The opening brace is on the same line as the
ifstatement, which differs from the rest of the file's style (braces on new lines).🎨 Style consistency fix
- if (string.IsNullOrWhiteSpace(pluginId)){ + if (string.IsNullOrWhiteSpace(pluginId)) + { throw new ArgumentException("Plugin ID cannot be null or whitespace.", nameof(pluginId)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Flow.Launcher/PluginSettingsWindow.xaml.cs` around lines 22 - 24, The if-statement checking pluginId uses a different brace style than the file; update the brace placement to match the project's convention (opening brace on a new line) for the pluginId validation block so it is consistent with surrounding code (the snippet with "if (string.IsNullOrWhiteSpace(pluginId)) { ... }" should be reformatted to place the '{' on the next line in the same method/constructor where pluginId is validated).
95-107: Consider usingCollapsedinstead ofHiddenfor button visibility.
Visibility.Hiddenreserves layout space whileVisibility.Collapseddoes not. If these buttons are in the same position, usingCollapsedwould be more appropriate to avoid invisible space reservation. However, if this is intentional to maintain consistent spacing, feel free to disregard.♻️ Suggested change if layout space isn't needed
private void RefreshMaximizeRestoreButton() { if (WindowState == WindowState.Maximized) { - MaximizeButton.Visibility = Visibility.Hidden; + MaximizeButton.Visibility = Visibility.Collapsed; RestoreButton.Visibility = Visibility.Visible; } else { MaximizeButton.Visibility = Visibility.Visible; - RestoreButton.Visibility = Visibility.Hidden; + RestoreButton.Visibility = Visibility.Collapsed; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Flow.Launcher/PluginSettingsWindow.xaml.cs` around lines 95 - 107, The RefreshMaximizeRestoreButton method currently sets MaximizeButton.Visibility and RestoreButton.Visibility to Visibility.Hidden which reserves layout space; change those to Visibility.Collapsed when toggling based on WindowState (e.g., in RefreshMaximizeRestoreButton use Visibility.Collapsed instead of Visibility.Hidden for the button you hide) so the hidden button does not consume layout space while keeping the visible button shown.Flow.Launcher/Helper/WindowVisibilityHelper.cs (1)
7-34: Redundant cast on return statement.The parameter
windowis already constrained to typeT, so the cast(T)windowon line 33 is unnecessary.♻️ Suggested simplification
- return (T)window; + return window;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Flow.Launcher/Helper/WindowVisibilityHelper.cs` around lines 7 - 34, The return statement in ShowOrActivate<T>(T window) uses a redundant cast (T)window even though the parameter is already of type T; remove the unnecessary cast and simply return window to clean up the code and avoid redundant casting in the method.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs`:
- Around line 165-173: The JsonRPCPublicAPI wrapper is missing the new
IPublicAPI method OpenPluginSettingsWindow; add a public method named
OpenPluginSettingsWindow(string pluginId) to the JsonRPCPublicAPI class (place
it after the existing OpenSettingDialog method) that simply returns
_api.OpenPluginSettingsWindow(pluginId) so JSON-RPC plugins can call through to
the underlying API.
---
Nitpick comments:
In `@Flow.Launcher/Helper/WindowVisibilityHelper.cs`:
- Around line 7-34: The return statement in ShowOrActivate<T>(T window) uses a
redundant cast (T)window even though the parameter is already of type T; remove
the unnecessary cast and simply return window to clean up the code and avoid
redundant casting in the method.
In `@Flow.Launcher/PluginSettingsWindow.xaml.cs`:
- Around line 22-24: The if-statement checking pluginId uses a different brace
style than the file; update the brace placement to match the project's
convention (opening brace on a new line) for the pluginId validation block so it
is consistent with surrounding code (the snippet with "if
(string.IsNullOrWhiteSpace(pluginId)) { ... }" should be reformatted to place
the '{' on the next line in the same method/constructor where pluginId is
validated).
- Around line 95-107: The RefreshMaximizeRestoreButton method currently sets
MaximizeButton.Visibility and RestoreButton.Visibility to Visibility.Hidden
which reserves layout space; change those to Visibility.Collapsed when toggling
based on WindowState (e.g., in RefreshMaximizeRestoreButton use
Visibility.Collapsed instead of Visibility.Hidden for the button you hide) so
the hidden button does not consume layout space while keeping the visible button
shown.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8486d2e6-60f0-4302-8376-dca61e472a65
📒 Files selected for processing (8)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.csFlow.Launcher/Helper/SingletonWindowOpener.csFlow.Launcher/Helper/WindowVisibilityHelper.csFlow.Launcher/Languages/en.xamlFlow.Launcher/PluginSettingsWindow.xamlFlow.Launcher/PluginSettingsWindow.xaml.csFlow.Launcher/PublicAPIInstance.csFlow.Launcher/ViewModel/MainViewModel.cs
✅ Files skipped from review due to trivial changes (2)
- Flow.Launcher/Languages/en.xaml
- Flow.Launcher/PluginSettingsWindow.xaml
Contributor
Author
|
I believe at this point it should be good enough for a human review. |
OpenPluginSettingsWindow, which opens new PluginSettingsWindow that holds an InstalledPluginDisplay
…ectly to PluginViewModel; reuse controls from InstalledPluginDisplay where applicable, add new rows for Enabled/Home/Priority/Search Delay
…tate and merge LoadPlugin Method into remaining constructor
… make delay 0 when disabled
…indow Title and Context Menu Result
- Update PluginSettingsWindow initial dimensions to fit new layout - Remove top border from enabled row (first row) - Simplify border Styling - Remove unnecessary containers/wrappers in xaml - Remove unnecessary properties in xaml - Move scrollbar to edge of window - Move margins from ScrollViewer to inner StackPanel - Adjust top margin of plugin header in xaml
…then catch to LogException and show localized error message, and make OpenPluginSettingsDialog use this to return success boolean
… with the same plugin id and reuse
…ener and OpenPluginSettingsWindow
DavidGBrett
force-pushed
the
plugin-settings-context-menu-option
branch
from
e54328e to
b513d30
Compare
Replaced static plugin settings title with dynamic one using plugin name, added SettingsIcon constant for settings.png, and updated context menu to use the new icon. Removed unused string resource from en.xaml.
Jack251970
approved these changes
Apr 4, 2026
Member
Jack251970
left a comment
Jack251970
left a comment
There was a problem hiding this comment.
Great work on this! Thanks for the contribution - the high code quality made this a pleasure to review!
Contributor
Author
|
Thank you for reviewing and merging @Jack251970 ! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a context menu option for all plugins, which calls a new api method, which opens a new plugin settings window for that plugin.
Closes #4262
UI Changes
I tried to match the style of the existing plugin settings ui in InstalledPluginDisplay, reusing components from it where possible.