diff --git a/src/DynamoCoreWpf/DynamoCoreWpf.csproj b/src/DynamoCoreWpf/DynamoCoreWpf.csproj index 483b81af869..5836dc0cac1 100644 --- a/src/DynamoCoreWpf/DynamoCoreWpf.csproj +++ b/src/DynamoCoreWpf/DynamoCoreWpf.csproj @@ -432,7 +432,6 @@ - diff --git a/src/DynamoCoreWpf/Utilities/ActionDebouncer.cs b/src/DynamoCoreWpf/Utilities/ActionDebouncer.cs deleted file mode 100644 index c1c89a3e2f6..00000000000 --- a/src/DynamoCoreWpf/Utilities/ActionDebouncer.cs +++ /dev/null @@ -1,61 +0,0 @@ -using Dynamo.Logging; -using System; -using System.Threading; -using System.Threading.Tasks; - -namespace Dynamo.Wpf.Utilities -{ - /// - /// 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. - /// - 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; - } - } - - /// - /// Delays the "action" for a "timeout" number of milliseconds - /// The input Action will run on same syncronization context as the Debounce method call. - /// - /// Number of milliseconds to wait - /// The action to execute after the timeout runs out. - /// A task that finishes when the deboucing is cancelled or the input action has completed (successfully or not). Should be discarded in most scenarios. - 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(); - } - } -} diff --git a/src/DynamoCoreWpf/ViewModels/Search/SearchViewModel.cs b/src/DynamoCoreWpf/ViewModels/Search/SearchViewModel.cs index ffe005440ec..4526b1f3afc 100644 --- a/src/DynamoCoreWpf/ViewModels/Search/SearchViewModel.cs +++ b/src/DynamoCoreWpf/ViewModels/Search/SearchViewModel.cs @@ -18,9 +18,7 @@ using Dynamo.UI; using Dynamo.Utilities; using Dynamo.Wpf.Services; -using Dynamo.Wpf.Utilities; using Dynamo.Wpf.ViewModels; -using DynamoUtilities; namespace Dynamo.ViewModels { @@ -75,11 +73,7 @@ public bool BrowserVisibility set { browserVisibility = value; RaisePropertyChanged("BrowserVisibility"); } } - internal int searchDelayTimeout = 150; - // Feature flags activated debouncer for the search UI. - internal ActionDebouncer searchDebouncer = null; - - private string searchText = string.Empty; + private string searchText; /// /// SearchText property /// @@ -92,28 +86,10 @@ 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); - } } } @@ -398,19 +374,9 @@ 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) { @@ -441,9 +407,6 @@ public override void Dispose() Model.EntryUpdated -= UpdateEntry; Model.EntryRemoved -= RemoveEntry; - searchDebouncer?.Dispose(); - DynamoFeatureFlagsManager.FlagsRetrieved -= TryInitializeDebouncer; - base.Dispose(); } @@ -471,9 +434,6 @@ private void InitializeCore() DefineFullCategoryNames(LibraryRootCategories, ""); InsertClassesIntoTree(LibraryRootCategories); - - // If feature flags are already cached, try to initialize the debouncer - TryInitializeDebouncer(); } private void AddEntry(NodeSearchElement entry) diff --git a/src/Tools/DynamoFeatureFlags/FeatureFlagsClient.cs b/src/Tools/DynamoFeatureFlags/FeatureFlagsClient.cs index 9966455d13c..1d618f5fcfb 100644 --- a/src/Tools/DynamoFeatureFlags/FeatureFlagsClient.cs +++ b/src/Tools/DynamoFeatureFlags/FeatureFlagsClient.cs @@ -85,9 +85,7 @@ internal FeatureFlagsClient(string userkey, string mobileKey = null, bool testMo AllFlags = LdValue.ObjectFrom(new Dictionary { { "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) }, - //in tests we want search debouncing on so we can test it. - { "searchbar_debounce", LdValue.Of(true) } }); + { "graphics-primitive-instancing", LdValue.Of(true) } }); return; } diff --git a/test/DynamoCoreWpfTests/CoreUITests.cs b/test/DynamoCoreWpfTests/CoreUITests.cs index 7ccd638edc6..7bdfeb586a4 100644 --- a/test/DynamoCoreWpfTests/CoreUITests.cs +++ b/test/DynamoCoreWpfTests/CoreUITests.cs @@ -26,7 +26,6 @@ using Dynamo.Utilities; using Dynamo.ViewModels; using Dynamo.Views; -using Dynamo.Wpf.Utilities; using DynamoCoreWpfTests.Utility; using Moq; using Moq.Protected; @@ -969,201 +968,10 @@ public void InCanvasSearchTextChangeTriggersOneSearchCommand() int count = 0; (searchControl.DataContext as SearchViewModel).SearchCommand = new Dynamo.UI.Commands.DelegateCommand((object _) => { count++; }); searchControl.SearchTextBox.Text = "dsfdf"; - DispatcherUtil.DoEventsLoop(() => count == 1); - - Assert.IsTrue(currentWs.InCanvasSearchBar.IsOpen); - Assert.AreEqual(count, 1); - } - - [Test] - [Category("UnitTests")] - public void InCanvasSearchTextChangeTriggersOneSearchCommandWithDebounce() - { - var currentWs = View.ChildOfType(); - - // open context menu - RightClick(currentWs.zoomBorder); - - // show in-canvas search - ViewModel.CurrentSpaceViewModel.ShowInCanvasSearchCommand.Execute(ShowHideFlags.Show); - - var searchControl = currentWs.ChildrenOfType().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(); - - // open context menu - RightClick(currentWs.zoomBorder); - - // show in-canvas search - ViewModel.CurrentSpaceViewModel.ShowInCanvasSearchCommand.Execute(ShowHideFlags.Show); - - var searchControl = currentWs.ChildrenOfType().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(); - - // open context menu - RightClick(currentWs.zoomBorder); - - // show in-canvas search - ViewModel.CurrentSpaceViewModel.ShowInCanvasSearchCommand.Execute(ShowHideFlags.Show); - - var searchControl = currentWs.ChildrenOfType().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"); + Assert.AreEqual(count, 1); } [Test]