Plugin Page Control Swap Feature & Restore "Search Delay" Field in General Page #3421
Conversation
- Adjust combobox
This comment has been minimized.
This comment has been minimized.
|
🥷 Code experts: Jack251970, Yusyuriv Jack251970, onesounds have most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
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 |
📝 WalkthroughWalkthroughThis pull request implements extensive updates to the Flow Launcher application. It adds new UI string resources and revises existing ones to reflect clearer display mode and search delay instructions. Several XAML files have been updated with additional controls, converters, and reorganized layouts. In the view models and settings, display mode properties and search delay settings have been restructured by switching from enumeration types to integer-based configurations. Obsolete files and components related to the previous search delay implementation have been removed. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsVM
participant UI
participant Dialog
User->>UI: Select display mode via ComboBox
UI->>SettingsVM: Update SelectedDisplayMode
SettingsVM->>SettingsVM: Execute UpdateDisplayModeFromSelection()
SettingsVM-->>UI: Update control visibilities (OnOff, Priority, SearchDelay)
User->>UI: Click Help Button
UI->>SettingsVM: Trigger OpenHelperCommand
SettingsVM->>Dialog: Open helper dialog (ContentDialog)
Dialog-->>SettingsVM: Return help information
SettingsVM-->>UI: Display help dialog to user
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (8)
Flow.Launcher/Resources/Dark.xaml (1)
116-116: Overlay Color Opacity Increased
TheContentDialogOverlayBGresource now uses#58000000(previously#4D000000), which increases its opacity. This should yield a more pronounced overlay effect when dialogs are displayed. Please verify that the enhanced opacity maintains sufficient contrast and integrates well with related UI elements (e.g., dialogs in the settings and plugin pages).Flow.Launcher/ViewModel/PluginViewModel.cs (1)
130-142: Unused private field_priorityshould be removed.The new implementation of the
Priorityproperty includes a private backing field_prioritythat is declared but never used. The getter still directly accessesPluginPair.Metadata.Priorityand the setter callsChangePrioritywithout using the backing field.- //public int Priority => PluginPair.Metadata.Priority; - private int _priority; + //public int Priority => PluginPair.Metadata.Priority; public int Priority { get => PluginPair.Metadata.Priority; set { if (PluginPair.Metadata.Priority != value) { ChangePriority(value); } } }Flow.Launcher/Resources/Controls/InstalledPluginSearchDelayCombobox.xaml.cs (1)
20-21: Translate Korean comments to English for consistency.The code contains Korean comments that should be translated to English for better maintainability and consistency.
- // 초기 값 설정 + // Set initial value int currentDelayMs = GetCurrentDelayMs(viewModel); ... - // SearchDelayTime enum 값을 int로 변환 + // Convert SearchDelayTime enum value to int SearchDelayTime? delayTime = viewModel.PluginPair.Metadata.SearchDelayTime; ... - return 0; // ̸Å|°ª + return 0; // Default valueAlso applies to: 35-36, 41-42
Flow.Launcher/SettingPages/Views/SettingsPanePlugins.xaml (1)
68-73: Enhance help button with a tooltip.The help button uses a symbol font character () which might not be immediately intuitive to all users. Adding a tooltip would improve usability.
<Button Width="34" Height="34" FontFamily="{DynamicResource SymbolThemeFontFamily}" Margin="0 0 20 0" Content="" - FontSize="14" Click="Help_Click"/> + FontSize="14" Click="Help_Click" + ToolTip="{DynamicResource helpButtonTooltip}" + ToolTipService.InitialShowDelay="200" + ToolTipService.Placement="Top"/>Flow.Launcher/SettingPages/Views/SettingsPanePlugins.xaml.cs (1)
44-85: Consider moving dialog content to XAML resources.The help dialog content is constructed programmatically, which works but makes it harder to maintain and style. Consider defining this content as a XAML resource or UserControl for better separation of concerns.
// Example of how to improve this: private async void Help_Click(object sender, RoutedEventArgs e) { // Create the dialog using a resource var helpDialog = new ContentDialog() { Content = Application.Current.Resources["HelpDialogContent"], PrimaryButtonText = (string)Application.Current.Resources["commonOK"], CornerRadius = new CornerRadius(8), Style = (Style)Application.Current.Resources["ContentDialog"] }; await helpDialog.ShowAsync(); } // Then define the content in a resource dictionary: // <DataTemplate x:Key="HelpDialogContent"> // <StackPanel> // <!-- Help content here --> // </StackPanel> // </DataTemplate>Flow.Launcher/Resources/CustomControlTemplate.xaml (1)
3586-3586: Potentially redundant marginYou're still using the dynamic resource
ContentDialogPaddingfor the Grid margin, but have removed it from the parent Border. This might lead to inconsistent spacing behavior.Consider whether you want padding to be applied at the Grid level or at the Border level to maintain a consistent spacing approach.
Flow.Launcher/Resources/Controls/InstalledPluginDisplay.xaml (1)
55-94: Consider removing commented code.While it's sometimes useful to keep old code temporarily during development, you should remove this commented-out code before finalizing the PR. It makes the codebase cleaner and more maintainable.
Flow.Launcher/SettingPages/ViewModels/SettingsPanePluginsViewModel.cs (1)
32-52: Consider using constants for display mode strings.The hard-coded strings "OnOff", "Priority", and "SearchDelay" appear in multiple places. Using constants or an enum would make the code more maintainable and less prone to typos.
+ public static class DisplayModes + { + public const string OnOff = "OnOff"; + public const string Priority = "Priority"; + public const string SearchDelay = "SearchDelay"; + } public void UpdateDisplayModeFromSelection() { switch (SelectedDisplayMode) { - case "OnOff": + case DisplayModes.OnOff: IsOnOffSelected = true; IsPrioritySelected = false; IsSearchDelaySelected = false; break; - case "Priority": + case DisplayModes.Priority: IsOnOffSelected = false; IsPrioritySelected = true; IsSearchDelaySelected = false; break; - case "SearchDelay": + case DisplayModes.SearchDelay: IsOnOffSelected = false; IsPrioritySelected = false; IsSearchDelaySelected = true; break; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
Flow.Launcher/Converters/StringEqualityToVisibilityConverter.cs(1 hunks)Flow.Launcher/Languages/en.xaml(1 hunks)Flow.Launcher/Resources/Controls/InstalledPluginDisplay.xaml(4 hunks)Flow.Launcher/Resources/Controls/InstalledPluginSearchDelayCombobox.xaml(1 hunks)Flow.Launcher/Resources/Controls/InstalledPluginSearchDelayCombobox.xaml.cs(1 hunks)Flow.Launcher/Resources/CustomControlTemplate.xaml(7 hunks)Flow.Launcher/Resources/Dark.xaml(1 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPanePluginsViewModel.cs(1 hunks)Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml(1 hunks)Flow.Launcher/SettingPages/Views/SettingsPanePlugins.xaml(2 hunks)Flow.Launcher/SettingPages/Views/SettingsPanePlugins.xaml.cs(2 hunks)Flow.Launcher/ViewModel/PluginViewModel.cs(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
Flow.Launcher/SettingPages/Views/SettingsPanePlugins.xaml.cs (2)
Flow.Launcher/SettingPages/ViewModels/SettingsPanePluginsViewModel.cs (3)
SettingsPanePluginsViewModel(13-150)SettingsPanePluginsViewModel(122-125)UpdateDisplayModeFromSelection(32-52)Flow.Launcher/SettingPages/Views/SettingsPanePluginStore.xaml.cs (1)
SettingsPanePlugins_OnKeyDown(43-47)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
🔇 Additional comments (20)
Flow.Launcher/Resources/Controls/InstalledPluginSearchDelayCombobox.xaml (1)
1-39: New UserControl looks well-structured for delay selection.This new UserControl implements a search delay combobox that will be used in the plugin management UI. The component is cleanly structured with proper XAML namespaces and data binding setup.
A few observations:
- The TextBlock is set to
Visibility="Collapsed"- is this intentional for this view, or will it be made visible conditionally?- The ComboBox has a fixed width of 100, which might limit display if option text is longer in some languages.
Please confirm that the hidden TextBlock is intentional, or if it's meant to be shown conditionally through a trigger or binding.
Flow.Launcher/Converters/StringEqualityToVisibilityConverter.cs (1)
1-23: Well-implemented visibility converter for string comparison.This converter follows best practices for implementing
IValueConverter:
- Properly handles null values by returning
Visibility.Collapsed- Uses efficient string comparison
- Clear implementation of the Convert method
The
ConvertBackmethod appropriately throwsNotImplementedExceptionsince the conversion is likely only needed in one direction.Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml (1)
199-218: New search delay settings section integrates well with existing UI.The new ExCard for search delay settings is properly implemented with:
- Toggle switch to enable/disable search delay functionality
- Nested card with combobox for selecting delay time
- Appropriate dynamic resources for localization
- Consistent styling with the rest of the settings page
This addition effectively restores the "Search Delay" field to the General page as mentioned in the PR objectives.
Flow.Launcher/Languages/en.xaml (1)
137-140: Added localization strings for new UI components.The new string resources provide localization support for the advanced settings interface mentioned in the PR objectives:
- FilterComboboxLabel for the combobox label
- EnabledCombobox, PriorityCombobox, and SearchDelayCombobox for display modes
These additions support the PR's goal of enhancing the plugin page with a ComboBox for switching controls.
Flow.Launcher/SettingPages/Views/SettingsPanePlugins.xaml (2)
47-67: Display mode selection UI is well-implemented.The ComboBox for selecting display modes provides a clear and intuitive way for users to switch between different views. The binding to the ViewModel's
SelectedDisplayModeproperty ensures proper state management.
119-123: Border styling improves visual consistency.Adding a Border with Background around the ListBox improves the visual consistency of the UI.
Flow.Launcher/SettingPages/Views/SettingsPanePlugins.xaml.cs (1)
29-35: Selection change handler correctly updates the view model.The
DisplayModeComboBox_SelectionChangedmethod properly delegates to the view model'sUpdateDisplayModeFromSelectionmethod, ensuring the state is updated when the user changes the display mode.Flow.Launcher/Resources/CustomControlTemplate.xaml (5)
3574-3578: Consider the implications of fixed sizingYou've changed the DialogSpace padding from a dynamic resource to a fixed value of 0, and changed the second row height from
Autoto a fixed80. While this creates a more consistent layout, it could create issues if:
- Content needs different padding based on themes/user preferences
- The button area needs more than 80px of height on certain displays or with certain content
Consider whether this dialog will be used in contexts where the button row might need more height, or where padding should be theme-dependent.
3629-3631: Good styling separation for button areaThe addition of a separate styled Border for the button area with specific background, border and corner radius creates a clear visual distinction for the command area.
3784-3790: Consistent button positioning changesThe changes to Grid.Column, ColumnSpan and margins create a more consistent button layout in the PrimaryVisible state.
3802-3813: Visual state consistency maintainedYou've made similar positioning and margin changes across multiple visual states, ensuring consistent behavior regardless of which buttons are visible.
3821-3835: Complete visual state patternThe changes to the CloseVisible state complete the pattern of updates across all visual states, maintaining consistency in the UI.
Flow.Launcher/Resources/Controls/InstalledPluginDisplay.xaml (4)
10-10: Added namespace for converters looks good.The addition of the converters namespace is appropriate for integrating the StringEqualityToVisibilityConverter needed for the display mode switching feature.
15-17: Converter resource implementation looks correct.The StringEqualityToVisibilityConverter resource is properly defined and will be used to control visibility of UI elements based on the current display mode.
115-131: Search Delay control implementation looks good.The Search Delay section is well-structured and correctly uses the custom InstalledPluginSearchDelayCombobox component. The visibility binding is properly set up to show this control only when the SearchDelay mode is selected.
133-142: ToggleSwitch visibility binding is correctly implemented.The OnOffControl now has proper visibility binding based on the selected display mode, which aligns with the PR objectives for the control swap feature.
Flow.Launcher/SettingPages/ViewModels/SettingsPanePluginsViewModel.cs (4)
17-30: SelectedDisplayMode property looks good.The property is well-implemented with proper change notification and triggers the UpdateDisplayModeFromSelection method when changed.
54-67: Boolean properties for display mode selection look good.The IsOnOffSelected, IsPrioritySelected, and IsSearchDelaySelected properties are well-implemented with proper change notification and call UpdateDisplayMode when changed.
Also applies to: 69-82, 84-97
99-111: CurrentDisplayMode property implementation is correct.This property provides the current display mode to the UI elements through bindings and handles change notification appropriately.
113-121: UpdateDisplayMode method has a potential logical issue.If multiple boolean properties are set to true (which shouldn't happen but is technically possible), the priority order is OnOff > Priority > SearchDelay. Consider adding validation to ensure only one mode is selected at a time or adding an else clause to handle invalid states.
private void UpdateDisplayMode() { if (IsOnOffSelected) CurrentDisplayMode = "OnOff"; else if (IsPrioritySelected) CurrentDisplayMode = "Priority"; else if (IsSearchDelaySelected) CurrentDisplayMode = "SearchDelay"; + else + // Set a default or log a warning about invalid state + CurrentDisplayMode = "OnOff"; }
Flow.Launcher/Resources/Controls/InstalledPluginSearchDelayCombobox.xaml.cs
Outdated
Show resolved
Hide resolved
Flow.Launcher/Resources/Controls/InstalledPluginSearchDelayCombobox.xaml.cs
Outdated
Show resolved
Hide resolved
Flow.Launcher/Resources/Controls/InstalledPluginSearchDelayCombobox.xaml.cs
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
Jack251970
added
kind/ui
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Flow.Launcher/Resources/Controls/InstalledPluginDisplay.xaml (1)
78-97: Good implementation of the Search Delay control.The SearchDelayControl successfully restores the "Search Delay" field as mentioned in the PR objectives, with:
- Clear visibility binding to IsSearchDelaySelected
- Appropriate tooltip and labeling
- NumberBox with proper minimum/maximum constraints
- Two-way binding to PluginSearchDelayTime
Consider adding the ValueChanged event handler to this NumberBox as well to handle potential NaN values consistently across all number inputs.
SpinButtonPlacementMode="Inline" ToolTip="{DynamicResource searchDelayToolTip}" - Value="{Binding PluginSearchDelayTime, Mode=TwoWay}" /> + Value="{Binding PluginSearchDelayTime, Mode=TwoWay}" + ValueChanged="NumberBox_OnValueChanged" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Flow.Launcher/Resources/Controls/InstalledPluginDisplay.xaml(4 hunks)Flow.Launcher/Resources/Controls/InstalledPluginDisplay.xaml.cs(1 hunks)
🔇 Additional comments (7)
Flow.Launcher/Resources/Controls/InstalledPluginDisplay.xaml (5)
5-6: Good addition of the converters namespace.Adding the converters namespace enables proper visibility management for the new display mode feature.
15-17: LGTM: Well-structured resource addition.The BooleanToVisibilityConverter is appropriately added to the UserControl.Resources section, making it available throughout the control.
56-76: Nicely implemented Priority control with proper binding.The PriorityControl implementation includes:
- Correct visibility binding using the converter
- Proper labeling with tooltip
- NumberBox with appropriate constraints and two-way binding to Priority
- Event handler for value changes
This effectively implements the requested priority control swapping feature.
99-106: LGTM: Toggle switch improvements and visibility control.The ToggleSwitch has been enhanced with:
- An explicit name for better code organization
- Appropriate visibility binding for display mode switching
- Good comment explaining the ordering relationship with other controls
This supports the new control swapping feature while maintaining existing functionality.
135-135: Fixed: BottomPart2 ContentControl is now properly uncommented.The ContentControl for BottomPart2 has been restored, resolving the issue identified in the previous review.
Flow.Launcher/Resources/Controls/InstalledPluginDisplay.xaml.cs (2)
1-2: Good addition of the required namespace.The ModernWpf.Controls namespace is correctly imported to support the NumberBox control.
12-18: LGTM: Well-implemented validation for NumberBox inputs.The NumberBox_OnValueChanged method provides essential validation by:
- Checking for NaN values
- Setting a default value of 0 when an invalid input is detected
This prevents UI errors from invalid user inputs and ensures consistent behavior.
- Adjust Numberbox Style - Add small change value(10) - Adjust strings
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Flow.Launcher/Resources/CustomControlTemplate.xaml (1)
3800-3807: Refined button positioning in different dialog statesThe updates to the button visibility states (PrimaryVisible, SecondaryVisible, CloseVisible) include:
- Repositioned buttons to use columns 1-2 instead of 0-2
- Removed margins between buttons for a cleaner look
- Updated grid positioning for a more balanced layout
Note that in the CloseVisible state (line 3849), the CloseButton is set to hidden, which seems counterintuitive. Consider reviewing this logic to ensure the button is properly displayed when needed.
- <ObjectAnimationUsingKeyFrames Storyboard.TargetName="CloseButton" Storyboard.TargetProperty="Visibility"> - <DiscreteObjectKeyFrame KeyTime="0:0:0" Value="{x:Static Visibility.Collapsed}" /> - </ObjectAnimationUsingKeyFrames> + <!-- Ensure CloseButton is visible in CloseVisible state -->Also applies to: 3819-3826, 3838-3851
📜 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/Languages/en.xaml(3 hunks)Flow.Launcher/Resources/Controls/InstalledPluginDisplay.xaml(4 hunks)Flow.Launcher/Resources/CustomControlTemplate.xaml(9 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs(3 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPanePluginsViewModel.cs(2 hunks)Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- Flow.Launcher.Infrastructure/UserSettings/Settings.cs
- Flow.Launcher/Languages/en.xaml
- Flow.Launcher/Resources/Controls/InstalledPluginDisplay.xaml
🧰 Additional context used
🧬 Code Definitions (1)
Flow.Launcher/SettingPages/ViewModels/SettingsPanePluginsViewModel.cs (2)
Flow.Launcher/SettingPages/ViewModels/DropdownDataGeneric.cs (2)
DropdownDataGeneric(7-35)UpdateLabels(28-34)Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (1)
UpdateEnumDropdownLocalizations(163-169)
🔇 Additional comments (18)
Flow.Launcher/SettingPages/ViewModels/SettingsPanePluginsViewModel.cs (7)
22-26: LGTM: DisplayModeData and DisplayModes implementationThe implementation of DisplayModeData as a subclass of DropdownDataGeneric and the initialization of DisplayModes using the GetValues method correctly leverages the existing dropdown framework. This approach ensures consistency with other dropdowns in the application.
27-40: LGTM: SelectedDisplayMode property with change notificationThe SelectedDisplayMode property is properly implemented with change tracking and calls UpdateDisplayModeFromSelection() when the value changes. This ensures that the UI stays in sync with the selected display mode.
42-82: LGTM: Boolean tracking properties for each display modeThe three boolean properties (IsOnOffSelected, IsPrioritySelected, IsSearchDelaySelected) are correctly implemented with proper change notification, which will enable binding to visibility properties in the XAML.
114-156: Content dialog structure follows best practicesThe OpenHelperAsync method creates a well-structured dialog with clear headings and explanations for Priority and Search Delay features. The implementation uses the ContentDialog pattern correctly.
One suggestion to consider - the dialog appears to use hard-coded strings retrieved from resources. For localization flexibility, you might want to:
- Text = (string)Application.Current.Resources["priority"], + Text = App.API.GetTranslation("priority"),This would ensure consistency with the localization approach used elsewhere.
158-161: LGTM: UpdateEnumDropdownLocalizations methodThis method ensures that display mode labels are updated when the language changes, maintaining UI consistency.
163-183: LGTM: UpdateDisplayModeFromSelection implementationThe method properly handles the mutual exclusivity of display modes, ensuring that only one mode is active at a time. The switch statement provides a clean approach to handling the different display modes.
186-191: LGTM: DisplayMode enum definitionThe DisplayMode enum is appropriately defined with the three modes mentioned in the PR objectives (OnOff, Priority, and SearchDelay). This enables the combo box at the top for switching controls as requested.
Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml (1)
199-226: LGTM: Search delay configuration UI componentsThe ExCard implementation for search delay settings follows the established pattern in the codebase. The structure with a toggle switch and nested card for detailed settings provides a clean, intuitive UI.
The NumberBox for search delay time has appropriate constraints:
- Minimum: 0
- Maximum: 1000
- SmallChange: 10
- Validation mode: InvalidInputOverwritten
This implementation successfully restores the "Search Delay" field on the General page as mentioned in the PR objectives.
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (3)
148-160: LGTM: SearchDelayTimeValue property implementationThe SearchDelayTimeValue property correctly updates the Settings.SearchDelayTime value and triggers property change notifications for both itself and the display property. This ensures the UI stays in sync with changes to the delay value.
161-161: LGTM: SearchDelayTimeDisplay propertyThe computed property provides a formatted string with "ms" suffix, making the value more user-friendly in the UI where needed.
163-169: LGTM: Updated UpdateEnumDropdownLocalizations methodThe method has been updated to remove any references to the previous SearchDelayTimes dropdown, reflecting the shift from an enum-based approach to a direct integer value for search delay time.
Flow.Launcher/Resources/CustomControlTemplate.xaml (7)
1506-1506: LGTM: Updating placeholder color for consistencyThis change sets the placeholder text to use the "CustomContextDisabled" color when the placeholder foreground is null, which provides better accessibility in disabled state.
2692-2705: Improved TextBox styling with nested bordersThe changes implement a more refined TextBox appearance for the NumberBox component by using nested borders with different styling properties. This creates a visual hierarchy with an outer border (CustomTextBoxOutline) and an inner border (CustomTextBoxInline), providing a more polished look that aligns with the plugin page control enhancements described in the PR.
2793-2804: Enhanced focus state styling for TextBoxThe focus state styling has been improved by applying different visual treatments to the inner and outer borders. When focused, the inner border now has a 2px bottom border while the outer border maintains its appearance, creating a subtle but effective focus indicator.
3538-3539: Updated ContentDialog color resourcesThe ContentDialog's foreground and background colors now use more specific dynamic resources (
PopupTextColorandPopupBGColor) rather than the generic content dialog resources. This change provides better consistency with the new popup styling throughout the application.
3586-3586: Removed padding from dialog content areaSetting the padding to 0 instead of using the ContentDialogPadding resource removes spacing around the content area, allowing for more control over the internal layout of dialog content. This aligns with the PR objective of enhancing the plugin page's Dialog controls.
3590-3590: Fixed height for button row in ContentDialogChanged the button row height from Auto to a fixed 80px, ensuring consistent spacing for the button area regardless of content. This creates a more predictable layout for the Help dialog mentioned in the PR.
3641-3687: Redesigned ContentDialog button areaThe button area in the ContentDialog has been completely redesigned with:
- Consistent horizontal padding (26px on each side)
- A dedicated background color and top border
- Rounded bottom corners (8px radius) to match the dialog shape
- Improved button positioning with a 4-column grid for more flexible layouts
These changes improve the visual appeal and usability of the dialog buttons, supporting the PR's goal of adding a help button that opens an information dialog.
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.
Copilot reviewed 13 out of 22 changed files in this pull request and generated no comments.
Files not reviewed (9)
- Flow.Launcher/Languages/en.xaml: Language not supported
- Flow.Launcher/PriorityChangeWindow.xaml: Language not supported
- Flow.Launcher/Resources/Controls/InstalledPluginDisplay.xaml: Language not supported
- Flow.Launcher/Resources/Controls/InstalledPluginSearchDelay.xaml: Language not supported
- Flow.Launcher/Resources/CustomControlTemplate.xaml: Language not supported
- Flow.Launcher/Resources/Dark.xaml: Language not supported
- Flow.Launcher/SearchDelayTimeWindow.xaml: Language not supported
- Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml: Language not supported
- Flow.Launcher/SettingPages/Views/SettingsPanePlugins.xaml: Language not supported
Comments suppressed due to low confidence (1)
Flow.Launcher/ViewModel/MainViewModel.cs:1260
- The removal of the mapping from enum values to milliseconds now assumes that both plugin.Metadata.SearchDelayTime and Settings.SearchDelayTime are stored as millisecond values. Please verify that these values are always provided in ms to avoid unintended delays.
var searchDelayTime = plugin.Metadata.SearchDelayTime ?? Settings.SearchDelayTime;
Jack251970
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Flow.Launcher/ViewModel/PluginViewModel.cs (1)
90-118: Consider notifying property changes in setters.Neither the Priority nor PluginSearchDelayTime property setters call OnPropertyChanged. While two-way bindings may update correctly, explicit notifications would ensure UI consistency.
public int Priority { get => PluginPair.Metadata.Priority; set { PluginPair.Metadata.Priority = value; PluginSettingsObject.Priority = value; + OnPropertyChanged(); } } public double PluginSearchDelayTime { get => PluginPair.Metadata.SearchDelayTime == null ? double.NaN : PluginPair.Metadata.SearchDelayTime.Value; set { if (double.IsNaN(value)) { PluginPair.Metadata.SearchDelayTime = null; PluginSettingsObject.SearchDelayTime = null; } else { PluginPair.Metadata.SearchDelayTime = (int)value; PluginSettingsObject.SearchDelayTime = (int)value; } + OnPropertyChanged(); + OnPropertyChanged(nameof(SearchDelayTimeText)); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Flow.Launcher/Languages/en.xaml(3 hunks)Flow.Launcher/Resources/Controls/InstalledPluginDisplay.xaml(4 hunks)Flow.Launcher/ViewModel/PluginViewModel.cs(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Flow.Launcher/Languages/en.xaml
🧰 Additional context used
🧬 Code Definitions (1)
Flow.Launcher/ViewModel/PluginViewModel.cs (2)
Flow.Launcher.Infrastructure/UserSettings/PluginSettings.cs (3)
Plugin(90-97)Plugin(99-103)Plugin(106-131)Flow.Launcher.Infrastructure/UserSettings/Settings.cs (1)
Settings(16-437)
🔇 Additional comments (9)
Flow.Launcher/Resources/Controls/InstalledPluginDisplay.xaml (5)
6-6: LGTM: Namespace addition for converters.The addition of the converters namespace is appropriate as it's needed for the BooleanToVisibilityConverter used throughout the file.
15-17: LGTM: UserControl resources setup.Adding the BooleanToVisibilityConverter as a resource at the UserControl level is a clean approach and follows standard WPF practices.
56-76: LGTM: Priority control implementation.The PriorityControl implementation is well-structured with:
- Appropriate visibility binding to the display mode
- Clear labeling with proper styling
- NumberBox with validation constraints and two-way binding
- Event handler for value changes
This addresses the previous review comment about missing data binding for the Priority NumberBox.
104-110: LGTM: Toggle switch control with display mode.The toggle switch has been properly named and its visibility is correctly bound to the IsOnOffSelected property. The comment above it explains the positioning logic.
139-139: LGTM: ContentControl for BottomPart2 has been uncommented.This change properly restores the ContentControl for BottomPart2, addressing the previous review comment about a mismatch between the XAML and ViewModel.
Flow.Launcher/ViewModel/PluginViewModel.cs (4)
18-19: LGTM: Settings service initialization.Properly retrieving the Settings service from the IoC container ensures consistent access to application settings.
90-98: LGTM: Priority property implementation.The Priority property implementation is good - it reads from and writes to both the runtime metadata and settings objects, maintaining consistency.
127-127: LGTM: Updated BottomPart2 initialization.The BottomPart2 property now correctly initializes with InstalledPluginDisplayBottomData, matching the uncommented ContentControl in the XAML file.
155-156: LGTM: Added properties for search delay settings.These new properties provide the SearchDelayControl with access to global settings, allowing it to show whether search delay is enabled and the default delay value.
This comment has been minimized.
This comment has been minimized.
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
What's the PR
Todo List
To Jack
memo
I spent a long time debating whether to implement this feature using a ComboBox or RadioButtons. Since this kind of layout isn’t commonly used, I even considered dropping the idea altogether. However, I prioritized the convenience of being able to quickly switch between multiple options at a glance. In the end, I chose the ComboBox for its compact size and aesthetic appeal.
For the Search Delay setting, I originally considered labeling the options as long, short, and normal. But since it's a setting users typically don’t need to adjust, and numerical values don’t require translation, I concluded that clearly explaining what each value does would be sufficient. (It also helped that ptr uses this same approach.)
I considered using an InfoBox to show guidance, but ultimately decided against it. Showing it only in certain situations caused the list to shift downward, and once users understand how it works, the extra information becomes unnecessary. Instead, I used a content dialog. Rather than creating a new window in XAML as was done in previous implementations, I handled everything in code. I also adjusted the style accordingly to match. I thought about adding an image, but in the end decided it would be better to simply provide a detailed explanation.