Render Vertical Participants and Lanes

[sombrek]

Hi there,

I propose an implementation for rendering vertical pools and lanes.
Fixes #57

The labels of vertical pools and lanes as well as the pool label separator are now correctly rendered on top.
Labels of vertical black-box pools are rotated.

The spec implies that horizontal is the default and I adhere to that, i.e. a missing isHorizontal attribute defaults to true.

Could you please provide some information how I can test this?
I thought about adding a vertical version of https://github.com/bpmn-io/bpmn-js/blob/7aee439f47cf0daebcb6131e84f0f9a9d2a51544/test/fixtures/bpmn/draw/pools.bpmn
...but I'm not quite sure if that'll do it.

Kind regards

Showcase

[philippfromme]

Thank you for your contribution. I tested it and it works.

As for the tests, something like

it('should render vertical pools', function() {
  var xml = require('../../fixtures/bpmn/draw/vertical-pools.bpmn');
  return bootstrapViewer(xml).call(this).then(function(result) {

    checkErrors(result.error, result.warnings);
  });
});

should be sufficient.

isHorizontal has no default value so a missing isHorizontal means the participant is vertical. There is a behavior (https://github.com/bpmn-io/bpmn-js/blob/develop/lib/features/modeling/behavior/IsHorizontalFix.js) that ensures that if isHorizontal is falsy it's set to true. Unfortunately this means, that isHorizontal="false" will be set to true. You'd need to adjust that behavior to only set it to true if it's undefined.

[sombrek]

Thank you for your feedback.

The suggested test has been added and it passes.
I also adjusted the IsHorizontalFix. However, I believe this affects only modeling, but not rendering.

isHorizontal has no default value

The spec is funny. Pools and lanes shall be horizontal, if either isVertical is false or missing (BPMN 2.0.2, page 402). However no other mention of isVertical appears in the spec, so I thought they changed it but forgot to update that part of the spec.

Anyway, is this the point where I should mark the pull request as ready?

[nikku]

Let's add a test case to verify this is being set to the proper thing, on move.

[philippfromme]

Label editing might have to be adjusted. This could be a follow-up.

[nikku]

@sombrek We'd need to adjust label editing here + add a test case.

[sombrek]

@nikku and @philippfromme Thanks for your support.

I'd like to ask you to reconsider your latest remarks as I feel they extend the scope of this pull request too much.

In this pull request I specifically wanted to target vertical rendering, i.e. issue #57 only.
The issue tracker separately lists issue #507 for vertical modeling. I consider labeling and moving stuff around modeling, so I didn't have a look at that yet. I probably diluted my intent by implementing the changes to IsHorizontalFix. Sorry for that.

As suggested earlier, I added test cases that ensure the attribute remains intact after moving.
Are you willing to accept that as a complete increment or do you prefer another approach?

[nikku]

@sombrek Thanks for getting back to us. We believe that supporting a feature end-to-end, especially as the remaining steps are low effort, is valuable.

You're right, you planned to contribute rendering. We'll only incorporate rendering once basic modeling works, too.

As I understand you you don't have capacity to work on the basic modeling part? If that is the case then we're happy to help out. Note that this may take a little bit longer then to get this merged.

[sombrek]

@nikku Thanks for your feedback.

The question is one of scope, not of capacity. I think basic modeling also includes creating, splitting and resizing lanes, which currently don't work vertically either.
I think I can implement that and labeling, all while learning what's needed along the way.

I don't feel confident yet about possible further additions like considering the direction when adding tasks/events, vertical-specific icons or a "turn pool vertical/horizontal" feature. Those may be out of scope, but I'd like to know where you draw the line, so I know when I'm done.

[nikku]

Let's really tackle label editing only first and consider everything else a follow-up.

Fun fact: Label editing was our first modeling feature, before we could do anything else.

[sombrek]

@nikku Labeling now works as expected. Thanks for pointing me to the code.

Fun fact: Label editing was our first modeling feature, before we could do anything else.

Well, you've got to give the baby a name.

[nikku]

Really nice contribution 👏. Solid, simple enough.

[dymkkk]

I implemented a demo, but there were some issues with the lane when adjusting the size
demo

[sombrek]

@dymkkk Thanks for trying.

This issue was only about rendering (drawing) vertical pools lanes.

I'm currently working on resize issues. Those are documented in issue #2062.
There are more issues that I plan to work on after that.
A summary of what's not working yet can be found in issue #507.

[dymkkk]

@dymkkk Thanks for trying.

This issue was only about rendering (drawing) vertical pools lanes.

I'm currently working on resize issues. Those are documented in issue #2062. There are more issues that I plan to work on after that. A summary of what's not working yet can be found in issue #507.

This demo covers more than just that. You probably didn't try to create swim lanes. Swimlanes can already be split and created vertically, and can be resized, but there are some problems with range limitations. I also noticed your plan. If you are continuing to do it, then I still look forward to your results. Thanks