Skip to content
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

Pm searchbar improvements #14553

Merged
merged 14 commits into from
Nov 10, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -304,8 +304,11 @@ public string SearchText
get { return _SearchText; }
set
{
_SearchText = value;
RaisePropertyChanged("SearchText");
if(_SearchText != value)
{
_SearchText = value;
RaisePropertyChanged("SearchText");
}
}
}

Expand Down Expand Up @@ -1059,19 +1062,24 @@ public bool TimedOut
}
}

private System.Timers.Timer aTimer;

private void StartTimer()
{
var aTimer = new System.Timers.Timer();
if(aTimer == null)
aTimer = new System.Timers.Timer();

aTimer.Elapsed += new ElapsedEventHandler(OnTimedEvent);
aTimer.Interval = MAX_LOAD_TIME;
aTimer.AutoReset = false;
aTimer.Enabled = true;
aTimer.Start();
}

private void OnTimedEvent(object sender, ElapsedEventArgs e)
{
var aTimer = (System.Timers.Timer)sender;
aTimer.Dispose();
aTimer.Stop();

// If we have managed to get all the results
// Simply dispose of the timer
Expand Down Expand Up @@ -1564,12 +1572,20 @@ public void DisableSearchTextBox()
/// <summary>
/// Clear after closing down
/// </summary>
internal void Close()
internal void PackageManagerViewClose()
reddyashish marked this conversation as resolved.
Show resolved Hide resolved
{
SearchAndUpdateResults(String.Empty); // reset the search text property
InitialResultsLoaded = false;
TimedOut = false;
}

/// <summary>
/// Remove PackageManagerSearchViewModel resources
/// </summary>
internal void Dispose()
{
TimedOut = false; // reset the timedout screen
InitialResultsLoaded = false; // reset the loading screen settings
RequestShowFileDialog -= OnRequestShowFileDialog;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still unsubscribed somewhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, no. This line never made sense in a way, because the RequestShowFileDialog is null here.

We are subscribing each individual PackageManagerSearchElementViewModel to OnRequestShowFileDialog here

. Speaking with @reddyashish these should automatically unsubscribe when the collection is destroyed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@QilongTang OnRequestShowFileDialog is subscribed to the elements in the packages list which should be disposed when the window is closed. So want to confirm if we still need to unsubscribe it.

nonHostFilter.ForEach(f => f.PropertyChanged -= filter_PropertyChanged);
dnenov marked this conversation as resolved.
Show resolved Hide resolved
nonHostFilter?.ForEach(f => f.PropertyChanged -= filter_PropertyChanged);
aTimer.Elapsed -= new ElapsedEventHandler(OnTimedEvent);
dnenov marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
3 changes: 3 additions & 0 deletions src/DynamoCoreWpf/Views/Core/DynamoView.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2014,6 +2014,7 @@ private void WindowClosed(object sender, EventArgs e)

this.Dispose();
sharedViewExtensionLoadedParams?.Dispose();
this._pkgSearchVM?.Dispose();
}

// the key press event is being intercepted before it can get to
Expand Down Expand Up @@ -2274,9 +2275,11 @@ private void DynamoViewModelRequestShowPackageManager(object s, EventArgs e)
{
packageManagerWindow.Navigate((e as OpenPackageManagerEventArgs).Tab);
}

_pkgSearchVM.RefreshAndSearchAsync();
}


internal void EnableEnvironment(bool isEnabled)
{
this.mainGrid.IsEnabled = isEnabled;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,8 @@
<!-- Search Bar -->
<local:SearchBoxControl x:Name="SearchBox"
DockPanel.Dock="Left"
Margin="0 5 0 0" />
Margin="0 5 0 0"
IsEnabled="{Binding Path=InitialResultsLoaded, UpdateSourceTrigger=PropertyChanged}" />

</DockPanel>
</Border>
Expand Down Expand Up @@ -387,18 +388,16 @@
Visibility="{Binding IsAnyFilterOn, Converter={StaticResource BoolToVisibilityCollapsedConverter}, UpdateSourceTrigger=PropertyChanged}" />
</DockPanel>


<local:PackageManagerPackageAnimationControl x:Name="loadingScreen"
<local:PackageManagerPackageAnimationControl x:Name="loadingAnimationSearchControlScreen"
Grid.Row="2"
Visibility="{Binding Path=InitialResultsLoaded, UpdateSourceTrigger=PropertyChanged,
Converter={StaticResource InverseBooleanToVisibilityCollapsedConverter}}" />
Visibility="Visible" />

<local:PackageManagerPackagesControl x:Name="packageManagerSearchPackages"
Grid.Row="2"
SearchItems="{Binding Path=SearchResults}"
Visibility="{Binding Path=InitialResultsLoaded, UpdateSourceTrigger=PropertyChanged,
Converter={StaticResource BoolToVisibilityCollapsedConverter}}" />


</Grid>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Windows.Controls;
using System.Windows.Controls.Primitives;
using System.Windows.Data;
using Dynamo.Controls;
using Dynamo.PackageManager.ViewModels;

namespace Dynamo.PackageManager.UI
Expand All @@ -23,6 +24,18 @@ public PackageManagerSearchControl()
private void InitializeContext(object sender, RoutedEventArgs e)
{
PkgSearchVM = this.DataContext as PackageManagerSearchViewModel;

dnenov marked this conversation as resolved.
Show resolved Hide resolved
if (PkgSearchVM != null)
{
// Create the binding once the DataContext is available
var binding = new Binding(nameof(PackageManagerSearchViewModel.InitialResultsLoaded))
{
Source = PkgSearchVM,
Converter = new InverseBooleanToVisibilityCollapsedConverter()
};

this.loadingAnimationSearchControlScreen.SetBinding(UIElement.VisibilityProperty, binding);
}
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,7 @@
HorizontalAlignment="Stretch"
IsEnabled="{Binding RelativeSource={RelativeSource AncestorType=UserControl}, Path=IsEnabled}"
TextChanged="SearchTextBox_OnTextChanged"
IsKeyboardFocusWithinChanged="SearchTextBox_OnKeyboardFocusWithinChanged"
Text="{Binding SearchText, UpdateSourceTrigger=PropertyChanged}"></TextBox>
dnenov marked this conversation as resolved.
Show resolved Hide resolved
IsKeyboardFocusWithinChanged="SearchTextBox_OnKeyboardFocusWithinChanged"></TextBox>

<!-- Clear Search -->

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
using Dynamo.PackageManager;
using HelixToolkit.Wpf.SharpDX;
using System;
using System.Globalization;
using System.Windows;
Expand All @@ -16,28 +14,28 @@ namespace Dynamo.PackageManager.UI
public partial class SearchBoxControl : UserControl
{
private DispatcherTimer delayTimer;
private string userSearchInput = ""; // Store user's input separately
private static int delayTime = 100; // set delay for event 100ms

dnenov marked this conversation as resolved.
Show resolved Hide resolved
// set delay for event 500ms
private static int delayTime = 500;
/// <summary>
/// Constructor
/// </summary>
public SearchBoxControl()
{
InitializeComponent();

delayTimer = new DispatcherTimer();
delayTimer.Interval = TimeSpan.FromMilliseconds(delayTime);
delayTimer.Tick += DelayTimer_Tick;
}
dnenov marked this conversation as resolved.
Show resolved Hide resolved
private static DispatcherTimer Debounce(DispatcherTimer dispatcher, TimeSpan interval, Action action)

private void DelayTimer_Tick(object sender, EventArgs e)
{
dispatcher?.Stop();
dispatcher = null;
dispatcher = new DispatcherTimer(interval, DispatcherPriority.ApplicationIdle, (s, e) =>
{
dispatcher?.Stop();
action.Invoke();
}, Dispatcher.CurrentDispatcher);
dispatcher?.Start();

return dispatcher;
delayTimer.Stop();
var textBox = this.SearchTextBox;
if (textBox == null) return;
(this.DataContext as PackageManagerSearchViewModel)?.SearchAndUpdateResults(userSearchInput);
textBox.Focus();
}

/// <summary>
Expand All @@ -48,15 +46,16 @@ private static DispatcherTimer Debounce(DispatcherTimer dispatcher, TimeSpan int
/// <exception cref="NotImplementedException"></exception>
private void SearchTextBox_OnTextChanged(object sender, TextChangedEventArgs e)
{
var textBox = sender as TextBox;
if (textBox == null) return;

Debounce(delayTimer, TimeSpan.FromMilliseconds(delayTime), () =>
{
var textBox = sender as TextBox;
if (textBox == null) return;
(this.DataContext as PackageManagerSearchViewModel)?.SearchAndUpdateResults(textBox.Text);
textBox.Focus();
});
userSearchInput = textBox.Text;

// When text changes, restart the timer
delayTimer.Stop();
delayTimer.Start();
}

private void OnSearchClearButtonClicked(object sender, System.Windows.Input.MouseButtonEventArgs e)
{
this.SearchTextBox.Clear();
Expand All @@ -76,6 +75,11 @@ private void SearchTextBox_OnKeyboardFocusWithinChanged(object sender, Dependenc
SearchTextBoxWatermark.Visibility = Visibility.Collapsed;
}
}

internal void Dispose()
{
delayTimer.Tick -= DelayTimer_Tick;
}
}

/// <summary>
Expand Down Expand Up @@ -106,7 +110,7 @@ public object Convert(object[] values, Type targetType, object parameter, Cultur
else
{
if (value != null && string.IsNullOrWhiteSpace(value.ToString()))
{
{
textVisible = true; // If the textbox has no text, we can show the Control..
}
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Dynamo.Controls;
using Dynamo.Logging;
using Dynamo.UI;
using Dynamo.Utilities;
using Dynamo.ViewModels;
using Dynamo.Wpf.Utilities;
using DynamoUtilities;
Expand Down Expand Up @@ -124,7 +125,13 @@ private void WindowClosed(object sender, EventArgs e)
{
this.packageManagerPublish.Dispose();
this.PackageManagerViewModel.PackageSearchViewModel.RequestShowFileDialog -= OnRequestShowFileDialog;
this.PackageManagerViewModel.PackageSearchViewModel.Close();
this.PackageManagerViewModel.PackageSearchViewModel.PackageManagerViewClose();

var searchBoxes = this.ChildrenOfType<SearchBoxControl>();
foreach(var searchBox in searchBoxes)
{
searchBox.Dispose();
}
}

private void SearchForPackagesButton_Click(object sender, RoutedEventArgs e)
Expand Down
27 changes: 27 additions & 0 deletions test/DynamoCoreWpfTests/PackageManager/PackageManagerUITests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1614,6 +1614,33 @@ public void PackageManagerClosesWithDynamo()
AssertWindowClosedWithDynamoView<PackageManagerView>();
}

[Test]
public void SearchBoxInactiveOnWindowOpened()
{
ViewModel.OnRequestPackageManagerDialog(null, null);

var windows = GetWindowEnumerable(View.OwnedWindows);
var packageManagerView = windows.First(x => x is PackageManagerView) as PackageManagerView;

Assert.IsNotNull(packageManagerView);

var searchBox = LogicalTreeHelper.FindLogicalNode(packageManagerView, "SearchBox") as UserControl;
Assert.IsNotNull(searchBox);
Assert.IsFalse(searchBox.IsEnabled);

packageManagerView.PackageManagerViewModel.PackageSearchViewModel.InitialResultsLoaded = true;
Assert.IsTrue(searchBox.IsEnabled);
}

[Test]
public void PackageManagerDialogDoesNotThrowExceptions()
{
Assert.DoesNotThrow(() => ViewModel.OnRequestPackageManagerDialog(null, null), "Package Manager View did not open without exceptions");

AssertWindowOwnedByDynamoView<PackageManagerView>();
}


/// <summary>
/// Asserts that the filter context menu will stay open while the user interacts with it
/// </summary>
Expand Down
Loading