DYN-5553 File from Path node remains in ghost warning state erroneously after graph fixup #13820
Conversation
|
|
|
Can we add a test for this |
| var previousNode = this.InputNodes.First().Value.Item2; | ||
|
|
||
|
|
||
| if (previousNode != null && previousNode.Name == fileFromPathNodeName) |
There was a problem hiding this comment.
so AssociatedWarning state only applies to this particular node for now? I cant help thinking that is it possible to address this without introducing this new warning state? Because I thought the reason for this bug is that the warning was over cached when zoomed out
There was a problem hiding this comment.
I have applied this fix bearing in mind only the responsible for display or not the ghost warning, its converter, it depended only on the zoom value, but it should be consider the node state also, that's why my fix consider both.
Even I couldn't trust on the status nodes, as we can see, both the FFP node and the IRFF node has Active as state (this second one displays ui alerts even), that's why I consider the info counts.
After checking your PR I agree with that, I can apply it here instead of the new status and update the unit test , or only keep here the unit test and consider your PR first, please let me know.
State doesn't match with warning UI
Unlinked nodes
Thoughts:
IMHO from an user perspective, the warning messages would be display in all the nodes that are part of this set because the goal is display an image, the would be treated as a total, even if I unlink any node the image is still displayed.
I consider it would be a good candidate to have a specific node to display an image, we can save space for the draw getting less visual noise.
I would love to know more about the nodes behavior and how they should act as a group in a .dyn file anatomy, thanks for your appreciate feedback.
There was a problem hiding this comment.
Thank @jesusalvino Check my other PR for comments and reasoning. I think for some reason, the file from path node got a warning during the computation and later the warning state was removed, but the color overlay was never removed due to issues in the code. So I think the existing states should be sufficient, if not, we should ask the Turbo team to look at VM and not remove the warning state after computation. Back to the issue, it seems naturally we would want to refresh the overlay states in this case after computation because that did not happen for this file from path node.
I would consider apply the fix in my proposed PR and combine it with your test, what do you think?
There was a problem hiding this comment.
sure, let's do it
There was a problem hiding this comment.
@jesusalvino Feel free to use the branch I created or use your own fork, looking forward to it
QilongTang
changed the title
* [DYN-4302 + DYN-5259] Add visual indication for deprecated packages (#13413) * add deprecated label * update extension * sync package manger and package view displays * finishes * set horizontal offset * update HD profile icon * Update PackageDetailsViewModel.cs * add comment * conflicts fix * Update PackageDetailItem.cs * Revert "conflicts fix" This reverts commit 2e60462. * conflict fix2 * DYN-3652 docs browser Artifakt font fails if not installed (#13654) * Embedded Artifakt Element as base64 - to ensure the DocumentationBrowser always loads with the correct font, the Artifakt Element font is now included in the css style sheet as base64 * Refactored font resource - now substitutes an embedded resource containing the font style and converts to base64 before rendering the html page * Revert unnecessary changes * FontFace Css rule reintroduced - brought back a fontface css rule to be whitelisted by the Md2HtmlSanitizer - all DocumentationBrowserExtension tests passing * DYN-5297 Import Excel Small Numbers (#13680) * read scientific notation * remove unecessary reference * Fix for situation with formulas and add unit tests for scientific notation * Dyn 5314 open xml (#13647) * Fix String display * Adding Unit test * Checking Formula * Fixing Ghost warning (#13820) * Fixing Ghost warning * Keeping the Warning Color for the new state * Adding the Unit test * Revert "Keeping the Warning Color for the new state" This reverts commit 40e9643. * Revert "Fixing Ghost warning" This reverts commit 04c14c1. * Updates, Implementation and Test * fix List.AllIndicesOf node (#13773) * Handle null values in watch node/preview bubble (#13855) * Handle null values in watch node/preview bubble * Add test * Update WatchNodeTests.cs * Add null check (#13908) * DYN-6073 civil3 d packages tour crashing (#14338) * DYN-6073 Civil3D Packages Guide Crashing It was crashing due that the user tried to close the Step 4 clicking the PackageSearch window and the Popup is not closed but seems that is already disposed. * DYN-6073 Civil3D Package Tour Crashing This change is disabling the close button in the PackageManagerSearch window when running the Packages tour, when passing to the next Step the button is enabled again (unless the next step also requires to disable the button). I've added a new icon image that will be shown when the close button is disabled. In this way we will be preventing the crash when the user try to close the Packages tour by closing the PackageManagerSearch window * DYN-6073 Civil3D Package Tour Crashing This fix will solve the problem of the packages guide crashing when clicking the Library (package installed) for passing from Step8 to Step9. * Update to remove extra code * Remove the test causing build failure * DYN-6123 Fix groups loose associations with notes and nested groups when a custom node is created (#14220) * Fix note in group in customnode * add test * merge test from other PR * DYN-6130 remove the toast notification (#14317) (#14332) * Make sure close the Toast notification when Dynamo closes * Removing extra file * Removing extra file * Relocate the responsibility about the toast notification to the DynamoViewModel * removing unnecessary using * take advantage of already implementation to apply minimal changes * Moving the code to a proper place --------- Co-authored-by: Jesus Alfredo Alviño <[email protected]> * DependencyType match (#14314) * remove extra code --------- Co-authored-by: Ashish Aggarwal <[email protected]> Co-authored-by: Deyan Nenov <[email protected]> Co-authored-by: filipeotero <[email protected]> Co-authored-by: jesusalvino <[email protected]> Co-authored-by: aparajit-pratap <[email protected]> Co-authored-by: reddyashish <[email protected]> Co-authored-by: Roberto T <[email protected]> Co-authored-by: Jesus Alfredo Alviño <[email protected]>
* Fixing Ghost warning * Keeping the Warning Color for the new state * Adding the Unit test * Revert "Keeping the Warning Color for the new state" This reverts commit 40e9643. * Revert "Fixing Ghost warning" This reverts commit 04c14c1. * Updates, Implementation and Test (cherry picked from commit 62ac05c)
* Fixing Ghost warning * Keeping the Warning Color for the new state * Adding the Unit test * Revert "Keeping the Warning Color for the new state" This reverts commit 40e9643. * Revert "Fixing Ghost warning" This reverts commit 04c14c1. * Updates, Implementation and Test (cherry picked from commit 62ac05c)
Purpose
Fix the bug https://jira.autodesk.com/browse/DYN-5553
Declarations
Check these if you believe they are true
*.resxfilesReviewers
@QilongTang
FYIs
@RobertGlobant20