Node auto layout for node autocomplete (#11177)

* trial commit

* update

* place suggested node wrt node height and width

* execute graph auto layout command after connecting input node

* node auto layout for node autocomplete

* revert unnecessary change

* revert unnecessary change

* cleanup

* cleanup

* rename parameter

* rename variables

* add code comments, null check

* revert unnecessary change

* more code comments

* refactor into delegate command on nodesearchelementviewmodel

* cleanup

* remove unused usings

Co-authored-by: tanga <[email protected]>

src/DynamoCore/Graph/Workspaces/LayoutExtensions.cs

@@ -18,7 +18,9 @@ public static class LayoutExtensions
         /// This function wraps a few methods on the workspace model layer
         /// to set up and run the graph layout algorithm.
         /// </summary>
-        internal static List<GraphLayout.Graph> DoGraphAutoLayout(this WorkspaceModel workspace)
+        /// <param name="workspace">Workspace on which graph layout will be performed.</param>
+        /// <param name="reuseUndoRedoGroup">If true, skip initializing new undo action group.</param>
+        internal static List<GraphLayout.Graph> DoGraphAutoLayout(this WorkspaceModel workspace, bool reuseUndoRedoGroup = false)
         {
             if (workspace.Nodes.Count() < 2) return null;
 
@@ -32,9 +34,11 @@ public static class LayoutExtensions
             List<GraphLayout.Graph> layoutSubgraphs;
             List<List<GraphLayout.Node>> subgraphClusters;
 
-            GenerateCombinedGraph(workspace, isGroupLayout,out layoutSubgraphs, out subgraphClusters);
+            GenerateCombinedGraph(workspace, isGroupLayout, out layoutSubgraphs, out subgraphClusters);
 
-            RecordUndoGraphLayout(workspace, isGroupLayout);
+
+            RecordUndoGraphLayout(workspace, isGroupLayout, reuseUndoRedoGroup);
+
 
             // Generate subgraphs separately for each cluster
             subgraphClusters.ForEach(
@@ -194,7 +198,8 @@ private static void GenerateCombinedGraph(this WorkspaceModel workspace, bool is
         /// </summary>
         /// <param name="workspace">A <see cref="WorkspaceModel"/>.</param>
         /// <param name="isGroupLayout">True if all the selected models are groups.</param>
-        private static void RecordUndoGraphLayout(this WorkspaceModel workspace, bool isGroupLayout)
+        /// <param name="reuseUndoRedoGroup">Skip creating new undo action group, reuse existing group if true.</param>
+        private static void RecordUndoGraphLayout(this WorkspaceModel workspace, bool isGroupLayout, bool reuseUndoRedoGroup)
         {
             List<ModelBase> undoItems = new List<ModelBase>();
 
@@ -221,7 +226,14 @@ private static void RecordUndoGraphLayout(this WorkspaceModel workspace, bool is
                 }
             }
 
-            WorkspaceModel.RecordModelsForModification(undoItems, workspace.UndoRecorder);
+            if (reuseUndoRedoGroup)
+            {
+                workspace.RecordModelsForModification(undoItems);
+            }
+            else
+            {
+                WorkspaceModel.RecordModelsForModification(undoItems, workspace.UndoRecorder);
+            }
         }
 
         /// <summary>

src/DynamoCore/Graph/Workspaces/UndoRedo.cs

@@ -110,6 +110,21 @@ internal static void RecordModelsForModification(List<ModelBase> models, UndoRed
             }
         }
 
+        /// <summary>
+        /// This method assumes an undo-redo action group already exists
+        /// and records in it models that are modified.
+        /// </summary>
+        /// <param name="models"></param>
+        internal void RecordModelsForModification(List<ModelBase> models)
+        {
+            if (null == UndoRecorder) return;
+
+            if (!ShouldProceedWithRecording(models)) return;
+
+            foreach (var model in models)
+                UndoRecorder.RecordModificationForUndo(model);
+        }
+
         internal static void RecordModelsForUndo(Dictionary<ModelBase, UndoRedoRecorder.UserAction> models, UndoRedoRecorder recorder)
         {
             if (null == recorder)

src/DynamoCore/Models/DynamoModelCommands.cs

@@ -83,43 +83,57 @@ private void CreateAndConnectNodeImpl(CreateAndConnectNodeCommand command)
         {
             using (CurrentWorkspace.UndoRecorder.BeginActionGroup())
             {
-                var newNode = CreateNodeFromNameOrType(command.ModelGuid, command.NewNodeName);
-                newNode.X = command.X;
-                newNode.Y = command.Y;
-                var existingNode = CurrentWorkspace.GetModelInternal(command.ModelGuids.ElementAt(1)) as NodeModel;
-
-                if(newNode == null || existingNode == null) return;
-
-                AddNodeToCurrentWorkspace(newNode, false, command.AddNewNodeToSelection);
-                CurrentWorkspace.UndoRecorder.RecordCreationForUndo(newNode);
-
-                PortModel inPortModel, outPortModel;
-                if (command.CreateAsDownstreamNode)
-                {
-                    // Connect output port of Existing Node to input port of New node
-                    outPortModel = existingNode.OutPorts[command.OutputPortIndex];
-                    inPortModel = newNode.InPorts[command.InputPortIndex];
-                }
-                else
-                {
-                    // Connect output port of New Node to input port of existing node
-                    outPortModel = newNode.OutPorts[command.OutputPortIndex];
-                    inPortModel = existingNode.InPorts[command.InputPortIndex];
-                }
+                CreateAndConnectNodeImplWithUndoGroup(command);
+            }
+        }
+
+        /// <summary>
+        /// This method assumes that there exists an undo-redo action group already
+        /// that can be used to record creation and deletion of models.
+        /// </summary>
+        /// <param name="command"></param>
+        private void CreateAndConnectNodeImplWithUndoGroup(CreateAndConnectNodeCommand command)
+        {
+            var newNode = CreateNodeFromNameOrType(command.ModelGuid, command.NewNodeName);
+
+            if (newNode == null) return;
 
-                var models = GetConnectorsToAddAndDelete(inPortModel, outPortModel);
+            newNode.X = command.X;
+            newNode.Y = command.Y;
 
-                foreach (var modelPair in models)
+            var existingNode = CurrentWorkspace.GetModelInternal(command.ModelGuids.ElementAt(1)) as NodeModel;
+
+            if (existingNode == null) return;
+
+            AddNodeToCurrentWorkspace(newNode, false, command.AddNewNodeToSelection);
+            CurrentWorkspace.UndoRecorder.RecordCreationForUndo(newNode);
+
+            PortModel inPortModel, outPortModel;
+            if (command.CreateAsDownstreamNode)
+            {
+                // Connect output port of Existing Node to input port of New node
+                outPortModel = existingNode.OutPorts[command.OutputPortIndex];
+                inPortModel = newNode.InPorts[command.InputPortIndex];
+            }
+            else
+            {
+                // Connect output port of New Node to input port of existing node
+                outPortModel = newNode.OutPorts[command.OutputPortIndex];
+                inPortModel = existingNode.InPorts[command.InputPortIndex];
+            }
+
+            var models = GetConnectorsToAddAndDelete(inPortModel, outPortModel);
+
+            foreach (var modelPair in models)
+            {
+                switch (modelPair.Value)
                 {
-                    switch (modelPair.Value)
-                    {
-                        case UndoRedoRecorder.UserAction.Creation:
-                            CurrentWorkspace.UndoRecorder.RecordCreationForUndo(modelPair.Key);
-                            break;
-                        case UndoRedoRecorder.UserAction.Deletion:
-                            CurrentWorkspace.UndoRecorder.RecordDeletionForUndo(modelPair.Key);
-                            break;
-                    }
+                    case UndoRedoRecorder.UserAction.Creation:
+                        CurrentWorkspace.UndoRecorder.RecordCreationForUndo(modelPair.Key);
+                        break;
+                    case UndoRedoRecorder.UserAction.Deletion:
+                        CurrentWorkspace.UndoRecorder.RecordDeletionForUndo(modelPair.Key);
+                        break;
                 }
             }
         }

src/DynamoCore/Models/RecordableCommands.cs

@@ -641,8 +641,6 @@ protected override void SerializeCore(XmlElement element)
         [DataContract]
         public class CreateNodeCommand : ModelBasedRecordableCommand
         {
-            #region Public Class Methods
-
             private void SetProperties(double x, double y, bool defaultPosition, bool transformCoordinates)
             {
                 X = x;
@@ -651,6 +649,8 @@ private void SetProperties(double x, double y, bool defaultPosition, bool transf
                 TransformCoordinates = transformCoordinates;
             }
 
+            #region Public Class Methods
+
             /// <summary>
             /// </summary>
             /// <param name="node">The node.</param>
@@ -817,6 +817,31 @@ internal override void TrackAnalytics()
         [DataContract]
         public class CreateAndConnectNodeCommand : ModelBasedRecordableCommand
         {
+            private readonly bool reuseUndoRedoGroup;
+
+            /// <summary>
+            /// Creates a new CreateAndConnectNodeCommand with the given inputs
+            /// </summary>
+            /// <param name="newNodeGuid"></param>
+            /// <param name="existingNodeGuid"></param>
+            /// <param name="newNodeName">The name of node to create</param>
+            /// <param name="outPortIndex"></param>
+            /// <param name="inPortIndex"></param>
+            /// <param name="x"></param>
+            /// <param name="y"></param>
+            /// <param name="createAsDownstreamNode">
+            /// new node to be created as downstream or upstream node wrt the existing node
+            /// </param>
+            /// <param name="addNewNodeToSelection">select the new node after it is created by default</param>
+            /// <param name="reuseUndoRedoGroup">Skip creating new undo action group and reuse existing one if true.</param>
+            internal CreateAndConnectNodeCommand(Guid newNodeGuid, Guid existingNodeGuid, string newNodeName,
+                int outPortIndex, int inPortIndex,
+                double x, double y, bool createAsDownstreamNode, bool addNewNodeToSelection, bool reuseUndoRedoGroup)
+                : this(newNodeGuid, existingNodeGuid, newNodeName, outPortIndex, inPortIndex,
+                    x, y, createAsDownstreamNode, addNewNodeToSelection)
+            {
+                this.reuseUndoRedoGroup = reuseUndoRedoGroup;
+            }
 
             #region Public Class Methods
 
@@ -879,7 +904,14 @@ public CreateAndConnectNodeCommand(Guid newNodeGuid, Guid existingNodeGuid, stri
 
             protected override void ExecuteCore(DynamoModel dynamoModel)
             {
-                dynamoModel.CreateAndConnectNodeImpl(this);
+                if (reuseUndoRedoGroup)
+                {
+                    dynamoModel.CreateAndConnectNodeImplWithUndoGroup(this);
+                }
+                else
+                {
+                    dynamoModel.CreateAndConnectNodeImpl(this);
+                }
             }
 
             protected override void SerializeCore(XmlElement element)

src/DynamoCoreWpf/Controls/NodeAutoCompleteSearchControl.xaml.cs

@@ -88,7 +88,10 @@ private void ExecuteSearchElement(ListBoxItem listBoxItem)
             {
                 searchElement.Position = ViewModel.InCanvasSearchPosition;
                 PortViewModel port = ViewModel.PortViewModel;
-                searchElement.CreateAndConnectCommand.Execute(port.PortModel);
+                if (searchElement.CreateAndConnectCommand.CanExecute(port.PortModel))
+                {
+                    searchElement.CreateAndConnectCommand.Execute(port.PortModel);
+                }
             }
         }
 

src/DynamoCoreWpf/ViewModels/Core/DynamoViewModel.cs

@@ -1924,7 +1924,10 @@ internal bool CanAlignSelected(object param)
 
         public void DoGraphAutoLayout(object parameter)
         {
-            this.CurrentSpaceViewModel.GraphAutoLayoutCommand.Execute(parameter);
+            if (CurrentSpaceViewModel.GraphAutoLayoutCommand.CanExecute(parameter))
+            {
+                CurrentSpaceViewModel.GraphAutoLayoutCommand.Execute(parameter);
+            }
         }
 
         internal bool CanDoGraphAutoLayout(object parameter)

src/DynamoCoreWpf/ViewModels/Search/NodeSearchElementViewModel.cs

@@ -5,9 +5,13 @@
 using System.Windows.Input;
 using System.Windows.Media;
 using Dynamo.Configuration;
+using Dynamo.Controls;
 using Dynamo.Graph.Nodes;
+using Dynamo.Graph.Workspaces;
 using Dynamo.Logging;
+using Dynamo.Models;
 using Dynamo.Search.SearchElements;
+using Dynamo.Selection;
 using Dynamo.ViewModels;
 using FontAwesome.WPF;
 using Microsoft.Practices.Prism.Commands;
@@ -20,6 +24,7 @@ public class NodeSearchElementViewModel : ViewModelBase, ISearchEntryViewModel
 
         private bool isSelected;
         private SearchViewModel searchViewModel;
+        private IDisposable undoRecorderGroup;
 
         public event RequestBitmapSourceHandler RequestBitmapSource;
         public void OnRequestBitmapSource(IconRequestEventArgs e)
@@ -44,10 +49,9 @@ public NodeSearchElementViewModel(NodeSearchElement element, SearchViewModel svm
             if (searchViewModel != null)
             {
                 Clicked += searchViewModel.OnSearchElementClicked;
-                CreateAndConnectToPort += searchViewModel.OnRequestConnectToPort;
             }
             ClickedCommand = new DelegateCommand(OnClicked);
-            CreateAndConnectCommand = new DelegateCommand<PortModel>(OnRequestCreateAndConnectToPort);
+            CreateAndConnectCommand = new DelegateCommand<PortModel>(CreateAndConnectToPort, CanCreateAndConnectToPort);
 
             LoadFonts();
         }
@@ -62,6 +66,10 @@ public override void Dispose()
             Model.VisibilityChanged -= ModelOnVisibilityChanged;
             if (searchViewModel != null)
             {
+                if (RequestBitmapSource != null)
+                {
+                    RequestBitmapSource -= searchViewModel.SearchViewModelRequestBitmapSource;
+                }
                 Clicked -= searchViewModel.OnSearchElementClicked;
                 searchViewModel = null;
             }
@@ -233,14 +241,85 @@ public ImageSource LargeIcon
         /// <summary>
         /// Create the search element as node and connect to target port
         /// </summary>
-        public ICommand CreateAndConnectCommand { get; private set; }
+        public ICommand CreateAndConnectCommand { get; }
 
-        public event Action<string, PortModel> CreateAndConnectToPort;
-        protected virtual void OnRequestCreateAndConnectToPort(PortModel portModel)
+        /// <summary>
+        /// Create new node for search element, connect to port and place using graph auto layout.
+        /// </summary>
+        /// <param name="parameter">Port model to connect to</param>
+        protected virtual void CreateAndConnectToPort(object parameter)
         {
-            if (CreateAndConnectToPort != null)
+            var portModel = (PortModel) parameter;
+            var dynamoViewModel = searchViewModel.dynamoViewModel;
+
+            // Initialize a new undo action group before calling 
+            // node CreateAndConnect and AutoLayout commands.
+            if (undoRecorderGroup == null)
             {
-                CreateAndConnectToPort(Model.CreationName, portModel);
+                undoRecorderGroup = dynamoViewModel.CurrentSpace.UndoRecorder.BeginActionGroup();
+
+                // Node auto layout can be performed correctly only when the positions and sizes
+                // of nodes are known, which is possible only after the node views are ready.
+                dynamoViewModel.NodeViewReady += AutoLayoutNodes;
+            }
+
+            var initialNode = portModel.Owner;
+            var initialNodeVm = dynamoViewModel.CurrentSpaceViewModel.Nodes.FirstOrDefault(x => x.Id == initialNode.GUID);
+            var id = Guid.NewGuid();
+
+            var adjustedX = initialNodeVm.X;
+
+            var createAsDownStreamNode = portModel.PortType == PortType.Output;
+            // Placing the new node based on which input port it is connecting to.
+            if (createAsDownStreamNode)
+            {
+                // Placing the new node to the right of initial node
+                adjustedX += initialNode.Width + 50;
+            }
+            else
+            {
+                // Placing the new node to the left of initial node
+                adjustedX -= initialNode.Width + 50;
+            }
+
+            // Create a new node based on node creation name and connection ports
+            dynamoViewModel.ExecuteCommand(new DynamoModel.CreateAndConnectNodeCommand(id, initialNode.GUID,
+                Model.CreationName, 0, portModel.Index, adjustedX, 0, createAsDownStreamNode, false, true));
+
+            // Clear current selections and select all input nodes as we need to perform Auto layout on only the input nodes.
+            DynamoSelection.Instance.ClearSelection();
+            var inputNodes = initialNode.InputNodes.Values.Where(x => x != null).Select(y => y.Item2);
+
+            foreach (var inputNode in inputNodes)
+            {
+                DynamoSelection.Instance.Selection.AddUnique(inputNode);
+            }
+        }
+
+        protected virtual bool CanCreateAndConnectToPort(object parameter)
+        {
+            // Do not auto connect code block node since default code block node do not have output port
+            if (Model.CreationName.Contains("Code Block")) return false;
+
+            return true;
+        }
+
+        private void AutoLayoutNodes(object sender, EventArgs e)
+        {
+            var nodeView = (NodeView) sender;
+            var dynamoViewModel = nodeView.ViewModel.DynamoViewModel;
+
+            dynamoViewModel.CurrentSpace.DoGraphAutoLayout(true);
+
+            DynamoSelection.Instance.ClearSelection();
+
+            // Close the undo action group once the node is created, connected and placed.
+            if (undoRecorderGroup != null)
+            {
+                undoRecorderGroup.Dispose();
+                undoRecorderGroup = null;
+
+                dynamoViewModel.NodeViewReady -= AutoLayoutNodes;
             }
         }