Skip to content

[FEAT] Render Catch Message Intermediate Event #238

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.

Sign up for GitHub

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

Jump to bottom
Merged
tbouffard merged 4 commits into from
May 12, 2020
Merged

[FEAT] Render Catch Message Intermediate Event #238

tbouffard merged 4 commits into from
May 12, 2020

Conversation

@csouchet
Copy link
Contributor

@csouchet csouchet commented May 6, 2020 •
edited
Loading

Closes #216

@csouchet csouchet added enhancement New feature or request depends on another PR ⚠️ Pull request depending on another one. The depending must be merged first BPMN rendering Something about the way the lib is rendering BPMN elements labels May 6, 2020
@csouchet
Copy link
Contributor Author

csouchet commented May 6, 2020

Final render
image

@csouchet csouchet marked this pull request as ready for review May 6, 2020 09:36
@csouchet csouchet requested review from aibcmars and tbouffard May 6, 2020 09:47
@csouchet csouchet force-pushed the 216-Render_Catch_Message_Intermediate_Event branch from 2be3244 to 1d76271 Compare May 6, 2020 16:48
@csouchet csouchet force-pushed the 216-Render_Catch_Message_Intermediate_Event branch from 1d76271 to 43ad04f Compare May 6, 2020 16:51
@csouchet csouchet removed the depends on another PR ⚠️ Pull request depending on another one. The depending must be merged first label May 6, 2020
@csouchet csouchet mentioned this pull request May 6, 2020
Merged
@tbouffard tbouffard marked this pull request as draft May 8, 2020 05:02
@tbouffard tbouffard self-assigned this May 8, 2020
@tbouffard
[REFACTOR] generalize icons management
a30ba36 
Introduce a declarative way for the bpmn event render icon method selection
This uses a more functional way of doing things instead of having several
if/else or switch which brings more clarity and will simplify extensions.

Simplify how we handle catch/throw icon render
This is now hold by the Shape class itself allowing icon render method to easily
know if a Throw or Catch icon has to be painted.
Prior this change, we had to pass argument to several methods which made the
code hard to follow.

Introduce a single place to manage events currently displayed with a arbitrary
fill color prior we implement their final icon.
This was prior done at several places which make the code hard to follow and was
error prone: we have broken the render of such BPMN events several times in the
past.
@tbouffard tbouffard force-pushed the 216-Render_Catch_Message_Intermediate_Event branch from d4e6f48 to a30ba36 Compare May 9, 2020 05:18
@tbouffard tbouffard removed their assignment May 9, 2020
@tbouffard tbouffard marked this pull request as ready for review May 9, 2020 05:18
@tbouffard
Copy link
Member

tbouffard commented May 9, 2020

Render with commit a30ba36
This confirms that the render of already supported BPMN events is ok.
BPMN events that are only displayed with an arbitrary colour (for instance timer events, message start and end events) are also rendered correctly.

BPMN example file: all_events_types.bpmn.zip

all_events_types_commit_a30ba36

csouchet
csouchet commented May 12, 2020
View reviewed changes
@tbouffard tbouffard merged commit 292c91b into master May 12, 2020
@tbouffard tbouffard deleted the 216-Render_Catch_Message_Intermediate_Event branch May 12, 2020 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tbouffard tbouffard tbouffard approved these changes

+1 more reviewer

@aibcmars aibcmars aibcmars approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

BPMN rendering Something about the way the lib is rendering BPMN elements enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[FEAT] Render Catch Message Intermediate Event

4 participants

@csouchet @tbouffard @aibcmars