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

handle positions of elements after a split #22

Merged
merged 3 commits into from
May 1, 2020

Conversation

baptistemesta
Copy link
Contributor

To do that, add rows above and below the element that have multiple
outgoing transition and then put each of the next element right of the
it.

Still not handled:

  • Elements that split + join
  • Overlapping of successive branches
  • Joins should be align to its split element

Added in this PR a ASCII exported to have a visual feedback when
assertions are failing (try to change an assert in ShapeLayouterTest)

Closes #9

To do that, add rows above and below the element that have multiple
outgoing transition and then put each of the next element right of the
it.

Still not handled:
* Elements that split + join
* Overlapping of successive branches
* Joins should be align to its split element

Added in this PR a ASCII exported to have a visual feedback when
assertions are failing (try to change an assert in ShapeLayouterTest)

Closes #9
import io.process.analytics.tools.bpmn.generator.model.Grid;
import io.process.analytics.tools.bpmn.generator.model.Position;

public class ASCIIExporter {
Copy link
Member

Choose a reason for hiding this comment

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

👍 like the idea to get feedbacks on failing tests

Copy link
Member

@tbouffard tbouffard left a comment

Choose a reason for hiding this comment

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

Everything looks good, I just have a few questions that doesn't block the merge

}
List<Edge> outgoingEdges = diagram.getOutgoingEdges(shape.getId());
if (outgoingEdges.size() > 1) {
//add rows to place elements of this split
Copy link
Member

Choose a reason for hiding this comment

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

❓ I may be messed by the comment about the split
at a 1st glance, this is strange to have something which is always run but which only relates to split
What I understood is that you always add rows to manage eventual splits. Is that correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At first, I started adding rows when adding the elements after the split:
i.e. when calling the addSplit method, I was adding rows to handle all elements.
Finally, I found it easier to add the rows when we are processing the elements that have multiple outgoing transitions.
I did not understood that we always add rows to manage an eventual split. It's only written that we need to add rows to put all elements after a split. It's not written how we do it.
We should check the implementation in oryx to see how they handled that.

Copy link
Member

Choose a reason for hiding this comment

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

ok, thanks for the info
As it is working for now, we can live with it (at least it is not a problem to me)

Co-authored-by: Thomas Bouffard <27200110+tbouffard@users.noreply.github.com>
@baptistemesta baptistemesta merged commit b911be4 into master May 1, 2020
@baptistemesta baptistemesta deleted the feat/implement_split branch May 1, 2020 13:39
@tbouffard tbouffard added the enhancement New feature or request label Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

create algorithm to position element with split
2 participants