-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Support modeling of collapsed subprocesses #1553
Conversation
1dbfd76
to
b68f5fd
Compare
587408a
to
72e4f67
Compare
72e4f67
to
c7b8bb6
Compare
test/spec/features/modeling/behavior/SubProcessPlaneBehaviorSpec.js
Outdated
Show resolved
Hide resolved
|
||
// old plane could have content, | ||
// we remove it so it is not recursively deleted from 'shape.delete' | ||
context.oldRoot = canvas.removeRootElement(planeId(oldShape)); |
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.
Shall we not modeling#removeRoot
here?
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.
That would imply that we move the children first before removing the root.
Played around with the implementation and could not break it, too. Just found #1564 (but was not quickly able to figure out where on |
Fixed the markers upstream: bpmn-io/diagram-js#609 |
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.
Looks good enough for me 💪. Let us use this as a base line and adapt where needed in the future 🚀.
// do not resize plane elements: | ||
// root elements, collapsed sub-processes | ||
if (is(target.di, 'bpmndi:BPMNPlane')) { | ||
return false; |
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.
We do have test to verify this, correct? I.e. creating or moving elements on a root element?
Found a follow up issue: #1565. |
Found another follow up issue: #1566. |
First up: sorry for the big Pull Request. With the new diagram-js API I had to rework all Behaviors to work accordingly. I already have them ready and they build on each other, so I decided to just create one big PR. I tried to split the work into into sensible commits to make reviewing a bit easier.
This PR fixes 3 Things listed in #1443 :
If you want, we can also have a quick call and I can give you an overview of the changes.
closes #1511
closes #1536
closes #1539