CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 658
DYN-8564: run autocomplete search in background thread #16265
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-8564
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR moves the ML autocomplete call to a background thread and adds a loading state flag to toggle between a spinner stripe and the search results UI.
- Introduces a
ResultsLoaded
property and wraps the ML request inTask.Run
- Adds error handling and marshals UI updates via the dispatcher
- Updates the XAML to show a
LoadingAnimationStripeControl
while results are loading
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
src/DynamoCoreWpf/ViewModels/Search/NodeAutoCompleteSearchViewModel.cs | Adds ResultsLoaded , moves ML call to a background thread, and dispatches UI updates |
src/DynamoCoreWpf/Controls/NodeAutoCompleteSearchControl.xaml | Defines an inverse visibility converter and binds the spinner/control visibility to ResultsLoaded |
Comments suppressed due to low confidence (1)
src/DynamoCoreWpf/ViewModels/Search/NodeAutoCompleteSearchViewModel.cs:335
- [nitpick] The method name
UpdateUIWithRresults
contains a typo (Rresults
). Consider renaming it toUpdateUIWithResults
for clarity and consistency.
private void UpdateUIWithRresults(MLNodeAutoCompletionResponse MLresults)
src/DynamoCoreWpf/ViewModels/Search/NodeAutoCompleteSearchViewModel.cs
Outdated
Show resolved
Hide resolved
@@ -47,6 +48,17 @@ public class NodeAutoCompleteSearchViewModel : SearchViewModel | |||
private const string nodeClusterAutocompleteMLEndpoint = "MLNodeClusterAutocomplete"; | |||
internal bool IsOpen { get; set; } | |||
|
|||
private bool resultsLoaded; | |||
public bool ResultsLoaded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like we need this to be public due to the binding. Let's add a comment an d also make it obsolete.
Made a few comments. |
Purpose
Run autocomplete search in background thread and hide search box with animation stripe.
Declarations
Check these if you believe they are true
*.resx
filesRelease Notes
Run autocomplete search in background thread and hide search box with animation stripe.
Reviewers
@BogdanZavu
@johnpierson
FYIs
(FILL ME IN, Optional) Names of anyone else you wish to be notified of