diff --git a/lib/features/auto-place/BpmnAutoPlace.js b/lib/features/auto-place/BpmnAutoPlace.js index 705cd018ae..f25533d0df 100644 --- 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' ]; \ No newline at end of file +AutoPlace.$inject = [ 'eventBus', 'elementRegistry' ]; \ No newline at end of file diff --git a/lib/features/auto-place/BpmnAutoPlaceUtil.js b/lib/features/auto-place/BpmnAutoPlaceUtil.js index b5ff912cd7..89fdb01d3f 100644 --- 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 index 79c13c4592..7e88f52056 100644 --- 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 index b194f8d91d..392778c0f8 100644 --- 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 index 5902f051a5..321961cd2f 100644 --- 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); } \ No newline at end of file diff --git a/test/spec/features/auto-place/BpmnAutoPlace.bpmn b/test/spec/features/auto-place/BpmnAutoPlace.bpmn index 1a07f9f41b..da38d40660 100644 --- a/test/spec/features/auto-place/BpmnAutoPlace.bpmn +++ b/test/spec/features/auto-place/BpmnAutoPlace.bpmn @@ -54,7 +54,9 @@ - + + + @@ -201,6 +203,10 @@ + + + + diff --git a/test/spec/features/auto-place/BpmnAutoPlace.subprocess.bpmn b/test/spec/features/auto-place/BpmnAutoPlace.subprocess.bpmn new file mode 100644 index 0000000000..11b6711007 --- /dev/null +++ b/test/spec/features/auto-place/BpmnAutoPlace.subprocess.bpmn @@ -0,0 +1,46 @@ + + + + + Flow_1qlbfsz + + + Flow_1qlbfsz + Flow_0au85uv + + + + + Flow_0au85uv + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/spec/features/auto-place/BpmnAutoPlace.subprocess.horizontal.bpmn b/test/spec/features/auto-place/BpmnAutoPlace.subprocess.horizontal.bpmn new file mode 100644 index 0000000000..81716b2bf5 --- /dev/null +++ b/test/spec/features/auto-place/BpmnAutoPlace.subprocess.horizontal.bpmn @@ -0,0 +1,53 @@ + + + + + + + + Flow_1jtt4p9 + + + Flow_06nuoit + + + Flow_1jtt4p9 + Flow_06nuoit + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/spec/features/auto-place/BpmnAutoPlace.subprocess.vertical.bpmn b/test/spec/features/auto-place/BpmnAutoPlace.subprocess.vertical.bpmn new file mode 100644 index 0000000000..708e54d910 --- /dev/null +++ b/test/spec/features/auto-place/BpmnAutoPlace.subprocess.vertical.bpmn @@ -0,0 +1,52 @@ + + + + + + + + Flow_1ijzj0g + + + + Flow_1ijzj0g + Flow_01mvjj3 + + + + Flow_01mvjj3 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/spec/features/auto-place/BpmnAutoPlace.vertical.bpmn b/test/spec/features/auto-place/BpmnAutoPlace.vertical.bpmn index e8f1c94096..943091d288 100644 --- a/test/spec/features/auto-place/BpmnAutoPlace.vertical.bpmn +++ b/test/spec/features/auto-place/BpmnAutoPlace.vertical.bpmn @@ -44,7 +44,9 @@ SequenceFlow_1 - + + + @@ -111,6 +113,10 @@ + + + + diff --git a/test/spec/features/auto-place/BpmnAutoPlaceSpec.js b/test/spec/features/auto-place/BpmnAutoPlaceSpec.js index 6bc4891e55..452bc2b2f3 100644 --- a/test/spec/features/auto-place/BpmnAutoPlaceSpec.js +++ b/test/spec/features/auto-place/BpmnAutoPlaceSpec.js @@ -71,6 +71,13 @@ describe('features/auto-place', function() { expectedBounds: { x: 925, y: 368, width: 100, height: 80 } })); + + it('after TASK_6', autoPlace({ + element: 'bpmn:Task', + behind: 'TASK_6', + expectedBounds: { x: 700, y: 370, width: 100, height: 80 } + })); + }); @@ -186,6 +193,13 @@ describe('features/auto-place', function() { expectedBounds: { x: 699, y: 930, width: 100, height: 80 } })); + + it('below V_TASK_6', autoPlace({ + element: 'bpmn:Task', + behind: 'V_TASK_6', + expectedBounds: { x: 700, y: 730, width: 100, height: 80 } + })); + }); @@ -474,6 +488,130 @@ describe('features/auto-place', function() { }); + + describe('nested element placement', function() { + + describe('in collapsed subprocess', function() { + + var diagramXML = require('./BpmnAutoPlace.subprocess.bpmn'); + + before(bootstrapModeler(diagramXML, { + modules: [ + coreModule, + modelingModule, + autoPlaceModule + ] + })); + + beforeEach(inject(function(canvas) { + canvas.setRootElement(canvas.findRoot('Sub_Process_plane')); + })); + + + it('should place node horizontally after Nested_Start_Event', autoPlace({ + element: 'bpmn:Task', + behind: 'Nested_Start_Event', + expectedBounds: { x: 265, y: 57, width: 100, height: 80 } + })); + + + it('should place annotation horizontally above Nested_Start_Event', autoPlace({ + element: 'bpmn:TextAnnotation', + behind: 'Nested_Start_Event', + expectedBounds: { x: 215, y: -1, width: 100, height: 30 } + })); + + + it('should place data store horizontally below Nested_Start_Event', autoPlace({ + element: 'bpmn:DataStoreReference', + behind: 'Nested_Start_Event', + expectedBounds: { x: 205, y: 155, width: 50, height: 50 } + })); + + }); + + + describe('in collapsed horizontal subprocess', function() { + + var diagramXML = require('./BpmnAutoPlace.subprocess.horizontal.bpmn'); + + before(bootstrapModeler(diagramXML, { + modules: [ + coreModule, + modelingModule, + autoPlaceModule + ] + })); + + beforeEach(inject(function(canvas) { + canvas.setRootElement(canvas.findRoot('Sub_Process_plane')); + })); + + + it('should place node horizontally after Nested_Start_Event', autoPlace({ + element: 'bpmn:Task', + behind: 'Nested_Start_Event', + expectedBounds: { x: 265, y: 77, width: 100, height: 80 } + })); + + + it('should place annotation horizontally above Nested_Start_Event', autoPlace({ + element: 'bpmn:TextAnnotation', + behind: 'Nested_Start_Event', + expectedBounds: { x: 215, y: 19, width: 100, height: 30 } + })); + + + it('should place data store horizontally below Nested_Start_Event', autoPlace({ + element: 'bpmn:DataStoreReference', + behind: 'Nested_Start_Event', + expectedBounds: { x: 205, y: 175, width: 50, height: 50 } + })); + + }); + + + describe('in collapsed vertical subprocess', function() { + + var diagramXML = require('./BpmnAutoPlace.subprocess.vertical.bpmn'); + + before(bootstrapModeler(diagramXML, { + modules: [ + coreModule, + modelingModule, + autoPlaceModule + ] + })); + + beforeEach(inject(function(canvas) { + canvas.setRootElement(canvas.findRoot('Sub_Process_plane')); + })); + + + it('should place node vertically after Nested_Start_Event', autoPlace({ + element: 'bpmn:Task', + behind: 'Nested_Start_Event', + expectedBounds: { x: 127, y: 165, width: 100, height: 80 } + })); + + + it('should place annotation vertically right of Nested_Start_Event', autoPlace({ + element: 'bpmn:TextAnnotation', + behind: 'Nested_Start_Event', + expectedBounds: { x: 245, y: 115, width: 100, height: 30 } + })); + + + it('should place data store vertically left of Nested_Start_Event', autoPlace({ + element: 'bpmn:DataStoreReference', + behind: 'Nested_Start_Event', + expectedBounds: { x: 69, y: 105, width: 50, height: 50 } + })); + + }); + + }); + });