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

Enable Node Autocompletion for output ports of Dynamo nodes. #11638

Merged
merged 10 commits into from
Apr 30, 2021

Conversation

reddyashish
Copy link
Contributor

Purpose

This PR is to address the task: https://jira.autodesk.com/browse/DYN-3344

This PR will enable the node autocompletion functionality on the output ports of the nodes. The node autocompletion model will find all the matching nodes which has atleast one input parameter matching the output port type of the selected node. When any of the search elements are clicked, it will create the node and connect the output port of the original node to the first available matching input port of the new node.

autocompletion output ports

Things to discuss

  1. When no matching search elements are found, do we want to return an empty list? Because the default nodes shown for the input nodes do not have any InPorts so we cannot show them for the output port.

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated

Reviewers

@QilongTang @zeusongit @mjkkirschner @aparajit-pratap

@reddyashish reddyashish changed the title Node autocompletion for output ports of the Dynamo nodes. Node autocompletion for output ports of Dynamo nodes. Apr 22, 2021
@reddyashish reddyashish changed the title Node autocompletion for output ports of Dynamo nodes. Enable Node Autocompletion for output ports of Dynamo nodes. Apr 22, 2021
@mjkkirschner
Copy link
Member

@reddyashish maybe when nothing matches we show a list with codeblock, watch node, or python?

@QilongTang
Copy link
Contributor

@reddyashish maybe when nothing matches we show a list with codeblock, watch node, or python?

Due to the convenience of creating CBN, I would maybe just add watch, watch 3D and Python, even watch3D may not be universal. My 2 cents.

@reddyashish
Copy link
Contributor Author

@mjkkirschner @QilongTang Addressed the comments.
For the output ports, I am modified it to only show the watch, watch 3D and Python nodes as the CBN node does not have any input parameters.
To store the data for Auto-completion node elements, I created a new class where we can add more attributes in the future. The NodeSearchElement is extending this class as the auto-completion API returns a list of NodeSearchElement and we would need this information in the NodeSearchElementViewModel to connect the port. What do you think about it?

/// <summary>
/// Class for all Auto-Completion Node search elements.
/// </summary>
public abstract class AutoCompletionNodeSearchElement
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you re-iterate the purpose of this class by comments here? We cant inherit the NodeSearchElement class because auto-completion API returns a list of NodeSearchElement and we would need this information in the NodeSearchElementViewModel to connect the port? I am fine with this, @mjkkirschner @aparajit-pratap what do you think? I think we plan to keep expanding here because we will have more properties coming in specific to AutoComplete function, e.g. user's rank, whether favored, ML rank

@reddyashish reddyashish merged commit 75f24d4 into DynamoDS:master Apr 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants