Merge pull request #69921 from sharwell/doc-outline-updates

Document outline updates

src/VisualStudio/Core/Def/DocumentOutline/DocumentOutlineView.xaml

@@ -11,6 +11,7 @@
              xmlns:vsui="clr-namespace:Microsoft.VisualStudio.PlatformUI;assembly=Microsoft.VisualStudio.Shell.15.0"
              xmlns:platformimaging="clr-namespace:Microsoft.VisualStudio.PlatformUI;assembly=Microsoft.VisualStudio.Imaging"
              xmlns:ComponentModel="clr-namespace:System.ComponentModel;assembly=WindowsBase"
+             d:DataContext="{d:DesignInstance Type=self:DocumentOutlineViewModel}"
              mc:Ignorable="d" 
              d:DesignHeight="450" d:DesignWidth="800"
              x:Name="DocumentOutline"
@@ -94,8 +95,9 @@
                   AutomationProperties.Name="{x:Static self:DocumentOutlineStrings.Document_Outline}"
                   VirtualizingStackPanel.IsVirtualizing="True"
                   VirtualizingStackPanel.VirtualizationMode="Recycling"
-                  TreeViewItem.SourceUpdated="SymbolTreeItem_SourceUpdated"
+                  SourceUpdated="SymbolTree_SourceUpdated"
                   TreeViewItem.Selected="SymbolTreeItem_Selected"
+                  Visibility="{Binding Visibility, Mode=OneWayToSource}"
                   ItemsSource="{Binding Source={StaticResource DocumentSymbolItems}}"> <!-- Binding to our CollectionViewSource -->
             <TreeView.Resources>
                 <HierarchicalDataTemplate DataType="{x:Type self:DocumentSymbolDataViewModel}">

src/VisualStudio/Core/Def/DocumentOutline/DocumentOutlineView.xaml.cs

@@ -18,6 +18,7 @@
 using Microsoft.VisualStudio.Shell;
 using Microsoft.VisualStudio.Shell.Interop;
 using Microsoft.VisualStudio.Text;
+using Microsoft.VisualStudio.Text.Outlining;
 using InternalUtilities = Microsoft.Internal.VisualStudio.PlatformUI.Utilities;
 using IOleCommandTarget = Microsoft.VisualStudio.OLE.Interop.IOleCommandTarget;
 using OLECMD = Microsoft.VisualStudio.OLE.Interop.OLECMD;
@@ -34,6 +35,7 @@ internal sealed partial class DocumentOutlineView : UserControl, IOleCommandTarg
     {
         private readonly IThreadingContext _threadingContext;
         private readonly IGlobalOptionService _globalOptionService;
+        private readonly IOutliningManagerService _outliningManagerService;
         private readonly VsCodeWindowViewTracker _viewTracker;
         private readonly DocumentOutlineViewModel _viewModel;
         private readonly IVsToolbarTrayHost _toolbarTrayHost;
@@ -44,11 +46,13 @@ public DocumentOutlineView(
             IVsWindowSearchHostFactory windowSearchHostFactory,
             IThreadingContext threadingContext,
             IGlobalOptionService globalOptionService,
+            IOutliningManagerService outliningManagerService,
             VsCodeWindowViewTracker viewTracker,
             DocumentOutlineViewModel viewModel)
         {
             _threadingContext = threadingContext;
             _globalOptionService = globalOptionService;
+            _outliningManagerService = outliningManagerService;
             _viewTracker = viewTracker;
             _viewModel = viewModel;
 
@@ -268,11 +272,18 @@ public static void UpdateSortDescription(SortDescriptionCollection sortDescripti
         /// <summary>
         /// When a symbol node in the window is selected via the keyboard, move the caret to its position in the latest active text view.
         /// </summary>
-        private void SymbolTreeItem_SourceUpdated(object sender, DataTransferEventArgs e)
+        private void SymbolTree_SourceUpdated(object sender, DataTransferEventArgs e)
         {
             _threadingContext.ThrowIfNotOnUIThread();
 
-            if (!_viewModel.IsNavigating && e.OriginalSource is TreeViewItem { DataContext: DocumentSymbolDataViewModel symbolModel })
+            // 🐉 In practice, this event was firing in cases where the user did not manually select an item in the
+            // tree view, resulting in sporadic/unexpected navigation while editing. To filter out these cases, we
+            // include a final check that keyboard focus in currently within the selected tree view item, which implies
+            // that the keyboard focus is _not_ within the editor (and thus, we will not be interfering with a user who
+            // is editing source code). See https://github.com/dotnet/roslyn/issues/69292.
+            if (!_viewModel.IsNavigating
+                && e.OriginalSource is TreeViewItem { DataContext: DocumentSymbolDataViewModel symbolModel } item
+                && FocusHelper.IsKeyboardFocusWithin(item))
             {
                 // This is a user-initiated navigation, and we need to prevent reentrancy.  Specifically: when a user
                 // does click on an item, we do navigate, and that does move the caret. This part happens synchronously.
@@ -282,7 +293,8 @@ private void SymbolTreeItem_SourceUpdated(object sender, DataTransferEventArgs e
                 {
                     var textView = _viewTracker.GetActiveView();
                     textView.TryMoveCaretToAndEnsureVisible(
-                        symbolModel.Data.SelectionRangeSpan.TranslateTo(textView.TextSnapshot, SpanTrackingMode.EdgeInclusive).Start);
+                        symbolModel.Data.SelectionRangeSpan.TranslateTo(textView.TextSnapshot, SpanTrackingMode.EdgeInclusive).Start,
+                        _outliningManagerService);
                 }
                 finally
                 {

src/VisualStudio/Core/Def/DocumentOutline/DocumentOutlineViewModel.cs

@@ -11,6 +11,7 @@
 using System.Runtime.CompilerServices;
 using System.Threading;
 using System.Threading.Tasks;
+using System.Windows;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.Collections;
 using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
@@ -63,6 +64,7 @@ internal sealed partial class DocumentOutlineViewModel : INotifyPropertyChanged,
 
         // Mutable state.  Should only update on UI thread.
 
+        private Visibility _visibility_doNotAccessDirectly = Visibility.Visible;
         private SortOption _sortOption_doNotAccessDirectly = SortOption.Location;
         private string _searchText_doNotAccessDirectly = "";
         private ImmutableArray<DocumentSymbolDataViewModel> _documentSymbolViewModelItems_doNotAccessDirectly = ImmutableArray<DocumentSymbolDataViewModel>.Empty;
@@ -111,7 +113,7 @@ public DocumentOutlineViewModel(
             _taggerEventSource.Connect();
 
             // queue initial model update
-            _workQueue.AddWork(default(VoidResult));
+            _workQueue.AddWork();
         }
 
         public void Dispose()
@@ -129,7 +131,7 @@ private static DocumentOutlineViewState CreateEmptyViewState(ITextSnapshot curre
                 IntervalTree<DocumentSymbolDataViewModel>.Empty);
 
         private void OnEventSourceChanged(object sender, TaggerEventArgs e)
-            => _workQueue.AddWork(default(VoidResult), cancelExistingWork: true);
+            => _workQueue.AddWork(cancelExistingWork: true);
 
         /// <summary>
         /// Keeps track if we're currently in the middle of navigating or not.  For example, when the user clicks on an
@@ -173,6 +175,23 @@ private DocumentOutlineViewState LastPresentedViewState
             }
         }
 
+        /// <remarks>This property is bound to the UI. However, it is only read/written by the UI. We only act as
+        /// storage for the value. When this value is true, UI updates are deferred.</remarks>
+        public Visibility Visibility
+        {
+            get
+            {
+                _threadingContext.ThrowIfNotOnUIThread();
+                return _visibility_doNotAccessDirectly;
+            }
+
+            set
+            {
+                _threadingContext.ThrowIfNotOnUIThread();
+                _visibility_doNotAccessDirectly = value;
+            }
+        }
+
         /// <remarks>This property is bound to the UI.  However, it is only read/written by the UI.  We only act as
         /// storage for the value.  When the value changes, the sorting is actually handled by
         /// DocumentSymbolDataViewModelSorter.</remarks>
@@ -208,7 +227,7 @@ public string SearchText
                 _threadingContext.ThrowIfNotOnUIThread();
                 _searchText_doNotAccessDirectly = value;
 
-                _workQueue.AddWork(default(VoidResult), cancelExistingWork: true);
+                _workQueue.AddWork(cancelExistingWork: true);
             }
         }
 
@@ -250,6 +269,17 @@ static void ExpandOrCollapse(ImmutableArray<DocumentSymbolDataViewModel> models,
 
         private async ValueTask ComputeViewStateAsync(CancellationToken cancellationToken)
         {
+            await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken);
+            if (_isDisposed)
+                return;
+
+            if (Visibility != Visibility.Visible)
+            {
+                // Retry the update after a delay
+                _workQueue.AddWork(cancelExistingWork: true);
+                return;
+            }
+
             // Do any expensive semantic/computation work in the background.
             await TaskScheduler.Default;
             cancellationToken.ThrowIfCancellationRequested();
@@ -262,6 +292,13 @@ private async ValueTask ComputeViewStateAsync(CancellationToken cancellationToke
             if (_isDisposed)
                 return;
 
+            if (Visibility != Visibility.Visible)
+            {
+                // Retry the update after a delay
+                _workQueue.AddWork(cancelExistingWork: true);
+                return;
+            }
+
             var searchText = this.SearchText;
             var sortOption = this.SortOption;
             var lastPresentedViewState = this.LastPresentedViewState;
@@ -306,6 +343,13 @@ private async ValueTask ComputeViewStateAsync(CancellationToken cancellationToke
             if (_isDisposed)
                 return;
 
+            if (Visibility != Visibility.Visible)
+            {
+                // Retry the update after a delay
+                _workQueue.AddWork(cancelExistingWork: true);
+                return;
+            }
+
             this.LastPresentedViewState = newViewState;
             this.DocumentSymbolViewModelItems = newViewModelItems;
 

src/VisualStudio/Core/Def/LanguageService/AbstractLanguageService`2.VsCodeWindowManager.cs

@@ -27,6 +27,7 @@
 using Microsoft.VisualStudio.OLE.Interop;
 using Microsoft.VisualStudio.Shell.Interop;
 using Microsoft.VisualStudio.Text;
+using Microsoft.VisualStudio.Text.Outlining;
 using Microsoft.VisualStudio.TextManager.Interop;
 using Roslyn.Utilities;
 
@@ -264,14 +265,15 @@ private void GetOutline(out IntPtr phwnd)
                 var asyncListenerProvider = _languageService.Package.ComponentModel.GetService<IAsynchronousOperationListenerProvider>();
                 var asyncListener = asyncListenerProvider.GetListener(FeatureAttribute.DocumentOutline);
                 var editorAdaptersFactoryService = _languageService.Package.ComponentModel.GetService<IVsEditorAdaptersFactoryService>();
+                var outliningManagerService = _languageService.Package.ComponentModel.GetService<IOutliningManagerService>();
 
                 // Assert that the previous Document Outline Control and host have been freed. 
                 Contract.ThrowIfFalse(_documentOutlineView is null);
                 Contract.ThrowIfFalse(_documentOutlineViewHost is null);
 
                 var viewTracker = new VsCodeWindowViewTracker(_codeWindow, threadingContext, editorAdaptersFactoryService);
                 _documentOutlineView = new DocumentOutlineView(
-                    uiShell, windowSearchHostFactory, threadingContext, _globalOptions, viewTracker,
+                    uiShell, windowSearchHostFactory, threadingContext, _globalOptions, outliningManagerService, viewTracker,
                     new DocumentOutlineViewModel(threadingContext, viewTracker, languageServiceBroker, asyncListener));
 
                 _documentOutlineViewHost = new ElementHost