Add new public api function BackToQueryResults for chaning the query list back to query results list #3087
Conversation
|
🥷 Code experts: no user matched threshold 10 jjw24 has most 🧠 knowledge in the files. See details
Knowledge based on git-blame:
Knowledge based on git-blame:
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 has been minimized.
This comment has been minimized.
📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request involve the addition of a new method, Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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: 1
🧹 Outside diff range and nitpick comments (3)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (1)
297-300: Enhance ReQuery method documentationWhile the added clarification is helpful, consider expanding the documentation to:
- Explain what happens if called when selected item is not from query results
- Add
@paramdocumentation for thereselectparameter explaining its behavior/// <summary> /// Reloads the query. /// This method should run when selected item is from query results. +/// Note: Behavior is undefined when called with selected items from context menu or history. /// </summary> +/// <param name="reselect">If true, selects the first result after reload. If false, attempts to maintain the current selection.</param> public void ReQuery(bool reselect = true);Flow.Launcher/ViewModel/MainViewModel.cs (2)
487-493: Add XML documentation for the public API method.Since this is a public API method, it should include XML documentation to describe its purpose, behavior, and usage scenarios.
+/// <summary> +/// Navigates back to the query results list from context menu or history. +/// This method should be called before invoking ReQuery() when the selected item +/// is from a context menu or history to ensure correct functionality. +/// </summary> public void BackToQueryResults() { if (!SelectedIsFromQueryResults()) { SelectedResults = Results; } }
487-493: Add unit tests for the new API method.Consider adding unit tests to verify:
- Navigation back to query results from context menu
- Navigation back to query results from history
- No-op when already in query results
Would you like me to help generate the unit test cases?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs(1 hunks)Flow.Launcher/PublicAPIInstance.cs(1 hunks)Flow.Launcher/ViewModel/MainViewModel.cs(2 hunks)
🔇 Additional comments (4)
Flow.Launcher/PublicAPIInstance.cs (2)
321-322: LGTM - Method placement and implementation are consistent.
The new method follows the established patterns in the codebase:
- Placed in the correct region (Public API)
- Located next to related functionality (ReQuery)
- Implementation matches the delegation pattern used by other API methods
321-322: Consider adding error handling or state validation.
The method delegates directly to _mainVM.BackToQueryResults() without any checks. Consider validating the current state or handling potential errors, especially since this method is part of the public API.
Let's check how other view model methods handle similar scenarios:
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (1)
302-306: 🛠️ Refactor suggestion
Enhance BackToQueryResults documentation and verify implementation
The new method addresses the workflow gap, but its documentation should be expanded to provide clearer guidance for plugin developers.
/// <summary>
/// Back to the query results.
/// This method should run when selected item is from context menu or history.
+/// <remarks>
+/// Use this method to return to the query results list before calling ReQuery when:
+/// - User has selected an item from a context menu
+/// - User has selected an item from history
+///
+/// Example workflow:
+/// ```csharp
+/// // When needing to refresh results after context menu action
+/// BackToQueryResults();
+/// ReQuery();
+/// ```
+/// </remarks>
/// </summary>
public void BackToQueryResults();Let's verify the implementation details:
Flow.Launcher/ViewModel/MainViewModel.cs (1)
487-493: LGTM! Well integrated with existing navigation logic.
The implementation follows established patterns and correctly integrates with the existing navigation flow between different result views.
@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 ...
|
Add new public api function for chaning the query list back to query results list.
As existing api function
ReQuery(bool reselect = true)function when selected item is from query results only, this api is for making this function work when selected item is from context menu or history.E.g.: In one context menu result, you need to invoke a function to return to query list and refresh the list. And you need to call
BackToQueryResults();before callingReQuery();.