Fix node overlap issue by skipping the original node from AutoLayout repositioning#11414
Fix node overlap issue by skipping the original node from AutoLayout repositioning#11414zeusongit merged 3 commits intoDynamoDS:masterfrom
Conversation
|
Seems in certain case, it is still covering a little bit. More testing on the way |
|
This solution does come with an advantage that if user autocomplete not based on the order of the ports, they will see our auto layout clean up the sequence of the nodes for them instead of have the two result nodes mingled.. |
| /// <summary> | ||
| /// This method pushes changes from the GraphLayout.Graph objects | ||
| /// back to the workspace models, but only for nodes placed by NodeAutocomplete. | ||
| /// </summary> |
There was a problem hiding this comment.
Both versions of the function are missing comments for input params, do you mind adding them back?
QilongTang
left a comment
QilongTang
left a comment
There was a problem hiding this comment.
LGTM with one comment. By comparing this solution with the other one by @aparajit-pratap, I see advantage for both. Due to the following reasons, I would like to ship this one for now:
- Node auto complete no longer depending on the order of the ports
- This solution is a bit simpler although less precise on the right amount to move, but do deliver a solution with less maintaining cost
|
IMHO this solution is not clean as it tampers with the original graph layout algorithm by adding a condition that if laying out for node-auto complete then do this other layout logic. Also, the placement of the nodes is not clean either as the nodes are placed on the top of the given node and not alongside it. I'm not sure if @Amoursol has any opinion on this but that's just me. Furthermore, I proposed a solution that I was hoping that the Morpheus team could take and continue working on/refining but it's unfortunate that no such attempt has not been made. |
…repositioning (DynamoDS#11414) * prevent overlap
4 participants
Purpose
Wider nodes tend to overlap the parent node, the fix is to include the parent node in the auto layout group, and then restrict original node's repositioning to prevent jumping effect.
Problem:
Fix:
Declarations
Check these if you believe they are true
Reviewers
@DynamoDS/dynamo