Skip to content

Commit

Permalink
Merge pull request #1414 from glopesdev/workflow-editor
Browse files Browse the repository at this point in the history 
Fixes and improvements to workflow sort order
  • Loading branch information
@glopesdev
glopesdev authored Jun 13, 2023
2 parents 4b8c0f3 + 5a90afb commit 3ad6c5d
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 47 deletions.
6 changes: 6 additions & 0 deletions Bonsai.Editor.Tests/EditorHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ internal static GraphNode FindNode(this WorkflowEditor editor, string name)
return editor.FindGraphNode(node.Value);
}

internal static GraphNode FindNode(this WorkflowEditor editor, ExpressionBuilder builder)
{
var node = editor.Workflow.First(n => ExpressionBuilder.Unwrap(n.Value) == builder);
return editor.FindGraphNode(node.Value);
}

internal static void ConnectNodes(this WorkflowEditor editor, string source, string target)
{
var sourceNodes = new[] { editor.FindNode(source) };
Expand Down
27 changes: 27 additions & 0 deletions Bonsai.Editor.Tests/WorkflowEditorTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,33 @@ public void UngroupGraphNodes_NestedPassthrough_KeepOuterEdges()
editor.UngroupGraphNodes(nodesToUngroup);
assertIsReversible();
}

[TestMethod]
public void DisconnectGraphNodes_TargetIsNotRoot_InsertAfterClosestRoot()
{
var workflow = EditorHelper.CreateEditorGraph("A", "B", "C", "D");
var (editor, assertIsReversible) = CreateMockEditor(workflow);
editor.ConnectNodes("A", "C");
editor.ConnectNodes("B", "C");
editor.ConnectNodes("C", "D");
var sourceNode = editor.FindNode("B");
var targetNode = editor.FindNode("C");
Assert.AreEqual(expected: editor.Workflow.Count - 1, editor.FindNode("D").Index);
editor.DisconnectGraphNodes(new[] { sourceNode }, targetNode);
Assert.AreEqual(expected: editor.Workflow.Count - 1, editor.FindNode("B").Index);
assertIsReversible();
}

[TestMethod]
public void CreateAnnotation_EmptySelection_InsertAfterClosestRoot()
{
var workflow = EditorHelper.CreateEditorGraph("A");
var (editor, assertIsReversible) = CreateMockEditor(workflow);
var annotationBuilder = new AnnotationBuilder();
editor.CreateGraphNode(annotationBuilder, null, CreateGraphNodeType.Successor, branch: false);
Assert.AreEqual(expected: editor.Workflow.Count - 1, editor.FindNode(annotationBuilder).Index);
assertIsReversible();
}
}

static class WorkflowEditorHelper
Expand Down
106 changes: 61 additions & 45 deletions Bonsai.Editor/GraphModel/WorkflowEditor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ private int GetInsertIndex(
Node<ExpressionBuilder, ExpressionBuilderArgument> target,
CreateGraphNodeType nodeType)
{
var insertOffset = target == null || nodeType == CreateGraphNodeType.Successor ? 1 : 0;
if (target == null) return workflow.Count;
var insertOffset = nodeType == CreateGraphNodeType.Successor ? 1 : 0;
return workflow.IndexOf(target) + insertOffset;
}

Expand Down Expand Up @@ -198,7 +199,7 @@ private void SortWorkflow()

private void SortWorkflow(ExpressionBuilderGraph workflow)
{
Workflow.InsertRange(0, Workflow.TopologicalSort());
workflow.InsertRange(0, workflow.TopologicalSort());
}

private GraphCommand GetSimpleSort()
Expand Down Expand Up @@ -688,7 +689,9 @@ where siblingEdge.Item2.Label.Index.CompareTo(edgeIndex) > 0
}

GraphCommand reorder;
var targetIndex = workflow.IndexOf(target) + 1;
var components = workflow.FindConnectedComponents();
var targetComponent = components.First(component => component.Contains(target));
var targetIndex = LastIndexOfComponentNode(workflow, targetComponent) + 1;
if (sinkNodes != null)
{
reorder = GetReversibleSort();
Expand Down Expand Up @@ -962,12 +965,56 @@ static string MakeGenericType(string typeName, ExpressionBuilder builder, out El
return genericType.MakeGenericType(inspectBuilder.ObservableType).AssemblyQualifiedName;
}

public void InsertGraphNode(string typeName, ElementCategory elementCategory, CreateGraphNodeType nodeType, bool branch, bool group)
private void ConfigureBuilder(ExpressionBuilder builder, GraphNode selectedNode, string arguments)
{
InsertGraphNode(typeName, elementCategory, nodeType, branch, group, null);
if (string.IsNullOrEmpty(arguments)) return;
// TODO: This special case for binary operator operands should be avoided in the future
if (builder is BinaryOperatorBuilder binaryOperator && selectedNode != null)
{
if (GetGraphNodeTag(selectedNode).Value is InspectBuilder inputBuilder &&
inputBuilder.ObservableType != null)
{
binaryOperator.Build(Expression.Parameter(typeof(IObservable<>).MakeGenericType(inputBuilder.ObservableType)));
}
}

var workflowElement = ExpressionBuilder.GetWorkflowElement(builder);
var defaultProperty = TypeDescriptor.GetDefaultProperty(workflowElement);
if (defaultProperty != null &&
!defaultProperty.IsReadOnly &&
defaultProperty.Converter != null &&
defaultProperty.Converter.CanConvertFrom(typeof(string)))
{
try
{
var context = new TypeDescriptorContext(workflowElement, defaultProperty, serviceProvider);
var propertyValue = defaultProperty.Converter.ConvertFromString(context, arguments);
defaultProperty.SetValue(workflowElement, propertyValue);
}
catch (Exception ex)
{
throw new SystemException(ex.Message, ex);
}
}
}

public void InsertGraphNode(string typeName, ElementCategory elementCategory, CreateGraphNodeType nodeType, bool branch, bool group, string arguments)
public void CreateGraphNode(
string typeName,
ElementCategory elementCategory,
CreateGraphNodeType nodeType,
bool branch,
bool group)
{
CreateGraphNode(typeName, elementCategory, nodeType, branch, group, arguments: null);
}

public void CreateGraphNode(
string typeName,
ElementCategory elementCategory,
CreateGraphNodeType nodeType,
bool branch,
bool group,
string arguments)
{
if (string.IsNullOrEmpty(typeName))
{
Expand Down Expand Up @@ -1017,11 +1064,6 @@ public void InsertGraphNode(string typeName, ElementCategory elementCategory, Cr

builder = CreateBuilder(typeName, elementCategory, group);
ConfigureBuilder(builder, selectedNode, arguments);
if (typeName == typeof(ExternalizedMappingBuilder).AssemblyQualifiedName ||
typeName == typeof(AnnotationBuilder).AssemblyQualifiedName)
{
nodeType = CreateGraphNodeType.Predecessor;
}
var commands = GetCreateGraphNodeCommands(builder, selectedNodes.Select(GetGraphNodeTag), nodeType, branch);
commandExecutor.BeginCompositeCommand();
commandExecutor.Execute(EmptyAction, commands.updateLayout.Undo);
Expand All @@ -1031,39 +1073,6 @@ public void InsertGraphNode(string typeName, ElementCategory elementCategory, Cr
commandExecutor.EndCompositeCommand();
}

private void ConfigureBuilder(ExpressionBuilder builder, GraphNode selectedNode, string arguments)
{
if (string.IsNullOrEmpty(arguments)) return;
// TODO: This special case for binary operator operands should be avoided in the future
if (builder is BinaryOperatorBuilder binaryOperator && selectedNode != null)
{
if (GetGraphNodeTag(selectedNode).Value is InspectBuilder inputBuilder &&
inputBuilder.ObservableType != null)
{
binaryOperator.Build(Expression.Parameter(typeof(IObservable<>).MakeGenericType(inputBuilder.ObservableType)));
}
}

var workflowElement = ExpressionBuilder.GetWorkflowElement(builder);
var defaultProperty = TypeDescriptor.GetDefaultProperty(workflowElement);
if (defaultProperty != null &&
!defaultProperty.IsReadOnly &&
defaultProperty.Converter != null &&
defaultProperty.Converter.CanConvertFrom(typeof(string)))
{
try
{
var context = new TypeDescriptorContext(workflowElement, defaultProperty, serviceProvider);
var propertyValue = defaultProperty.Converter.ConvertFromString(context, arguments);
defaultProperty.SetValue(workflowElement, propertyValue);
}
catch (Exception ex)
{
throw new SystemException(ex.Message, ex);
}
}
}

public void CreateGraphNode(
ExpressionBuilder builder,
GraphNode selectedNode,
Expand Down Expand Up @@ -1154,6 +1163,13 @@ void ConfigureWorkflowBuilder(
var inspectParameter = new ExpressionBuilderArgument();

var targetNodes = selectedNodes.ToArray();
if (targetNodes.Length > 0 &&
(builder is ExternalizedMappingBuilder ||
builder is AnnotationBuilder))
{
nodeType = CreateGraphNodeType.Predecessor;
}

var restoreSelectedNodes = CreateUpdateSelectionDelegate(targetNodes);
if (insertIndex < 0)
{
Expand Down Expand Up @@ -1193,7 +1209,7 @@ void ConfigureWorkflowBuilder(
_ => false
}));

var reorder = GetSimpleSort();
var reorder = GetReversibleSort();
var updateGraphLayout = CreateUpdateGraphLayoutDelegate();
var updateSelectedNode = CreateUpdateSelectionDelegate(builder);
var insertCommands = GetInsertGraphNodeCommands(inspectNode, inspectNode, targetNodes, nodeType, branch, validateInsert);
Expand Down
4 changes: 2 additions & 2 deletions Bonsai.Editor/GraphView/WorkflowGraphView.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ public void CreateGraphNode(
bool group,
string arguments)
{
try { Editor.InsertGraphNode(typeName, elementCategory, nodeType, branch, group, arguments); }
try { Editor.CreateGraphNode(typeName, elementCategory, nodeType, branch, group, arguments); }
catch (TargetInvocationException e)
{
var message = string.Format(Resources.CreateTypeNode_Error, name, e.InnerException.Message);
Expand Down Expand Up @@ -1692,7 +1692,7 @@ private ToolStripMenuItem CreatePropertySourceMenuItem(
valueProperty.SetValue(propertySource, memberValue);
propertySource.MemberName = memberName;

Editor.CreateGraphNode(propertySource, default(GraphNode), CreateGraphNodeType.Predecessor, branch: true);
Editor.CreateGraphNode(propertySource, default(GraphNode), CreateGraphNodeType.Successor, branch: true);
contextMenuStrip.Close(ToolStripDropDownCloseReason.ItemClicked);
});

Expand Down

0 comments on commit 3ad6c5d

Please sign in to comment.