-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Delete Vertical Lanes #2082
Delete Vertical Lanes #2082
Conversation
Could you please share a diagram in which you got this result? I cannot reproduce it on my own. |
Sure, it's test/spec/features/modeling/lanes/lanes.bpmn I cannot reproduce it with a newly created pool. Maybe it's because of the odd dimensions in that model. |
Thank you! I can reproduce it now. We should make sure that the child lanes don't exceed the parent lane bounds. |
I moved this to a separate issue as it's not required for this PR: #2083 |
var isHorizontalLane = isHorizontal(shape); | ||
|
||
compensateLaneDelete(shape, oldParent, isHorizontalLane); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather move the isHorizontal
call to the inside of compensateLaneDelete
. There's no case where we'd pass isHorizontalLane=false
for horizontal lanes to this function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will do that as a quick change.
BTW this bug can be noticed on the vertical lanes fixture that you're adding in this PR. |
There is a width missmatch in the DI:
Notice that the child lanes' widths sum up to 604 while the parent is 603. |
I will fix the widths as well. |
a47867b
to
b2a8aee
Compare
b2a8aee
to
d306dcd
Compare
We will make it a |
Thank you @sombrek for your another great contribution! |
Closes #2081
This is a small independent fix.