Workflow Diagram: "edge merging" (new edge creation)

[josephjclark]

Allow new edges to be created in the workflow diagram. Also enables the removal of edges (removing an edge should not remove the steps at either end)

Edges can only be created between two steps:

• If they would not create a circular workflow
• If no edge exists

Changes in this PR

• Use dagre layout instead of hierachy
• Add a new hover button for creating edges
• Create new edges on drop
• Visual indicator for valid drop targets
• Don't allow edges that would create loops, or duplicate edges, or connection to triggers
• Show an error message if an edge cannot be created

Issue

#2008 #2006

TODO

• Turns out edges are still created if you drop on an invalid node!
• Allow edges to be removed
• Get save working properly
• Don't auto layout
• edges don't connect to the node now
• tweak layout because it's a bit spacey
• Maybe we should style the source edge and links while dragging
• it would be neat for the edge to not be drawn over any nodes (the source or target), ie, the line goes behind or is clipped by the source and target handles
• Better feedback when hovering the connector over an invalid node (a message would be ideal)
• Use curved edges generally
• Layout doesn't behave nicely on adding of new nodes (improved but a dagre bug is problematic)
• Add control bar (zoom fit, disable autofit, zoom in/out, maybe layout)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.61%. Comparing base (68c8c04) to head (8eacc59).
Report is 10 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2160      +/-   ##
==========================================
+ Coverage   89.60%   89.61%   +0.01%     
==========================================
  Files         269      269              
  Lines        8947     8958      +11     
==========================================
+ Hits         8017     8028      +11     
  Misses        930      930              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[josephjclark]

Removal of edges is going to have to happen from the lightning side I think - just like how we remove steps. I'll need help with that.

[josephjclark]

Encountered a big problem with the layout.

We are using the dagre algorithm to run layouts now. This can handle simple graphs in a way that d3's tree layout cannot (to be fair I haven't actually tested this..)

The problem with dagre is that when you add nodes, it has tendency to switch nodes orders around more or less at random. Looks like d3 is doing this in production too - maybe not quite as badly as dagre on this branch.

Now we can fix this by setting disableOptimalOrderHeuristic on the dagre layout, like: Dagre.layout(g, { disableOptimalOrderHeuristic: true });. The layout is much, much more stable like this.

This option was exposed in fairly recent PR: dagrejs/dagre#263. I've reproduced this locally by hacking the code, so I know it works.

But here's the fun bit! Due to what looks like a bug in the release, the options object doesn't actually get passed through to the layout algorithm. It basically looks like half the fix PR got merge conflicted out, or something. It's totally borked.

Dagre is basically in maintenance mode and only gets very sporadic updates. I don't expect much of a response if we turn to the community.

So here are our options:

• We fork dagre into the openfn org and make the changes we need. This is likely to be fine and won't take long.
• We use someone else'se fork - there is at least one on npm. We have to vet it to be sure what's in it.
• We copy/paste a build of dagre into the lightning repo and support it like a lib file. We could copy source or a dist file (it'll get built into our eventual production distribution anyway).
• We live with the annoying ordering thing
• We look for an alternative to dagre

What do you reckon @stuartc ? I say FORK IT

[josephjclark]

I've added controls. I kinda think the options we need are:

• Fit view
• Autofit view (toggle)
• Layout
• Auto layout

Which is too many buttons really. But both layout and view you need to be able to separately run once and set the behaviour when adding new nodes/edges.

The behaviour for adding new nodes with view-fit off is terrible by the way. Especially if the layout goes haywire.

I wonder if the auto-view behavior is: zoom to fit all nodes visible now, PLUS this new one. That seems like a good compromise but it's a new feature really. Might be hard to implement.

I'd really, really like to avoid supporting manual layout.

[josephjclark]

Just for the record, if we use d3 hierarchy, it seems to only use the first edge to work out the hierarchy. So it basically won't layout at all, it assumes a single parent

[josephjclark]

So we just need to support edge removal and this is good to go.

I am going to take a look at a better autofit in another PR tomorrow - I have a feeling I can turn it around quickly. But it should not block this release.

Suggest we stick with autolayout for now