Merge branch 'master' into stabilize_tests_pt2

src/DynamoCoreWpf/DynamoCoreWpf.csproj

@@ -432,6 +432,7 @@
     <Compile Include="ViewModels\Preview\ConnectorAnchorViewModel.cs" />
     <Compile Include="ViewModels\Preview\ConnectorContextMenuViewModel.cs" />
     <Compile Include="ViewModels\RunSettingsViewModel.cs" />
+    <Compile Include="Utilities\ActionDebouncer.cs" />
     <Compile Include="ViewModels\Search\BrowserInternalElementViewModel.cs" />
     <Compile Include="ViewModels\Search\BrowserItemViewModel.cs" />
     <Compile Include="ViewModels\Search\NodeAutoCompleteSearchViewModel.cs" />

src/DynamoCoreWpf/Utilities/ActionDebouncer.cs

@@ -0,0 +1,61 @@
+using Dynamo.Logging;
+using System;
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace Dynamo.Wpf.Utilities
+{
+    /// <summary>
+    /// The ActionDebouncer class offers a means to reduce the number of UI notifications for a specified time.
+    /// It is meant to be used in UI elements where too many UI updates can cause perfomance issues.
+    /// </summary>
+    internal class ActionDebouncer(ILogger logger) : IDisposable
+    {
+        private readonly ILogger logger = logger;
+        private CancellationTokenSource cts;
+
+        public void Cancel()
+        {
+            if (cts != null)
+            {
+                cts.Cancel();
+                cts.Dispose();
+                cts = null;
+            }
+        }
+
+        /// <summary>
+        /// Delays the "action" for a "timeout" number of milliseconds
+        /// The input Action will run on same syncronization context as the Debounce method call.
+        /// </summary>
+        /// <param name="timeout">Number of milliseconds to wait</param>
+        /// <param name="action">The action to execute after the timeout runs out.</param>
+        /// <returns>A task that finishes when the deboucing is cancelled or the input action has completed (successfully or not). Should be discarded in most scenarios.</returns>
+        public void Debounce(int timeout, Action action)
+        {
+            Cancel();
+            cts = new CancellationTokenSource();
+
+            Task.Delay(timeout, cts.Token).ContinueWith((t) =>
+            {
+                try
+                {
+                    if (t.Status == TaskStatus.RanToCompletion)
+                    {
+                        action();
+                    }
+                }
+                catch (Exception ex)
+                {
+                    logger?.Log("Failed to run debounce action with the following error:");
+                    logger?.Log(ex.ToString());
+                }
+            }, TaskScheduler.FromCurrentSynchronizationContext());
+        }
+
+        public void Dispose()
+        {
+            Cancel();
+        }
+    }
+}

src/DynamoCoreWpf/ViewModels/Search/SearchViewModel.cs

@@ -18,7 +18,9 @@
 using Dynamo.UI;
 using Dynamo.Utilities;
 using Dynamo.Wpf.Services;
+using Dynamo.Wpf.Utilities;
 using Dynamo.Wpf.ViewModels;
+using DynamoUtilities;
 
 namespace Dynamo.ViewModels
 {
@@ -73,7 +75,11 @@ public bool BrowserVisibility
             set { browserVisibility = value; RaisePropertyChanged("BrowserVisibility"); }
         }
 
-        private string searchText;
+        internal int searchDelayTimeout = 150;
+        // Feature flags activated debouncer for the search UI.
+        internal ActionDebouncer searchDebouncer = null;
+
+        private string searchText = string.Empty;
         /// <summary>
         ///     SearchText property
         /// </summary>
@@ -86,10 +92,28 @@ public string SearchText
             set
             {
                 searchText = value;
-                OnSearchTextChanged(this, EventArgs.Empty);
+
                 RaisePropertyChanged("SearchText");
                 RaisePropertyChanged("BrowserRootCategories");
                 RaisePropertyChanged("CurrentMode");
+
+                // The searchText is set multiple times before the control becomes visible and interactable.
+                // To prevent any debounces from triggering at some unexpected point before or after the control
+                // becomes visible, this flag is only set once the searchText value is set by the user
+                // (unless it somehow gets set somewhere else)
+                //
+                // pinzart: The search text is set multiple times with an empty value. Seems sufficient to only use the debouncer
+                // if we get a non-empty value.
+                if (!string.IsNullOrEmpty(searchText) && searchDebouncer != null)
+                {
+                    searchDebouncer.Debounce(searchDelayTimeout, () => OnSearchTextChanged(this, EventArgs.Empty));
+                }
+                else
+                {
+                    // Make sure any previously scheduled debounces are cancelled
+                    searchDebouncer?.Cancel();
+                    OnSearchTextChanged(this, EventArgs.Empty);
+                }
             }
         }
 
@@ -374,9 +398,19 @@ internal SearchViewModel(DynamoViewModel dynamoViewModel)
 
             iconServices = new IconServices(pathManager);
 
+            DynamoFeatureFlagsManager.FlagsRetrieved += TryInitializeDebouncer;
+
             InitializeCore();
         }
 
+        private void TryInitializeDebouncer()
+        {
+            if (DynamoModel.FeatureFlags?.CheckFeatureFlag("searchbar_debounce", false) ?? false)
+            {
+                searchDebouncer ??= new ActionDebouncer(dynamoViewModel?.Model?.Logger);
+            }
+        }
+
         // Just for tests. Please refer to LibraryTests.cs
         internal SearchViewModel(NodeSearchModel model)
         {
@@ -407,6 +441,9 @@ public override void Dispose()
             Model.EntryUpdated -= UpdateEntry;
             Model.EntryRemoved -= RemoveEntry;
 
+            searchDebouncer?.Dispose();
+            DynamoFeatureFlagsManager.FlagsRetrieved -= TryInitializeDebouncer;
+
             base.Dispose();
         }
 
@@ -434,6 +471,9 @@ private void InitializeCore()
 
             DefineFullCategoryNames(LibraryRootCategories, "");
             InsertClassesIntoTree(LibraryRootCategories);
+
+            // If feature flags are already cached, try to initialize the debouncer
+            TryInitializeDebouncer();
         }
 
         private void AddEntry(NodeSearchElement entry)

src/Tools/DynamoFeatureFlags/FeatureFlagsClient.cs

@@ -85,7 +85,9 @@ internal FeatureFlagsClient(string userkey, string mobileKey = null, bool testMo
                 AllFlags = LdValue.ObjectFrom(new Dictionary<string,LdValue> { { "TestFlag1",LdValue.Of(true) },
                     { "TestFlag2", LdValue.Of("I am a string") },
                     //in tests we want instancing on so we can test it.
-                    { "graphics-primitive-instancing", LdValue.Of(true) } });
+                    { "graphics-primitive-instancing", LdValue.Of(true) },
+                    //in tests we want search debouncing on so we can test it.
+                    { "searchbar_debounce", LdValue.Of(true) } });
                 return;
             }
 

test/DynamoCoreWpfTests/CoreUITests.cs

@@ -26,6 +26,7 @@
 using Dynamo.Utilities;
 using Dynamo.ViewModels;
 using Dynamo.Views;
+using Dynamo.Wpf.Utilities;
 using DynamoCoreWpfTests.Utility;
 using Moq;
 using Moq.Protected;
@@ -968,12 +969,203 @@ public void InCanvasSearchTextChangeTriggersOneSearchCommand()
             int count = 0;
             (searchControl.DataContext as SearchViewModel).SearchCommand = new Dynamo.UI.Commands.DelegateCommand((object _) => { count++; });
             searchControl.SearchTextBox.Text = "dsfdf";
-            DispatcherUtil.DoEvents();
+            DispatcherUtil.DoEventsLoop(() => count == 1);
 
             Assert.IsTrue(currentWs.InCanvasSearchBar.IsOpen);
             Assert.AreEqual(count, 1);
         }
 
+        [Test]
+        [Category("UnitTests")]
+        public void InCanvasSearchTextChangeTriggersOneSearchCommandWithDebounce()
+        {
+            var currentWs = View.ChildOfType<WorkspaceView>();
+
+            // open context menu
+            RightClick(currentWs.zoomBorder);
+
+            // show in-canvas search
+            ViewModel.CurrentSpaceViewModel.ShowInCanvasSearchCommand.Execute(ShowHideFlags.Show);
+
+            var searchControl = currentWs.ChildrenOfType<Popup>().Select(x => (x as Popup)?.Child as InCanvasSearchControl).Where(c => c != null).FirstOrDefault();
+            Assert.IsNotNull(searchControl);
+
+            DispatcherUtil.DoEvents();
+
+            int count = 0;
+            var vm = searchControl.DataContext as SearchViewModel;
+            Assert.IsNotNull(vm);
+            vm.SearchCommand = new Dynamo.UI.Commands.DelegateCommand((object _) => { count++; });
+
+            // run without debouncer
+            vm.searchDebouncer.Dispose();
+            vm.searchDebouncer = null; // disable the debouncer
+            searchControl.SearchTextBox.Text = "dsfdf";
+            DispatcherUtil.DoEventsLoop(() => count == 1);
+
+            Assert.IsTrue(currentWs.InCanvasSearchBar.IsOpen);
+            Assert.AreEqual(1, count, "changing the text once should cause a single update");
+
+            int currThreadId = Environment.CurrentManagedThreadId;
+            int debouncerThreadId = -1;
+            var debouncer = new ActionDebouncer(null);
+
+            int dbCount = 0;
+            debouncer.Debounce(100, () =>
+            {
+                dbCount++;
+                debouncerThreadId = Environment.CurrentManagedThreadId;
+            });
+
+            DispatcherUtil.DoEventsLoop(() => debouncerThreadId != -1);
+            Assert.AreEqual(currThreadId, debouncerThreadId);
+
+            vm.searchDebouncer = debouncer;
+            searchControl.SearchTextBox.Text = "dsfdf";
+            DispatcherUtil.DoEventsLoop(() => dbCount == 1);
+            Assert.AreEqual(1, dbCount);
+
+            // Empty strings should not hit the debounce action
+            searchControl.SearchTextBox.Text = "";
+            Assert.AreEqual(1, dbCount);
+            DispatcherUtil.DoEventsLoop(() => count == 2);
+        }
+
+        [Test]
+        [Category("UnitTests")]
+        public void InCanvasSearchTextWithDebouncer()
+        {
+            var currentWs = View.ChildOfType<WorkspaceView>();
+
+            // open context menu
+            RightClick(currentWs.zoomBorder);
+
+            // show in-canvas search
+            ViewModel.CurrentSpaceViewModel.ShowInCanvasSearchCommand.Execute(ShowHideFlags.Show);
+
+            var searchControl = currentWs.ChildrenOfType<Popup>().Select(x => (x as Popup)?.Child as InCanvasSearchControl).Where(c => c != null).FirstOrDefault();
+            Assert.IsNotNull(searchControl);
+
+            DispatcherUtil.DoEvents();
+
+            int count = 0;
+            var vm = searchControl.DataContext as SearchViewModel;
+            Assert.IsNotNull(vm);
+
+            // Check that the default debouncer is setup.
+            Assert.IsNotNull(vm.searchDebouncer);
+
+            void Vm_SearchTextChanged(object sender, EventArgs e)
+            {
+                count++;
+                throw new Exception("Failure that should be logged");
+            }
+
+            vm.searchDelayTimeout = 50;
+            vm.SearchTextChanged += Vm_SearchTextChanged;
+
+            vm.SearchText = "point";
+            DispatcherUtil.DoEventsLoop(() => count == 1);
+            Assert.AreEqual(1, count, "Search updates were sent out");
+
+
+            vm.SearchText = "point.by";
+            DispatcherUtil.DoEventsLoop(() => count == 2);
+            Assert.AreEqual(2, count, "thread sees updated count");
+
+            vm.SearchText = "abcde";
+            DispatcherUtil.DoEventsLoop(() => count == 3);
+            Assert.AreEqual(3, count, "thread sees updated count");
+
+            searchControl.SearchTextBox.Text = "";
+            DispatcherUtil.DoEventsLoop(() => currentWs.InCanvasSearchBar.IsOpen);
+
+            Assert.IsTrue(currentWs.InCanvasSearchBar.IsOpen);
+            Assert.AreEqual(4, count, "main sees updated count");
+        }
+
+        [Test]
+        [Category("UnitTests")]
+        public void InCanvasSearchTextChangeTriggersDebouncer()
+        {
+            var currentWs = View.ChildOfType<WorkspaceView>();
+
+            // open context menu
+            RightClick(currentWs.zoomBorder);
+
+            // show in-canvas search
+            ViewModel.CurrentSpaceViewModel.ShowInCanvasSearchCommand.Execute(ShowHideFlags.Show);
+
+            var searchControl = currentWs.ChildrenOfType<Popup>().Select(x => (x as Popup)?.Child as InCanvasSearchControl).Where(c => c != null).FirstOrDefault();
+            Assert.IsNotNull(searchControl);
+
+            DispatcherUtil.DoEvents();
+
+            int count = 0;
+            var vm = searchControl.DataContext as SearchViewModel;
+            Assert.IsNotNull(vm);
+            vm.SearchCommand = new Dynamo.UI.Commands.DelegateCommand((object _) => { count++; });
+
+            // prepare debounce tests
+            vm.searchDelayTimeout = 50;
+            var sleepTime = vm.searchDelayTimeout * 2;
+            Assert.NotNull(vm.searchDebouncer);
+
+            // run with debouncer
+            count = 0;
+            searchControl.SearchTextBox.Text = "dsfdfdsfdf";
+            Thread.Sleep(sleepTime);
+            DispatcherUtil.DoEventsLoop(() => count == 1);
+
+            Assert.IsTrue(currentWs.InCanvasSearchBar.IsOpen);
+            Assert.AreEqual(1, count, "changing the text once should cause a single update after timeout expires");
+
+            // multiple updates with debouncer
+            count = 0;
+            searchControl.SearchTextBox.Text = "dsfdf";
+            searchControl.SearchTextBox.Text = "dsfdfdsfdf";
+            searchControl.SearchTextBox.Text = "wer";
+            searchControl.SearchTextBox.Text = "dsfdf";
+            DispatcherUtil.DoEventsLoop(() => count == 1);
+            // Do another events loop to make sure no other debouncer actions are triggered
+            DispatcherUtil.DoEventsLoop(null, 10);
+            Assert.IsTrue(currentWs.InCanvasSearchBar.IsOpen);
+            Assert.AreEqual(1, count, "changing the text multiple times in quick succession should cause a single update once timeout expires");
+
+            // multiple updates with empty string debouncer
+            count = 0;
+            searchControl.SearchTextBox.Text = "dsfdf";
+            searchControl.SearchTextBox.Text = "";
+            searchControl.SearchTextBox.Text = "dsfdf";
+            DispatcherUtil.DoEventsLoop(() => count == 2);
+            Assert.IsTrue(currentWs.InCanvasSearchBar.IsOpen);
+            Assert.AreEqual(2, count, "changing the text to empty should not trigger the debouncer");
+
+            // test timeout expiration
+            count = 0;
+            searchControl.SearchTextBox.Text = "dsfdfdsfdf";
+            Thread.Sleep(sleepTime);
+            searchControl.SearchTextBox.Text = "dsfdf";
+            Thread.Sleep(sleepTime);
+            DispatcherUtil.DoEventsLoop(() => count == 2);
+
+            Assert.IsTrue(currentWs.InCanvasSearchBar.IsOpen);
+            Assert.AreEqual(2, count, "2 timeout expirations should cause 2 updates");
+
+            // run with debouncer, then without
+            count = 0;
+            searchControl.SearchTextBox.Text = "dsfdfdsfdf";
+            vm.searchDebouncer.Dispose();
+            vm.searchDebouncer = null; // disable debounce
+            searchControl.SearchTextBox.Text = "dsfdf";
+
+            DispatcherUtil.DoEventsLoop(() => count == 1);
+            DispatcherUtil.DoEventsLoop(null, 10);
+
+            Assert.IsTrue(currentWs.InCanvasSearchBar.IsOpen);
+            Assert.AreEqual(1, count, "the debounced update should have been cancelled by the immediate set");
+        }
+
         [Test]
         public void WarningShowsWhenSavingWithLinterWarningsOrErrors()
         {