Merge if node changes #11728
Merge if node changes #11728
Conversation
…evalue based on the test condition. (DynamoDS#11481) * attempt1 at creating partially applied function * change implementation of If NodeModel node * Move the functionality to a new if node that will be renamed. * Update IfTest.cs * Hiding the old If node and renaming to the new one to have the same name. * Throw a warning message when the old node is opened. * add comments. * Update the warning message and move it to the resources file. * Fix failing test Co-authored-by: Aparajit Pratap <[email protected]>
…DS#11598) * Add lacing feature to the "If" node. * Addressing comments * additional comments * Move code into a private function.
| [OutPortTypes("Function")] | ||
| [IsDesignScriptCompatible] | ||
| [AlsoKnownAs("DSCoreNodesUI.Logic.If")] | ||
| public class RefactoredIf : NodeModel |
There was a problem hiding this comment.
Since this is going to be a public class, is "RefactoredIf" name fine with everyone? Are we going to obsolete the old node in 3.0 and then rename this class name to "If"?
There was a problem hiding this comment.
Maybe a more descriptive name and summary? Is there any reason not to obsolete the old node right now?
There was a problem hiding this comment.
Sorry I meant, if we are going to remove this old node completely in 3.0. Currently the old one is obsoleted and the new node has a summary under BuildOutputAst(). Do you want me to add any other information?
There was a problem hiding this comment.
@reddyashish there is still work to be done to apply the existing If node icon to this new node. I tried your branch locally and the icon doesn't work for the new node.
|
@aparajit-pratap looking into it. |
|
@reddyashish can you also change the names of the small and large png files from the old class name to the new class name. You should find them in |
|
You might also need to update this file for the icons accordingly for the library tests to pass: |
|
@aparajit-pratap changed the png file name in both the locations and updated libraryItems.json. Let me know if those changes look good to you. |
|
Addressed the failing test, but somehow the self-serve job validation here is still showing it is unstable. |
Purpose
This PR is to merge the "IF" node implementation changes from feature branch to master.
#11481
#11598
#11721
Declarations
Check these if you believe they are true
*.resxfilesReviewers
@QilongTang @zeusongit @aparajit-pratap @mjkkirschner