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

[FEAT] Render signal event #408

Merged

Conversation

benjaminParisel
Copy link
Contributor

@benjaminParisel benjaminParisel commented Jul 8, 2020

When you merge your refactoring on the painter, i guess you need to manage conflicts 👀

closes #220
closes #221
closes #401
closes #402

Remaining tasks

  • icon position: centered
  • BPMN support doc update

* MISC icon is not really center
@tbouffard tbouffard added BPMN rendering Something about the way the lib is rendering BPMN elements enhancement New feature or request external contribution 👤 Pull requests provided by someone who is not a core maintainer WIP 🚧 Pull request in progress and/or not ready for review. Used in addition to marking it as draft labels Jul 8, 2020
@tbouffard
Copy link
Member

tbouffard commented Jul 8, 2020

Rendering with commit c8670ab

image

It includes a preview of the #404 refactoring. The icon is currently not centered, I have worked with @benjaminParisel on the topic but we currently don't know why the icon is not fully centered (issue on the scaling factor computation? something else?)
The stroke width of the triangle will be managed later

@tbouffard tbouffard changed the title [FEAT] Render signal start event [FEAT] Render signal event Jul 16, 2020
@tbouffard
Copy link
Member

tbouffard commented Jul 16, 2020

Render with 29cb3fd

boundary events
image

other events
image

Implementation Notes

We should rework the scaling and origin position computing in the future

  • this is a pain to center the icon
  • I didn't find a largerratio from parent allowing me center easily the icon, so the icon is small IMHO
  • the strokeWidth influences the scaling and makes things worse, so I left it out of the canvas wrapper configuration and set it directly on the mxgraph canvas

@tbouffard tbouffard removed the WIP 🚧 Pull request in progress and/or not ready for review. Used in addition to marking it as draft label Jul 16, 2020
@tbouffard tbouffard requested review from aibcmars and csouchet and removed request for aibcmars July 16, 2020 15:33
@tbouffard tbouffard marked this pull request as ready for review July 16, 2020 15:33
style: icon,
},
});
c.setStrokeWidth(StyleDefault.STROKE_WIDTH_THIN); // TODO should be done via icon style, but in that case, the strokeWidth change the ratio from shape
Copy link
Member

Choose a reason for hiding this comment

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

It's normal that the strokeWidth change the ratio from the shape, because MxGraph increase the size of the icon when the stroke width is greater than 1.
So to keep consistency and have the same size of an icon (whatever its stroke width), the ratio from the shape change 😉

@csouchet
Copy link
Member

At commit 398e088

image
image

@csouchet csouchet merged commit 2e07ef0 into process-analytics:master Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BPMN rendering Something about the way the lib is rendering BPMN elements enhancement New feature or request external contribution 👤 Pull requests provided by someone who is not a core maintainer
Projects
None yet
3 participants