Pm searchbar improvements (#14553)

dnenov · reddyashish · web-flow · commit ce4c9b32b03f · 2023-11-10T14:16:02.000-08:00
* disable search bar when loading

- disables the search bar functionality while the results are being loaded

* search box update

- removed the double-binding through textchanged AND property binding
- simplified code for timed trigger of search via the DispatcherTimer
- added value difference check to stop reapplying the same one over and over
- removed unnecessary un-subscriptions (to explain in PR)

* refreshes search result only when initializing VM

- now only does the initial (slow) search when loading the model for the first time
- since we are not destroying the model when closing the window, we should not have to refreshandsearch again and again. We are using the cached results we have loaded the first time
- timed-out timer fix
- removed reset events on windows close as no longer needed

* revert change

* disposing of handlers

* tests added

* revert RefreshAndSearchAsync changes

- RefreshAndSearchAsync will be repeated every time you open the PM window

* Update PackageManagerSearchViewModel.cs

* Update PackageManagerSearchViewModel.cs

missed space

* replaced delayed timer

- replaced delay timer with wpf native delay property (thank you!)
- removed old code

---------

Co-authored-by: reddyashish &lt;43763136+reddyashish@users.noreply.github.com&gt;
diff --git a/src/DynamoCoreWpf/ViewModels/PackageManager/PackageManagerSearchViewModel.cs b/src/DynamoCoreWpf/ViewModels/PackageManager/PackageManagerSearchViewModel.cs
@@ -304,8 +304,12 @@ public string SearchText
             get { return _SearchText; }
             set
             {
-                _SearchText = value;
-                RaisePropertyChanged("SearchText");
+                if(_SearchText != value)
+                {
+                    _SearchText = value;
+                    SearchAndUpdateResults();
+                    RaisePropertyChanged("SearchText");
+                }
             }
         }
 
@@ -1073,19 +1077,24 @@ public bool TimedOut
             }
         }
 
+        private System.Timers.Timer aTimer;
+
         private void StartTimer()
         {
-            var aTimer = new System.Timers.Timer();
-            aTimer.Elapsed += new ElapsedEventHandler(OnTimedEvent);
+            if(aTimer == null) 
+                aTimer = new System.Timers.Timer();
+
+            aTimer.Elapsed += 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
@@ -1577,12 +1586,23 @@ public void DisableSearchTextBox()
         /// <summary>
         /// Clear after closing down
         /// </summary>
-        internal void Close()
+        internal void PackageManagerViewClose()
+        {
+            SearchAndUpdateResults(String.Empty); // reset the search text property
+            InitialResultsLoaded = false;
+            TimedOut = false;
+        }
+
+        /// <summary>
+        /// Remove PackageManagerSearchViewModel resources
+        /// </summary>
+        internal void Dispose()
         {
+            nonHostFilter?.ForEach(f => f.PropertyChanged -= filter_PropertyChanged);
+            aTimer.Elapsed -= OnTimedEvent;
+
             TimedOut = false;   // reset the timedout screen 
             InitialResultsLoaded = false;   // reset the loading screen settings
-            RequestShowFileDialog -= OnRequestShowFileDialog;
-            nonHostFilter.ForEach(f => f.PropertyChanged -= filter_PropertyChanged);
 
             ClearMySearchResults();
         }
diff --git a/src/DynamoCoreWpf/Views/Core/DynamoView.xaml.cs b/src/DynamoCoreWpf/Views/Core/DynamoView.xaml.cs
@@ -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
@@ -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;
diff --git a/src/DynamoCoreWpf/Views/PackageManager/Controls/PackageManagerSearchControl.xaml b/src/DynamoCoreWpf/Views/PackageManager/Controls/PackageManagerSearchControl.xaml
@@ -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>
@@ -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>
 
diff --git a/src/DynamoCoreWpf/Views/PackageManager/Controls/PackageManagerSearchControl.xaml.cs b/src/DynamoCoreWpf/Views/PackageManager/Controls/PackageManagerSearchControl.xaml.cs
@@ -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
@@ -23,6 +24,18 @@ public PackageManagerSearchControl()
         private void InitializeContext(object sender, RoutedEventArgs e)
         {
             PkgSearchVM = this.DataContext as PackageManagerSearchViewModel;
+
+            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);
+            }
         }
 
 
diff --git a/src/DynamoCoreWpf/Views/PackageManager/Controls/SearchBoxControl.xaml b/src/DynamoCoreWpf/Views/PackageManager/Controls/SearchBoxControl.xaml
@@ -115,9 +115,8 @@
                             Cursor="IBeam"
                             HorizontalAlignment="Stretch"
                             IsEnabled="{Binding RelativeSource={RelativeSource AncestorType=UserControl}, Path=IsEnabled}"
-                            TextChanged="SearchTextBox_OnTextChanged"
-                            IsKeyboardFocusWithinChanged="SearchTextBox_OnKeyboardFocusWithinChanged"
-                            Text="{Binding SearchText, UpdateSourceTrigger=PropertyChanged}"></TextBox>
+                            Text="{Binding SearchText, Delay=100, Mode=TwoWay, UpdateSourceTrigger=PropertyChanged}"
+                            IsKeyboardFocusWithinChanged="SearchTextBox_OnKeyboardFocusWithinChanged"></TextBox>
                         
                         <!-- Clear Search -->
 
diff --git a/src/DynamoCoreWpf/Views/PackageManager/Controls/SearchBoxControl.xaml.cs b/src/DynamoCoreWpf/Views/PackageManager/Controls/SearchBoxControl.xaml.cs
@@ -1,11 +1,8 @@
-using Dynamo.PackageManager;
-using HelixToolkit.Wpf.SharpDX;
 using System;
 using System.Globalization;
 using System.Windows;
 using System.Windows.Controls;
 using System.Windows.Data;
-using System.Windows.Threading;
 
 namespace Dynamo.PackageManager.UI
 {
@@ -15,48 +12,14 @@ namespace Dynamo.PackageManager.UI
     /// </summary>
     public partial class SearchBoxControl : UserControl
     {
-        private DispatcherTimer delayTimer;
-
-        // set delay for event 500ms
-        private static int delayTime = 500;
         /// <summary>
         /// Constructor
         /// </summary>
         public SearchBoxControl()
         {
             InitializeComponent();
         }
-        private static DispatcherTimer Debounce(DispatcherTimer dispatcher, TimeSpan interval, Action action)
-        {
-            dispatcher?.Stop();
-            dispatcher = null;
-            dispatcher = new DispatcherTimer(interval,  DispatcherPriority.ApplicationIdle, (s, e) =>
-            {
-                dispatcher?.Stop();
-                action.Invoke();
-            }, Dispatcher.CurrentDispatcher);
-            dispatcher?.Start();
-
-            return dispatcher;
-        }
-
-        /// <summary>
-        /// Handles Search Box Text Changed Event
-        /// </summary>
-        /// <param name="sender"></param>
-        /// <param name="e"></param>
-        /// <exception cref="NotImplementedException"></exception>
-        private void SearchTextBox_OnTextChanged(object sender, TextChangedEventArgs e)
-        {
 
-            Debounce(delayTimer, TimeSpan.FromMilliseconds(delayTime), () =>
-            {
-                var textBox = sender as TextBox;
-                if (textBox == null) return;
-                (this.DataContext as PackageManagerSearchViewModel)?.SearchAndUpdateResults(textBox.Text);
-                textBox.Focus();
-            });
-        }
         private void OnSearchClearButtonClicked(object sender, System.Windows.Input.MouseButtonEventArgs e)
         {
             this.SearchTextBox.Clear();
@@ -78,86 +41,6 @@ private void SearchTextBox_OnKeyboardFocusWithinChanged(object sender, Dependenc
         }
     }
 
-    /// <summary>
-    /// A multivalue visibilty converter
-    /// value 0 - boolean
-    /// value 1 - text
-    /// </summary>
-    public class MultiBooleanToVisibilityConverter : IMultiValueConverter
-    {
-        public object Convert(object[] values, Type targetType, object parameter, CultureInfo culture)
-        {
-            bool focusVisible = true;
-            bool textVisible = true;
-
-            foreach (object value in values)
-            {
-                if (value is bool)
-                {
-                    if ((bool)value)
-                    {
-                        focusVisible = false;   // If the textbox has the focus, don't show the Control..
-                    }
-                    else
-                    {
-                        focusVisible = true;    // If the textbox don't have focus, we can show the Control..
-                    }
-                }
-                else
-                {
-                    if (value != null && string.IsNullOrWhiteSpace(value.ToString()))
-                    {
-                        textVisible = true; // If the textbox has no text, we can show the Control..
-                    }
-                    else
-                    {
-                        textVisible = false;    // If the textbox has some text, don't show the Control..
-                    }
-                }
-            }
-
-            // Only of both conditions are true, show the Control..
-            if (focusVisible && textVisible)
-                return Visibility.Visible;
-            else
-                return Visibility.Collapsed;
-        }
-
-        public object[] ConvertBack(object value, Type[] targetTypes, object parameter, CultureInfo culture)
-        {
-            throw new NotImplementedException();
-        }
-    }
-
-    /// <summary>
-    /// Converts null or empty string to Visibility Collapsed
-    /// </summary>
-    public class NonEmptyStringToCollapsedConverter : IValueConverter
-    {
-        enum Parameters
-        {
-            Normal, Inverted
-        }
-
-        public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
-        {
-            parameter = parameter ?? "Normal";
-
-            var boolValue = value is string s && !string.IsNullOrEmpty(s);
-            var direction = (Parameters)Enum.Parse(typeof(Parameters), (string)parameter);
-
-            if (direction == Parameters.Inverted)
-                return !boolValue ? Visibility.Visible : Visibility.Collapsed;
-
-            return boolValue ? Visibility.Visible : Visibility.Collapsed;
-        }
-
-        public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
-        {
-            return null;
-        }
-    }
-
     /// <summary>
     /// Converts null or empty string to Visibility Collapsed
     /// </summary>
diff --git a/src/DynamoCoreWpf/Views/PackageManager/PackageManagerView.xaml.cs b/src/DynamoCoreWpf/Views/PackageManager/PackageManagerView.xaml.cs
@@ -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;
@@ -124,7 +125,7 @@ private void WindowClosed(object sender, EventArgs e)
         {
             this.packageManagerPublish.Dispose();
             this.PackageManagerViewModel.PackageSearchViewModel.RequestShowFileDialog -= OnRequestShowFileDialog;
-            this.PackageManagerViewModel.PackageSearchViewModel.Close();
+            this.PackageManagerViewModel.PackageSearchViewModel.PackageManagerViewClose();
         }
 
         private void SearchForPackagesButton_Click(object sender, RoutedEventArgs e)
diff --git a/test/DynamoCoreWpfTests/PackageManager/PackageManagerUITests.cs b/test/DynamoCoreWpfTests/PackageManager/PackageManagerUITests.cs
@@ -1613,6 +1613,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>

Original file line number	Diff line number	Diff line change
`@@ -2014,6 +2014,7 @@ private void WindowClosed(object sender, EventArgs e)`
`2014`	`2014`
`2015`	`2015`	`this.Dispose();`
`2016`	`2016`	`sharedViewExtensionLoadedParams?.Dispose();`
	`2017`	`+ this._pkgSearchVM?.Dispose();`
`2017`	`2018`	`}`
`2018`	`2019`
`2019`	`2020`	`// the key press event is being intercepted before it can get to`
`@@ -2274,9 +2275,11 @@ private void DynamoViewModelRequestShowPackageManager(object s, EventArgs e)`
`2274`	`2275`	`{`
`2275`	`2276`	`packageManagerWindow.Navigate((e as OpenPackageManagerEventArgs).Tab);`
`2276`	`2277`	`}`
	`2278`	`+`
`2277`	`2279`	`_pkgSearchVM.RefreshAndSearchAsync();`
`2278`	`2280`	`}`
`2279`	`2281`
	`2282`	`+`
`2280`	`2283`	`internal void EnableEnvironment(bool isEnabled)`
`2281`	`2284`	`{`
`2282`	`2285`	`this.mainGrid.IsEnabled = isEnabled;`