Skip to content

feat: render event subprocess icons#2293

Merged
barmac merged 1 commit intobpmn-io:developfrom
sombrek:event-subprocess-icons
Mar 17, 2025
Merged

feat: render event subprocess icons#2293
barmac merged 1 commit intobpmn-io:developfrom
sombrek:event-subprocess-icons

Conversation

@sombrek
Copy link
Copy Markdown
Contributor

@sombrek sombrek commented Mar 10, 2025

Closes #50

Draw event icons for collapsed event subprocesses.
No icon is drawn when a start event is missing or when there's more than one start event (forbidden by the spec).

Reuses existing code with few minor adjustments.

Demo (spec on the right):

evsub-demo

event-subprocess-icons.bpmn contains all possible icons.

@sombrek
Copy link
Copy Markdown
Contributor Author

sombrek commented Mar 10, 2025

There is one bug: When the start event type changes, the icon is not updated. It's only updated when the collapsed subprocess is moved afterwards. I have no idea how to fix this. Do you?

Either way I'd like to argue that this is a separate issue that can be fixed separately, because modeling collapsed event subprocesses is not promoted by bpmn-js and users are unlikely to encounter this problem.

@barmac barmac self-requested a review March 10, 2025 13:03
@barmac
Copy link
Copy Markdown
Member

barmac commented Mar 10, 2025

Hi @sombrek! Thanks for your contribution.

There is one bug: When the start event type changes, the icon is not updated. It's only updated when the collapsed subprocess is moved afterwards. I have no idea how to fix this. Do you?

The renderer is reactive in a sense that it redraws only the elements which it is prompted to redraw. The elements are passed by the Command Stack, and it receives them from the Command Handlers (e.g. https://github.com/bpmn-io/diagram-js/blob/develop/lib/features/modeling/cmd/CreateShapeHandler.js#L65). I checked and the event subprocess is not passed when event is replaced, and that's why it's not redrawn.

image

So the solution would be to mark the event subprocess as dirty when its child start event's type changes. It can be difficult since we are in a different diagram then the one where we need to redraw.

@barmac
Copy link
Copy Markdown
Member

barmac commented Mar 17, 2025

@sombrek do you still want to work on this PR? I am converting this to draft since some changes are still required, but feel free to comment/mark it as ready when you have an update.

@barmac barmac marked this pull request as draft March 17, 2025 14:05
@sombrek
Copy link
Copy Markdown
Contributor Author

sombrek commented Mar 17, 2025

@barmac I'd like to get the PR merged, but I do not see a way to fix the remaining issue.

How can re-drawing the parent diagram be triggered?
I've tried using modeling.updateProperties(newShape, {}) in a postExecuted function when the event subprocess changes, but it had no effect.

@barmac
Copy link
Copy Markdown
Member

barmac commented Mar 17, 2025

Hmm if a simple solution does not work, we could as well merge the pull request, and create an issue for a missing rendering update.

@barmac barmac force-pushed the event-subprocess-icons branch from 223b0e9 to 974a8b5 Compare March 17, 2025 14:39
@barmac barmac marked this pull request as ready for review March 17, 2025 14:58
@barmac barmac merged commit cfec0a9 into bpmn-io:develop Mar 17, 2025
8 checks passed
@barmac
Copy link
Copy Markdown
Member

barmac commented Mar 17, 2025

Follow-up issue: #2297

@barmac
Copy link
Copy Markdown
Member

barmac commented Mar 19, 2025

This is published in v18.4.0. Thank you @sombrek!

@sombrek
Copy link
Copy Markdown
Contributor Author

sombrek commented Mar 19, 2025

Cool. Thanks!

@sombrek sombrek deleted the event-subprocess-icons branch March 19, 2025 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Render start event marker in collapsed event subprocesses

2 participants