Add user style selection page with CLI/GUI modes and Delete Word Backwards #3462
Conversation
This comment has been minimized.
This comment has been minimized.
onesounds
added
kind/ui
This comment has been minimized.
This comment has been minimized.
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughThis update introduces a new "Delete Word" hotkey feature to the application, allowing users to delete the last word or path segment in the query box using configurable hotkeys. Two new hotkey settings are added, with corresponding UI controls and default values. The hotkey management logic, settings, and key bindings are extended to support these new actions. Additionally, the welcome flow is refactored to use a strongly typed enum for page navigation, and a new welcome page for user type selection is introduced, along with related UI resources and converters. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MainWindow
participant MainViewModel
participant Settings
User->>MainWindow: Presses DeleteWordHotkey/DeleteWordHotkey2
MainWindow->>MainViewModel: Execute DeleteWord command
MainViewModel->>MainViewModel: Check if query is a file path
alt Path-like
MainViewModel->>MainViewModel: Remove last path segment or clear
else
MainViewModel->>MainViewModel: Delete last word (Ctrl+Backspace logic)
end
MainViewModel->>MainWindow: Update query text
sequenceDiagram
participant User
participant WelcomeWindow
participant WelcomeViewModel
User->>WelcomeWindow: Navigates welcome pages (Next/Back)
WelcomeWindow->>WelcomeViewModel: Update CurrentPage (enum)
WelcomeViewModel->>WelcomeWindow: Update UI and navigation state
WelcomeWindow->>WelcomeWindow: Show corresponding page (via PageTypeSelector)
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ 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 (
|
- Update the sorting logic to allow page names to be sorted regardless of the numbers they contain.
This comment has been minimized.
This comment has been minimized.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
Flow.Launcher/Resources/Pages/WelcomePageUserType.xaml (4)
40-56: Consider translating non-English comments.There are several non-English comments in the XAML code (appears to be Korean). For consistency with the rest of the codebase, consider translating these to English.
- <!-- 아이콘 --> + <!-- Icon --> ... - <!-- 제목 --> + <!-- Title -->
58-114: Consider translating non-English comments.For consistency with the rest of the codebase, consider translating the Korean comment to English.
- <!-- 설명 및 bullet 목록 --> + <!-- Description and bullet list -->
166-179: Consider translating non-English comment.For consistency with the rest of the codebase, consider translating the Korean comment to English.
- <!-- CLI 카드 --> + <!-- CLI card -->
181-195: Consider translating non-English comment.For consistency with the rest of the codebase, consider translating the Korean comment to English.
- <!-- Windows 카드 (Bullet 3개) --> + <!-- Windows card (3 Bullets) -->Flow.Launcher/Resources/Pages/WelcomePageUserType.xaml.cs (1)
29-36: Consider using enum-based page navigation for consistencyConsider updating the navigation to use
CurrentPage = WelcomePage.UserTypeinstead ofPageNum = 3to align with the enum-based navigation approach used in other files.Also, calling
InitializeComponent()a second time may be unnecessary and could potentially cause UI initialization issues, as it's already called in the constructor.protected override void OnNavigatedTo(NavigationEventArgs e) { Settings = Ioc.Default.GetRequiredService<Settings>(); // Sometimes the navigation is not triggered by button click, // so we need to reset the page number - Ioc.Default.GetRequiredService<WelcomeViewModel>().PageNum = 3; + Ioc.Default.GetRequiredService<WelcomeViewModel>().CurrentPage = WelcomePage.UserType; - InitializeComponent(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
Flow.Launcher.Infrastructure/UserSettings/Settings.cs(2 hunks)Flow.Launcher/Converters/NullToVisibilityConverter.cs(1 hunks)Flow.Launcher/Languages/en.xaml(2 hunks)Flow.Launcher/Resources/Controls/HotkeyDisplay.xaml(2 hunks)Flow.Launcher/Resources/Dark.xaml(2 hunks)Flow.Launcher/Resources/Light.xaml(2 hunks)Flow.Launcher/Resources/Pages/WelcomePage3.xaml.cs(1 hunks)Flow.Launcher/Resources/Pages/WelcomePage4.xaml.cs(1 hunks)Flow.Launcher/Resources/Pages/WelcomePage5.xaml.cs(1 hunks)Flow.Launcher/Resources/Pages/WelcomePageUserType.xaml(1 hunks)Flow.Launcher/Resources/Pages/WelcomePageUserType.xaml.cs(1 hunks)Flow.Launcher/ViewModel/WelcomeViewModel.cs(3 hunks)Flow.Launcher/WelcomeWindow.xaml.cs(3 hunks)
✅ Files skipped from review due to trivial changes (5)
- Flow.Launcher/Resources/Light.xaml
- Flow.Launcher/Converters/NullToVisibilityConverter.cs
- Flow.Launcher/Resources/Controls/HotkeyDisplay.xaml
- Flow.Launcher/Resources/Dark.xaml
- Flow.Launcher/Languages/en.xaml
🚧 Files skipped from review as they are similar to previous changes (1)
- Flow.Launcher.Infrastructure/UserSettings/Settings.cs
🧰 Additional context used
🧬 Code Graph Analysis (3)
Flow.Launcher/Resources/Pages/WelcomePage4.xaml.cs (1)
Flow.Launcher/ViewModel/WelcomeViewModel.cs (1)
WelcomeViewModel(16-99)
Flow.Launcher/Resources/Pages/WelcomePage3.xaml.cs (1)
Flow.Launcher/ViewModel/WelcomeViewModel.cs (1)
WelcomeViewModel(16-99)
Flow.Launcher/Resources/Pages/WelcomePage5.xaml.cs (1)
Flow.Launcher/ViewModel/WelcomeViewModel.cs (1)
WelcomeViewModel(16-99)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: gitStream.cm
🔇 Additional comments (13)
Flow.Launcher/Resources/Pages/WelcomePage4.xaml.cs (1)
15-15: Correctly updated page numbering for welcome page navigation.The page number has been updated from 4 to 5 to accommodate the new WelcomePageUserType in the welcome page sequence. This change aligns with the refactored WelcomeViewModel that now uses a strongly typed WelcomePage enum with a sequence of 6 pages.
Flow.Launcher/Resources/Pages/WelcomePageUserType.xaml (1)
1-199: Well-structured new welcome page for user style selection.This new page provides an intuitive UI for users to choose between CLI-friendly and Windows-friendly interaction styles, which directly relates to the PR objectives of changing default shortcuts and behaviors.
Flow.Launcher/Resources/Pages/WelcomePage5.xaml.cs (1)
22-22: Correctly updated page numbering for welcome page navigation.The page number has been updated from 5 to 6 to accommodate the new WelcomePageUserType in the welcome page sequence. This change aligns with the refactored WelcomeViewModel that now uses a strongly typed WelcomePage enum with a sequence of 6 pages.
Flow.Launcher/Resources/Pages/WelcomePage3.xaml.cs (1)
15-15: Correctly updated page numbering for welcome page navigation.The page number has been updated from 3 to 4 to accommodate the new WelcomePageUserType in the welcome page sequence. This change aligns with the refactored WelcomeViewModel that now uses a strongly typed WelcomePage enum with a sequence of 6 pages.
Flow.Launcher/WelcomeWindow.xaml.cs (4)
35-44: Well-implemented navigation refactoring!The refactoring from numeric indices to a strongly typed enum approach makes the navigation logic more maintainable and robust. Using
Array.IndexOfto find the current position in the sequence is a good practice.
63-76: Good use of switch expression with comprehensive case handlingThe refactored
PageTypeSelectormethod using a switch expression with theWelcomePageenum improves type safety and maintainability. The explicit handling of all enum cases with a default case for unexpected values follows best practices.
78-82: Well-designed adapter method for backward compatibilityThis overload provides a clean bridge between the old integer-based page navigation and the new enum-based approach, facilitating a smooth transition while maintaining compatibility with any existing code.
101-101: Improved initialization logicUsing the view model's current page rather than a hardcoded page number improves consistency and makes the code more maintainable. This change aligns with the overall refactoring approach.
Flow.Launcher/Resources/Pages/WelcomePageUserType.xaml.cs (1)
12-19: Clean data model implementationThe
RadioCardDataclass provides a clear and concise data structure for the radio card UI elements with appropriate properties for all the visual elements.Flow.Launcher/ViewModel/WelcomeViewModel.cs (4)
6-14: Well-structured enum with clear documentationThe
WelcomePageenum is well-defined with intuitive names and helpful comments indicating the corresponding page classes. This improves code readability and maintainability.
20-28: Logical page sequence with good encapsulationThe static
PageSequencearray encapsulates the welcome flow order, making it easy to modify the sequence in the future without changing navigation logic. The inclusion of the new UserType page in the appropriate position is well thought out.
32-46: Well-implemented bidirectional property synchronizationThe
CurrentPageproperty implementation ensures bidirectional synchronization with thePageNumproperty, maintaining backward compatibility while enabling the transition to the enum-based approach. The property change notifications are handled correctly.
91-98: Simplified and more maintainable view update logicThe refactored
UpdateViewmethod uses the page sequence index to determine button states, making it more maintainable than explicit numeric checks. This approach automatically adapts to any changes in the page sequence.
This comment has been minimized.
This comment has been minimized.
|
🥷 Code experts: Jack251970 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:
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.
onesounds
changed the title
@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 ...
|
There was a problem hiding this comment.
For bullet hiding.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
Flow.Launcher/Resources/Pages/WelcomePageUserType.xaml.cs (6)
54-56: Extract file path logic to a helper method or consider using a settings manager.Directly constructing file paths in the event handler makes the code less maintainable. Consider extracting this logic to a helper method or using a dedicated settings manager if one exists.
- var pluginDirectory = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "Plugins", "Flow.Launcher.Plugin.Explorer"); - var explorerSettingsPath = Path.Combine(pluginDirectory, "Settings.json"); + var explorerSettingsPath = GetExplorerSettingsPath(); // Add this method to the class: + private string GetExplorerSettingsPath() + { + var pluginDirectory = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "Plugins", "Flow.Launcher.Plugin.Explorer"); + return Path.Combine(pluginDirectory, "Settings.json"); + }
60-60: Consider using a strongly-typed settings model instead of Dictionary.Using
Dictionary<string, object>for settings deserialization is not type-safe and could lead to runtime errors if the settings format changes. Consider creating a strongly-typed model for Explorer plugin settings.- var explorerSettings = JsonSerializer.Deserialize<Dictionary<string, object>>(explorerSettingsJson); + var explorerSettings = JsonSerializer.Deserialize<ExplorerPluginSettings>(explorerSettingsJson); // Add this class: + private class ExplorerPluginSettings + { + public bool UseLocationAsWorkingDir { get; set; } + // Add other properties as needed + }Then update the usage:
- if (explorerSettings != null) - explorerSettings["UseLocationAsWorkingDir"] = false; + if (explorerSettings != null) + explorerSettings.UseLocationAsWorkingDir = false;
62-75: Isolate settings logic to separate methods for better readability.The switch statement handling different user types would be more maintainable if the settings logic for each type was isolated in separate methods.
- switch (data.Key) - { - case "CLI": - if (explorerSettings != null) - explorerSettings["UseLocationAsWorkingDir"] = false; - settings.AutoCompleteHotkey = "Tab"; - break; - - case "GUI": - if (explorerSettings != null) - explorerSettings["UseLocationAsWorkingDir"] = true; - settings.AutoCompleteHotkey = $"{KeyConstant.Alt} + Right"; - break; - } + switch (data.Key) + { + case "CLI": + ApplyCliSettings(explorerSettings, settings); + break; + + case "GUI": + ApplyGuiSettings(explorerSettings, settings); + break; + } // Add these methods: + private void ApplyCliSettings(Dictionary<string, object> explorerSettings, Settings settings) + { + if (explorerSettings != null) + explorerSettings["UseLocationAsWorkingDir"] = false; + settings.AutoCompleteHotkey = "Tab"; + } + + private void ApplyGuiSettings(Dictionary<string, object> explorerSettings, Settings settings) + { + if (explorerSettings != null) + explorerSettings["UseLocationAsWorkingDir"] = true; + settings.AutoCompleteHotkey = $"{KeyConstant.Alt} + Right"; + }
73-73: Use a constant for the AutoCompleteHotkey value to ensure consistency.The string
"{KeyConstant.Alt} + Right"could potentially be inconsistent with other hotkey string representations in the application. Consider using a constant or helper method to ensure consistent hotkey formatting.- settings.AutoCompleteHotkey = $"{KeyConstant.Alt} + Right"; + settings.AutoCompleteHotkey = HotkeyFormattingHelper.FormatHotkey(KeyConstant.Alt, "Right");Where
HotkeyFormattingHelperwould be a utility class ensuring consistent hotkey string formatting across the application.
77-77: Consider adding a backup mechanism when writing settings.When overwriting settings files, it's a good practice to create a backup first in case the write operation fails.
+ // Create a backup of the original settings file + if (File.Exists(explorerSettingsPath)) + { + File.Copy(explorerSettingsPath, $"{explorerSettingsPath}.bak", true); + } File.WriteAllText(explorerSettingsPath, JsonSerializer.Serialize(explorerSettings, new JsonSerializerOptions { WriteIndented = true }));
20-28: Consider adding XML documentation to the RadioCardData class.Adding XML documentation comments to the class and its properties would improve code maintainability and help other developers understand its purpose and usage.
+ /// <summary> + /// Data class for radio button cards in the user type selection UI + /// </summary> public class RadioCardData { + /// <summary> + /// Unique identifier for the radio card option (e.g., "CLI" or "GUI") + /// </summary> public string Key { get; set; } + /// <summary> + /// Icon to display for this option + /// </summary> public string Icon { get; set; } + /// <summary> + /// Primary description text + /// </summary> public string Description1 { get; set; } + /// <summary> + /// First bullet point text + /// </summary> public string Bullet1 { get; set; } + /// <summary> + /// Second bullet point text + /// </summary> public string Bullet2 { get; set; } + /// <summary> + /// Third bullet point text + /// </summary> public string Bullet3 { get; set; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Flow.Launcher/Resources/Pages/WelcomePageUserType.xaml(1 hunks)Flow.Launcher/Resources/Pages/WelcomePageUserType.xaml.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Flow.Launcher/Resources/Pages/WelcomePageUserType.xaml
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: build
| protected override void OnNavigatedTo(NavigationEventArgs e) | ||
| { | ||
| Settings = Ioc.Default.GetRequiredService<Settings>(); | ||
| // Sometimes the navigation is not triggered by button click, | ||
| // so we need to reset the page number | ||
| Ioc.Default.GetRequiredService<WelcomeViewModel>().PageNum = 3; | ||
| InitializeComponent(); | ||
| } |
There was a problem hiding this comment.
Redundant InitializeComponent call may cause UI issues.
There's a second call to InitializeComponent() in the OnNavigatedTo method, but this has already been called in the constructor. This redundant initialization could potentially cause UI elements to be recreated unexpectedly.
protected override void OnNavigatedTo(NavigationEventArgs e)
{
Settings = Ioc.Default.GetRequiredService<Settings>();
// Sometimes the navigation is not triggered by button click,
// so we need to reset the page number
Ioc.Default.GetRequiredService<WelcomeViewModel>().PageNum = 3;
- InitializeComponent();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| protected override void OnNavigatedTo(NavigationEventArgs e) | |
| { | |
| Settings = Ioc.Default.GetRequiredService<Settings>(); | |
| // Sometimes the navigation is not triggered by button click, | |
| // so we need to reset the page number | |
| Ioc.Default.GetRequiredService<WelcomeViewModel>().PageNum = 3; | |
| InitializeComponent(); | |
| } | |
| protected override void OnNavigatedTo(NavigationEventArgs e) | |
| { | |
| Settings = Ioc.Default.GetRequiredService<Settings>(); | |
| // Sometimes the navigation is not triggered by button click, | |
| // so we need to reset the page number | |
| Ioc.Default.GetRequiredService<WelcomeViewModel>().PageNum = 3; | |
| } |
| Settings = Ioc.Default.GetRequiredService<Settings>(); | ||
| // Sometimes the navigation is not triggered by button click, | ||
| // so we need to reset the page number | ||
| Ioc.Default.GetRequiredService<WelcomeViewModel>().PageNum = 3; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use enum instead of magic number for page navigation.
The hardcoded value 3 for PageNum is a magic number that makes the code less maintainable. Since the PR mentions a strongly typed enum for page navigation, you should use the enum value instead.
- Ioc.Default.GetRequiredService<WelcomeViewModel>().PageNum = 3;
+ Ioc.Default.GetRequiredService<WelcomeViewModel>().PageNum = (int)WelcomePage.UserType;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Ioc.Default.GetRequiredService<WelcomeViewModel>().PageNum = 3; | |
| Ioc.Default.GetRequiredService<WelcomeViewModel>().PageNum = (int)WelcomePage.UserType; |
| if (File.Exists(explorerSettingsPath)) | ||
| { | ||
| var explorerSettingsJson = File.ReadAllText(explorerSettingsPath); | ||
| var explorerSettings = JsonSerializer.Deserialize<Dictionary<string, object>>(explorerSettingsJson); | ||
|
|
||
| switch (data.Key) | ||
| { | ||
| case "CLI": | ||
| if (explorerSettings != null) | ||
| explorerSettings["UseLocationAsWorkingDir"] = false; | ||
| settings.AutoCompleteHotkey = "Tab"; | ||
| break; | ||
|
|
||
| case "GUI": | ||
| if (explorerSettings != null) | ||
| explorerSettings["UseLocationAsWorkingDir"] = true; | ||
| settings.AutoCompleteHotkey = $"{KeyConstant.Alt} + Right"; | ||
| break; | ||
| } | ||
|
|
||
| File.WriteAllText(explorerSettingsPath, JsonSerializer.Serialize(explorerSettings, new JsonSerializerOptions { WriteIndented = true })); | ||
| } |
There was a problem hiding this comment.
Add error handling for file operations and JSON deserialization.
The current implementation lacks proper error handling for file operations and JSON deserialization. If any of these operations fail, the application might crash or behave unexpectedly.
- if (File.Exists(explorerSettingsPath))
- {
- var explorerSettingsJson = File.ReadAllText(explorerSettingsPath);
- var explorerSettings = JsonSerializer.Deserialize<Dictionary<string, object>>(explorerSettingsJson);
+try
+{
+ if (File.Exists(explorerSettingsPath))
+ {
+ var explorerSettingsJson = File.ReadAllText(explorerSettingsPath);
+ var explorerSettings = JsonSerializer.Deserialize<Dictionary<string, object>>(explorerSettingsJson);
// ... existing switch and settings update code ...
- File.WriteAllText(explorerSettingsPath, JsonSerializer.Serialize(explorerSettings, new JsonSerializerOptions { WriteIndented = true }));
-}
+ File.WriteAllText(explorerSettingsPath, JsonSerializer.Serialize(explorerSettings, new JsonSerializerOptions { WriteIndented = true }));
+ }
+}
+catch (Exception ex)
+{
+ // Log the error and possibly show a user-friendly message
+ Log.Exception($"Failed to update Explorer plugin settings: {ex.Message}", ex);
+ // Consider notifying the user that settings might not be fully applied
+}
| @@ -44,7 +44,7 @@ | |||
| <system:String x:Key="GameMode">Game Mode</system:String> | |||
| <system:String x:Key="GameModeToolTip">Suspend the use of Hotkeys.</system:String> | |||
| <system:String x:Key="PositionReset">Position Reset</system:String> | |||
| <system:String x:Key="PositionResetToolTip">Reset search window position</system:String> | |||
| <system:String x:Key="PositionResetToolTip">Reset search window location</system:String> | |||
There was a problem hiding this comment.
Small Fix. requested by CitizenDee
| @@ -51,14 +51,14 @@ | |||
| <Border.Style> | |||
| <Style TargetType="{x:Type Border}"> | |||
| <Setter Property="Background" Value="{DynamicResource SystemControlBackgroundBaseLowBrush}" /> | |||
| <Setter Property="Padding" Value="10,5,10,5" /> | |||
| <Setter Property="Margin" Value="2,5,2,5" /> | |||
| <Setter Property="Padding" Value="10 5 10 5" /> | |||
There was a problem hiding this comment.
Minor fix: Corrected vertical alignment
Add user style selection page with CLI/GUI modes
What’s Changed
Details
Depending on the selected mode, initial settings are adjusted:
CLI Mode
GUI Mode
Why
This gives users an immediate way to tailor the interface to their workflow—whether they prefer keyboard‑driven CLI or a more explorer‑style GUI.
Testing
Delete Word Backwards (New Feature)
A new feature has been implemented to delete the previous word or path segment, similar to system
Ctrl + Backspace.Issue:
Ctrl + Backspaceis reserved by the systemSolution:
Implement a custom version of this behavior in Flow and map it to a new shortcut:
Alt + Left.Behavior:
Alt + Left: Deletes the previous word or path segment (e.g., moves up one folder level in a path).Alt + Right: Accepts autocomplete suggestion or navigates deeper into a path.Todos
Test Cases
Alt + Leftshould closely replicateCtrl + Backspace, deleting one word or path segment at a time.