Correctly lay out boundary to activity loops

lib/features/modeling/BpmnLayouter.js

@@ -23,6 +23,8 @@ import {
 import { is } from '../../util/ModelUtil';
 
 
+var BOUNDARY_TO_HOST_THRESHOLD = 40;
+
 export default function BpmnLayouter() {}
 
 inherits(BpmnLayouter, BaseLayouter);
@@ -86,7 +88,7 @@ BpmnLayouter.prototype.layoutConnection = function(connection, hints) {
     if (is(source, 'bpmn:BoundaryEvent')) {
 
       manhattanOptions = {
-        preferredLayouts: getBoundaryEventPreferredLayouts(source, target)
+        preferredLayouts: getBoundaryEventPreferredLayouts(source, target, end)
       };
 
     } else
@@ -262,7 +264,7 @@ function isHorizontalOrientation(orientation) {
   return orientation === 'right' || orientation === 'left';
 }
 
-function getBoundaryEventPreferredLayouts(source, target) {
+function getBoundaryEventPreferredLayouts(source, target, end) {
   var sourceMid = getMid(source),
       targetMid = getMid(target),
       attachOrientation = getAttachOrientation(source),
@@ -278,16 +280,52 @@ function getBoundaryEventPreferredLayouts(source, target) {
     y: source.height / 2 + target.height / 2
   });
 
+  if (isLoop) {
+    return getBoundaryEventLoopLayout(attachOrientation, attachedToSide, source, target, end);
+  }
+
   // source layout
-  sourceLayout = getBoundaryEventSourceLayout(attachOrientation, targetOrientation, attachedToSide, isLoop);
+  sourceLayout = getBoundaryEventSourceLayout(attachOrientation, targetOrientation, attachedToSide);
 
   // target layout
-  targetLayout = getBoundaryEventTargetLayout(attachOrientation, targetOrientation, attachedToSide, isLoop);
+  targetLayout = getBoundaryEventTargetLayout(attachOrientation, targetOrientation, attachedToSide);
 
   return [ sourceLayout + ':' + targetLayout ];
 }
 
-function getBoundaryEventSourceLayout(attachOrientation, targetOrientation, attachedToSide, isLoop) {
+function getBoundaryEventLoopLayout(attachOrientation, attachedToSide, source, target, end) {
+
+  var sourceLayout = orientationDirectionMapping[attachedToSide ? attachOrientation : getVerticalOrientation(attachOrientation)],
+      targetLayout;
+
+  if (attachedToSide) {
+    if (isHorizontalOrientation(attachOrientation)) {
+      targetLayout = shouldConnectToSameSide('y', source, target, end) ? 'h' : 'b';
+    } else {
+      targetLayout = shouldConnectToSameSide('x', source, target, end) ? 'v' : 'l';
+    }
+  } else {
+    targetLayout = 'v';
+  }
+
+  return [ sourceLayout + ':' + targetLayout ];
+}
+
+function shouldConnectToSameSide(axis, source, target, end) {
+  var threshold = BOUNDARY_TO_HOST_THRESHOLD;
+
+  return !(
+    areCloseOnAxis(axis, end, target, threshold) ||
+    areCloseOnAxis(axis, end, { x: target.x + target.width, y: target.y + target.height }, threshold) ||
+    areCloseOnAxis(axis, end, getMid(source), threshold)
+  );
+}
+
+function areCloseOnAxis(axis, a, b, threshold) {
+  return Math.abs(a[axis] - b[axis]) < threshold;
+}
+
+function getBoundaryEventSourceLayout(attachOrientation, targetOrientation, attachedToSide) {
 
   // attached to either top, right, bottom or left side
   if (attachedToSide) {
@@ -296,31 +334,28 @@ function getBoundaryEventSourceLayout(attachOrientation, targetOrientation, atta
 
   // attached to either top-right, top-left, bottom-right or bottom-left corner
 
-  // loop, same vertical or opposite horizontal orientation
-  if (isLoop ||
-    isSame(
-      getVerticalOrientation(attachOrientation), getVerticalOrientation(targetOrientation)
-    ) ||
-    isOppositeOrientation(
-      getHorizontalOrientation(attachOrientation), getHorizontalOrientation(targetOrientation)
-    )) {
+  // same vertical or opposite horizontal orientation
+  if (isSame(
+    getVerticalOrientation(attachOrientation), getVerticalOrientation(targetOrientation)
+  ) || isOppositeOrientation(
+    getHorizontalOrientation(attachOrientation), getHorizontalOrientation(targetOrientation)
+  )) {
     return orientationDirectionMapping[getVerticalOrientation(attachOrientation)];
   }
 
   // fallback
   return orientationDirectionMapping[getHorizontalOrientation(attachOrientation)];
 }
 
-function getBoundaryEventTargetLayout(attachOrientation, targetOrientation, attachedToSide, isLoop) {
+function getBoundaryEventTargetLayout(attachOrientation, targetOrientation, attachedToSide) {
 
   // attached to either top, right, bottom or left side
   if (attachedToSide) {
     if (isHorizontalOrientation(attachOrientation)) {
       // orientation is 'right' or 'left'
 
-      // loop or opposite horizontal orientation or same orientation
+      // opposite horizontal orientation or same orientation
       if (
-        isLoop ||
         isOppositeHorizontalOrientation(attachOrientation, targetOrientation) ||
         isSame(attachOrientation, targetOrientation)
       ) {
@@ -332,9 +367,8 @@ function getBoundaryEventTargetLayout(attachOrientation, targetOrientation, atta
     } else {
       // orientation is 'top' or 'bottom'
 
-      // loop or opposite vertical orientation or same orientation
+      // opposite vertical orientation or same orientation
       if (
-        isLoop ||
         isOppositeVerticalOrientation(attachOrientation, targetOrientation) ||
         isSame(attachOrientation, targetOrientation)
       ) {

lib/features/snapping/BpmnConnectSnapping.js

@@ -5,12 +5,23 @@ import {
 
 import { isCmd } from 'diagram-js/lib/features/keyboard/KeyboardUtil';
 
+import {
+  getOrientation
+} from 'diagram-js/lib/layout/LayoutUtil';
+
 import { is } from '../../util/ModelUtil';
 
 import { some } from 'min-dash';
 
 var HIGHER_PRIORITY = 1250;
 
+var BOUNDARY_TO_HOST_THRESHOLD = 40;
+
+var TARGET_BOUNDS_PADDING = 20;
+
+var AXES = [ 'x', 'y' ];
+
+var abs = Math.abs;
 
 /**
  * Snap during connect.
@@ -41,6 +52,10 @@ export default function BpmnConnectSnapping(eventBus, rules) {
       target: target
     });
 
+    if (target && connectionAttrs) {
+      snapInsideTarget(event, target);
+    }
+
     if (target && isAnyType(connectionAttrs, [
       'bpmn:Association',
       'bpmn:DataInputAssociation',
@@ -51,8 +66,9 @@ export default function BpmnConnectSnapping(eventBus, rules) {
       // snap source
       context.sourcePosition = mid(source);
 
-      // snap target
-      snapToPosition(event, mid(target));
+      if (is(source, 'bpmn:BoundaryEvent') && target === source.host) {
+        snapBoundaryEventLoop(event, source, target);
+      }
     } else if (isType(connectionAttrs, 'bpmn:MessageFlow')) {
 
       if (is(source, 'bpmn:Event')) {
@@ -80,6 +96,54 @@ BpmnConnectSnapping.$inject = [
   'rules'
 ];
 
+function snapInsideTarget(event, target) {
+
+  AXES.forEach(function(axis) {
+    var matchingTargetDimension = getDimensionForAxis(axis, target),
+        newCoordinate;
+
+    if (event[axis] < target[axis] + TARGET_BOUNDS_PADDING) {
+      newCoordinate = target[axis] + TARGET_BOUNDS_PADDING;
+    } else if (event[axis] > target[axis] + matchingTargetDimension - TARGET_BOUNDS_PADDING) {
+      newCoordinate = target[axis] + matchingTargetDimension - TARGET_BOUNDS_PADDING;
+    }
+
+    if (newCoordinate) {
+      setSnapped(event, axis, newCoordinate);
+    }
+  });
+}
+
+// snap outside of Boundary Event surroundings
+function snapBoundaryEventLoop(event, source, target) {
+  var sourceMid = mid(source),
+      orientation = getOrientation(sourceMid, target, -10),
+      snappingAxes = [];
+
+  if (/top|bottom/.test(orientation)) {
+    snappingAxes.push('x');
+  }
+
+  if (/left|right/.test(orientation)) {
+    snappingAxes.push('y');
+  }
+
+  snappingAxes.forEach(function(axis) {
+    var coordinate = event[axis], newCoordinate;
+
+    if (abs(coordinate - sourceMid[axis]) < BOUNDARY_TO_HOST_THRESHOLD) {
+      if (coordinate > sourceMid[axis]) {
+        newCoordinate = sourceMid[axis] + BOUNDARY_TO_HOST_THRESHOLD;
+      }
+      else {
+        newCoordinate = sourceMid[axis] - BOUNDARY_TO_HOST_THRESHOLD;
+      }
+
+      setSnapped(event, axis, newCoordinate);
+    }
+  });
+}
+
 // helpers //////////
 
 function snapToPosition(event, position) {
@@ -95,4 +159,8 @@ function isAnyType(attrs, types) {
   return some(types, function(type) {
     return isType(attrs, type);
   });
-}
+}
+
+function getDimensionForAxis(axis, element) {
+  return axis === 'x' ? element.width : element.height;
+}

test/spec/features/modeling/layout/LayoutSequenceFlowSpec.boundaryEventsLoops.bpmn

@@ -0,0 +1,49 @@
+<?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" id="Definitions_1" targetNamespace="http://bpmn.io/schema/bpmn" exporter="Camunda Modeler" exporterVersion="3.1.1">
+  <bpmn:process id="Process_1" isExecutable="false">
+    <bpmn:subProcess id="SubProcess" />
+    <bpmn:boundaryEvent id="BoundaryEvent_TopLeft" attachedToRef="SubProcess" />
+    <bpmn:boundaryEvent id="BoundaryEvent_BottomRight" attachedToRef="SubProcess" />
+    <bpmn:boundaryEvent id="BoundaryEvent_BottomLeft" attachedToRef="SubProcess" />
+    <bpmn:boundaryEvent id="BoundaryEvent_TopRight" attachedToRef="SubProcess" />
+    <bpmn:subProcess id="SubProcess_2" />
+    <bpmn:boundaryEvent id="BoundaryEvent_CenterRight" attachedToRef="SubProcess_2" />
+    <bpmn:boundaryEvent id="BoundaryEvent_BottomCenter" attachedToRef="SubProcess_2" />
+    <bpmn:boundaryEvent id="BoundaryEvent_TopCenter" attachedToRef="SubProcess_2" />
+    <bpmn:boundaryEvent id="BoundaryEvent_CenterLeft" attachedToRef="SubProcess_2" />
+  </bpmn:process>
+  <bpmndi:BPMNDiagram id="BPMNDiagram_1">
+    <bpmndi:BPMNPlane id="BPMNPlane_1" bpmnElement="Process_1">
+      <bpmndi:BPMNShape id="SubProcess_12qmapm_di" bpmnElement="SubProcess" isExpanded="true">
+        <dc:Bounds x="200" y="200" width="350" height="200" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="BoundaryEvent_0s0nl1k_di" bpmnElement="BoundaryEvent_TopLeft">
+        <dc:Bounds x="182" y="220" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="BoundaryEvent_0nomac7_di" bpmnElement="BoundaryEvent_BottomRight">
+        <dc:Bounds x="532" y="350" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="BoundaryEvent_1spolhy_di" bpmnElement="BoundaryEvent_BottomLeft">
+        <dc:Bounds x="182" y="382" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="BoundaryEvent_13iwzlu_di" bpmnElement="BoundaryEvent_TopRight">
+        <dc:Bounds x="532" y="182" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="SubProcess_2_di" bpmnElement="SubProcess_2" isExpanded="true">
+        <dc:Bounds x="200" y="460" width="350" height="200" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="BoundaryEvent_CenterRight_di" bpmnElement="BoundaryEvent_CenterRight">
+        <dc:Bounds x="532" y="542" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="BoundaryEvent_BottomCenter_di" bpmnElement="BoundaryEvent_BottomCenter">
+        <dc:Bounds x="357" y="642" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="BoundaryEvent_TopCenter_di" bpmnElement="BoundaryEvent_TopCenter">
+        <dc:Bounds x="357" y="442" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="BoundaryEvent_CenterLeft_di" bpmnElement="BoundaryEvent_CenterLeft">
+        <dc:Bounds x="182" y="542" width="36" height="36" />
+      </bpmndi:BPMNShape>
+    </bpmndi:BPMNPlane>
+  </bpmndi:BPMNDiagram>
+</bpmn:definitions>