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

Auto Placement should not put elements on top of existing flows #757

Open
1 task
romero83 opened this issue Jan 29, 2018 · 18 comments
Open
1 task

Auto Placement should not put elements on top of existing flows #757

romero83 opened this issue Jan 29, 2018 · 18 comments
Labels
backlog Queued in backlog modeling spring cleaning Could be cleaned up one day ux

Comments

@romero83
Copy link

romero83 commented Jan 29, 2018

The auto placement feature may place elements on top of existing sequence flows:

element_auto_placement

We should circumvent this issue, as this particular situation is hard to grasp for users.

Tasks

  • Update Auto Placement to not put elements on top of existing sequence flows

Original Bug Report:

Hi!

I am using bpmn-js version 0.26.4.
When I want to create a new inner element from the previous element context pad (which already has an output sequence flow line), if there is enough space to the element creation but there is already an sequence flow line placed, it creates the element on the sequence flow line which is annoying and can lead to hardly interpretable and visible business flow.

Expected Behavior

  • Should merge the newly created element into the already placed sequence flow.
  • Can be that the new element placed beside the sequence line.
  • The user cannot create element from the element context pad if it already has an outgoing sequence flow. It is still possible to create new element from the left menu.
  • I don't know.

Actual Behavior
Newly created element placed on the sequence flow line.

Steps to reproduce the Behavior

  1. Create a new BPMN
  2. From the start event, create and end event from the element's context pad
  3. Move the end event horizontally far enough to let space to the element which will be created
  4. Click on the start event and from the context pad, create a new element

See
element_auto_placement

@nikku
Copy link
Member

nikku commented Jan 30, 2018

Thanks for reporting this issue.

From the alternatives you've provided I'd personally go with:

Expected Behavior

  • Task is placed below the existing sequence flow

@nikku nikku added this to the Backlog milestone Jan 30, 2018
@nikku nikku added the backlog Queued in backlog label Feb 14, 2018
@nikku nikku removed this from the Backlog milestone Feb 14, 2018
@romero83
Copy link
Author

Hi!

Unfortunately I found a new major problem in this particular bug which is not anymore just an UX problem.
Stick to the example gif what I reported. When user creates the task between the start and the end event the program creates a new sequence flow from the start event to the newly created task.
However when user "merge" this task after back to the original flow it duplicates the sequence flow while on the surface you see only one sequence flow line! It cause major problem when user tries to run this kind of workflow process!
It might can be fixed also in engine side (backend) to validate proper source XML before send it into engine to filter out these kind of problems.

@nikku
Copy link
Member

nikku commented Mar 22, 2018

Yea, that is indeed an issue. A different one though.

@nikku
Copy link
Member

nikku commented Mar 22, 2018

How would you address this from the UI side of things?

@romero83
Copy link
Author

The auto placement stuff works if the task has no out sequence flow. In that case it cannot be a problem because it will be sure that only one out sequence flow will be placed. But If a task has an out sequence flow in that case program should fall back to user side to manually drop the task.

@nikku
Copy link
Member

nikku commented Mar 23, 2018

[...] But If a task has an out sequence flow in that case program should fall back to user side to manually drop the task.

We cannot go with this option, as it is not gonna be understandable for the user.

However, I consider changing the drop on flow behavior to remove duplicate incoming and outgoing connections (as I cannot figure out a scenario where these would make sense).

@romero83
Copy link
Author

Multiple outcoming sequence flows can be at start gateways.
Multiple incoming sequence flows can be at end gateways.

@romero83
Copy link
Author

Or do not take it intolerant its just an option but please consider also to drop this kind of auto placement stuff and revert back to that method where user decided manually where to put the task in the flow.
Yes, in that case user has to do an extra click and cannot fast build a "one-liner" process, but on the other side user get extra safety and precision to build the process which I think is the most important thing in a business flow.

@nikku
Copy link
Member

nikku commented Mar 23, 2018

Did you try dragging out elements from the context menu? It achieves exactly what you mentioned:

  • dragging out = full control where to put it (old behavior)
  • clicking = placing automatically at an appropriate place

@nikku
Copy link
Member

nikku commented Mar 23, 2018

A small visualization regarding my previous comment:

However, I consider changing the drop on flow behavior to remove duplicate incoming and outgoing connections (as I cannot figure out a scenario where these would make sense).

image

As you can see, there are certain cases where sequence flows are not equivalent (i.e. because they are default or have conditions assigned). There are other cases though where it makes absolutely no sense to keep these (scenario two). From the BPMN prespective this would otherwise be equivalent to a AND gateway.

@nikku
Copy link
Member

nikku commented Mar 23, 2018

As mentioned, I believe both issues should be tackled / investigated separately.

I've opened #774 for the sequence flow duplication issue.

@romero83
Copy link
Author

dragging out = full control where to put it (old behavior)
clicking = placing automatically at an appropriate place

I did not know about it. Thanks for pointing it out.

@nikku
Copy link
Member

nikku commented Mar 23, 2018

So I guess we can close this issue?

@romero83
Copy link
Author

It is still a problem that the automatic placement render the task on the sequence flow (like it was merged) and hard to see what is happening.

@nikku
Copy link
Member

nikku commented Apr 11, 2018

Additional feedback we got convinced me that this is an actual issue.

We'll improve auto-placement to ensure elements are not being dropped on incoming or outgoing sequence flows.

@nikku
Copy link
Member

nikku commented Jul 25, 2018

Parts of this issue, namely the duplication of sequence flows got addressed via #774.

@nikku nikku changed the title Element Auto Placement rendering confusion Auto Placement should not put elements on top of existing flows Jul 25, 2018
@nikku
Copy link
Member

nikku commented Jul 25, 2018

I've updated the issue description to reflect the work left to close this issue.

@pinussilvestrus pinussilvestrus added the spring cleaning Could be cleaned up one day label Feb 28, 2020
@christian-konrad
Copy link

This is part of refactoring actions (as it applies mostly to the "modify" phase of the developer/modeler job journey).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Queued in backlog modeling spring cleaning Could be cleaned up one day ux
Development

No branches or pull requests

4 participants