-
Notifications
You must be signed in to change notification settings - Fork 7.9k
CmdPal: Fix first-open top-level context menus for slow providers #46626
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: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,8 @@ public sealed partial class ListPage : Page, | |
|
|
||
| private ListItemViewModel? _stickySelectedItem; | ||
| private ListItemViewModel? _lastPushedToVm; | ||
| private long _pendingContextMenuOpenRequestId; | ||
| private Action? _cancelPendingContextMenuOpen; | ||
|
|
||
| // A single search-text change can produce multiple ItemsUpdated calls | ||
| // dispatched as separate UI-thread callbacks. A later "soft" update | ||
|
|
@@ -124,6 +126,8 @@ protected override void OnNavigatingFrom(NavigatingCancelEventArgs e) | |
| { | ||
| base.OnNavigatingFrom(e); | ||
|
|
||
| CancelPendingContextMenuOpen(); | ||
|
|
||
| WeakReferenceMessenger.Default.Unregister<NavigateNextCommand>(this); | ||
| WeakReferenceMessenger.Default.Unregister<NavigatePreviousCommand>(this); | ||
| WeakReferenceMessenger.Default.Unregister<NavigateLeftCommand>(this); | ||
|
|
@@ -283,17 +287,7 @@ private void Items_RightTapped(object sender, RightTappedRoutedEventArgs e) | |
| ViewModel?.UpdateSelectedItemCommand.Execute(item); | ||
|
|
||
| var pos = e.GetPosition(element); | ||
|
|
||
| _ = DispatcherQueue.TryEnqueue( | ||
| () => | ||
| { | ||
| WeakReferenceMessenger.Default.Send<OpenContextMenuMessage>( | ||
| new OpenContextMenuMessage( | ||
| element, | ||
| FlyoutPlacementMode.BottomEdgeAlignedLeft, | ||
| pos, | ||
| ContextMenuFilterLocation.Top)); | ||
| }); | ||
| RequestContextMenuOpen(item, element, pos); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1014,21 +1008,14 @@ private void Items_OnContextRequested(UIElement sender, ContextRequestedEventArg | |
| pos = new(0, element.ActualHeight); | ||
| } | ||
|
|
||
| _ = DispatcherQueue.TryEnqueue( | ||
| () => | ||
| { | ||
| WeakReferenceMessenger.Default.Send<OpenContextMenuMessage>( | ||
| new OpenContextMenuMessage( | ||
| element, | ||
| FlyoutPlacementMode.BottomEdgeAlignedLeft, | ||
| pos, | ||
| ContextMenuFilterLocation.Top)); | ||
| }); | ||
| ViewModel?.UpdateSelectedItemCommand.Execute(item); | ||
| RequestContextMenuOpen(item, element, pos); | ||
| e.Handled = true; | ||
|
Comment on lines
+1011
to
1013
|
||
| } | ||
|
|
||
| private void Items_OnContextCanceled(UIElement sender, RoutedEventArgs e) | ||
| { | ||
| CancelPendingContextMenuOpen(); | ||
| _ = DispatcherQueue.TryEnqueue(() => WeakReferenceMessenger.Default.Send<CloseContextMenuMessage>()); | ||
| } | ||
|
|
||
|
|
@@ -1210,6 +1197,87 @@ private void ResetScrollToTop() | |
| scroll.ChangeView(horizontalOffset: null, verticalOffset: 0, zoomFactor: null, disableAnimation: true); | ||
| } | ||
|
|
||
| private void RequestContextMenuOpen(ListItemViewModel item, FrameworkElement element, Point pos) | ||
| { | ||
| // BEAR LOADING: Right-click can arrive before the selected item's slow | ||
| // context-menu hydration completes, especially for out-of-proc | ||
| // providers. Keep this exact open request alive until the same | ||
| // selected item becomes context-openable instead of dropping the first | ||
| // click. | ||
| CancelPendingContextMenuOpen(); | ||
| var requestId = Interlocked.Increment(ref _pendingContextMenuOpenRequestId); | ||
|
|
||
| System.ComponentModel.PropertyChangedEventHandler? onItemChanged = null; | ||
| Action? detach = null; | ||
| detach = () => | ||
| { | ||
| if (onItemChanged is not null) | ||
| { | ||
| item.PropertyChanged -= onItemChanged; | ||
| } | ||
|
|
||
| if (ReferenceEquals(_cancelPendingContextMenuOpen, detach)) | ||
| { | ||
| _cancelPendingContextMenuOpen = null; | ||
| } | ||
| }; | ||
|
|
||
| onItemChanged = (_, args) => | ||
| { | ||
| if (args.PropertyName is nameof(ListItemViewModel.CanOpenContextMenu) or nameof(ListItemViewModel.AllCommands) && | ||
| TryOpenContextMenuIfReady(item, element, pos, requestId)) | ||
| { | ||
| detach(); | ||
| } | ||
| }; | ||
|
|
||
| item.PropertyChanged += onItemChanged; | ||
| _cancelPendingContextMenuOpen = detach; | ||
|
|
||
| if (TryOpenContextMenuIfReady(item, element, pos, requestId)) | ||
| { | ||
| detach(); | ||
| } | ||
| } | ||
|
|
||
| private bool TryOpenContextMenuIfReady(ListItemViewModel item, FrameworkElement element, Point pos, long requestId) | ||
| { | ||
| // Ignore stale requests so rapid selection changes or cancelled opens | ||
| // can't resurrect an old context menu on the wrong item. | ||
| if (requestId != Volatile.Read(ref _pendingContextMenuOpenRequestId) || | ||
| !ReferenceEquals(ItemView.SelectedItem, item) || | ||
| !item.CanOpenContextMenu) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| _ = DispatcherQueue.TryEnqueue( | ||
| () => | ||
| { | ||
| if (requestId != Volatile.Read(ref _pendingContextMenuOpenRequestId) || | ||
| !ReferenceEquals(ItemView.SelectedItem, item)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| WeakReferenceMessenger.Default.Send<OpenContextMenuMessage>( | ||
| new OpenContextMenuMessage( | ||
| element, | ||
| FlyoutPlacementMode.BottomEdgeAlignedLeft, | ||
| pos, | ||
| ContextMenuFilterLocation.Top)); | ||
| }); | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| private void CancelPendingContextMenuOpen() | ||
| { | ||
| Interlocked.Increment(ref _pendingContextMenuOpenRequestId); | ||
| _cancelPendingContextMenuOpen?.Invoke(); | ||
| _cancelPendingContextMenuOpen = null; | ||
| } | ||
|
Comment on lines
+1274
to
+1279
|
||
|
|
||
| private IDisposable SuppressSelectionChangedScope() | ||
| { | ||
| _suppressSelectionChanged = true; | ||
|
|
||
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.
Creating
_defaultCommandContextItemViewModelhere changes the computedCanOpenContextMenuvalue, but this method doesn’t raise aCanOpenContextMenunotification when the synthetic primary gets created. In the fast-init path this can leave listeners that react specifically toCanOpenContextMenu(e.g., CommandBarViewModel.SelectedItemPropertyChanged) out of date. Consider raisingCanOpenContextMenualongsideAllCommandswhen the synthetic primary is first created.