CmdPal: Fix first-open top-level context menus for slow providers#46626
CmdPal: Fix first-open top-level context menus for slow providers#46626jiripolasek wants to merge 1 commit intomainfrom
Conversation
- Keeps the first context-menu request alive when a top-level item is selected but its out-of-proc MoreCommands are still hydrating. - Short-circuits CanOpenContextMenu when a valid synthetic primary command is already available, so items with a usable primary action can open immediately without waiting for late menu hydration.
jiripolasek
added
Product-Command Palette
There was a problem hiding this comment.
Pull request overview
Fixes CmdPal’s first-attempt context menu opening for slow / out-of-proc providers by keeping the initial open request alive until the selected item becomes context-openable, and by making a usable synthetic primary command available earlier in item initialization.
Changes:
- Add “pending context menu open” tracking in
ListPageto retry opening once the selected item’s context menu becomes available. - Update
CommandItemViewModelto allowCanOpenContextMenuto succeed immediately when a visible synthetic primary command exists (even ifMoreCommandshydration is still in progress). - Add a unit test validating that fast init creates the synthetic primary context item.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/CommandItemViewModelTests.cs | Adds coverage for fast-init creation of the synthetic primary context item. |
| src/modules/cmdpal/Microsoft.CmdPal.UI/ExtViews/ListPage.xaml.cs | Keeps first context-menu open request pending and opens once the selected item becomes context-openable. |
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandItemViewModel.cs | Enables context menu opening via a visible synthetic primary command before MoreCommands hydration completes. |
| private void CancelPendingContextMenuOpen() | ||
| { | ||
| Interlocked.Increment(ref _pendingContextMenuOpenRequestId); | ||
| _cancelPendingContextMenuOpen?.Invoke(); | ||
| _cancelPendingContextMenuOpen = null; | ||
| } |
There was a problem hiding this comment.
CancelPendingContextMenuOpen is only invoked on navigation, explicit context-cancel, or a new context-menu request. If the user changes selection while an open request is pending (e.g., slow provider never hydrates, or selection changes due to list updates), the PropertyChanged handler remains attached to the old item indefinitely. Consider canceling pending requests on selection changes (and/or when the selected item no longer matches) so the handler is detached promptly and doesn’t keep doing work for stale requests.
| ViewModel?.UpdateSelectedItemCommand.Execute(item); | ||
| RequestContextMenuOpen(item, element, pos); | ||
| e.Handled = true; |
There was a problem hiding this comment.
Both Items_RightTapped and Items_OnContextRequested are wired up in XAML and each calls UpdateSelectedItemCommand.Execute(item) and RequestContextMenuOpen(...). If these two events can fire for the same mouse right-click, this will run selection updates twice and can cancel/restart the background slow-init work in ListViewModel.SetSelectedItem, potentially delaying menu hydration. Consider consolidating to a single code path or adding a guard to ensure the selection/open logic runs only once per user action.
| // We only synthesize the primary entry when the command is already | ||
| // usable; a null/empty primary must still fall back to late | ||
| // MoreCommands-based opening. | ||
| if (string.IsNullOrEmpty(Command.Name) || commandModel is null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| _defaultCommandContextItemViewModel = new CommandContextItemViewModel(new CommandContextItem(model.Command!), PageContext) | ||
| _defaultCommandContextItemViewModel = new CommandContextItemViewModel(new CommandContextItem(commandModel), PageContext) | ||
| { |
There was a problem hiding this comment.
Creating _defaultCommandContextItemViewModel here changes the computed CanOpenContextMenu value, but this method doesn’t raise a CanOpenContextMenu notification when the synthetic primary gets created. In the fast-init path this can leave listeners that react specifically to CanOpenContextMenu (e.g., CommandBarViewModel.SelectedItemPropertyChanged) out of date. Consider raising CanOpenContextMenu alongside AllCommands when the synthetic primary is first created.
Summary of the Pull Request
This PR fixes opening of the list item context menu through right-click, which affected mainly 3rd party out-of-process extensions.
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed