-
Notifications
You must be signed in to change notification settings - Fork 635
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
Dyn 5008 watch node height 4 dictionary #12989
Dyn 5008 watch node height 4 dictionary #12989
Conversation
@@ -59,7 +60,7 @@ private void _vm_PropertyChanged(object sender, System.ComponentModel.PropertyCh | |||
this.Height = minHeightSize; | |||
if (_vm.Children.Count !=0) | |||
{ | |||
if (_vm.Children[0].NodeLabel.Contains(Environment.NewLine)) | |||
if (_vm.Children[0].NodeLabel.Contains(Environment.NewLine) || _vm.Children[0].NodeLabel.ToUpper() == nameof(WatchViewModel.DICTIONARY)) |
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.
Could Children[0]
ever be null?
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.
If just a null value is passed to the watch node, i think the watch node doesn't consider it a child and children could be zero. Can you verify?
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.
Could
Children[0]
ever be null?
After a lot of debugging at least in the more common scenarios of the WatchNode, In this section of the code when we are dealing with the Iscollection property, no.
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.
If just a null value is passed to the watch node, i think the watch node doesn't consider it a child and children could be zero. Can you verify?
The only way that I found to assign a null value to the WatchNode is being explicit from a Code Block. Per my understanding all the data types that a WatchNode receives always is handled in the first Child[0].
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.
QilongTang reddyashish Please your thoughts/worries, Maybe I'm missing something like others scenarios, complex data types or whatever, It will be nice to test it in order to be on the same page and prevent Dynamo crashes or unexpected behaviors.
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.
It would be hard to tell accurately considering all cases. To be on the safe side, you can just add a additional check before that like:
if(_vm.Children.Count > 0 && ....)
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.
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.
Oh, I did not see that. Looks good to be merged.
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.
In line 63, you can just use the variable NodeLabel you created instead of _vm.Children[0].NodeLabel.
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.
Done
@jesusalvino Please cherry-pick this to RC2.15.0_master branch |
* Dyn 5008 watch node height 4 dictionary (#12989) * WatchTree f Dictionary and Testing * Watchtree 4 Dictionary Testing resources * Use of NodeLabel property * Update the License styling (#13021) * Update the License styling * Update License txt and rtf * Removing duplicate entries (#13027) * Removing duplicate entries * Removing duplicate entries * Cleaning the txt LICENSE
Purpose
WatchNode height 4 a Dictionary
https://jira.autodesk.com/browse/DYN-5008
Declarations
Check these if you believe they are true
*.resx
files###Reviewers
@QilongTang
###FYIs
@RobertGlobant20 @filipeotero
Show less