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.

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

[FIX] do not force usage orthogonal edge segments #351

Closed
wants to merge 1 commit into from
Closed

[FIX] do not force usage orthogonal edge segments #351

wants to merge 1 commit into from

Conversation

tbouffard
Copy link
Member

In a conform BPMN file, segments are defined by waypoints, so forcing orthogonal
segments prevent rendering edges correctly when the waypoints don't defined such
orthogonality.

closes #295

@tbouffard tbouffard added the bug Something isn't working label Jun 24, 2020
@tbouffard tbouffard force-pushed the 295-fix_do_not_always_use_orthogonal_segments branch from 9b16b52 to 8a9482c Compare June 24, 2020 21:23
@tbouffard
Copy link
Member Author

I have removed the edge style configuration that generated the orthogonal segment. This has been introduced with #264
In this PR, I didn't find any remark explaining why it has been added.
My 1st intention was to keep it for edge without waypoints, but as this is not something we are supposed to support for now and as I have detected that it doesn't cover some use case (see #349), I left it removed.
Please comment if you have extra information explaining that we need to keep it in some cases.

@csouchet
Copy link
Member

csouchet commented Jul 2, 2020

all_activity_types.bpmn

image

label_sequence_flows.bpmn

image

B.2.0.bpmn

image

B.1.0.bpmn

image

BPMN.Bonita.bpmn from the drive

image

aibcmars
aibcmars approved these changes Jul 2, 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

@tbouffard
Copy link
Member Author

ok, please don't merge for now, I want to do some testing with other BPMN files that behave like what we see with the Bonita example: some sequence flows don't go straight, we see a glitch near to the arrow.

image

@tbouffard tbouffard marked this pull request as draft July 2, 2020 14:37
@csouchet csouchet mentioned this pull request Jul 6, 2020
Merged
@csouchet csouchet added the rebase needed 💥 Pull request that must be rebased on the latest master commit prior being reviewed or merged label Jul 7, 2020
@tbouffard tbouffard added the WIP 🚧 Pull request in progress and/or not ready for review. Used in addition to marking it as draft label Jul 8, 2020
@tbouffard
[FIX] do not force usage orthogonal edge segments
084cd72 
In a conform BPMN file, segments are defined by waypoints, so forcing orthogonal
segments prevent rendering edges correctly when the waypoints don't defined such
orthogonality.
@tbouffard tbouffard force-pushed the 295-fix_do_not_always_use_orthogonal_segments branch from 8cc4a72 to 084cd72 Compare July 8, 2020 16:53
@tbouffard tbouffard removed the rebase needed 💥 Pull request that must be rebased on the latest master commit prior being reviewed or merged label Jul 8, 2020
@tbouffard
Copy link
Member Author

I have noticed a side effect on message flows

Using our all_message_flow_types.bpmn test file, the message flow position at the right of the diagram is now correct but the start circle and end arrow rendering have changed.
We see the line inside of the terminal parts.

image

@tbouffard
Copy link
Member Author

As there are too much side effects, I postponed this fix
We need more investigations as suggested in #349

@tbouffard tbouffard closed this Jul 9, 2020
@tbouffard tbouffard deleted the 295-fix_do_not_always_use_orthogonal_segments branch July 9, 2020 09:07
@tbouffard tbouffard mentioned this pull request Dec 29, 2020
Closed
This was referenced Jun 14, 2021
Closed
Merged
@tbouffard tbouffard mentioned this pull request Jan 28, 2022
Closed
This was referenced Mar 14, 2022
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aibcmars aibcmars aibcmars approved these changes

@csouchet csouchet csouchet approved these changes

Assignees
No one assigned
Labels
bug Something isn't working WIP 🚧 Pull request in progress and/or not ready for review. Used in addition to marking it as draft
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] Message/Sequence flow segments shouldn't always be orthogonal
3 participants
@tbouffard @csouchet @aibcmars