fix: auto-place elements vertically in sub-processes

Closes #2127

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' ];

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

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 //////////

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' ];

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);
 }