fix: auto-place elements vertically in sub-processes

Closes #2127
bpmn-io · Sep 9, 2024 · 3153b08 · 3153b08
1 parent cdf7461
commit 3153b08
Show file tree

Hide file tree

Showing 11 changed files with 376 additions and 28 deletions.
diff --git a/lib/features/auto-place/BpmnAutoPlace.js b/lib/features/auto-place/BpmnAutoPlace.js
@@ -2,20 +2,22 @@ import { getNewShapePosition } from './BpmnAutoPlaceUtil';
 
 /**
  * @typedef {import('diagram-js/lib/core/EventBus').default} EventBus
+ * @typedef {import('diagram-js/lib/core/ElementRegistry').default} ElementRegistry
  */
 
 /**
  * BPMN auto-place behavior.
  *
  * @param {EventBus} eventBus
+ * @param {ElementRegistry} elementRegistry
  */
-export default function AutoPlace(eventBus) {
+export default function AutoPlace(eventBus, elementRegistry) {
   eventBus.on('autoPlace', function(context) {
     var shape = context.shape,
         source = context.source;
 
-    return getNewShapePosition(source, shape);
+    return getNewShapePosition(source, shape, elementRegistry);
   });
 }
 
-AutoPlace.$inject = [ 'eventBus' ];
+AutoPlace.$inject = [ 'eventBus', 'elementRegistry' ];
diff --git a/lib/features/auto-place/BpmnAutoPlaceUtil.js b/lib/features/auto-place/BpmnAutoPlaceUtil.js
@@ -22,6 +22,7 @@ import { isConnection } from 'diagram-js/lib/util/ModelUtil';
 /**
  * @typedef {import('../../model/Types').Shape} Shape
  *
+ * @typedef {import('diagram-js/lib/core/ElementRegistry').default} ElementRegistry
  * @typedef {import('diagram-js/lib/util/Types').Point} Point
  * @typedef {import('diagram-js/lib/util/Types').DirectionTRBL} DirectionTRBL
  */
@@ -32,21 +33,24 @@ import { isConnection } from 'diagram-js/lib/util/ModelUtil';
  *
  * @param  {Shape} source
  * @param  {Shape} element
+ * @param  {ElementRegistry} elementRegistry
  *
  * @return {Point}
  */
-export function getNewShapePosition(source, element) {
+export function getNewShapePosition(source, element, elementRegistry) {
+
+  var placeHorizontally = isDirectionHorizontal(source, elementRegistry);
 
   if (is(element, 'bpmn:TextAnnotation')) {
-    return getTextAnnotationPosition(source, element);
+    return getTextAnnotationPosition(source, element, placeHorizontally);
   }
 
   if (isAny(element, [ 'bpmn:DataObjectReference', 'bpmn:DataStoreReference' ])) {
-    return getDataElementPosition(source, element);
+    return getDataElementPosition(source, element, placeHorizontally);
   }
 
   if (is(element, 'bpmn:FlowNode')) {
-    return getFlowNodePosition(source, element);
+    return getFlowNodePosition(source, element, placeHorizontally);
   }
 }
 
@@ -56,16 +60,15 @@ export function getNewShapePosition(source, element) {
  *
  * @param {Shape} source
  * @param {Shape} element
+ * @param {boolean} placeHorizontally Whether to place the new element horizontally
  *
  * @return {Point}
  */
-export function getFlowNodePosition(source, element) {
+export function getFlowNodePosition(source, element, placeHorizontally) {
 
   var sourceTrbl = asTRBL(source);
   var sourceMid = getMid(source);
 
-  var placeHorizontally = isDirectionHorizontal(source);
-
   var placement = placeHorizontally ? {
     directionHint: 'e',
     minDistance: 80,
@@ -147,15 +150,14 @@ function getDistance(orientation, minDistance, placement) {
  *
  * @param {Shape} source
  * @param {Shape} element
+ * @param {boolean} placeHorizontally Whether to place the new element horizontally
  *
  * @return {Point}
  */
-export function getTextAnnotationPosition(source, element) {
+export function getTextAnnotationPosition(source, element, placeHorizontally) {
 
   var sourceTrbl = asTRBL(source);
 
-  var placeHorizontally = isDirectionHorizontal(source);
-
   var position = placeHorizontally ? {
     x: sourceTrbl.right + element.width / 2,
     y: sourceTrbl.top - 50 - element.height / 2
@@ -195,15 +197,14 @@ export function getTextAnnotationPosition(source, element) {
  *
  * @param {Shape} source
  * @param {Shape} element
+ * @param {boolean} placeHorizontally Whether to place the new element horizontally
  *
  * @return {Point}
  */
-export function getDataElementPosition(source, element) {
+export function getDataElementPosition(source, element, placeHorizontally) {
 
   var sourceTrbl = asTRBL(source);
 
-  var placeHorizontally = isDirectionHorizontal(source);
-
   var position = placeHorizontally ? {
     x: sourceTrbl.right - 10 + element.width / 2,
     y: sourceTrbl.bottom + 40 + element.width / 2

diff --git a/lib/features/grid-snapping/behavior/GridSnappingAutoPlaceBehavior.js b/lib/features/grid-snapping/behavior/GridSnappingAutoPlaceBehavior.js
@@ -5,6 +5,7 @@ import { is } from '../../../util/ModelUtil';
 
 /**
  * @typedef {import('diagram-js/lib/core/EventBus').default} EventBus
+ * @typedef {import('diagram-js/lib/core/ElementRegistry').default} ElementRegistry
  * @typedef {import('diagram-js/lib/features/grid-snapping/GridSnapping').default} GridSnapping
  *
  * @typedef {import('diagram-js/lib/util/Types').Axis} Axis
@@ -15,14 +16,15 @@ var HIGH_PRIORITY = 2000;
 /**
  * @param {EventBus} eventBus
  * @param {GridSnapping} gridSnapping
+ * @param {ElementRegistry} elementRegistry
  */
-export default function GridSnappingAutoPlaceBehavior(eventBus, gridSnapping) {
+export default function GridSnappingAutoPlaceBehavior(eventBus, gridSnapping, elementRegistry) {
   eventBus.on('autoPlace', HIGH_PRIORITY, function(context) {
     var source = context.source,
         sourceMid = getMid(source),
         shape = context.shape;
 
-    var position = getNewShapePosition(source, shape);
+    var position = getNewShapePosition(source, shape, elementRegistry);
 
     [ 'x', 'y' ].forEach(function(axis) {
       var options = {};
@@ -59,7 +61,8 @@ export default function GridSnappingAutoPlaceBehavior(eventBus, gridSnapping) {
 
 GridSnappingAutoPlaceBehavior.$inject = [
   'eventBus',
-  'gridSnapping'
+  'gridSnapping',
+  'elementRegistry'
 ];
 
 // helpers //////////

diff --git a/lib/features/modeling/BpmnLayouter.js b/lib/features/modeling/BpmnLayouter.js
@@ -25,6 +25,8 @@ import { is } from '../../util/ModelUtil';
 import { isDirectionHorizontal } from './util/ModelingUtil';
 
 /**
+ * @typedef {import('diagram-js/lib/core/ElementRegistry').default} ElementRegistry
+ *
  * @typedef {import('diagram-js/lib/util/Types').Point} Point
  *
  * @typedef {import('../../model/Types').Connection} Connection
@@ -107,7 +109,9 @@ var orientationDirectionMapping = {
   left: 'l'
 };
 
-export default function BpmnLayouter() {}
+export default function BpmnLayouter(elementRegistry) {
+  this._elementRegistry = elementRegistry;
+}
 
 inherits(BpmnLayouter, BaseLayouter);
 
@@ -128,7 +132,8 @@ BpmnLayouter.prototype.layoutConnection = function(connection, hints) {
       target = hints.target || connection.target,
       waypoints = hints.waypoints || connection.waypoints,
       connectionStart = hints.connectionStart,
-      connectionEnd = hints.connectionEnd;
+      connectionEnd = hints.connectionEnd,
+      elementRegistry = this._elementRegistry;
 
   var manhattanOptions,
       updatedWaypoints;
@@ -149,7 +154,7 @@ BpmnLayouter.prototype.layoutConnection = function(connection, hints) {
     }
   }
 
-  var layout = isDirectionHorizontal(source) ? PREFERRED_LAYOUTS_HORIZONTAL : PREFERRED_LAYOUTS_VERTICAL;
+  var layout = isDirectionHorizontal(source, elementRegistry) ? PREFERRED_LAYOUTS_HORIZONTAL : PREFERRED_LAYOUTS_VERTICAL;
 
   if (is(connection, 'bpmn:MessageFlow')) {
     manhattanOptions = getMessageFlowManhattanOptions(source, target, layout);
@@ -479,3 +484,5 @@ function getBoundaryEventTargetLayout(attachOrientation, targetOrientation, atta
     }
   }
 }
+
+BpmnLayouter.$inject = [ 'elementRegistry' ];
diff --git a/lib/features/modeling/util/ModelingUtil.js b/lib/features/modeling/util/ModelingUtil.js
@@ -2,11 +2,16 @@ import { isString } from 'min-dash';
 
 export { is, isAny } from '../../../util/ModelUtil';
 
-import { isAny } from '../../../util/ModelUtil';
+import {
+  is,
+  isAny,
+  getBusinessObject
+} from '../../../util/ModelUtil';
 
 import { isHorizontal } from '../../../util/DiUtil';
 
 /**
+ * @typedef {import('diagram-js/lib/core/ElementRegistry').default} ElementRegistry
  * @typedef {import('../../../model/Types').Element} Element
  */
 
@@ -37,19 +42,48 @@ export function getParent(element, anyType) {
  * Determines if the local modeling direction is vertical or horizontal.
  *
  * @param {Element} element
+ * @param {ElementRegistry} [elementRegistry] - provide to consider parent diagram direction
  *
  * @return {boolean} false for vertical pools, lanes and their children. true otherwise
  */
-export function isDirectionHorizontal(element) {
+export function isDirectionHorizontal(element, elementRegistry) {
+
+  var parent = getParent(element, 'bpmn:Process');
+  if (parent) {
+    return true;
+  }
 
   var types = [ 'bpmn:Participant', 'bpmn:Lane' ];
 
-  var parent = getParent(element, types);
+  parent = getParent(element, types);
   if (parent) {
     return isHorizontal(parent);
   } else if (isAny(element, types)) {
     return isHorizontal(element);
   }
 
-  return true;
+  var process;
+  for (process = getBusinessObject(element); process; process = process.$parent) {
+    if (is(process, 'bpmn:Process')) {
+      break;
+    }
+  }
+
+  if (!elementRegistry) {
+    return true;
+  }
+
+  // The direction may be specified in another diagram. We ignore that there
+  // could be multiple diagrams with contradicting properties based on the
+  // assumption that such BPMN files are unusual.
+  var pool = elementRegistry.find(function(shape) {
+    var businessObject = getBusinessObject(shape);
+    return businessObject && businessObject.get('processRef') === process;
+  });
+
+  if (!pool) {
+    return true;
+  }
+
+  return isHorizontal(pool);
 }
diff --git a/test/spec/features/auto-place/BpmnAutoPlace.bpmn b/test/spec/features/auto-place/BpmnAutoPlace.bpmn
@@ -54,7 +54,9 @@
     </bpmn:intermediateThrowEvent>
     <bpmn:sequenceFlow id="SequenceFlow_19p2kv6" sourceRef="IntermediateThrowEvent_0yy98gf" targetRef="TASK_0" />
     <bpmn:dataStoreReference id="DataStoreReference_0r0lie7" />
-    <bpmn:subProcess id="SUBPROCESS_1" name="SUBPROCESS_1" />
+    <bpmn:subProcess id="SUBPROCESS_1" name="SUBPROCESS_1">
+      <bpmn:task id="TASK_6" name="TASK_6" />
+    </bpmn:subProcess>
     <bpmn:textAnnotation id="TextAnnotation_0czqc1j" />
     <bpmn:association id="Association_1ebqqnb" sourceRef="TASK_3" targetRef="TextAnnotation_0czqc1j" />
   </bpmn:process>
@@ -201,6 +203,10 @@
       <bpmndi:BPMNShape id="SUBPROCESS_1_di" bpmnElement="SUBPROCESS_1" isExpanded="true">
         <dc:Bounds x="525" y="308" width="350" height="200" />
       </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="TASK_6_di" bpmnElement="TASK_6">
+        <dc:Bounds x="550" y="370" width="100" height="80" />
+        <bpmndi:BPMNLabel />
+      </bpmndi:BPMNShape>
     </bpmndi:BPMNPlane>
   </bpmndi:BPMNDiagram>
 </bpmn:definitions>
diff --git a/test/spec/features/auto-place/BpmnAutoPlace.subprocess.bpmn b/test/spec/features/auto-place/BpmnAutoPlace.subprocess.bpmn
@@ -0,0 +1,46 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<bpmn:definitions xmlns:bpmn="http://www.omg.org/spec/BPMN/20100524/MODEL" xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI" xmlns:dc="http://www.omg.org/spec/DD/20100524/DC" xmlns:di="http://www.omg.org/spec/DD/20100524/DI" xmlns:modeler="http://camunda.org/schema/modeler/1.0" id="Definitions_1phrppp" targetNamespace="http://bpmn.io/schema/bpmn" exporter="Camunda Modeler" exporterVersion="5.25.0" modeler:executionPlatform="Camunda Cloud" modeler:executionPlatformVersion="8.5.0">
+  <bpmn:process id="Process_0e3fgm1" isExecutable="true">
+    <bpmn:startEvent id="StartEvent_1">
+      <bpmn:outgoing>Flow_1qlbfsz</bpmn:outgoing>
+    </bpmn:startEvent>
+    <bpmn:subProcess id="Sub_Process">
+      <bpmn:incoming>Flow_1qlbfsz</bpmn:incoming>
+      <bpmn:outgoing>Flow_0au85uv</bpmn:outgoing>
+      <bpmn:startEvent id="Nested_Start_Event" />
+    </bpmn:subProcess>
+    <bpmn:sequenceFlow id="Flow_1qlbfsz" sourceRef="StartEvent_1" targetRef="Sub_Process" />
+    <bpmn:endEvent id="Event_06byytn">
+      <bpmn:incoming>Flow_0au85uv</bpmn:incoming>
+    </bpmn:endEvent>
+    <bpmn:sequenceFlow id="Flow_0au85uv" sourceRef="Sub_Process" targetRef="Event_06byytn" />
+  </bpmn:process>
+  <bpmndi:BPMNDiagram id="BPMNDiagram_1">
+    <bpmndi:BPMNPlane id="BPMNPlane_1" bpmnElement="Process_0e3fgm1">
+      <bpmndi:BPMNShape id="_BPMNShape_StartEvent_2" bpmnElement="StartEvent_1">
+        <dc:Bounds x="179" y="159" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="Activity_0aamzwi_di" bpmnElement="Sub_Process">
+        <dc:Bounds x="270" y="137" width="100" height="80" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="Event_06byytn_di" bpmnElement="Event_06byytn">
+        <dc:Bounds x="432" y="159" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNEdge id="Flow_1qlbfsz_di" bpmnElement="Flow_1qlbfsz">
+        <di:waypoint x="215" y="177" />
+        <di:waypoint x="270" y="177" />
+      </bpmndi:BPMNEdge>
+      <bpmndi:BPMNEdge id="Flow_0au85uv_di" bpmnElement="Flow_0au85uv">
+        <di:waypoint x="370" y="177" />
+        <di:waypoint x="432" y="177" />
+      </bpmndi:BPMNEdge>
+    </bpmndi:BPMNPlane>
+  </bpmndi:BPMNDiagram>
+  <bpmndi:BPMNDiagram id="BPMNDiagram_01vxuwr">
+    <bpmndi:BPMNPlane id="BPMNPlane_0kcojux" bpmnElement="Sub_Process">
+      <bpmndi:BPMNShape id="Event_08tsh87_di" bpmnElement="Nested_Start_Event">
+        <dc:Bounds x="179" y="79" width="36" height="36" />
+      </bpmndi:BPMNShape>
+    </bpmndi:BPMNPlane>
+  </bpmndi:BPMNDiagram>
+</bpmn:definitions>
diff --git a/test/spec/features/auto-place/BpmnAutoPlace.subprocess.horizontal.bpmn b/test/spec/features/auto-place/BpmnAutoPlace.subprocess.horizontal.bpmn
@@ -0,0 +1,53 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<bpmn:definitions xmlns:bpmn="http://www.omg.org/spec/BPMN/20100524/MODEL" xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI" xmlns:dc="http://www.omg.org/spec/DD/20100524/DC" xmlns:di="http://www.omg.org/spec/DD/20100524/DI" xmlns:modeler="http://camunda.org/schema/modeler/1.0" id="Definitions_1ovis1u" targetNamespace="http://bpmn.io/schema/bpmn" exporter="Camunda Modeler" exporterVersion="5.25.0" modeler:executionPlatform="Camunda Cloud" modeler:executionPlatformVersion="8.5.0">
+  <bpmn:collaboration id="Collaboration_0dgrh13">
+    <bpmn:participant id="Participant_0c4fywu" name="Horizontal Participant" processRef="Process_1vizv6s" />
+  </bpmn:collaboration>
+  <bpmn:process id="Process_1vizv6s" isExecutable="true">
+    <bpmn:startEvent id="StartEvent_1">
+      <bpmn:outgoing>Flow_1jtt4p9</bpmn:outgoing>
+    </bpmn:startEvent>
+    <bpmn:endEvent id="Event_1qgvgxz">
+      <bpmn:incoming>Flow_06nuoit</bpmn:incoming>
+    </bpmn:endEvent>
+    <bpmn:subProcess id="Sub_Process">
+      <bpmn:incoming>Flow_1jtt4p9</bpmn:incoming>
+      <bpmn:outgoing>Flow_06nuoit</bpmn:outgoing>
+      <bpmn:startEvent id="Nested_Start_Event" />
+    </bpmn:subProcess>
+    <bpmn:sequenceFlow id="Flow_1jtt4p9" sourceRef="StartEvent_1" targetRef="Sub_Process" />
+    <bpmn:sequenceFlow id="Flow_06nuoit" sourceRef="Sub_Process" targetRef="Event_1qgvgxz" />
+  </bpmn:process>
+  <bpmndi:BPMNDiagram id="BPMNDiagram_1">
+    <bpmndi:BPMNPlane id="BPMNPlane_1" bpmnElement="Collaboration_0dgrh13">
+      <bpmndi:BPMNShape id="Participant_0c4fywu_di" bpmnElement="Participant_0c4fywu" isHorizontal="true">
+        <dc:Bounds x="130" y="82" width="600" height="250" />
+        <bpmndi:BPMNLabel />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="_BPMNShape_StartEvent_2" bpmnElement="StartEvent_1">
+        <dc:Bounds x="189" y="189" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="Event_1qgvgxz_di" bpmnElement="Event_1qgvgxz">
+        <dc:Bounds x="442" y="189" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="Activity_1mo1y9x_di" bpmnElement="Sub_Process">
+        <dc:Bounds x="280" y="167" width="100" height="80" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNEdge id="Flow_1jtt4p9_di" bpmnElement="Flow_1jtt4p9">
+        <di:waypoint x="225" y="207" />
+        <di:waypoint x="280" y="207" />
+      </bpmndi:BPMNEdge>
+      <bpmndi:BPMNEdge id="Flow_06nuoit_di" bpmnElement="Flow_06nuoit">
+        <di:waypoint x="380" y="207" />
+        <di:waypoint x="442" y="207" />
+      </bpmndi:BPMNEdge>
+    </bpmndi:BPMNPlane>
+  </bpmndi:BPMNDiagram>
+  <bpmndi:BPMNDiagram id="BPMNDiagram_0gq0ymm">
+    <bpmndi:BPMNPlane id="BPMNPlane_0xsxkoi" bpmnElement="Sub_Process">
+      <bpmndi:BPMNShape id="Event_1j73q1w_di" bpmnElement="Nested_Start_Event">
+        <dc:Bounds x="179" y="99" width="36" height="36" />
+      </bpmndi:BPMNShape>
+    </bpmndi:BPMNPlane>
+  </bpmndi:BPMNDiagram>
+</bpmn:definitions>