[DYN-4493] fix behavior with cleanup node layout feature and groups (#12535)

SHKnudsen · web-flow · commit 0166f1315fd7 · 2022-01-20T11:53:49.000-05:00
* Fix nested groups auto layout bug

* fix collapsed groups auto layout issue

* fix clean up node layout issue with collapsed groups

* Update AnnotationViewModel.cs
diff --git a/src/DynamoCore/Graph/Annotations/AnnotationModel.cs b/src/DynamoCore/Graph/Annotations/AnnotationModel.cs
@@ -681,6 +681,7 @@ protected override void DeserializeCore(XmlElement element, SaveContext context)
             RaisePropertyChanged("FontSize");
             RaisePropertyChanged("AnnotationText");
             RaisePropertyChanged("Nodes");
+            this.ReportPosition();
         }
 
         /// <summary>
diff --git a/src/DynamoCore/Graph/Workspaces/LayoutExtensions.cs b/src/DynamoCore/Graph/Workspaces/LayoutExtensions.cs
@@ -86,6 +86,14 @@ private static void GenerateCombinedGraph(this WorkspaceModel workspace, bool is
             {
                 foreach (AnnotationModel group in workspace.Annotations)
                 {
+                    // We dont care about nested groups here,
+                    // as the parent group is treated as one big
+                    // node.
+                    if (workspace.Annotations.ContainsModel(group))
+                    {
+                        continue;
+                    }
+
                     // Treat a group as a graph layout node/vertex
                     combinedGraph.AddNode(group.GUID, group.Width, group.Height, group.X, group.Y,
                         group.IsSelected || DynamoSelection.Instance.Selection.Count == 0);
@@ -96,15 +104,15 @@ private static void GenerateCombinedGraph(this WorkspaceModel workspace, bool is
             {
                 if (!isGroupLayout)
                 {
-                    AnnotationModel group = workspace.Annotations.Where(
-                        g => g.Nodes.Contains(node)).ToList().FirstOrDefault();
+                    AnnotationModel group = workspace.Annotations
+                        .Where(g => g.ContainsModel(node))
+                        .FirstOrDefault();
 
                     // Do not process nodes within groups
-                    if (group == null)
-                    {
-                        combinedGraph.AddNode(node.GUID, node.Width, node.Height, node.X, node.Y,
+                    if (group != null) continue;
+
+                    combinedGraph.AddNode(node.GUID, node.Width, node.Height, node.X, node.Y,
                             node.IsSelected || DynamoSelection.Instance.Selection.Count == 0);
-                    }
                 }
                 else
                 {
@@ -114,7 +122,7 @@ private static void GenerateCombinedGraph(this WorkspaceModel workspace, bool is
                 }
             }
 
-            ///Adding all connectorPins (belonging to all connectors) as graph.nodes to the combined graph.
+            // Adding all connectorPins (belonging to all connectors) as graph.nodes to the combined graph.
             foreach (ConnectorModel edge in workspace.Connectors)
             {
                 foreach (var pin in edge.ConnectorPinModels)
@@ -133,10 +141,22 @@ private static void GenerateCombinedGraph(this WorkspaceModel workspace, bool is
                 if (!isGroupLayout)
                 {
                     AnnotationModel startGroup = null, endGroup = null;
-                    startGroup = workspace.Annotations.Where(
-                        g => g.Nodes.Contains(edge.Start.Owner)).ToList().FirstOrDefault();
-                    endGroup = workspace.Annotations.Where(
-                        g => g.Nodes.Contains(edge.End.Owner)).ToList().FirstOrDefault();
+
+                    // To get the start/end group we first make sure
+                    // that all nested groups are filterd out as we dont
+                    // care about them. We then check if the edge
+                    // start/end owner is in either the parent or child group
+                    var groupsFiltered = workspace.Annotations.Where(g => !workspace.Annotations.ContainsModel(g));
+
+                    startGroup = groupsFiltered
+                        .Where(g => g.ContainsModel(edge.Start.Owner) || 
+                                    g.Nodes.OfType<AnnotationModel>().SelectMany(x => x.Nodes).Contains(edge.Start.Owner))
+                        .FirstOrDefault();
+
+                    endGroup = groupsFiltered
+                        .Where(g => g.ContainsModel(edge.End.Owner) || 
+                                    g.Nodes.OfType<AnnotationModel>().SelectMany(x => x.Nodes).Contains(edge.End.Owner))
+                        .FirstOrDefault();
 
                     // Treat a group as a node, but do not process edges within a group
                     if (startGroup == null || endGroup == null || startGroup != endGroup)
@@ -163,13 +183,15 @@ private static void GenerateCombinedGraph(this WorkspaceModel workspace, bool is
                     // We add this note to the LinkedNotes on the 
                     // pinned node. 
                     var graphNode = combinedGraph.FindNode(note.PinnedNode.GUID);
+                    if (graphNode is null) continue;
                     var height = note.PinnedNode.Rect.Top - note.Rect.Top;
                     graphNode.LinkNote(note, note.Width, height);
                     continue;
                 }
 
-                AnnotationModel group = workspace.Annotations.Where(
-                    g => g.Nodes.Contains(note)).ToList().FirstOrDefault();
+                AnnotationModel group = workspace.Annotations
+                    .Where(g => g.Nodes.Contains(note))
+                    .FirstOrDefault();
 
                 GraphLayout.Node nd = null;
 
@@ -289,6 +311,7 @@ private static void RecordUndoGraphLayout(this WorkspaceModel workspace, bool is
             if (!isGroupLayout)
             {
                 // Add all selected items to the undo recorder
+                undoItems.AddRange(workspace.Annotations);
                 undoItems.AddRange(workspace.Connectors.SelectMany(conn => conn.ConnectorPinModels));
                 undoItems.AddRange(workspace.Nodes);
                 undoItems.AddRange(workspace.Notes);
@@ -475,7 +498,13 @@ private static void SaveLayoutGraph(this WorkspaceModel workspace, List<GraphLay
 
                     double deltaX = n.X - group.X;
                     double deltaY = n.Y - group.Y + graph.OffsetY;
-                    foreach (var node in group.Nodes.OfType<NodeModel>())
+
+                    // We update the posistion of all nodes in the
+                    // parent group + all nodes in any potential
+                    // nested groups.
+                    foreach (var node in group.Nodes
+                        .OfType<NodeModel>()
+                        .Union(group.Nodes.OfType<AnnotationModel>().SelectMany(x => x.Nodes.OfType<NodeModel>())))
                     {
                         node.X += deltaX;
                         node.Y += deltaY;
@@ -492,6 +521,8 @@ private static void SaveLayoutGraph(this WorkspaceModel workspace, List<GraphLay
                             note.ReportPosition();
                         }
                     }
+
+                    group.ReportPosition();
                 }
             }
 
diff --git a/src/DynamoCoreWpf/ViewModels/Core/StateMachine.cs b/src/DynamoCoreWpf/ViewModels/Core/StateMachine.cs
@@ -838,7 +838,9 @@ internal bool HandleMouseMove(object sender, Point mouseCursor)
                     // the nested group and the parent group, as the mouse coursor will be inside
                     // both of there rects. In these cases we want to get group that is nested
                     // inside the parent group.
-                    var dropGroup = dropGroups.FirstOrDefault(x => !x.AnnotationModel.HasNestedGroups) ?? dropGroups.FirstOrDefault();
+                    var dropGroup = dropGroups
+                        .FirstOrDefault(x => !x.AnnotationModel.HasNestedGroups) ?? dropGroups.FirstOrDefault();
+
 
                     // If the dropGroup is null or any of the selected items is already in the dropGroup,
                     // we disable the drop border by setting NodeHoveringState to false
@@ -881,6 +883,15 @@ internal bool HandleMouseMove(object sender, Point mouseCursor)
                             .ToList()
                             .ForEach(x => x.NodeHoveringState = false);
 
+                        // If the dropGroup belongs to another group
+                        // we need to check if the parent group is collapsed
+                        // if it is we dont want to be able to add new
+                        // models to the drop group.
+                        var parentGroup = owningWorkspace.Annotations
+                            .Where(x => x.AnnotationModel.ContainsModel(dropGroup.AnnotationModel))
+                            .FirstOrDefault();
+                        if (parentGroup != null && !parentGroup.IsExpanded) return false;
+                        
                         dropGroup.NodeHoveringState = true;
                     }
 
diff --git a/src/DynamoCoreWpf/ViewModels/Core/WorkspaceViewModel.cs b/src/DynamoCoreWpf/ViewModels/Core/WorkspaceViewModel.cs
@@ -906,7 +906,7 @@ internal void SelectInRegion(Rect2D region, bool isCrossSelect)
 
             foreach (var n in childlessModels)
             {
-                if (IsInRegion(region, n, fullyEnclosed))
+                if (IsInRegion(region, n, fullyEnclosed) && !IsCollapsed)
                 {
                     selection.AddUnique(n);
                 }
@@ -918,7 +918,7 @@ internal void SelectInRegion(Rect2D region, bool isCrossSelect)
 
             foreach (var n in Model.Annotations)
             {
-                if (IsInRegion(region, n, fullyEnclosed))
+                if (IsInRegion(region, n, fullyEnclosed) && !IsCollapsed)
                 {
                     selection.AddUnique(n);
                     // if annotation is selected its children should be added to selection too

Original file line number	Diff line number	Diff line change
`@@ -681,6 +681,7 @@ protected override void DeserializeCore(XmlElement element, SaveContext context)`
`681`	`681`	`RaisePropertyChanged("FontSize");`
`682`	`682`	`RaisePropertyChanged("AnnotationText");`
`683`	`683`	`RaisePropertyChanged("Nodes");`
	`684`	`+ this.ReportPosition();`
`684`	`685`	`}`
`685`	`686`
`686`	`687`	`/// <summary>`
Original file line number	Diff line number	Diff line change
`@@ -906,7 +906,7 @@ internal void SelectInRegion(Rect2D region, bool isCrossSelect)`
`906`	`906`
`907`	`907`	`foreach (var n in childlessModels)`
`908`	`908`	`{`
`909`		`- if (IsInRegion(region, n, fullyEnclosed))`
	`909`	`+ if (IsInRegion(region, n, fullyEnclosed) && !IsCollapsed)`
`910`	`910`	`{`
`911`	`911`	`selection.AddUnique(n);`
`912`	`912`	`}`
`@@ -918,7 +918,7 @@ internal void SelectInRegion(Rect2D region, bool isCrossSelect)`
`918`	`918`
`919`	`919`	`foreach (var n in Model.Annotations)`
`920`	`920`	`{`
`921`		`- if (IsInRegion(region, n, fullyEnclosed))`
	`921`	`+ if (IsInRegion(region, n, fullyEnclosed) && !IsCollapsed)`
`922`	`922`	`{`
`923`	`923`	`selection.AddUnique(n);`
`924`	`924`	`// if annotation is selected its children should be added to selection too`