Skip to content

[FEAT] Render Service task #208

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
csouchet merged 21 commits into from
Apr 29, 2020
Merged

[FEAT] Render Service task #208

csouchet merged 21 commits into from
Apr 29, 2020

Conversation

@csouchet
Copy link
Contributor

@csouchet csouchet commented Apr 22, 2020 •
edited by tbouffard
Loading

Closes #196

@csouchet csouchet added enhancement New feature or request BPMN rendering Something about the way the lib is rendering BPMN elements depends on another PR ⚠️ Pull request depending on another one. The depending must be merged first labels Apr 22, 2020
@csouchet csouchet mentioned this pull request Apr 22, 2020
Merged
@csouchet csouchet force-pushed the 196-Render_Service_Task branch from d507870 to 6a04488 Compare April 22, 2020 15:30
@tbouffard tbouffard added the rebase needed 💥 Pull request that must be rebased on the latest master commit prior being reviewed or merged label Apr 22, 2020
@tbouffard tbouffard force-pushed the 196-Render_Service_Task branch from ec361ee to 6af4423 Compare April 22, 2020 21:48
@tbouffard tbouffard removed depends on another PR ⚠️ Pull request depending on another one. The depending must be merged first rebase needed 💥 Pull request that must be rebased on the latest master commit prior being reviewed or merged labels Apr 22, 2020
@tbouffard
Copy link
Member

tbouffard commented Apr 27, 2020

Pior service task rendering
step-02__prior_service_task

New service task rendering
step-03__with_service_task

@tbouffard
Copy link
Member

tbouffard commented Apr 27, 2020 •
edited
Loading

Implementation information about icon adjustments done in this PR

In the example process, it was initially hard to distinguish the 2 gears of the Service Task icon
service_task_icon_adjustments_00_original_process

Here is a zoom on the Service Task and its original icon
service_task_icon_adjustments_01_original_icon

Let's fill the background and foreground icon elements; this highlights the part of the background element displayed within the 'hole' of the foreground element
service_task_icon_adjustments_02_highlighted

Translate the foreground element to the right bottom direction: we see more of the background element
service_task_icon_adjustments_03_foreground_gear_translation

Finally, let fill the 'hole' of the foreground element to hide the background in that area
service_task_icon_adjustments_04_foreground_gear_fill_inner_circle

Fill everything in white, and Voilà!
service_task_icon_adjustments_05_final_white

About the icon size
The icon fully depends on the size of the BPMN Element for both its position (top left coordinates depends on the width and heigth of the element) and its size
See also #211

@tbouffard tbouffard mentioned this pull request Apr 27, 2020
Merged
@tbouffard tbouffard requested a review from NathalieC April 27, 2020 15:03
@tbouffard tbouffard mentioned this pull request Apr 28, 2020
Open
@tbouffard tbouffard marked this pull request as ready for review April 28, 2020 04:26
@tbouffard tbouffard mentioned this pull request Apr 28, 2020
Merged
@csouchet csouchet requested a review from aibcmars April 28, 2020 13:19
csouchet and others added 13 commits April 28, 2020 15:26
@csouchet
Add Service task render without icon
c823522
@csouchet
Add e2e for Service Task render
414d84f
@csouchet
Remove bad refactor
a565d8a
@csouchet
Refactor tasks shape
1b91288
@csouchet
Fix lint check
c6ef190
@csouchet
Update doc
90bf212
@csouchet
Update doc
c16d610
@tbouffard @csouchet
refactor: ShapeUtil - rename the EVENT_KINDS const
908d70c 
the plural was not put at the right place
@tbouffard @csouchet
refactor: task styles generalization
c5154ed 
Store the array of tasks in ShapeUtil
allow to ignore the suffix case when building array of bpmn kinds
@tbouffard @csouchet
refactor: rename base task shape
148edb8 
Do not use 'abstract' in the shape name: this could lead to misunderstanding
as BPMN uses 'AbstractTask' for the generic 'Task'
@tbouffard @csouchet
e2e: rename ids of service tasks elements and shapes for consistency
972de45
@tbouffard @csouchet
1st service task icon rendering
6828006
@tbouffard @csouchet
refactoring: introduce a mxXmlCanvas2D to manage scale factor
6153439 
The adapter passes the scale values to the underlying canvas, so writing code is
much more simpler: we pass regular values to the adapter instead of computing
the scaled values each time prior to pass them
@csouchet csouchet force-pushed the 196-Render_Service_Task branch from 10e9159 to 0f431df Compare April 28, 2020 13:31
aibcmars
aibcmars approved these changes Apr 28, 2020
View reviewed changes
Copy link
Contributor

@aibcmars aibcmars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@csouchet csouchet merged commit 26011b8 into master Apr 29, 2020
@csouchet csouchet deleted the 196-Render_Service_Task branch April 29, 2020 07:41
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

@NathalieC NathalieC Awaiting requested review from NathalieC

+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 Service task

4 participants

@csouchet @tbouffard @aibcmars