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

Collapsing a pool breaks sequence flow layout #1033

Closed
Abendstolz opened this Issue Nov 27, 2018 · 12 comments

Comments

Projects
None yet
4 participants
@Abendstolz
Copy link

Abendstolz commented Nov 27, 2018

Describe the Bug

Collapsing a pool (kinda) breaks rendering (increases height of whole diagram by an unhealthy amount)

process before collapsing

process after collapsing - rendering bug

Steps to Reproduce

Steps to reproduce the behaviour:

  1. Open example diagram
  2. Collapse any participant
  3. Left over sequence flow has funny behavior

Expected Behaviour

Sequence flow is re-layouted or removed entirely ?.

Environment

Please complete the following information:

  • OS: 16.04.1-Ubuntu (but same on MacOS something)
  • Camunda Modeler Version: >= 2.0

@nevries fyi, not that I think it's important, but maybe the MacOS version can be added by you, if necessary.

Archive containing original broken diagram: camunda_modeler_2.2.1_bug_report.zip

@barmac

This comment has been minimized.

Copy link
Contributor

barmac commented Nov 29, 2018

Thank you for your bug report.

Confirmed also with 2.2.0, 2.2.2, 2.1.2, 2.0.3 and 2.0.0 on macOS@10.14.1. It seems like the last version which didn't have this bug was 1.16.2.

@barmac barmac added bug ready labels Nov 29, 2018

@nikku

This comment has been minimized.

Copy link
Member

nikku commented Nov 29, 2018

I'll have a look into this one.

@nikku

This comment has been minimized.

Copy link
Member

nikku commented Nov 29, 2018

image

😯

Most simple diagram to reproduce this: BpmnReplace.poolMessageFlows.bpmn.txt

@nikku

This comment has been minimized.

Copy link
Member

nikku commented Nov 29, 2018

Update: This is not a bug but a feature.

Consider the rendered diagram:

image

Both message flows are being joined as Task_A is being removed. They are not parallel to each other but cross each other at { x: 366, y: -7906 }. Because of this the are being joined at that coordinate before being transformed to a bpmn:SequenceFlow later in the process.

You could argue that what is shown to the user is not a practical though.

🚶

@nikku nikku self-assigned this Nov 29, 2018

@nikku

This comment has been minimized.

Copy link
Member

nikku commented Nov 29, 2018

Moving this back to our backlog and flagging it as a potential improvement.

@nikku nikku changed the title Collapsing a pool breaks rendering in version 2.2.1 Collapsing a pool breaks sequence flow layout Nov 29, 2018

@nikku

This comment has been minimized.

Copy link
Member

nikku commented Nov 29, 2018

@Abendstolz, @nevries What should be the desired behavior after the pool is collapsed, given the example diagram I pasted above: (A) Sequence Flow is gone or (B) Sequence Flow is now looping around remaining task.

@nikku nikku added help wanted and removed backlog labels Nov 29, 2018

@nevries

This comment has been minimized.

Copy link
Member

nevries commented Nov 29, 2018

@nikku I'm a bit lost, why does the current implementation create a sequence flow out of the two message flows at all? From a modelling perspective, that makes no sense to me, or am I missing something?

@nikku

This comment has been minimized.

Copy link
Member

nikku commented Nov 30, 2018

You are opting for: (A) Sequence Flow is gone.

In this case you could argue there is a bug that causes this behavior. We should not attempt to join message flows and remove them on pool collapsing.

@nikku

This comment has been minimized.

Copy link
Member

nikku commented Nov 30, 2018

Relevant toolkit bug ticket: bpmn-io/bpmn-js#917

@nikku nikku added bug and removed enhancement help wanted labels Nov 30, 2018

@nikku

This comment has been minimized.

Copy link
Member

nikku commented Nov 30, 2018

Reclassified this as a bug.

nikku added a commit that referenced this issue Nov 30, 2018

@nikku

This comment has been minimized.

Copy link
Member

nikku commented Nov 30, 2018

Closed via 66d579c.

@nikku nikku closed this Nov 30, 2018

@nikku

This comment has been minimized.

Copy link
Member

nikku commented Nov 30, 2018

Fixed in Camunda Modeler v2.2.3.

@nikku nikku added this to the M25 milestone Nov 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment