Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add dynamic drag-and-drop connections between objects on designer workflows (with x6) #3404

Conversation

karavajka
Copy link

No description provided.

karavajka and others added 23 commits October 13, 2022 16:25
@tanelkuhi
Copy link

So by default x6 graphs will not be enabled. To enable them, you need to edit package.json and add "--x6graph" argument to the start and build scripts.

@sfmskywalker
Copy link
Member

Hi @karavajka, I tested your work and it is looking really good. I did find the following two issues:

  1. I'm unable to draw a selection rectangle to select activities.
  2. When I add a new activity, the zoom level of the canvas appears to be set to "zoom to fit" (see recording)

Thanks for the hard work!

@sfmskywalker
Copy link
Member

I'm unable to draw a selection rectangle to select activities.

I just found out that it works when I press & hold the SHIFT key, so it is actually possible to draw a selection rectangle.
I suggest that this is enabled by default when a user starts dragging.

@sfmskywalker
Copy link
Member

A few more items (some are taken from the other PR):

  • Need auto-layout (for workflows created from code) - this feature was recently pushed to the v3 branch via 46d4b4d
  • When executing Test, an orange bar appears where it does not belong when zooming & panning

image

image

@sfmskywalker
Copy link
Member

  • When executing Test, an orange bar appears where it does not belong when zooming & panning

This does not seem to be a problem for me. The picture is also not from the recent changes it seems, since the down arrow does not exist on the activity node.

Indeed, it only happens when zooming out it seems. The screenshot was take from the other PR, so it's true that it' not exactly the same.

@tanelkuhi
Copy link

tanelkuhi commented Nov 9, 2022

  • When executing Test, an orange bar appears where it does not belong when zooming & panning

This does not seem to be a problem for me. The picture is also not from the recent changes it seems, since the down arrow does not exist on the activity node.

Indeed, it only happens when zooming out it seems. The screenshot was take from the other PR, so it's true that it' not exactly the same.

Okay I can see it now, when scrolling 100% out, but only when the 0-border exists I think.

@sfmskywalker
Copy link
Member

About the zoom to fit, when adding a new activity is what I made by design, since it seemed like a good solution. The problem was that when adding a new activity, it needs to be created to a predictable position, and the top left corner is probably a good one. But since I could not figure out how to easily move the view to the created node, I made it zoom to fit the created node. But if you have a better idea how to solve this, please let me know.

Understood. The problem with this however is that it is not a good UX for the diagram to change like that when adding activities.

A better approach would be to position the activity by taking a screen coordinate of e.g. 250,250 and translate that to local coordinates (local to the design surface).

For example, if the design surface is 10000x10000 pixels, and the user has panned to e.g. 5000x1000, the position of the new activity would be 5250x1250. The zoom level has to be taken into account also.

The v3 branch contains logic to translate from the "drop" position to local coordinates, so it would be the same. The only difference is that in your case, you hard-code the "drop position" to e.g. 250x250.

An alternative approach could be to get rid of the green Add Activity button entirely and add a panel to the left that represents the activity picker, from which users can now drag & drop activities directly Obviously, this would be more work, but more consistent with the V3 designer also.

@tanelkuhi
Copy link

tanelkuhi commented Nov 10, 2022

Now with getting auto-layout working, there is a small/big problem. In v3 this @antv/layout plays along fine, but in v2 there is a conflict of packages. Namely because @antv/layout has a dependency for dagre-compound.
In v2 we already are using a package named dagre. The problem now arises due to dagre-compound declaring a module named dagre. Now, the dagre-d3 package, that exists in v2, imports things from dagre and the module that dagre-compound creates with the same name gets found before the dagre package and now the build breaks inside the dagre-d3 index file.
image

We put some hours into trying to somehow getting it to cooperate, but to no avail. Maybe you have some ideas, which do not require days to implement. Because otherwise, this feature should perhaps be left to another time.

@tanelkuhi
Copy link

tanelkuhi commented Nov 10, 2022

Except for the layout one, the issues you mentioned, should be solved. I solved the zoom to fit problem, by just creating the activity node to the corner of the view.

@tanelkuhi
Copy link

tanelkuhi commented Nov 11, 2022

About the layout problem. I think I found a solution by using the dagre library that already exists inside v2 directly. It looks like it is giving very good results. I will commit it soon.

@sfmskywalker
Copy link
Member

About the layout problem. I think I found a solution by using the dagre library that already exists inside v2 directly. It looks like it is giving very good results. I will commit it soon.

That's a clever solution 👍🏻

@sfmskywalker
Copy link
Member

sfmskywalker commented Nov 11, 2022

I noticed the following behavior when opening a workflow definition where the connections appear to be starting from the top-left corner of an activity. But as soon as I move around an activity, the connections are replaced correctly. Please take a look at the following recording to see what I mean:

Screen.Recording.2022-11-11.at.21.38.29.mov

@sfmskywalker
Copy link
Member

I tried the switching between workflow versions, and although it now indeed reflects the currently selected version in the designer, the autolayout logic gets applied as well, which is not desirable: when a user positions activities, those positions should not be overridden by autolayout. Here are the rules for applying autolayout:

  1. When opening a workflow definition in the designer, autolayout should only be applied manually, when the user clicks a button (like "Autolayout" in Elsa 3 designer).
  2. When opening a programmatic workflow (via Workflow Registry), autolayout should be applied automatically, since a programmatic workflow does not contain positional information about its activities.
  3. As with 2, when the user opens the workflow instance viewer, autolayout should be applied automatically only if the workflow is a programmatic workflow.

Here's a recording to demonstrate the issue:

Screen.Recording.2022-11-11.at.21.46.10.mov

@sfmskywalker
Copy link
Member

When running the workflow from the designer, the status icons have disappeared. To show what it should look like, please take a look at the following recording:

Screen.Recording.2022-11-11.at.21.58.45.mov

Notice the symbols on the top-right of each activity to indicate that an activity has executed, is executing or has faulted.

@sfmskywalker
Copy link
Member

When I create a workflow and add the RunJavaScript activity, run the workflow and try to view the workflow instance, the design canvas appears to be empty. This is most likely due to the JS error visible in the console (duplicate port ID detected).

Please see the following recording to reproduce the issue:

Screen.Recording.2022-11-11.at.22.10.03.mov

@tanelkuhi
Copy link

I think I managed to fix these issues to a satisfactory degree. Please have a look.

@sfmskywalker
Copy link
Member

@tanelkuhi I just pulled the latest updates, and noticed the following:
image

Are you getting this too?

@sfmskywalker
Copy link
Member

@tanelkuhi I can confirm that you fixed all the issues mentioned 🎉
If you could take a look at the last one I just posted, then that would seal the deal ;)

Thank you for the hard work!

@tanelkuhi
Copy link

Actually the graph was centering content even before, but just like the connections being broken, when opened from the list (not when refreshing on the page), it was broken as well. So I fixed it. But I realize, centering, when there are no activities, is probably not the best idea.

@sfmskywalker
Copy link
Member

Nice. Then let's get this merged in 👍🏻

@tanelkuhi
Copy link

It seems, I don't have the rights to merge it, so can you do it, please?

@sfmskywalker sfmskywalker merged commit 1f9bf18 into elsa-workflows:master Nov 16, 2022
@sfmskywalker
Copy link
Member

Yeah, sorry that's what I meant: I'll get it merged :)

@tomy2105
Copy link
Contributor

tomy2105 commented Nov 16, 2022

@tanelkuhi can you please, just for reference, write here a clear example what needs to be changed in html (or wherever else) for this change to be activated?

@tanelkuhi
Copy link

tanelkuhi commented Nov 17, 2022

@tomy2105 For x6 graphs to be activated, in the workflow studio index.html file, you have to attach an attribute named use-x6-graphs to the elsa-studio-root element. And that is it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants