-
-
Notifications
You must be signed in to change notification settings - Fork 548
Add max suggestions limit for web search plugin #4198
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
Add max suggestions limit for web search plugin #4198
Conversation
Introduced a MaxSuggestions property (min 1, max 10) to control the number of autocomplete suggestions shown in web search plugins. Updated the UI to allow user configuration and added localized labels for the new setting in all supported languages. Suggestions in results are now limited according to this setting.
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughAdds a configurable MaxSuggestions setting, a localized UI string, a NumberBox in the settings UI, wiring to Settings.MaxSuggestions, enforcement of the cap in query results via Take(_settings.MaxSuggestions), and a package reference for a UI library; a NumberBox value-changed handler appears duplicated. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant SettingsUI as SettingsControl (UI)
participant Settings
participant WebSearch as WebSearch.Main
participant Provider as SuggestionProvider
participant ResultsUI as Results List
User->>SettingsUI: change MaxSuggestions (NumberBox)
SettingsUI->>Settings: update MaxSuggestions (TwoWay binding / ValueChanged)
Note over Settings,WebSearch: Settings value persisted/read by plugin
User->>WebSearch: enter query
WebSearch->>Provider: request suggestions
Provider-->>WebSearch: yields suggestions (stream)
WebSearch->>WebSearch: apply .Take(Settings.MaxSuggestions)
WebSearch-->>ResultsUI: return capped suggestions
ResultsUI->>User: display suggestions
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
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)
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml (1)
157-170: UI binding is correct, but consider adding range indication.The TextBox binding to
Settings.MaxSuggestionswithUpdateSourceTrigger=PropertyChangedensures immediate validation. The layout integrates well with existing controls.However, users have no visual indication that the valid range is 1–10. Consider adding a ToolTip to improve discoverability:
🔎 Optional: Add ToolTip for better UX
<TextBox Width="40" Height="30" Margin="{StaticResource SettingPanelItemLeftMargin}" VerticalAlignment="Center" HorizontalAlignment="Center" TextAlignment="Center" FontSize="11" + ToolTip="Valid range: 1-10" Text="{Binding Settings.MaxSuggestions, UpdateSourceTrigger=PropertyChanged}" />Plugins/Flow.Launcher.Plugin.WebSearch/Languages/pt-br.xaml (1)
22-22: Consider "Máx. Sugestões:" for semantic consistency.While "Qtd Sugestões:" is understandable, it emphasizes quantity rather than maximum limit. Other locales use variations of "maximum" (en: "Max Suggestions:", de: "Max. Vorschläge:", es: "Max sugerencias:", fr: "Max suggestions :"). Using "Máx. Sugestões:" would better align with the concept of a ceiling/limit rather than a count.
🔎 Suggested alternative translation
- <system:String x:Key="flowlauncher_plugin_websearch_max_suggestions">Qtd Sugestões:</system:String> + <system:String x:Key="flowlauncher_plugin_websearch_max_suggestions">Máx. Sugestões:</system:String>
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
Plugins/Flow.Launcher.Plugin.WebSearch/Languages/de.xamlPlugins/Flow.Launcher.Plugin.WebSearch/Languages/en.xamlPlugins/Flow.Launcher.Plugin.WebSearch/Languages/es.xamlPlugins/Flow.Launcher.Plugin.WebSearch/Languages/fr.xamlPlugins/Flow.Launcher.Plugin.WebSearch/Languages/pt-br.xamlPlugins/Flow.Launcher.Plugin.WebSearch/Main.csPlugins/Flow.Launcher.Plugin.WebSearch/Settings.csPlugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml.cs:143-165
Timestamp: 2025-09-13T06:04:26.977Z
Learning: In Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml, the ListView_SizeChanged handler in the C# file assumes 5 GridViewColumns but the XAML may have more columns. The user Jack251970 indicated this mismatch doesn't need to be fixed when pointed out during PR #3593 review.
📚 Learning: 2025-09-05T11:56:27.267Z
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.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/Languages/es.xamlPlugins/Flow.Launcher.Plugin.WebSearch/Languages/en.xamlPlugins/Flow.Launcher.Plugin.WebSearch/Languages/de.xamlPlugins/Flow.Launcher.Plugin.WebSearch/Languages/pt-br.xaml
📚 Learning: 2025-09-13T06:04:26.977Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml.cs:143-165
Timestamp: 2025-09-13T06:04:26.977Z
Learning: In Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml, the ListView_SizeChanged handler in the C# file assumes 5 GridViewColumns but the XAML may have more columns. The user Jack251970 indicated this mismatch doesn't need to be fixed when pointed out during PR #3593 review.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/Languages/en.xamlPlugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xamlPlugins/Flow.Launcher.Plugin.WebSearch/Main.csPlugins/Flow.Launcher.Plugin.WebSearch/Settings.cs
📚 Learning: 2025-09-11T08:30:29.487Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Flow.Launcher/SettingPages/Views/SettingsPaneTheme.xaml:0-0
Timestamp: 2025-09-11T08:30:29.487Z
Learning: In Flow.Launcher's SettingsPaneTheme.xaml theme editor panel, the inner ui:ScrollViewerEx within the Border (Grid.Column="1") is intentionally kept because its height cannot be expanded to the whole page due to layout constraints. This serves a different scrolling purpose than the outer ScrollViewerEx and is necessary for the theme editor panel functionality.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml
📚 Learning: 2025-03-28T21:12:13.386Z
Learnt from: onesounds
Repo: Flow-Launcher/Flow.Launcher PR: 3394
File: Flow.Launcher/Themes/Darker Glass.xaml:134-141
Timestamp: 2025-03-28T21:12:13.386Z
Learning: In Flow.Launcher, hotkey styling is implemented with a two-component structure: a Border element with style `ItemHotkeyBGStyle` that provides background and border styling, containing a TextBlock with style `ItemHotkeyStyle` that handles the text styling.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml
📚 Learning: 2025-03-28T21:20:54.978Z
Learnt from: onesounds
Repo: Flow-Launcher/Flow.Launcher PR: 3394
File: Flow.Launcher/Themes/Darker Glass.xaml:134-141
Timestamp: 2025-03-28T21:20:54.978Z
Learning: In WPF applications like Flow.Launcher, Border elements cannot directly display text content and require a child element like TextBlock to handle text rendering. This separation of concerns (Border for visual container styling, TextBlock for text display) follows WPF best practices and provides greater styling flexibility.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml
📚 Learning: 2025-09-04T11:52:29.096Z
Learnt from: jjw24
Repo: Flow-Launcher/Flow.Launcher PR: 3932
File: Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs:48-55
Timestamp: 2025-09-04T11:52:29.096Z
Learning: In Flow Launcher's PluginsManifest.cs, when dealing with version parsing for the MinimumAppVersion feature, maintainer jjw24 prefers to keep the solution simple rather than implementing comprehensive helper methods for SemVer parsing normalization.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/Main.cs
📚 Learning: 2025-11-06T12:57:11.574Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 4072
File: Plugins/Flow.Launcher.Plugin.Program/Main.cs:100-111
Timestamp: 2025-11-06T12:57:11.574Z
Learning: In Flow Launcher's Program plugin Main.cs QueryAsync method, the semaphore pattern where WaitAsync(token) is inside a try-catch that only catches OperationCanceledException, followed by an explicit Release() call after the try-catch block, is safe and doesn't cause semaphore leaks. When WaitAsync throws OperationCanceledException, the semaphore is not acquired, so no release is needed. When WaitAsync succeeds, the Release() call after the try-catch executes normally.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/Main.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: gitStream.cm
🔇 Additional comments (6)
Plugins/Flow.Launcher.Plugin.WebSearch/Languages/es.xaml (1)
22-22: LGTM! Spanish translation is correct.The localization string for the max suggestions setting is properly translated and consistent with the format used in other language files.
Plugins/Flow.Launcher.Plugin.WebSearch/Main.cs (1)
130-130: LGTM! Suggestions limit is correctly applied.The
.Take(_settings.MaxSuggestions)call is properly positioned before the.Select()transformation, ensuring the limit is enforced efficiently. The null-conditional operator handles the case where suggestions might be null.Plugins/Flow.Launcher.Plugin.WebSearch/Languages/fr.xaml (1)
22-22: LGTM! French translation follows proper conventions.The localization string is correctly translated, and the space before the colon follows proper French typography rules.
Plugins/Flow.Launcher.Plugin.WebSearch/Languages/en.xaml (1)
24-24: LGTM! English localization string is clear.The base English translation for the max suggestions setting follows the project's localization conventions and is consistent with other strings in the file.
Plugins/Flow.Launcher.Plugin.WebSearch/Languages/de.xaml (1)
22-22: LGTM!The German localization is accurate and consistent with the existing translation style in the file.
Plugins/Flow.Launcher.Plugin.WebSearch/Settings.cs (1)
208-220: LGTM!The
MaxSuggestionsproperty implementation is well-designed:
- Proper validation with clamping to the [1, 10] range
- Correct property change notification for UI binding
- Appropriate default value of 1
- Will be persisted in settings as expected
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: ✨ Comment |
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: 2
🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml (1)
6-6: Remove unused namespace declaration.The
ikwnamespace is declared but never referenced in the XAML. Consider removing it to keep the code clean.🔎 Proposed fix
- xmlns:ikw="http://schemas.inkore.net/lib/ui/wpf"
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Plugins/Flow.Launcher.Plugin.WebSearch/Flow.Launcher.Plugin.WebSearch.csprojPlugins/Flow.Launcher.Plugin.WebSearch/Settings.csPlugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml
🚧 Files skipped from review as they are similar to previous changes (1)
- Plugins/Flow.Launcher.Plugin.WebSearch/Settings.cs
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml.cs:143-165
Timestamp: 2025-09-13T06:04:26.977Z
Learning: In Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml, the ListView_SizeChanged handler in the C# file assumes 5 GridViewColumns but the XAML may have more columns. The user Jack251970 indicated this mismatch doesn't need to be fixed when pointed out during PR #3593 review.
📚 Learning: 2024-10-08T15:52:58.573Z
Learnt from: taooceros
Repo: Flow-Launcher/Flow.Launcher PR: 2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/Flow.Launcher.Plugin.WebSearch.csproj
📚 Learning: 2025-09-04T11:52:29.096Z
Learnt from: jjw24
Repo: Flow-Launcher/Flow.Launcher PR: 3932
File: Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs:48-55
Timestamp: 2025-09-04T11:52:29.096Z
Learning: In Flow Launcher's PluginsManifest.cs, when dealing with version parsing for the MinimumAppVersion feature, maintainer jjw24 prefers to keep the solution simple rather than implementing comprehensive helper methods for SemVer parsing normalization.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/Flow.Launcher.Plugin.WebSearch.csprojPlugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml
📚 Learning: 2025-07-01T05:46:13.251Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/Flow.Launcher.Plugin.WebSearch.csproj
📚 Learning: 2025-09-05T11:56:27.267Z
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.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/Flow.Launcher.Plugin.WebSearch.csprojPlugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml
📚 Learning: 2025-09-13T06:04:26.977Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml.cs:143-165
Timestamp: 2025-09-13T06:04:26.977Z
Learning: In Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml, the ListView_SizeChanged handler in the C# file assumes 5 GridViewColumns but the XAML may have more columns. The user Jack251970 indicated this mismatch doesn't need to be fixed when pointed out during PR #3593 review.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml
📚 Learning: 2025-09-11T08:30:29.487Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Flow.Launcher/SettingPages/Views/SettingsPaneTheme.xaml:0-0
Timestamp: 2025-09-11T08:30:29.487Z
Learning: In Flow.Launcher's SettingsPaneTheme.xaml theme editor panel, the inner ui:ScrollViewerEx within the Border (Grid.Column="1") is intentionally kept because its height cannot be expanded to the whole page due to layout constraints. This serves a different scrolling purpose than the outer ScrollViewerEx and is necessary for the theme editor panel functionality.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml
📚 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:
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml
📚 Learning: 2025-03-28T21:20:54.978Z
Learnt from: onesounds
Repo: Flow-Launcher/Flow.Launcher PR: 3394
File: Flow.Launcher/Themes/Darker Glass.xaml:134-141
Timestamp: 2025-03-28T21:20:54.978Z
Learning: In WPF applications like Flow.Launcher, Border elements cannot directly display text content and require a child element like TextBlock to handle text rendering. This separation of concerns (Border for visual container styling, TextBlock for text display) follows WPF best practices and provides greater styling flexibility.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml
📚 Learning: 2025-09-05T12:00:58.678Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Flow.Launcher/Resources/Pages/WelcomePage3.xaml:45-50
Timestamp: 2025-09-05T12:00:58.678Z
Learning: In Flow.Launcher's WelcomePage3.xaml (and likely other welcome pages), the ScrollViewerEx uses a fixed Height="478" intentionally to ensure the page remains scrollable. This fixed height is necessary for proper scrolling behavior in the welcome page context, rather than being a responsive design oversight.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml
🔇 Additional comments (2)
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml (2)
151-193: Layout restructuring looks good.The reorganization of the bottom panel into vertical StackPanels with proper grouping of the MaxSuggestions control and existing suggestion provider controls improves the UI structure and maintains proper data bindings.
50-60: TheListView_Dropevent handler is already implemented in SettingsControl.xaml.cs (line 197) with proper drag-and-drop logic. No action needed.Likely an incorrect or invalid review comment.
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: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Plugins/Flow.Launcher.Plugin.WebSearch/Flow.Launcher.Plugin.WebSearch.csprojPlugins/Flow.Launcher.Plugin.WebSearch/Settings.csPlugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xamlPlugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml.cs:143-165
Timestamp: 2025-09-13T06:04:26.977Z
Learning: In Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml, the ListView_SizeChanged handler in the C# file assumes 5 GridViewColumns but the XAML may have more columns. The user Jack251970 indicated this mismatch doesn't need to be fixed when pointed out during PR #3593 review.
📚 Learning: 2025-09-13T06:04:26.977Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml.cs:143-165
Timestamp: 2025-09-13T06:04:26.977Z
Learning: In Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml, the ListView_SizeChanged handler in the C# file assumes 5 GridViewColumns but the XAML may have more columns. The user Jack251970 indicated this mismatch doesn't need to be fixed when pointed out during PR #3593 review.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/Settings.csPlugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml.cs
📚 Learning: 2024-10-08T15:52:58.573Z
Learnt from: taooceros
Repo: Flow-Launcher/Flow.Launcher PR: 2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/Flow.Launcher.Plugin.WebSearch.csproj
📚 Learning: 2025-09-04T11:52:29.096Z
Learnt from: jjw24
Repo: Flow-Launcher/Flow.Launcher PR: 3932
File: Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs:48-55
Timestamp: 2025-09-04T11:52:29.096Z
Learning: In Flow Launcher's PluginsManifest.cs, when dealing with version parsing for the MinimumAppVersion feature, maintainer jjw24 prefers to keep the solution simple rather than implementing comprehensive helper methods for SemVer parsing normalization.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/Flow.Launcher.Plugin.WebSearch.csproj
📚 Learning: 2025-07-01T05:46:13.251Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/Flow.Launcher.Plugin.WebSearch.csproj
📚 Learning: 2025-09-05T11:56:27.267Z
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.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/Flow.Launcher.Plugin.WebSearch.csproj
📚 Learning: 2025-09-23T04:39:31.981Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 4009
File: Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml.cs:135-139
Timestamp: 2025-09-23T04:39:31.981Z
Learning: In Flow.Launcher plugin settings UI code, setting ComboBox.SelectedItem directly to an integer value after populating ItemsSource with a List<int> works correctly and doesn't require additional validation checks. User Jack251970 confirmed this pattern is acceptable in the codebase.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml.cs
🔇 Additional comments (2)
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml.cs (1)
244-252: LGTM!The event handler correctly prevents invalid state by resetting cleared values to 1 and synchronizing with the settings model.
Plugins/Flow.Launcher.Plugin.WebSearch/Flow.Launcher.Plugin.WebSearch.csproj (1)
53-55: Package version 0.10.2.1 is valid and has no known public vulnerabilities.Version 0.10.2.1 of
iNKORE.UI.WPF.Modernis published on NuGet and maintains proper package metadata. No public CVEs or security advisories exist for this version.
Jack251970
left a comment
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.
LGTM! Thanks for your contribution!
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
♻️ Duplicate comments (2)
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml (2)
161-163: Fix NumberBox Maximum to match specification.The PR description states that MaxSuggestions should have a range of 1–10, but the NumberBox specifies
Maximum="1000"andSmallChange="10". This inconsistency could allow users to configure values far beyond the intended limit, potentially causing performance or UX issues.🔎 Proposed fix
<ui:NumberBox Width="120" MinWidth="120" Margin="{StaticResource SettingPanelItemRightTopBottomMargin}" - Maximum="1000" + Maximum="10" Minimum="1" - SmallChange="10" + SmallChange="1" SpinButtonPlacementMode="Compact" ValidationMode="InvalidInputOverwritten" ValueChanged="NumberBox_ValueChanged" Value="{Binding Settings.MaxSuggestions, Mode=OneWay}" />
156-156: Add the missing localization resource to all language files.The resource key
flowlauncher_plugin_websearch_max_suggestionsis defined only inen.xamlbut is missing from all other language files (de, fr, es, pt-br, ar, cs, da, es-419, he, it, ja, ko, nb, nl, pl, pt-pt, ru, sk, sr-Cyrl-RS, sr, tr, uk-UA, vi, zh-cn, zh-tw). This will cause missing translations and broken UI labels in non-English locales.
🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml (1)
166-167: Consider using TwoWay binding instead of OneWay + ValueChanged handler.The NumberBox uses
Mode=OneWaybinding combined with aValueChangedevent handler. This pattern is unconventional and requires the ValueChanged handler to manually update the Settings.MaxSuggestions property. Typically, you would use either:
Mode=TwoWaybinding (automatic synchronization), orMode=OneWaywith a ValueChanged handler (manual control)Using both adds complexity. If the ValueChanged handler is necessary for validation or side effects, the OneWay binding is appropriate. Otherwise, consider simplifying to TwoWay binding and removing the handler.
🔎 Proposed fix (if no special validation is needed)
<ui:NumberBox Width="120" MinWidth="120" Margin="{StaticResource SettingPanelItemRightTopBottomMargin}" Maximum="10" Minimum="1" SmallChange="1" SpinButtonPlacementMode="Compact" ValidationMode="InvalidInputOverwritten" - ValueChanged="NumberBox_ValueChanged" - Value="{Binding Settings.MaxSuggestions, Mode=OneWay}" /> + Value="{Binding Settings.MaxSuggestions, Mode=TwoWay}" />Then remove the
NumberBox_ValueChangedhandler from the code-behind file.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml.cs:143-165
Timestamp: 2025-09-13T06:04:26.977Z
Learning: In Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml, the ListView_SizeChanged handler in the C# file assumes 5 GridViewColumns but the XAML may have more columns. The user Jack251970 indicated this mismatch doesn't need to be fixed when pointed out during PR #3593 review.
📚 Learning: 2025-09-13T06:04:26.977Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml.cs:143-165
Timestamp: 2025-09-13T06:04:26.977Z
Learning: In Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml, the ListView_SizeChanged handler in the C# file assumes 5 GridViewColumns but the XAML may have more columns. The user Jack251970 indicated this mismatch doesn't need to be fixed when pointed out during PR #3593 review.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml
📚 Learning: 2025-09-11T08:30:29.487Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Flow.Launcher/SettingPages/Views/SettingsPaneTheme.xaml:0-0
Timestamp: 2025-09-11T08:30:29.487Z
Learning: In Flow.Launcher's SettingsPaneTheme.xaml theme editor panel, the inner ui:ScrollViewerEx within the Border (Grid.Column="1") is intentionally kept because its height cannot be expanded to the whole page due to layout constraints. This serves a different scrolling purpose than the outer ScrollViewerEx and is necessary for the theme editor panel functionality.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml
📚 Learning: 2025-09-05T11:56:27.267Z
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.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml
📚 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:
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml
📚 Learning: 2025-09-04T11:52:29.096Z
Learnt from: jjw24
Repo: Flow-Launcher/Flow.Launcher PR: 3932
File: Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs:48-55
Timestamp: 2025-09-04T11:52:29.096Z
Learning: In Flow Launcher's PluginsManifest.cs, when dealing with version parsing for the MinimumAppVersion feature, maintainer jjw24 prefers to keep the solution simple rather than implementing comprehensive helper methods for SemVer parsing normalization.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml
📚 Learning: 2025-09-23T04:39:31.981Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 4009
File: Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml.cs:135-139
Timestamp: 2025-09-23T04:39:31.981Z
Learning: In Flow.Launcher plugin settings UI code, setting ComboBox.SelectedItem directly to an integer value after populating ItemsSource with a List<int> works correctly and doesn't require additional validation checks. User Jack251970 confirmed this pattern is acceptable in the codebase.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml
📚 Learning: 2025-03-28T21:20:54.978Z
Learnt from: onesounds
Repo: Flow-Launcher/Flow.Launcher PR: 3394
File: Flow.Launcher/Themes/Darker Glass.xaml:134-141
Timestamp: 2025-03-28T21:20:54.978Z
Learning: In WPF applications like Flow.Launcher, Border elements cannot directly display text content and require a child element like TextBlock to handle text rendering. This separation of concerns (Border for visual container styling, TextBlock for text display) follows WPF best practices and provides greater styling flexibility.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml
📚 Learning: 2025-09-05T12:00:58.678Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Flow.Launcher/Resources/Pages/WelcomePage3.xaml:45-50
Timestamp: 2025-09-05T12:00:58.678Z
Learning: In Flow.Launcher's WelcomePage3.xaml (and likely other welcome pages), the ScrollViewerEx uses a fixed Height="478" intentionally to ensure the page remains scrollable. This fixed height is necessary for proper scrolling behavior in the welcome page context, rather than being a responsive design oversight.
Applied to files:
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml
⏰ 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)
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml (3)
6-6: LGTM!The namespace additions are necessary for the
ui:NumberBoxcontrol used in the max suggestions setting.Also applies to: 8-8
50-50: Verify that drag-and-drop changes are intentional.The addition of
AllowDrop="True"andDrop="ListView_Drop"enables drag-and-drop functionality on the search sources list. However, the PR description focuses solely on adding a max suggestions limit and doesn't mention drag-and-drop features.Are these changes intentional or should they be moved to a separate PR?
Also applies to: 53-53
151-188: Approve the WrapPanel layout structure.The refactoring from DockPanel to WrapPanel provides a more flexible responsive layout for the settings controls. The organization of max suggestions, enable suggestion, and suggestion provider into separate horizontal groups is clean and maintainable.
Description
Adds a configurable maximum suggestions setting for the WebSearch plugin, allowing users to limit the number of autocomplete suggestions displayed.
Changes
MaxSuggestionsproperty to Settings with validation (range 1-10, default: 1).Take()Files Modified
Plugins/Flow.Launcher.Plugin.WebSearch/Settings.cs- Added MaxSuggestions propertyPlugins/Flow.Launcher.Plugin.WebSearch/Main.cs- Updated to use MaxSuggestionsPlugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml- Added TextBox for user input and spacingPlugins/Flow.Launcher.Plugin.WebSearch/Languages/*.xaml- Added localization stringsLocales Updated
Type of Change
Before
After