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

Support modeling of collapsed subprocesses #1553

Merged
merged 13 commits into from
Jan 12, 2022
Merged

Conversation

marstamm
Copy link
Contributor

@marstamm marstamm commented Dec 14, 2021

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 :

  • Creation through replace
  • Toggle Collapse/Expand
  • deletion (recursive)

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

@bpmn-io-tasks bpmn-io-tasks bot added the in progress Currently worked on label Dec 14, 2021
@marstamm marstamm force-pushed the 1539-collapse-expand branch 2 times, most recently from 1dbfd76 to b68f5fd Compare December 15, 2021 17:44
@marstamm marstamm force-pushed the 1539-collapse-expand branch 2 times, most recently from 587408a to 72e4f67 Compare December 15, 2021 18:17
@marstamm marstamm changed the title Add collapse/expand behavior Support modeling of collapsed subprocesses Dec 15, 2021
@marstamm marstamm force-pushed the 1539-collapse-expand branch from 72e4f67 to c7b8bb6 Compare December 15, 2021 18:44
@marstamm marstamm marked this pull request as ready for review December 15, 2021 18:56
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Dec 15, 2021
@marstamm marstamm requested a review from nikku December 15, 2021 18:56

// old plane could have content,
// we remove it so it is not recursively deleted from 'shape.delete'
context.oldRoot = canvas.removeRootElement(planeId(oldShape));
Copy link
Member

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?

Copy link
Member

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.

@nikku
Copy link
Member

nikku commented Jan 10, 2022

Played around with the implementation and could not break it, too.

Just found #1564 (but was not quickly able to figure out where on bpmn-js@develop that broke).

@marstamm
Copy link
Contributor Author

Fixed the markers upstream: bpmn-io/diagram-js#609

@marstamm marstamm requested a review from nikku January 10, 2022 15:34
Copy link
Member

@nikku nikku left a 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;
Copy link
Member

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?

@nikku nikku merged commit 695c261 into develop Jan 12, 2022
@nikku nikku deleted the 1539-collapse-expand branch January 12, 2022 08:14
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Jan 12, 2022
@nikku
Copy link
Member

nikku commented Jan 12, 2022

Found a follow up issue: #1565.

@nikku
Copy link
Member

nikku commented Jan 12, 2022

Found another follow up issue: #1566.

@kurtsys-howest kurtsys-howest mentioned this pull request Jan 28, 2022
33 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants