Add search functionality to Node Autocomplete window. #11192
Conversation
|
@reddyashish could you add info to this PR about how this works? |
src/DynamoCoreWpf/ViewModels/Search/NodeAutoCompleteSearchViewModel.cs
Outdated
Show resolved
Hide resolved
src/DynamoCoreWpf/ViewModels/Search/NodeAutoCompleteSearchViewModel.cs
Outdated
Show resolved
Hide resolved
src/DynamoCoreWpf/ViewModels/Search/NodeAutoCompleteSearchViewModel.cs
Outdated
Show resolved
Hide resolved
reddyashish
changed the title
|
One last comment |
src/DynamoCoreWpf/ViewModels/Search/NodeAutoCompleteSearchViewModel.cs
Outdated
Show resolved
Hide resolved
src/DynamoCoreWpf/ViewModels/Search/NodeAutoCompleteSearchViewModel.cs
Outdated
Show resolved
Hide resolved
src/DynamoCoreWpf/ViewModels/Search/NodeAutoCompleteSearchViewModel.cs
Outdated
Show resolved
Hide resolved
src/DynamoCoreWpf/ViewModels/Search/NodeAutoCompleteSearchViewModel.cs
Outdated
Show resolved
Hide resolved
| if (PortViewModel == null) return; | ||
|
|
||
| FilteredResults = searchElementsCache.Where(e => | ||
| QuerySearchElements(e, input)).Select(e => |
There was a problem hiding this comment.
Can we not use the same search algorithm as the one used for in-canvas search?
There was a problem hiding this comment.
For the incanvas search we use the search command on the model and search for all the nodes in the library. It doesn't search in a specific list of nodes.
There was a problem hiding this comment.
Yes, I got that but can we not use the same algorithm/methods, refactor some code, etc?
There was a problem hiding this comment.
@aparajit-pratap Can you be more specific? We are not using the same algorithm here right?
There was a problem hiding this comment.
Yes, Aparajit is asking if we could use the same algorithm to search. https://github.com/DynamoDS/Dynamo/blob/master/src/DynamoCore/Search/SearchDictionary.cs#L327
We can use it with some changes but since the list of search elements won't be that long and we are checking just the name, I kept it simple without using the weights and time. What do you guys think?
There was a problem hiding this comment.
I think this looks good for now.
There was a problem hiding this comment.
I think we should keep the search algorithm consistent irrespective of how many search elements there might be. I think one of @Amoursol's points of emphasis was that search is key for the node autocomplete feature. We should at least create an improvement task for this to address in the near future.
There was a problem hiding this comment.
Indeed @aparajit-pratap - now that I'm back we will spin up a conversation around Search :) For Node Autocomplete to be as successful as we want it to be, Search also needs to be robust.
|
@QilongTang Refactored the code and fixed the tests. |
|
@reddyashish Can we merge and cherry-pick to RC2.9.0? |
* First commit * Cache the seeach elements after the matching nodes have been calculated. * Add comments * some comments. * Adding a test * Addressing some comments. * Some refactoring. (cherry picked from commit f00b2f7)
* First commit * Cache the seeach elements after the matching nodes have been calculated. * Add comments * some comments. * Adding a test * Addressing some comments. * Some refactoring. (cherry picked from commit f00b2f7)
Purpose
This PR adds the search functionality to the Node Autocomplete window.
Task: https://jira.autodesk.com/browse/DYN-3136
TODO:
We are caching the list of matching nodes after they are calculated for the first time and re-use them.
As of now, we are just matching the node name with the user input. If we want to match for any other node property, we can add the necessary condition.
Declarations
Check these if you believe they are true
*.resxfilesReviewers
@DynamoDS/dynamo