From 1e6186e3ce2e774daa2afa17ec378a49aacc948f Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Mon, 13 Nov 2017 11:11:07 +0100 Subject: [PATCH] feat(draw): render sequence flows behind tasks * ordering provider ensures sequence flows appear behind tasks * importer makes sure we render elements in the order lanes > sequence flows > other flow elements * consistent minimal opacity ensures elements in front of tasks don't look _connected_ Closes #727 --- lib/draw/BpmnRenderer.js | 58 ++++----- lib/features/ordering/BpmnOrderingProvider.js | 21 +++- lib/import/BpmnImporter.js | 16 ++- lib/import/BpmnTreeWalker.js | 28 +++-- .../ordering/BpmnOrderingProviderSpec.js | 20 ++++ test/spec/features/ordering/Helper.js | 41 ++++++- test/spec/import/ImporterSpec.js | 89 ++++++++++++-- test/spec/import/sequenceFlow-ordering.bpmn | 110 ++++++++++++++++++ 8 files changed, 325 insertions(+), 58 deletions(-) create mode 100644 test/spec/import/sequenceFlow-ordering.bpmn diff --git a/lib/draw/BpmnRenderer.js b/lib/draw/BpmnRenderer.js index 627a055f..c01caf62 100644 --- a/lib/draw/BpmnRenderer.js +++ b/lib/draw/BpmnRenderer.js @@ -50,6 +50,9 @@ var LABEL_STYLE = { fontSize: 12 }; +var DEFAULT_FILL_OPACITY = .95, + HIGH_FILL_OPACITY = .35; + function BpmnRenderer(eventBus, styles, pathMap, canvas, priority) { @@ -487,6 +490,8 @@ function BpmnRenderer(eventBus, styles, pathMap, canvas, priority) { var handlers = this.handlers = { 'bpmn:Event': function(parentGfx, element, attrs) { + attrs.fillOpacity = attrs.fillOpacity || DEFAULT_FILL_OPACITY; + return drawCircle(parentGfx, element.width, element.height, attrs); }, 'bpmn:StartEvent': function(parentGfx, element) { @@ -801,6 +806,10 @@ function BpmnRenderer(eventBus, styles, pathMap, canvas, priority) { 'bpmn:IntermediateThrowEvent': as('bpmn:IntermediateEvent'), 'bpmn:Activity': function(parentGfx, element, attrs) { + + attrs = attrs || {}; + attrs.fillOpacity = attrs.fillOpacity || DEFAULT_FILL_OPACITY; + return drawRect(parentGfx, element.width, element.height, TASK_BORDER_RADIUS, attrs); }, @@ -1035,7 +1044,6 @@ function BpmnRenderer(eventBus, styles, pathMap, canvas, priority) { }, 'bpmn:SubProcess': function(parentGfx, element, attrs) { attrs = assign({ - fillOpacity: 0.95, fill: getFillColor(element), stroke: getStrokeColor(element) }, attrs); @@ -1084,7 +1092,7 @@ function BpmnRenderer(eventBus, styles, pathMap, canvas, priority) { 'bpmn:Participant': function(parentGfx, element) { var attrs = { - fillOpacity: 0.95, + fillOpacity: DEFAULT_FILL_OPACITY, fill: getFillColor(element), stroke: getStrokeColor(element) }; @@ -1124,6 +1132,7 @@ function BpmnRenderer(eventBus, styles, pathMap, canvas, priority) { 'bpmn:Lane': function(parentGfx, element, attrs) { var rect = drawRect(parentGfx, element.width, element.height, 0, assign({ fill: getFillColor(element), + fillOpacity: HIGH_FILL_OPACITY, stroke: getStrokeColor(element) }, attrs)); @@ -1137,12 +1146,7 @@ function BpmnRenderer(eventBus, styles, pathMap, canvas, priority) { return rect; }, 'bpmn:InclusiveGateway': function(parentGfx, element) { - var attrs = { - fill: getFillColor(element), - stroke: getStrokeColor(element) - }; - - var diamond = drawDiamond(parentGfx, element.width, element.height, attrs); + var diamond = renderer('bpmn:Gateway')(parentGfx, element); /* circle path */ drawCircle(parentGfx, element.width, element.height, element.height * 0.24, { @@ -1154,12 +1158,7 @@ function BpmnRenderer(eventBus, styles, pathMap, canvas, priority) { return diamond; }, 'bpmn:ExclusiveGateway': function(parentGfx, element) { - var attrs = { - fill: getFillColor(element), - stroke: getStrokeColor(element) - }; - - var diamond = drawDiamond(parentGfx, element.width, element.height, attrs); + var diamond = renderer('bpmn:Gateway')(parentGfx, element); var pathData = pathMap.getScaledPath('GATEWAY_EXCLUSIVE', { xScaleFactor: 0.4, @@ -1183,12 +1182,7 @@ function BpmnRenderer(eventBus, styles, pathMap, canvas, priority) { return diamond; }, 'bpmn:ComplexGateway': function(parentGfx, element) { - var attrs = { - fill: getFillColor(element), - stroke: getStrokeColor(element) - }; - - var diamond = drawDiamond(parentGfx, element.width, element.height, attrs); + var diamond = renderer('bpmn:Gateway')(parentGfx, element); var pathData = pathMap.getScaledPath('GATEWAY_COMPLEX', { xScaleFactor: 0.5, @@ -1210,12 +1204,7 @@ function BpmnRenderer(eventBus, styles, pathMap, canvas, priority) { return diamond; }, 'bpmn:ParallelGateway': function(parentGfx, element) { - var attrs = { - fill: getFillColor(element), - stroke: getStrokeColor(element) - }; - - var diamond = drawDiamond(parentGfx, element.width, element.height, attrs); + var diamond = renderer('bpmn:Gateway')(parentGfx, element); var pathData = pathMap.getScaledPath('GATEWAY_PARALLEL', { xScaleFactor: 0.6, @@ -1240,12 +1229,7 @@ function BpmnRenderer(eventBus, styles, pathMap, canvas, priority) { var semantic = getSemantic(element); - var attrs = { - fill: getFillColor(element), - stroke: getStrokeColor(element) - }; - - var diamond = drawDiamond(parentGfx, element.width, element.height, attrs); + var diamond = renderer('bpmn:Gateway')(parentGfx, element); /* outer circle path */ drawCircle(parentGfx, element.width, element.height, element.height * 0.20, { strokeWidth: 1, @@ -1314,7 +1298,13 @@ function BpmnRenderer(eventBus, styles, pathMap, canvas, priority) { return diamond; }, 'bpmn:Gateway': function(parentGfx, element) { - return drawDiamond(parentGfx, element.width, element.height); + var attrs = { + fill: getFillColor(element), + fillOpacity: DEFAULT_FILL_OPACITY, + stroke: getStrokeColor(element) + }; + + return drawDiamond(parentGfx, element.width, element.height, attrs); }, 'bpmn:SequenceFlow': function(parentGfx, element) { var pathData = createPathFromConnection(element); @@ -1452,6 +1442,7 @@ function BpmnRenderer(eventBus, styles, pathMap, canvas, priority) { var elementObject = drawPath(parentGfx, pathData, { fill: getFillColor(element), + fillOpacity: DEFAULT_FILL_OPACITY, stroke: getStrokeColor(element) }); @@ -1503,6 +1494,7 @@ function BpmnRenderer(eventBus, styles, pathMap, canvas, priority) { var elementStore = drawPath(parentGfx, DATA_STORE_PATH, { strokeWidth: 2, fill: getFillColor(element), + fillOpacity: DEFAULT_FILL_OPACITY, stroke: getStrokeColor(element) }); diff --git a/lib/features/ordering/BpmnOrderingProvider.js b/lib/features/ordering/BpmnOrderingProvider.js index b887c54e..e5c32bcb 100644 --- a/lib/features/ordering/BpmnOrderingProvider.js +++ b/lib/features/ordering/BpmnOrderingProvider.js @@ -26,7 +26,7 @@ function BpmnOrderingProvider(eventBus, translate) { { type: 'bpmn:SequenceFlow', order: { - level: 5, + level: 3, containers: [ 'bpmn:Participant', 'bpmn:FlowElementsContainer' @@ -34,8 +34,22 @@ function BpmnOrderingProvider(eventBus, translate) { } }, // handle DataAssociation(s) like message flows and render them always on top - { type: 'bpmn:DataAssociation', order: { level: 9, containers: [ 'bpmn:Collaboration', 'bpmn:Process' ] } }, - { type: 'bpmn:MessageFlow', order: { level: 9, containers: [ 'bpmn:Collaboration' ] } }, + { + type: 'bpmn:DataAssociation', + order: { + level: 9, + containers: [ + 'bpmn:Collaboration', + 'bpmn:Process' + ] + } + }, + { + type: 'bpmn:MessageFlow', order: { + level: 9, + containers: [ 'bpmn:Collaboration' ] + } + }, { type: 'bpmn:Association', order: { @@ -48,6 +62,7 @@ function BpmnOrderingProvider(eventBus, translate) { } }, { type: 'bpmn:BoundaryEvent', order: { level: 8 } }, + { type: 'bpmn:FlowElement', order: { level: 5 } }, { type: 'bpmn:Participant', order: { level: -2 } }, { type: 'bpmn:Lane', order: { level: -1 } } ]; diff --git a/lib/import/BpmnImporter.js b/lib/import/BpmnImporter.js index 230a9f8d..0fa07f36 100644 --- a/lib/import/BpmnImporter.js +++ b/lib/import/BpmnImporter.js @@ -73,6 +73,8 @@ BpmnImporter.prototype.add = function(semantic, parentElement) { translate = this._translate, hidden; + var parentIndex; + // ROOT ELEMENT // handle the special case that we deal with a // invisible root element (process or collaboration) @@ -105,7 +107,12 @@ BpmnImporter.prototype.add = function(semantic, parentElement) { this._attachBoundary(semantic, element); } - this._canvas.addShape(element, parentElement); + // insert lanes behind other flow nodes (cf. #727) + if (is(semantic, 'bpmn:Lane')) { + parentIndex = 0; + } + + this._canvas.addShape(element, parentElement, parentIndex); } // CONNECTION @@ -132,7 +139,12 @@ BpmnImporter.prototype.add = function(semantic, parentElement) { parentElement = null; } - this._canvas.addConnection(element, parentElement); + // insert sequence flows behind other flow nodes (cf. #727) + if (is(semantic, 'bpmn:SequenceFlow')) { + parentIndex = 0; + } + + this._canvas.addConnection(element, parentElement, parentIndex); } else { throw new Error(translate('unknown di {di} for element {semantic}', { di: elementToString(di), diff --git a/lib/import/BpmnTreeWalker.js b/lib/import/BpmnTreeWalker.js index 9cd748b5..6bc79718 100644 --- a/lib/import/BpmnTreeWalker.js +++ b/lib/import/BpmnTreeWalker.js @@ -229,7 +229,15 @@ function BpmnTreeWalker(handler, translate) { } function handleDeferred(deferred) { - forEach(deferred, function(d) { d(); }); + + var fn; + + // drain deferred until empty + while (deferred.length) { + fn = deferred.shift(); + + fn(); + } } function handleProcess(process, context) { @@ -348,13 +356,17 @@ function BpmnTreeWalker(handler, translate) { } function handleLane(lane, context) { - var newContext = visitIfDi(lane, context); - if (lane.childLaneSet) { - handleLaneSet(lane.childLaneSet, newContext || context); - } + deferred.push(function() { - wireFlowNodeRefs(lane); + var newContext = visitIfDi(lane, context); + + if (lane.childLaneSet) { + handleLaneSet(lane.childLaneSet, newContext || context); + } + + wireFlowNodeRefs(lane); + }); } function handleLaneSet(laneSet, context) { @@ -366,11 +378,11 @@ function BpmnTreeWalker(handler, translate) { } function handleFlowElementsContainer(container, context) { + handleFlowElements(container.flowElements, context); + if (container.laneSets) { handleLaneSets(container.laneSets, context); } - - handleFlowElements(container.flowElements, context); } function handleFlowElements(flowElements, context) { diff --git a/test/spec/features/ordering/BpmnOrderingProviderSpec.js b/test/spec/features/ordering/BpmnOrderingProviderSpec.js index 0c3e0e70..1a940763 100644 --- a/test/spec/features/ordering/BpmnOrderingProviderSpec.js +++ b/test/spec/features/ordering/BpmnOrderingProviderSpec.js @@ -6,6 +6,7 @@ var Helper = require('./Helper'); var move = Helper.move, attach = Helper.attach, + connect = Helper.connect, expectZOrder = Helper.expectZOrder; var modelingModule = require('../../../../lib/features/modeling'), @@ -228,4 +229,23 @@ describe('features/modeling - ordering', function() { }); + + describe('connections', function() { + + var diagramXML = require('./ordering.bpmn'); + + beforeEach(bootstrapModeler(diagramXML, { modules: testModules })); + + + it('should render sequence flows behind tasks', inject(function() { + + // when + var connection = connect('BoundaryEvent', 'Task'); + + // then + expectZOrder(connection, 'Task', 'BoundaryEvent', connection.label); + })); + + }); + }); diff --git a/test/spec/features/ordering/Helper.js b/test/spec/features/ordering/Helper.js index 34181228..7d142896 100644 --- a/test/spec/features/ordering/Helper.js +++ b/test/spec/features/ordering/Helper.js @@ -85,6 +85,33 @@ function add(attrs, position, target, isAttach) { module.exports.add = add; +function connect(source, target) { + + return TestHelper.getBpmnJS().invoke(function(canvas, elementRegistry, modeling) { + + function getElement(id) { + + var element = elementRegistry.get(id); + + expect(element).to.exist; + + return element; + } + + if (typeof target === 'string') { + target = getElement(target); + } + + if (typeof source === 'string') { + source = getElement(source); + } + + return modeling.connect(source, target); + }); +} + +module.exports.connect = connect; + function attach(attrs, position, target) { return add(attrs, position, target, true); } @@ -105,7 +132,7 @@ function getAncestors(element) { } -function compareZOrder(aId, bId) { +function compareZOrder(a, b) { var elementA, elementB; @@ -120,8 +147,16 @@ function compareZOrder(aId, bId) { return element; } - elementA = getElement(aId); - elementB = getElement(bId); + if (typeof a === 'string') { + a = getElement(a); + } + + if (typeof b === 'string') { + b = getElement(b); + } + + elementA = a; + elementB = b; }); diff --git a/test/spec/import/ImporterSpec.js b/test/spec/import/ImporterSpec.js index 63b13efd..b92614e8 100644 --- a/test/spec/import/ImporterSpec.js +++ b/test/spec/import/ImporterSpec.js @@ -1,8 +1,5 @@ 'use strict'; -require('../../TestHelper'); - - var TestContainer = require('mocha-test-container-support'); var Diagram = require('diagram-js/lib/Diagram'), @@ -10,8 +7,14 @@ var Diagram = require('diagram-js/lib/Diagram'), importBpmnDiagram = require('../../../lib/import/Importer').importBpmnDiagram, Viewer = require('../../../lib/Viewer'); +var domMatches = require('min-dom/lib/matches'); + +var getChildrenGfx = require('diagram-js/lib/util/GraphicsUtil').getChildren; + var find = require('lodash/collection/find'); +var is = require('../../../lib/util/ModelUtil').is; + describe('import - Importer', function() { @@ -164,12 +167,12 @@ describe('import - Importer', function() { expect(events).to.eql([ { type: 'add', semantic: '_Collaboration_2', di: 'BPMNPlane_1', diagramElement: '_Collaboration_2' }, { type: 'add', semantic: 'Participant_2', di: '_BPMNShape_Participant_2', diagramElement: 'Participant_2' }, - { type: 'add', semantic: 'Lane_1', di: '_BPMNShape_Lane_2', diagramElement: 'Lane_1' }, - { type: 'add', semantic: 'Lane_2', di: '_BPMNShape_Lane_3', diagramElement: 'Lane_2' }, - { type: 'add', semantic: 'Lane_3', di: '_BPMNShape_Lane_4', diagramElement: 'Lane_3' }, { type: 'add', semantic: 'Task_1', di: '_BPMNShape_Task_3', diagramElement: 'Task_1' }, { type: 'add', semantic: 'Participant_1', di: '_BPMNShape_Participant_3', diagramElement: 'Participant_1' }, - { type: 'add', semantic: 'StartEvent_1', di: '_BPMNShape_StartEvent_3', diagramElement: 'StartEvent_1' } + { type: 'add', semantic: 'StartEvent_1', di: '_BPMNShape_StartEvent_3', diagramElement: 'StartEvent_1' }, + { type: 'add', semantic: 'Lane_1', di: '_BPMNShape_Lane_2', diagramElement: 'Lane_1' }, + { type: 'add', semantic: 'Lane_2', di: '_BPMNShape_Lane_3', diagramElement: 'Lane_2' }, + { type: 'add', semantic: 'Lane_3', di: '_BPMNShape_Lane_4', diagramElement: 'Lane_3' } ]); done(err); @@ -184,7 +187,7 @@ describe('import - Importer', function() { var xml = require('../../fixtures/bpmn/import/position/position-testcase.bpmn'); - it('should round shape\'s x and y coordinates', function(done) { + it('should round shape coordinates', function(done) { // given var events = {}; @@ -210,7 +213,7 @@ describe('import - Importer', function() { }); - it('should round shape\'s height and width', function(done) { + it('should round shape dimensions', function(done) { // given var events = {}; @@ -234,6 +237,42 @@ describe('import - Importer', function() { }); + describe('order', function() { + + it('should import sequence flows and lanes behind other flow nodes', function(done) { + + var xml = require('./sequenceFlow-ordering.bpmn'); + + // given + var elementRegistry = diagram.get('elementRegistry'); + + + runImport(diagram, xml, function(err, warnings) { + + // when + var processShape = elementRegistry.get('Participant_1jxpy8o'); + + var children = processShape.children; + + // lanes + // connections + // other elements + var correctlyOrdered = [].concat( + children.filter(function(c) { return is(c, 'bpmn:Lane'); }), + children.filter(function(c) { return c.waypoints; }), + children.filter(function(c) { return !is(c, 'bpmn:Lane') && !c.waypoints; }) + ); + + // then + expectChildren(diagram, processShape, correctlyOrdered); + + done(err); + }); + }); + + }); + + describe('elements', function() { it('should import boundary events', function(done) { @@ -422,3 +461,35 @@ describe('import - Importer', function() { }); }); + + + +////////// helpers ///////////////////////////////////// + +function expectChildren(diagram, parent, children) { + + return diagram.invoke(function(elementRegistry) { + + // verify model is consistent + expect(parent.children).to.eql(children); + + // verify SVG is consistent + var parentGfx = elementRegistry.getGraphics(parent); + + var expectedChildrenGfx = children.map(function(c) { + return elementRegistry.getGraphics(c); + }); + + var childrenContainerGfx = + domMatches(parentGfx, '[data-element-id="Process_1"]') + ? parentGfx + : getChildrenGfx(parentGfx); + + var existingChildrenGfx = Array.prototype.map.call(childrenContainerGfx.childNodes, function(c) { + return c.querySelector('.djs-element'); + }); + + expect(existingChildrenGfx).to.eql(expectedChildrenGfx); + }); + +} \ No newline at end of file diff --git a/test/spec/import/sequenceFlow-ordering.bpmn b/test/spec/import/sequenceFlow-ordering.bpmn new file mode 100644 index 00000000..f9a3b2a3 --- /dev/null +++ b/test/spec/import/sequenceFlow-ordering.bpmn @@ -0,0 +1,110 @@ + + + + + + + + + Task_16z77et + StartEvent_1 + Task_14uuigv + EndEvent_0exo24i + + + Task_1y8k07l + ExclusiveGateway_0xyt6ot + + + + SequenceFlow_1q8max9 + + + SequenceFlow_1q8max9 + + + + SequenceFlow_01mqkl9 + + + SequenceFlow_01mqkl9 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +