-
-
Notifications
You must be signed in to change notification settings - Fork 352
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Input Smooth & Search delay #3350
Conversation
🥷 Code experts: onesounds, Yusyuriv 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:
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 |
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughThis pull request implements configurable search delay settings across the application. It updates the Settings class to add new properties for search delay and query behavior, revises UI elements and string resources to support these features, and adjusts event handling and query execution logic to incorporate a delay parameter. Additionally, it introduces per-plugin delay overrides through changes in plugin metadata, settings, and display components. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🪧 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 (
|
This comment has been minimized.
This comment has been minimized.
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: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
Flow.Launcher.Infrastructure/UserSettings/Settings.cs
(1 hunks)Flow.Launcher/Flow.Launcher.csproj
(1 hunks)Flow.Launcher/Languages/en.xaml
(1 hunks)Flow.Launcher/MainWindow.xaml.cs
(5 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs
(1 hunks)Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml
(1 hunks)Flow.Launcher/ViewModel/MainViewModel.cs
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
🔇 Additional comments (11)
Flow.Launcher.Infrastructure/UserSettings/Settings.cs (1)
278-279
: Good implementation of the search delay settingsThe addition of these two settings properties is clean and fits well within the existing settings structure. The default values (delay disabled by default with a 120ms duration when enabled) are reasonable choices.
Flow.Launcher/Flow.Launcher.csproj (1)
107-107
: Appropriate package addition for implementing reactive search behaviorAdding System.Reactive.Linq is a good choice for implementing the delayed search functionality, as it provides the tools needed for throttling input events.
Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml (1)
175-195
: UI elements for search delay are well structuredThe UI elements follow the application's design patterns with appropriate use of CardGroup and Cards. The toggle switch and ComboBox are correctly bound to the corresponding ViewModel properties, and the use of localized strings for labels and tooltips supports internationalization.
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (1)
153-156
: Range of delay values is comprehensiveThe delay range from 30ms to 300ms provides good flexibility for users to find their preferred responsiveness level. The 120ms default is a reasonable middle ground.
Flow.Launcher/Languages/en.xaml (1)
97-100
: Well-formatted and clear string resources for search delay feature.The added string resources for the new search delay functionality follow the existing naming conventions and provide helpful tooltips that clearly explain the feature's purpose and default values.
Flow.Launcher/ViewModel/MainViewModel.cs (2)
633-633
: Explicit query call for better control of search execution.This change separates setting the query text from executing the query, which enables the delayed search functionality. Now the query is explicitly triggered after setting the text rather than being automatically triggered by the property setter.
698-698
: Ensures query is executed when selecting results with empty query text.Adds an explicit call to Query() after clearing the query text, which maintains the expected behavior while supporting the new delayed search implementation.
Flow.Launcher/MainWindow.xaml.cs (4)
29-29
: Added required namespace for reactive programming.The System.Reactive.Linq namespace provides the essential extensions for implementing delayed search using the Observable pattern and Throttle operator.
71-71
: Initializes search delay functionality when window loads.Properly sets up the reactive behavior of the search text box based on user settings when the window is loaded.
209-265
: Improved code organization for window visibility and progress bar handling.The refactored property change handlers flatten the nested conditionals and improve the organization of the code, making it more readable while maintaining the same functionality.
864-932
: Well-implemented search delay using reactive programming.The search delay implementation:
- Uses the Observable pattern with Throttle operator to delay search execution
- Properly handles subscription lifecycle (creation and disposal)
- Maintains auto-complete functionality
- Separates concerns into well-defined methods
- Follows good reactive programming practices derived from PowerToys
The code effectively implements the search delay functionality requested in issue #1677.
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
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 (1)
Flow.Launcher/MainWindow.xaml.cs (1)
865-936
: Well-implemented search delay functionalityThe implementation of search delay using Reactive Extensions is well-structured and handles all edge cases:
- Properly cleans up previous subscriptions before creating new ones
- Uses appropriate throttling mechanism to delay search execution
- Correctly handles both delayed and immediate search modes
- Properly manages auto-complete text clearing
- Includes attribution to the original PowerToys implementation
Consider adding XML documentation to these methods to improve code maintainability for future developers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Flow.Launcher.Infrastructure/UserSettings/Settings.cs
(1 hunks)Flow.Launcher/MainWindow.xaml.cs
(6 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs
(1 hunks)Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml
- Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: gitStream.cm
🔇 Additional comments (5)
Flow.Launcher.Infrastructure/UserSettings/Settings.cs (1)
278-288
: New search delay functionality looks goodThe implementation adds two well-designed properties to support search query delay functionality:
SearchQueryResultsWithDelay
- toggle to enable/disable the delay featureSearchInputDelay
- configurable delay value (default 120ms)The properties follow proper MVVM patterns with appropriate property change notification.
Flow.Launcher/MainWindow.xaml.cs (4)
29-29
: Added Reactive Extensions dependencyAdded the necessary dependency to implement reactive programming for the search delay functionality.
71-71
: Appropriate initialization of search reactivenessThe search reactiveness setup is called during window loading, which is the correct place to initialize this functionality.
205-264
: Improved property change handlers structureThe property change handlers for
MainWindowVisibilityStatus
andProgressBarVisibility
have been refactored to be more readable with better structure and explicit break statements.
300-302
: Good handling of settings property changeThe code properly responds to settings changes by setting up the search reactiveness when the delay toggle changes.
This comment has been minimized.
This comment has been minimized.
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
🧹 Nitpick comments (1)
Flow.Launcher/MainWindow.xaml.cs (1)
926-926
: Consider using case-insensitive comparison for autocompleteThe code uses StringComparison.Ordinal for autocomplete prefix matching, which is case-sensitive. For better user experience, consider using StringComparison.OrdinalIgnoreCase to match autocomplete suggestions regardless of case.
- return string.IsNullOrEmpty(queryText) || !autoCompleteText.StartsWith(queryText, StringComparison.Ordinal); + return string.IsNullOrEmpty(queryText) || !autoCompleteText.StartsWith(queryText, StringComparison.OrdinalIgnoreCase);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/MainWindow.xaml.cs
(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: gitStream.cm
🔇 Additional comments (6)
Flow.Launcher/MainWindow.xaml.cs (6)
29-29
: Added Reactive Extensions dependencyThe code now includes System.Reactive.Linq for implementing throttled search functionality.
198-199
: Search reactiveness initialization addedGood addition of search reactiveness setup during window loading, which allows for delayed search functionality.
210-246
: Improved window visibility handlingThe code has been reorganized for better readability while maintaining the same functionality. The conditional structure is now clearer with better indentation.
249-265
: Refactored progress bar visibility logicThe progress bar visibility handling has been improved with clearer conditional statements and better structure.
301-303
: Added handler for search delay setting changesGood implementation of a property change handler that reconfigures search reactiveness when the user toggles the delay setting.
889-889
:✅ Verification successful
Verify search delay configuration
The throttle delay is set from user settings, which is a good approach for configurability.
🌐 Web query:
What is a good throttle delay for search input in UI applications?
💡 Result:
Throttling delays for search inputs in UI applications typically range between 200-500 milliseconds, but the optimal choice depends on balancing responsiveness and performance. Here's a breakdown of key considerations:
Throttle vs. Debounce Preference
- Debouncing is generally preferred for search inputs[1][5][9], as it waits until typing stops completely (e.g., 300ms pause) before executing, reducing unnecessary API calls.
- Throttling forces regular execution intervals, which can feel more responsive but might generate more network traffic than needed[2][3].
When Using Throttle
For scenarios where immediate feedback is critical:
- 200-300ms: Provides near real-time updates for short queries
- 400-500ms: Better for complex searches or slower backends
Performance Considerations
Delay Pros Cons 200ms Feels instantaneous High server load 300ms Balanced UX Slight lag 500ms Server-friendly Noticeable delay Implementation Example
const throttleSearch = throttle((query) => { // API call here }, 300); // Adjust delay based on testing inputElement.addEventListener('input', (e) => { throttleSearch(e.target.value); });In practice, most teams opt for debouncing with 300ms delay for search inputs[5][7][9], reserving throttling for scroll/resize handlers or rate-limited APIs[4][6]. Always test with real users to find the sweet spot between perceived performance and system load[8].
Citations:
- 1: https://dev.to/abhirupa/the-art-of-smooth-ux-debouncing-and-throttling-for-a-more-performant-ui-m0h
- 2: http://devsign.co/notes/throttle-and-debounce
- 3: https://www.youtube.com/watch?v=cjIswDCKgu0
- 4: https://weblog.west-wind.com/posts/2017/jul/02/debouncing-and-throttling-dispatcher-events
- 5: https://dev.to/kartikbudhraja/debouncing-and-throttling-choosing-the-right-strategy-for-your-web-projects-5cjh
- 6: https://www.telerik.com/blogs/debouncing-and-throttling-in-javascript
- 7: https://forums.meteor.com/t/best-way-to-throttle-a-live-search-input/32133
- 8: https://www.developerway.com/posts/debouncing-in-react
- 9: https://blog.codeminer42.com/codetips10-throttle-and-debounce/
Search Delay Configuration Verified
The current implementation sets the throttle delay from user settings, which is a flexible approach. Based on industry guidance, a throttle delay in the range of 200–500 ms is generally ideal for search inputs. As a best practice, please ensure that the default value in
_settings.SearchInputDelay
falls within this range (around 300 ms is common). Additionally, consider whether a debouncing strategy might better suit your use case if minimizing unnecessary API calls is a priority.
@@ -631,6 +630,7 @@ public void ChangeQueryText(string queryText, bool isReQuery = false) | |||
{ | |||
// re-query is done in QueryText's setter method | |||
QueryText = queryText; | |||
Query(); |
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.
why do we need this?
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.
From original codes, when QueryText is changed, it will call Query()
.
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.
And I remove Query()
in QueryText
setter, so we need to manually call it.
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.
Why do we do that?
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.
QueryText
is a property used by others functions in MainViewModel
like auto complete text, backspace event, change query text event, etc. So we need to update it once users input.
Query
is a function to load results and we can delay them.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
Flow.Launcher/Languages/en.xaml
(4 hunks)Flow.Launcher/SearchDelaySpeedWindow.xaml
(1 hunks)Flow.Launcher/SearchDelaySpeedWindow.xaml.cs
(1 hunks)Flow.Launcher/SettingPages/ViewModels/DropdownDataGeneric.cs
(3 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs
(3 hunks)Flow.Launcher/ViewModel/PluginViewModel.cs
(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs
- Flow.Launcher/Languages/en.xaml
🧰 Additional context used
🧬 Code Definitions (1)
Flow.Launcher/ViewModel/PluginViewModel.cs (3)
Flow.Launcher.Infrastructure/UserSettings/PluginSettings.cs (3)
Plugin
(90-97)Plugin
(99-103)Plugin
(106-132)Flow.Launcher/Resources/Controls/InstalledPluginSearchDelay.xaml.cs (2)
InstalledPluginSearchDelay
(5-11)InstalledPluginSearchDelay
(7-10)Flow.Launcher/SearchDelaySpeedWindow.xaml.cs (2)
SearchDelaySpeedWindow
(10-55)SearchDelaySpeedWindow
(14-18)
🔇 Additional comments (11)
Flow.Launcher/SearchDelaySpeedWindow.xaml.cs (2)
22-36
: Good job implementing dropdown population logic.The dropdown population logic is well-structured with proper handling for both default and null cases. The code correctly handles inserting a default value at the beginning of the list and setting the selected item based on the current plugin search delay value.
44-54
: LGTM! Well-implemented button handling.The method properly updates the search delay speed based on the user's selection, handles the special case for default values, and triggers the necessary event to refresh UI components before closing the window.
Flow.Launcher/SettingPages/ViewModels/DropdownDataGeneric.cs (2)
21-22
: Consistent use of translation API.Good refactoring to use
App.API.GetTranslation
instead ofInternationalizationManager.Instance.GetTranslation
. This maintains consistency with other parts of the codebase.Also applies to: 32-33
11-11
:❓ Verification inconclusive
Property accessibility change might have broader implications.
Changed from
private init
topublic set
, making the property mutable after initialization. While this is needed for the search delay functionality (when inserting new items in the dropdown), consider if this change has any unintended consequences elsewhere in the codebase.
🏁 Script executed:
#!/bin/bash # Checking for other usages of LocalizationKey setter rg -n "\.LocalizationKey\s*=\s*" --glob "*.cs" | grep -v "DropdownDataGeneric\.cs" | grep -v "SearchDelaySpeedWindow\.cs"Length of output: 123
Let's run an additional check covering all files (without exclusions) to ensure we don't miss any assignments to
LocalizationKey
.
🏁 Script executed:
#!/bin/bash # Re-checking the entire repo for assignments to LocalizationKey without any file exclusions rg -n "\.LocalizationKey\s*=" --glob "*.cs"Length of output: 45
Action Required: Confirm No Unintended Side Effects from Increased Property Mutability
The change in
LocalizationKey
from aprivate init
to apublic set
is necessary for implementing the search delay functionality when new items are added to the dropdown. Repository-wide searches for assignments toLocalizationKey
(excluding and then including all files) did not reveal any unexpected usage outside of the known contexts. However, since the search outputs were empty—which might suggest a low signal-to-noise ratio—it is advisable to manually verify that no other parts of the codebase rely on the previous immutability of this property.
- File:
Flow.Launcher/SettingPages/ViewModels/DropdownDataGeneric.cs
- Concern: Increased mutability of
LocalizationKey
could potentially affect how and where this property is assigned.- Recommendation: Manually review assignment points and usage of
LocalizationKey
across the codebase to ensure no unintended side effects exist.Flow.Launcher/SearchDelaySpeedWindow.xaml (1)
1-138
: Well-structured UI layout for search delay settings.The window is well-designed with:
- Appropriate width, height, and non-resizable properties
- Clear display of current and new search delay speeds
- Properly organized controls with good spacing and alignment
- Consistent use of dynamic resources for text and colors
The window provides a clean interface for users to modify search delay settings.
Flow.Launcher/ViewModel/PluginViewModel.cs (6)
88-96
: Well-implemented search delay property.The
PluginSearchDelay
property is correctly implemented to:
- Get the search delay speed from the plugin metadata
- Update both the metadata and settings object when set
- Use nullable enum to allow for "not set" value
This maintains consistency between the UI and the underlying configuration.
105-108
: Component reorganization appears appropriate.The changes to
BottomPart2
and addition ofBottomPart3
appropriately reorganize the UI components to accommodate the search delay settings. The lazy initialization pattern (??=
) is correctly used to create controls only when needed.
132-132
: Good implementation of SearchDelaySpeedText property.The property correctly handles both null and non-null cases for the search delay speed, providing appropriate localized text using the DefaultLocalizationKey constant where needed.
133-133
: Appropriate property modifier change.Changing
PluginSettingsObject
fromget; set;
toget; init;
is a good practice that ensures the property can only be set during object initialization. This helps prevent accidental modifications after initialization.
140-143
: Proper implementation of change notification.The
OnSearchDelaySpeedChanged
method correctly triggers property change notification for theSearchDelaySpeedText
property, ensuring that UI components are updated when the search delay speed changes.
187-192
: Clean implementation of search delay settings dialog.The
SetSearchDelaySpeed
method correctly creates and displays the search delay speed window, making it modal to prevent interaction with the main window until settings are confirmed or canceled.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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 (1)
Flow.Launcher/SearchDelayTimeWindow.xaml.cs (1)
25-26
: Correct the comment referencing a non-existent enum value.The comment references "SearchDelayTime.Slow" which doesn't exist in the
SearchDelayTime
enum. The enum has "Long" as its highest value, not "Slow".- // Because default value is SearchDelayTime.Slow, we need to get selected value before adding default value + // Because default value is SearchDelayTime.Medium, we need to get selected value before adding default value
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
Flow.Launcher.Infrastructure/UserSettings/PluginSettings.cs
(3 hunks)Flow.Launcher.Infrastructure/UserSettings/Settings.cs
(3 hunks)Flow.Launcher.Plugin/PluginMetadata.cs
(1 hunks)Flow.Launcher.Plugin/SearchDelayTime.cs
(1 hunks)Flow.Launcher/Languages/en.xaml
(4 hunks)Flow.Launcher/Resources/Controls/InstalledPluginSearchDelay.xaml
(1 hunks)Flow.Launcher/SearchDelayTimeWindow.xaml
(1 hunks)Flow.Launcher/SearchDelayTimeWindow.xaml.cs
(1 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs
(3 hunks)Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml
(1 hunks)Flow.Launcher/ViewModel/MainViewModel.cs
(15 hunks)Flow.Launcher/ViewModel/PluginViewModel.cs
(8 hunks)Plugins/Flow.Launcher.Plugin.WebSearch/plugin.json
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- Flow.Launcher.Plugin/PluginMetadata.cs
- Plugins/Flow.Launcher.Plugin.WebSearch/plugin.json
- Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml
- Flow.Launcher/Resources/Controls/InstalledPluginSearchDelay.xaml
- Flow.Launcher.Infrastructure/UserSettings/Settings.cs
- Flow.Launcher/ViewModel/MainViewModel.cs
- Flow.Launcher/Languages/en.xaml
- Flow.Launcher/ViewModel/PluginViewModel.cs
- Flow.Launcher.Infrastructure/UserSettings/PluginSettings.cs
🔇 Additional comments (10)
Flow.Launcher.Plugin/SearchDelayTime.cs (1)
1-32
: Well-structured enum definition with clear documentation.The
SearchDelayTime
enum is well-defined with appropriate value names and comprehensive XML documentation that includes specific millisecond values for each option. This is a good approach for configurable delay times.Flow.Launcher/SearchDelayTimeWindow.xaml.cs (4)
14-18
: Constructor implementation looks good.The constructor properly initializes the window and stores the plugin view model reference.
20-37
: Well-implemented window loading handler.The method correctly populates the UI with current settings and handles the default value case appropriately.
39-42
: Simple cancel button handler is correctly implemented.The cancel button handler appropriately closes the window without saving changes.
44-54
: Done button handler correctly updates plugin settings.The handler properly extracts the selected value, handles the default case correctly, and updates the view model before closing the window.
Flow.Launcher/SearchDelayTimeWindow.xaml (1)
1-138
: Well-structured XAML with proper styling and accessibility.The window UI is well-designed with appropriate layout, styling, and localization. The controls are properly arranged and event handlers are correctly connected.
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (4)
34-34
: New SearchDelayTimeData class follows established pattern.The
SearchDelayTimeData
class correctly extends the generic dropdown data class, following the same pattern as other dropdown data classes in this file.
147-149
: SearchDelayTimes property correctly initialized.The property is properly initialized using the same pattern as other dropdown lists in this class.
150-165
: Well-implemented SearchDelayTime property with proper null handling.The getter uses
FirstOrDefault
with fallbacks to avoid exceptions, and the setter checks for null before updating the value. This follows best practices for null safety.
173-173
: Correctly updated localization method.The
UpdateEnumDropdownLocalizations
method is properly updated to include the new search delay times.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
Pull Request Overview
This PR introduces configurable search delay times for plugins along with UI and settings updates to support smooth input and search delay functionality. Key changes include updating the Plugin and Main view models to handle search delay settings, adding a new SearchDelayTimeWindow for user selection, and updating plugin metadata and settings to include delay values.
Reviewed Changes
Copilot reviewed 15 out of 21 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
Flow.Launcher/ViewModel/PluginViewModel.cs | Added PluginSearchDelayTime property and updated icon loading to be asynchronous. |
Flow.Launcher/ViewModel/MainViewModel.cs | Modified query methods to account for search delay and adjusted lambda parameter naming. |
Flow.Launcher/SettingPages/ViewModels/SettingsPanePluginsViewModel.cs | Updated plugin settings retrieval using a helper method for null-safety. |
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs | Added dropdown support and binding for search delay time in general settings. |
Flow.Launcher/SettingPages/ViewModels/DropdownDataGeneric.cs | Updated localization calls to use App.API instead of InternationalizationManager. |
Flow.Launcher/SearchDelayTimeWindow.xaml.cs | Implemented new window for user selection of search delay time with default value support. |
Flow.Launcher/MainWindow.xaml.cs | Added event handler to trigger query with search delay when text changes. |
Flow.Launcher.Plugin/SearchDelayTime.cs | Introduced SearchDelayTime enum with various delay options. |
Flow.Launcher.Plugin/PluginMetadata.cs | Added nullable SearchDelayTime property with JSON converter support. |
Flow.Launcher.Infrastructure/UserSettings/Settings.cs & PluginSettings.cs | Updated settings to include search delay configuration and initialized default values. |
Flow.Launcher.Core/Plugin/PluginsLoader.cs & PluginManager.cs | Made internal helper methods private and updated plugin removal to use the new settings method. |
Files not reviewed (6)
- Flow.Launcher/Languages/en.xaml: Language not supported
- Flow.Launcher/MainWindow.xaml: Language not supported
- Flow.Launcher/Resources/Controls/InstalledPluginDisplay.xaml: Language not supported
- Flow.Launcher/Resources/Controls/InstalledPluginSearchDelay.xaml: Language not supported
- Flow.Launcher/SearchDelayTimeWindow.xaml: Language not supported
- Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml: Language not supported
Comments suppressed due to low confidence (2)
Flow.Launcher/ViewModel/MainViewModel.cs:1043
- [nitpick] Consider renaming the 'searchDelay' parameter to 'applySearchDelay' for improved clarity about its behavior.
public void Query(bool searchDelay, bool isReQuery = false)
Flow.Launcher/SearchDelayTimeWindow.xaml.cs:32
- [nitpick] Consider extracting the magic string 'default' into a constant for better reuse and maintainability.
searchDelayTimes.Insert(0, new SearchDelayTimeData { Display = App.API.GetTranslation("default"), LocalizationKey = "default" });
This comment has been minimized.
This comment has been minimized.
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 (4)
Flow.Launcher/SearchDelayTimeWindow.xaml.cs (4)
34-36
: Consider separating the logic for default option.The insertion of the default option and the null coalescing assignment could be structured more clearly.
- searchDelayTimes.Insert(0, new SearchDelayTimeData { Display = App.API.GetTranslation("default"), LocalizationKey = "default" }); - selected ??= searchDelayTimes.FirstOrDefault(); + // Add default option + var defaultOption = new SearchDelayTimeData { Display = App.API.GetTranslation("default"), LocalizationKey = "default" }; + searchDelayTimes.Insert(0, defaultOption); + + // Select default if none was found + if (selected == null) + { + selected = defaultOption; + }
41-44
: Method naming convention inconsistency.The event handler
BtnCancel_OnClick
starts with an uppercase letter, whilebtnDone_OnClick
starts with a lowercase letter.Use consistent naming conventions for all event handlers, preferably the
ControlName_EventName
pattern with consistent casing.
49-51
: Complex conditional logic could be simplified.The conditional logic for setting the
changedValue
is somewhat dense and could be made more readable.- var selected = cbDelay.SelectedItem as SearchDelayTimeData; - SearchDelayTime? changedValue = selected?.LocalizationKey != "default" ? selected.Value : null; - _pluginViewModel.PluginSearchDelayTime = changedValue; + var selectedData = cbDelay.SelectedItem as SearchDelayTimeData; + + // Set to null if default is selected, otherwise use the selected value + SearchDelayTime? newDelayTime = null; + if (selectedData?.LocalizationKey != "default") + { + newDelayTime = selectedData.Value; + } + + _pluginViewModel.PluginSearchDelayTime = newDelayTime;
54-55
: Add comment about OnSearchDelayTimeChanged.There is no documentation about what
OnSearchDelayTimeChanged()
does. This would help future developers understand the notification flow.Add a descriptive comment explaining the purpose and side effects of calling
OnSearchDelayTimeChanged()
.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Flow.Launcher/SearchDelayTimeWindow.xaml
(1 hunks)Flow.Launcher/SearchDelayTimeWindow.xaml.cs
(1 hunks)Flow.Launcher/ViewModel/PluginViewModel.cs
(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- Flow.Launcher/SearchDelayTimeWindow.xaml
- Flow.Launcher/ViewModel/PluginViewModel.cs
🔇 Additional comments (3)
Flow.Launcher/SearchDelayTimeWindow.xaml.cs (3)
1-7
: Using statements are well-organized.The imports are logically grouped and include all necessary references for the window's functionality.
10-18
: Class structure looks good.The window class is properly defined with a constructor that accepts a PluginViewModel instance. The initialization flow is clear and follows standard practices.
27-28
: Inconsistency between comment and actual default.The comment mentions that the default value is
SearchDelayTime.Slow
, but according to the AI summary, the default in theSettings
class isSearchDelayTime.Medium
. This could lead to confusion.Please verify which is the actual default value and update the comment accordingly.
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. Forbidden patterns 🙅 (1)In order to address this, you could change the content to not match the forbidden patterns (comments before forbidden patterns may help explain why they're forbidden), add patterns for acceptable instances, or adjust the forbidden patterns themselves. These forbidden patterns matched content: s.b. workaround(s)
If the flagged items are 🤯 false positivesIf items relate to a ...
|
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 (1)
Flow.Launcher/Languages/en.xaml (1)
130-131
: Fix typos in plugin search delay stringsThere are spelling errors in the plugin search delay strings.
-<system:String x:Key="pluginSearchDelayTime">Plugin seach delay time</system:String> -<system:String x:Key="pluginSearchDelayTimeTooltip">Change Plugin Seach Delay Time</system:String> +<system:String x:Key="pluginSearchDelayTime">Plugin search delay time</system:String> +<system:String x:Key="pluginSearchDelayTimeTooltip">Change Plugin Search Delay Time</system:String>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
Flow.Launcher.Infrastructure/UserSettings/Settings.cs
(3 hunks)Flow.Launcher.Plugin/SearchDelayTime.cs
(1 hunks)Flow.Launcher/Languages/en.xaml
(4 hunks)Flow.Launcher/SearchDelayTimeWindow.xaml.cs
(1 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs
(3 hunks)Flow.Launcher/ViewModel/MainViewModel.cs
(15 hunks)Plugins/Flow.Launcher.Plugin.WebSearch/plugin.json
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- Flow.Launcher/SearchDelayTimeWindow.xaml.cs
- Flow.Launcher.Infrastructure/UserSettings/Settings.cs
- Flow.Launcher/ViewModel/MainViewModel.cs
🧰 Additional context used
🧬 Code Definitions (2)
Flow.Launcher.Plugin/SearchDelayTime.cs (1)
Flow.Launcher.Infrastructure/UserSettings/PluginSettings.cs (3)
Plugin
(90-97)Plugin
(99-103)Plugin
(106-132)
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (2)
Flow.Launcher/SettingPages/ViewModels/DropdownDataGeneric.cs (3)
DropdownDataGeneric
(7-35)List
(13-26)UpdateLabels
(28-34)Flow.Launcher.Infrastructure/UserSettings/Settings.cs (2)
List
(401-438)Settings
(16-439)
🔇 Additional comments (8)
Flow.Launcher.Plugin/SearchDelayTime.cs (1)
1-32
: Well-structured enum implementationThe
SearchDelayTime
enum is well-documented with clear XML comments that specify the delay duration in milliseconds for each option. The naming is intuitive and follows a logical progression from very long to very short delays.Plugins/Flow.Launcher.Plugin.WebSearch/plugin.json (1)
34-35
: Appropriate delay time for Web Search pluginSetting a "VeryLong" search delay for the Web Search plugin makes sense, as web searches typically require more processing time and network operations. This should help reduce unnecessary API calls during typing.
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (4)
34-34
: Good implementation of dropdown data classThe
SearchDelayTimeData
class follows the established pattern in the codebase by extendingDropdownDataGeneric<SearchDelayTime>
for UI representation.
147-149
: Consistent collection initializationThe
SearchDelayTimes
property follows the same initialization pattern as other dropdown collections in the file, maintaining consistency throughout the codebase.
150-165
: Null-safe property implementationThe
SearchDelayTime
property implementation includes proper null-checking and fallback mechanisms to prevent potential NullReferenceExceptions. This follows best practices for safe property access.
173-173
: Consistent update of localization labelsThe addition to
UpdateEnumDropdownLocalizations()
ensures that the search delay dropdown labels are properly updated when the language changes.Flow.Launcher/Languages/en.xaml (2)
105-114
: Clear and descriptive string resourcesThe main search delay string resources are well-defined with clear labels and helpful tooltips that explain the feature's purpose and behavior to users.
368-373
: Comprehensive search delay settings dialog stringsThe string resources for the search delay settings dialog are well-organized and provide clear guidance to users when configuring plugin-specific delay settings.
Delay time when typing to search
Delay plugin query when typing. We can set one default search delay time, and we can set search delay time for each plugin so that they will not follow default search delay time.
Close #1677.
Demo
Delay:
2025-03-30.16-24-10.mp4
No delay:
2025-03-30.16-24-32.mp4
TODO
plugin.json
. (You can add"SearchDelaySpeed": "Slow"
inplugin.json
to set default search delay speed value)Plugin
project like Improve Plugin Metadata & Path Management #3272.// TODO: Remove debug codes.
(Remove search delay debug codes #3405)Test
plugin.json
can be applied.