Support select file & folder in quick link access add button #3738
Conversation
This comment has been minimized.
This comment has been minimized.
|
🥷 Code experts: no user but you matched threshold 10 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: To learn more about /:\ gitStream - Visit our Docs |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
📝 WalkthroughWalkthroughThe changes introduce new localized UI strings for prompting file or folder selection in the Explorer plugin and enhance the quick access link settings logic. The code now explicitly tracks and determines the type (file, folder, or volume) of access links, prompting users as needed and centralizing type determination logic. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant QuickAccessLinkSettings
participant Dialogs (File/Folder)
participant MessageBox
User->>QuickAccessLinkSettings: Click "Select Path"
alt Editing existing link
QuickAccessLinkSettings->>Dialogs: Show dialog based on existing type
else Adding new link
QuickAccessLinkSettings->>MessageBox: Prompt "Select file or folder?"
alt User selects File
QuickAccessLinkSettings->>Dialogs: Show OpenFileDialog
else User selects Folder
QuickAccessLinkSettings->>Dialogs: Show FolderBrowserDialog
end
end
Dialogs-->>QuickAccessLinkSettings: Return selected path
QuickAccessLinkSettings->>QuickAccessLinkSettings: Determine type via GetResultType(path)
QuickAccessLinkSettings->>User: Update UI with selected path and type
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 1
🔭 Outside diff range comments (1)
Plugins/Flow.Launcher.Plugin.Explorer/Views/QuickAccessLinkSettings.xaml.cs (1)
91-98: Duplicate-path check should ignore the nameCurrent check only flags duplicates when both path and name match.
Two different names pointing at the same path is still a duplicate entry from a user-perspective.-if (QuickAccessLinks.Any(x => - x.Path.Equals(SelectedPath, StringComparison.OrdinalIgnoreCase) && - x.Name.Equals(SelectedName, StringComparison.OrdinalIgnoreCase))) +if (QuickAccessLinks.Any(x => + x.Path.Equals(SelectedPath, StringComparison.OrdinalIgnoreCase)))
🧹 Nitpick comments (3)
Plugins/Flow.Launcher.Plugin.Explorer/Views/QuickAccessLinkSettings.xaml.cs (3)
148-166: Cancel option not handled
ShowMsgBoxis invoked withMessageBoxButton.YesNo, but a third state (MessageBoxResult.None/Cancel) can still be returned by some helper wrappers.
Treating anything other thanYesasFoldermay surprise users if they close the box.var result = Main.Context.API.ShowMsgBox(...); - type = result == MessageBoxResult.Yes ? ResultType.File : ResultType.Folder; + if (result == MessageBoxResult.Yes) + type = ResultType.File; + else if (result == MessageBoxResult.No) + type = ResultType.Folder; + else + return; // user aborted
168-196: Consider WPF-native pickers instead of WinFormsUsing
Microsoft.Win32.OpenFileDialogandSystem.Windows.Forms.FolderBrowserDialogtogether in a WPF window pulls in WinForms dependencies and inconsistent UI styling.
If feasible, swap to the WPF versions (or the modern Windows API picker) for a more coherent user experience.
198-225: Minor: early return simplifiesGetResultTypeCurrent nested
else if/elsechain can be flattened for readability, e.g.:if (string.IsNullOrEmpty(path)) return ResultType.Folder; if (File.Exists(path)) return ResultType.File; if (!Directory.Exists(path)) return ResultType.Folder; return string.Equals(Path.GetPathRoot(path), path, StringComparison.OrdinalIgnoreCase) ? ResultType.Volume : ResultType.Folder;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Plugins/Flow.Launcher.Plugin.Explorer/Languages/en.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Views/QuickAccessLinkSettings.xaml.cs(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (1)
Plugins/Flow.Launcher.Plugin.Explorer/Languages/en.xaml (1)
25-26: New resources look correct & consistentKeys, placeholder usage (
{0}forEnvironment.NewLine), and wording follow the existing pattern in this dictionary.
No issues from my side.
Plugins/Flow.Launcher.Plugin.Explorer/Views/QuickAccessLinkSettings.xaml.cs
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the Quick Access Link Settings dialog to let users choose between files, folders, or volumes when adding or editing links.
- Tracks a new AccessLinkType property and applies it in both add and edit flows
- Prompts the user with a Yes/No dialog to select file vs. folder, then uses OpenFileDialog/FolderBrowserDialog
- Adds a GetResultType helper to classify paths, plus corresponding translation keys
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Plugins/Flow.Launcher.Plugin.Explorer/Views/QuickAccessLinkSettings.xaml.cs | Introduced AccessLinkType, refactored OnDoneButtonClick to use it, added SelectPath_OnClick prompt logic and GetResultType helper |
| Plugins/Flow.Launcher.Plugin.Explorer/Languages/en.xaml | Added plugin_explorer_quickAccessLinkFileOrFolderTitle and plugin_explorer_quickAccessLinkFileOrFolderSubtitle translation keys |
Comments suppressed due to low confidence (5)
Plugins/Flow.Launcher.Plugin.Explorer/Views/QuickAccessLinkSettings.xaml.cs:109
- After adding a new AccessLink, the dialog isn’t closed and DialogResult isn’t set. Consider setting
DialogResult = trueand callingClose()insideAddNewAccessLinkto match the edit flow.
QuickAccessLinks.Add(newAccessLink);
Plugins/Flow.Launcher.Plugin.Explorer/Views/QuickAccessLinkSettings.xaml.cs:146
- [nitpick] The handler parameter is named
commandParameterinstead ofsender, which may break conventional WPF event wiring. Rename it toobject senderto align with the expected signature.
private void SelectPath_OnClick(object commandParameter, RoutedEventArgs e)
Plugins/Flow.Launcher.Plugin.Explorer/Views/QuickAccessLinkSettings.xaml.cs:149
- [nitpick] The local variable
typeshadows the keyword and is ambiguous. Consider renaming it toselectedTypeorpickedTypefor clarity.
ResultType type;
Plugins/Flow.Launcher.Plugin.Explorer/Views/QuickAccessLinkSettings.xaml.cs:198
- The
GetResultTypemethod introduces branching logic for files, folders, and volumes but currently lacks unit tests. Add tests covering each branch to ensure correct classification.
private static ResultType GetResultType(string path)
Plugins/Flow.Launcher.Plugin.Explorer/Views/QuickAccessLinkSettings.xaml.cs:198
- [nitpick] Consider adding XML documentation to
GetResultTypeto explain how file, folder, and volume paths are determined, including the fallback assumption.
private static ResultType GetResultType(string path)
| AddNewAccessLink(); | ||
| } | ||
|
|
||
| void AddNewAccessLink() |
There was a problem hiding this comment.
[nitpick] The AddNewAccessLink local function is nested inside OnDoneButtonClick, which can reduce readability. Consider extracting it to a private method so it's easier to test and maintain.
| void AddNewAccessLink() | |
| private void AddNewAccessLink() |
Copilot uses AI. Check for mistakes.
@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 ...
|
|
Continue in #3739. |
Since quick link access paths support volumn / file / folder type, we should support these three type in quick link access add button.
Now when we click this button, we will get a message box to choose whether to select a file or a folder:
Test: