Feature: History context menu with delete & info item & Refactor context menu logic - #4609
Conversation
Implemented Remove method in QueryHistory to delete entries. Added delete icon and context menu option in MainViewModel. Updated Constant.cs for icon reference. Added unit tests for single and multiple entry removal.
Remember and restore previous results view and query text when opening/closing the context menu. Add fields to track context menu state. Refactor context menu logic for regular/history results. Implement ReturnFromContextMenu() to centralize UI restoration. Update Esc command to close context menu and restore state. Improve context menu population and handle history result deletion with results refresh.
Added a "History Info" option to the history item context menu in MainViewModel. The new ContextMenuHistoryInfo method creates this entry, allowing users to view information about a history item. The menu now includes both "Delete History" and "History Info" options.
The context menu now loads when a history item is selected and the caret is at the end of the text box, in addition to query results. Made HistorySelected internal in MainViewModel.cs to enable this behavior from MainWindow.xaml.cs.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds history-entry removal, history-specific context-menu actions, and context restoration. Right Arrow opens context menus for history selections when the caret is at the query end. Escape returns from context menus before changing views or hiding the launcher. ChangesHistory context-menu flow
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant MainWindow
participant MainViewModel
participant History
MainWindow->>MainViewModel: Open context menu for selected history result
MainViewModel->>History: Remove history item
History-->>MainViewModel: Return removal count
MainViewModel->>History: Save updated history
MainViewModel-->>MainWindow: Restore source view and refresh results
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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.
Pull request overview
This PR enhances Flow Launcher’s history UX by adding a dedicated context menu for history results (with delete and info actions) and refactors context menu navigation so users can reliably return to the prior results view (query results vs. history) after interacting with the context menu.
Changes:
- Added deletion support for history entries via
History.Remove(...)and surfaced it as a “Delete” context menu item for history results. - Refactored context menu flow to preserve/restore the originating results view and target item for context menu generation.
- Added unit tests covering single-entry and “remove all matching” deletion behavior; added a
DeleteIconconstant for UI usage.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Flow.Launcher/ViewModel/MainViewModel.cs | Refactors context menu navigation, adds history context menu items (delete/info), and wires deletion + UI refresh behavior. |
| Flow.Launcher/Storage/QueryHistory.cs | Adds History.Remove(...) to support deleting individual history entries or all matching entries. |
| Flow.Launcher/MainWindow.xaml.cs | Enables opening context menu from the history view via keyboard (Right arrow). |
| Flow.Launcher.Test/QueryHistoryTest.cs | Adds unit tests for the new history removal behavior. |
| Flow.Launcher.Infrastructure/Constant.cs | Introduces DeleteIcon path for the new context menu item icon. |
Suppressed comments (1)
Flow.Launcher/ViewModel/MainViewModel.cs:1894
- The lambda parameter in the delete-history context menu action is unused. Using
_ =>makes this explicit and avoids unused-parameter warnings from analyzers.
OriginQuery = result.OriginQuery
};
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Refactored ReturnFromContextMenu to ReturnFromContextMenuAsync and updated all call sites to use await. Related methods like ChangeQueryText and UpdatePreviewAsync are now awaited. Updated context menu delete action to use an async lambda, ensuring UI and data updates complete asynchronously before proceeding.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Flow.Launcher/ViewModel/MainViewModel.cs:1355
- History context menus return before applying the context-menu query filter. As a result, typing while this menu is open always leaves both Delete and Info visible, whereas regular context-menu items are filtered below. Filter these history actions with
querybefore adding them.
ContextMenu.AddResults([
ContextMenuDeleteHistory(historyItem),
ContextMenuHistoryInfo(historyItem)
], id);
Refactored Dispatcher.BeginInvoke usage to an async lambda that checks window visibility before calling Activate() and focusing QueryTextBox. This prevents activation and focus when the window is not visible.
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
@codex Review it |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e350dc23b8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Introduce _contextMenuTargetIndex to track selected result index when opening context menu. Restore selection to the targeted result or fallback to previous index after returning. Add ResultEqual helper for result comparison. Reset context menu tracking fields after use.
|
@codex Review it |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
Flow.Launcher/ViewModel/MainViewModel.cs:446
- The new context-menu return state machine is untested. The added
QueryHistoryTestonly coversHistory.Remove, so regressions in restoring the Results/History source, query text, selected target/fallback index, and preview would not be caught. Add focused tests for returning via Esc and after deletion from both source views.
private async Task ReturnFromContextMenuAsync()
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2dc41eee79
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| private Result ContextMenuHistoryInfo(LastOpenedHistoryResult historyItem) | ||
| { | ||
| return new Result | ||
| { | ||
| Title = Settings.HistoryStyle == HistoryStyle.Query ? Localize.queryHistory() : Localize.executedHistory(), | ||
| IcoPath = Constant.HistoryIcon, | ||
| Glyph = new GlyphInfo(FontFamily: "/Resources/#Segoe Fluent Icons", Glyph: "\uE81C"), | ||
| PluginDirectory = Constant.ProgramDirectory, | ||
| Action = _ => false, | ||
| OriginQuery = historyItem.OriginQuery | ||
| }; | ||
| } |
There was a problem hiding this comment.
What is the intention of this context menu result?
It looks to do nothing to me...
Perhaps we could change the name or add comments to make this clearer
There was a problem hiding this comment.
What is the intention of this context menu result? It looks to do nothing to me... Perhaps we could change the name or add comments to make this clearer
Here I want to add the result info items as the plugin info items of other items. I know this could be little useless, but I want to make this behaviour consistent with the items from other plugins.
There was a problem hiding this comment.
Ok I see,
I added a comment above to make it's purpose a little clearer.
ToggleContextMenuCommand now toggles context menu state, replacing all usages of LoadContextMenuCommand in XAML and ViewModel. Updated key bindings and command references in MainWindow.xaml and MainViewModel.cs. Replaced LoadContextMenuAsync with ToggleContextMenuAsync to handle both opening and closing the context menu.
Replaced multiple context menu state fields in MainViewModel with a sealed ResultsQuerySelection record. This encapsulates source, query text, selected result, and index, simplifying state management and improving maintainability. Updated all related logic to use the new structure.
|
@DavidGBrett Hi, thanks for your comments! I have polished my codes to meet your suggestions and they works for me. Could you please check if they meet your ideas and work correctly? Also please help me confirm if #4626 is resolved. I cannot reproduce this issue on my side. |
Clarified that this result method exists to label the history context menu and say what the history style is.
This better matches the existing plugin context menu labels
Summary by cubic
Adds a context menu for history items with Delete and Info, and refactors the context menu flow to be fully async and toggleable. Previously, history items had no context menu and Right Arrow only opened the menu for query results; now Right Arrow works for both, options are fuzzy-filtered by your query, window activation respects visibility, and returning restores your query, preview, and selection.
Summary of changes
LoadContextMenuCommandwithToggleContextMenuCommand, so the same command opens and closes the menu.ContextData; returning restores the preview, and home results refresh after deletion.ResultsQuerySelectionrecord.History.Remove(...)to delete a single entry or all matching entries (LastOpened style) with storage persisted, a delete icon asset withConstant.DeleteIcon, and title-case localization strings for the history labels.Flow.Launcher.Test/QueryHistoryTest.csfor single-entry and multi-entry removal across matching results.Release Note
You can open a menu on history items to delete entries or view info, with options filtered by your query and your previous view restored after closing.
Written for commit bed0e19. Summary will update on new commits.
TEST