Skip to content

Commit

Permalink
Revert "Search bar stuttering improvements (#15726)"
Browse files Browse the repository at this point in the history 
This reverts commit 7334501.
  • Loading branch information
@pinzart90
pinzart90 committed Jan 23, 2025
1 parent 10c145b commit 47629b3
Show file tree
Hide file tree
Showing 5 changed files with 4 additions and 300 deletions.
1 change: 0 additions & 1 deletion src/DynamoCoreWpf/DynamoCoreWpf.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,6 @@
<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" />
Expand Down
61 changes: 0 additions & 61 deletions src/DynamoCoreWpf/Utilities/ActionDebouncer.cs
View file

This file was deleted.

44 changes: 2 additions & 42 deletions src/DynamoCoreWpf/ViewModels/Search/SearchViewModel.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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;
/// <summary>
/// SearchText property
/// </summary>
Expand All @@ -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);
}
}
}

Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -441,9 +407,6 @@ public override void Dispose()
Model.EntryUpdated -= UpdateEntry;
Model.EntryRemoved -= RemoveEntry;

searchDebouncer?.Dispose();
DynamoFeatureFlagsManager.FlagsRetrieved -= TryInitializeDebouncer;

base.Dispose();
}

Expand Down Expand Up @@ -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)
Expand Down
4 changes: 1 addition & 3 deletions src/Tools/DynamoFeatureFlags/FeatureFlagsClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,7 @@ 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) },
//in tests we want search debouncing on so we can test it.
{ "searchbar_debounce", LdValue.Of(true) } });
{ "graphics-primitive-instancing", LdValue.Of(true) } });
return;
}

Expand Down
194 changes: 1 addition & 193 deletions test/DynamoCoreWpfTests/CoreUITests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<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");
Assert.AreEqual(count, 1);
}

[Test]
Expand Down

0 comments on commit 47629b3

Please sign in to comment.