-
-
Notifications
You must be signed in to change notification settings - Fork 470
[FEATURE] Topmost automatically #4159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
|
🥷 Code experts: Jack251970, VictoriousRaptor 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: ✨ Comment |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughAdds a new boolean setting Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 2 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Flow.Launcher/ViewModel/MainViewModel.cs (1)
536-543: Consider the UX implications of automatically marking every selected result as topmost.When
AutoTopmostis enabled, every result a user selects from the query results will be automatically added to the topmost tracker for that query. This could lead to:
Rapid accumulation of topmost records: If a user frequently explores different results, many items could be marked as topmost, potentially diminishing the value of the feature.
Loss of intentional curation: The manual topmost feature (accessed via context menu, lines 1756-1797) allows users to deliberately mark important results. Auto-marking removes this intentionality.
No way to undo easily: Unlike the manual feature which has "Cancel topmost in this query" option, auto-marked results would need to be removed through the context menu, which users might not discover.
Consider adding guidance in the tooltip or implementing one of these enhancements:
- Add a frequency threshold (e.g., only mark as topmost if selected 2+ times)
- Limit the number of auto-topmost results per query
- Add a separate UI indicator or notification when a result is auto-marked as topmost
- Document this behavior more clearly in the feature description
📜 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/Languages/en.xaml(1 hunks)Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml(1 hunks)Flow.Launcher/ViewModel/MainViewModel.cs(2 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: Yusyuriv
Repo: Flow-Launcher/Flow.Launcher PR: 3057
File: Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs:0-0
Timestamp: 2024-11-03T07:40:11.014Z
Learning: In Flow Launcher, when using Windows Forms dialogs (e.g., in `JsonRPCPluginSettings.cs`), path validation is enabled by default in `OpenFileDialog` and `FolderBrowserDialog`, preventing users from selecting invalid paths, but it's possible to opt out of this validation on individual dialogs.
📚 Learning: 2025-05-01T05:38:25.673Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3500
File: Flow.Launcher/Storage/TopMostRecord.cs:145-149
Timestamp: 2025-05-01T05:38:25.673Z
Learning: For the MultipleTopMostRecord implementation in Flow.Launcher, sequence order of records in the ConcurrentBag does not need to be preserved, as confirmed by the developer. The unordered nature of ConcurrentBag is acceptable for this implementation.
Applied to files:
Flow.Launcher/ViewModel/MainViewModel.cs
📚 Learning: 2025-09-05T11:56:27.267Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Flow.Launcher/HotkeyControlDialog.xaml:6-6
Timestamp: 2025-09-05T11:56:27.267Z
Learning: In Flow.Launcher's migration to iNKORE.UI.WPF.Modern UI framework, dialog resource keys like PopuBGColor, PopupButtonAreaBGColor, PopupButtonAreaBorderColor, and PopupTextColor are provided by the iNKORE.UI.WPF.Modern framework itself, not defined locally in the codebase theme files.
Applied to files:
Flow.Launcher/Languages/en.xaml
📚 Learning: 2024-10-08T15:52:58.573Z
Learnt from: taooceros
Repo: Flow-Launcher/Flow.Launcher PR: 2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
Applied to files:
Flow.Launcher/Languages/en.xaml
📚 Learning: 2025-06-08T14:12:21.348Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3672
File: Flow.Launcher/MainWindow.xaml.cs:244-247
Timestamp: 2025-06-08T14:12:21.348Z
Learning: In Flow.Launcher, App.NotifyIcon is created before MainWindow creation, so null checks for App.NotifyIcon are not necessary when accessing it from MainWindow code.
Applied to files:
Flow.Launcher/Languages/en.xaml
📚 Learning: 2024-11-03T07:40:11.014Z
Learnt from: Yusyuriv
Repo: Flow-Launcher/Flow.Launcher PR: 3057
File: Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs:0-0
Timestamp: 2024-11-03T07:40:11.014Z
Learning: In Flow Launcher, when using Windows Forms dialogs (e.g., in `JsonRPCPluginSettings.cs`), path validation is enabled by default in `OpenFileDialog` and `FolderBrowserDialog`, preventing users from selecting invalid paths, but it's possible to opt out of this validation on individual dialogs.
Applied to files:
Flow.Launcher/Languages/en.xaml
📚 Learning: 2025-09-28T03:57:43.995Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3770
File: Flow.Launcher/Helper/HotKeyMapper.cs:0-0
Timestamp: 2025-09-28T03:57:43.995Z
Learning: In Flow.Launcher/Helper/HotKeyMapper.cs, the resource descriptions for arrow keys are intentionally swapped - Up/Down keys use "HotkeyLeftRightDesc" and Left/Right keys use "HotkeyUpDownDesc". This is documented with an inline comment and is intended behavior, not a bug.
Applied to files:
Flow.Launcher/Languages/en.xaml
🧬 Code graph analysis (1)
Flow.Launcher/ViewModel/MainViewModel.cs (3)
Flow.Launcher/Storage/QueryHistory.cs (1)
Add(36-65)Flow.Launcher.Infrastructure/UserSettings/Settings.cs (1)
Settings(16-646)Flow.Launcher/Storage/TopMostRecord.cs (3)
AddOrUpdate(99-102)AddOrUpdate(130-140)AddOrUpdate(216-246)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (2)
Flow.Launcher.Infrastructure/UserSettings/Settings.cs (1)
499-511: LGTM!The property implementation follows the established pattern used throughout the Settings class, includes proper change notification, and has a safe default value of
false.Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml (1)
99-111: LGTM!The SettingsCard implementation follows the established patterns in this file, uses appropriate spacing and styling, and is logically placed next to the related "Show At Topmost" setting. The icon reuse (glyph
) is appropriate since both features relate to topmost behavior.Note: The misleading feature description flagged in en.xaml applies to this UI card as well, but the issue originates from the resource strings, not the XAML structure itself.
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/SettingPages/Views/SettingsPaneGeneral.xaml (1)
99-111: Consider using a distinct icon for the AutoTopmostResults setting.The new setting uses the same icon glyph (
) as the ShowAtTopmost setting (Line 90). While these features are related, they serve different purposes—ShowAtTopmost controls whether the launcher window stays on top, while AutoTopmostResults appears to automatically prioritize frequently selected results. Using distinct icons would help users differentiate between these features at a glance.
📜 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/Languages/en.xaml(1 hunks)Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml(1 hunks)Flow.Launcher/ViewModel/MainViewModel.cs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- Flow.Launcher/ViewModel/MainViewModel.cs
- Flow.Launcher/Languages/en.xaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (1)
Flow.Launcher.Infrastructure/UserSettings/Settings.cs (1)
499-512: LGTM!The AutoTopmostResults property implementation correctly follows the established pattern for boolean settings in this class, including proper INotifyPropertyChanged support and a sensible default value of false.
Co-authored-by: VictoriousRaptor <[email protected]>
Issue: #3984
Example:
2025-12-14.19-05-17.mp4