From 53e7198a47df9d05ae5b183bae1a5ea8fafc641a Mon Sep 17 00:00:00 2001 From: Maciej Barelkowski Date: Mon, 9 Mar 2020 10:34:58 +0100 Subject: [PATCH 1/7] fix(modeling): assign id prefix matching element type Closes #1285 --- lib/features/modeling/BpmnFactory.js | 2 +- .../spec/features/modeling/BpmnFactorySpec.js | 60 +++++++++++++++---- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/lib/features/modeling/BpmnFactory.js b/lib/features/modeling/BpmnFactory.js index 4a2fcc91..b334d4e8 100644 --- a/lib/features/modeling/BpmnFactory.js +++ b/lib/features/modeling/BpmnFactory.js @@ -53,7 +53,7 @@ BpmnFactory.prototype._ensureId = function(element) { prefix = 'Event'; } else if (is(element, 'bpmn:Gateway')) { prefix = 'Gateway'; - } else if (is(element, 'bpmn:FlowElement')) { + } else if (isAny(element, [ 'bpmn:SequenceFlow', 'bpmn:MessageFlow' ])) { prefix = 'Flow'; } else { prefix = (element.$type || '').replace(/^[^:]*:/g, ''); diff --git a/test/spec/features/modeling/BpmnFactorySpec.js b/test/spec/features/modeling/BpmnFactorySpec.js index 96e42ea4..a388ff13 100644 --- a/test/spec/features/modeling/BpmnFactorySpec.js +++ b/test/spec/features/modeling/BpmnFactorySpec.js @@ -52,27 +52,67 @@ describe('features - bpmn-factory', function() { it('should assign id with generic semantic prefix (Gateway)', inject(function(bpmnFactory) { - var task = bpmnFactory.create('bpmn:ParallelGateway'); + var gateway = bpmnFactory.create('bpmn:ParallelGateway'); - expect(task.$type).to.equal('bpmn:ParallelGateway'); - expect(task.id).to.match(/^Gateway_/g); + expect(gateway.$type).to.equal('bpmn:ParallelGateway'); + expect(gateway.id).to.match(/^Gateway_/g); })); it('should assign id with generic semantic prefix (Event)', inject(function(bpmnFactory) { - var task = bpmnFactory.create('bpmn:EndEvent'); + var event = bpmnFactory.create('bpmn:EndEvent'); - expect(task.$type).to.equal('bpmn:EndEvent'); - expect(task.id).to.match(/^Event_/g); + expect(event.$type).to.equal('bpmn:EndEvent'); + expect(event.id).to.match(/^Event_/g); })); - it('should assign id with generic semantic prefix (FlowElement)', inject( + it('should assign id with generic semantic prefix (Flow)', inject( function(bpmnFactory) { - var task = bpmnFactory.create('bpmn:SequenceFlow'); + var flow = bpmnFactory.create('bpmn:SequenceFlow'); - expect(task.$type).to.equal('bpmn:SequenceFlow'); - expect(task.id).to.match(/^Flow_/g); + expect(flow.$type).to.equal('bpmn:SequenceFlow'); + expect(flow.id).to.match(/^Flow_/g); + }) + ); + + + it('should assign id with generic semantic prefix (Flow)', inject( + function(bpmnFactory) { + var flow = bpmnFactory.create('bpmn:MessageFlow'); + + expect(flow.$type).to.equal('bpmn:MessageFlow'); + expect(flow.id).to.match(/^Flow_/g); + }) + ); + + + it('should assign id with specific semantic prefix (DataStore)', inject( + function(bpmnFactory) { + var dataStore = bpmnFactory.create('bpmn:DataStore'); + + expect(dataStore.$type).to.equal('bpmn:DataStore'); + expect(dataStore.id).to.match(/^DataStore_/g); + }) + ); + + + it('should assign id with specific semantic prefix (DataObject)', inject( + function(bpmnFactory) { + var dataObject = bpmnFactory.create('bpmn:DataObject'); + + expect(dataObject.$type).to.equal('bpmn:DataObject'); + expect(dataObject.id).to.match(/^DataObject_/g); + }) + ); + + + it('should assign id with specific semantic prefix (DataObjectReference)', inject( + function(bpmnFactory) { + var dataObjectReference = bpmnFactory.create('bpmn:DataObjectReference'); + + expect(dataObjectReference.$type).to.equal('bpmn:DataObjectReference'); + expect(dataObjectReference.id).to.match(/^DataObjectReference_/g); }) ); }); From d902a970e87b2505efdc3b3cbfb91f48b703a2b9 Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Mon, 9 Mar 2020 10:48:05 +0100 Subject: [PATCH 2/7] fix(modeling): update AddLaneHandler to new spaceTool API Related to https://github.com/camunda/camunda-modeler/issues/1715 --- lib/features/modeling/cmd/AddLaneHandler.js | 8 ++- .../features/modeling/lanes/AddLaneSpec.js | 54 +++++++++++++++++-- 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/lib/features/modeling/cmd/AddLaneHandler.js b/lib/features/modeling/cmd/AddLaneHandler.js index ba2a34ad..711de3d7 100644 --- a/lib/features/modeling/cmd/AddLaneHandler.js +++ b/lib/features/modeling/cmd/AddLaneHandler.js @@ -77,7 +77,13 @@ AddLaneHandler.prototype.preExecute = function(context) { var adjustments = spaceTool.calculateAdjustments(allAffected, 'y', offset, spacePos); - spaceTool.makeSpace(adjustments.movingShapes, adjustments.resizingShapes, { x: 0, y: offset }, direction); + spaceTool.makeSpace( + adjustments.movingShapes, + adjustments.resizingShapes, + { x: 0, y: offset }, + direction, + spacePos + ); // (2) create new lane at open space context.newLane = modeling.createShape({ type: 'bpmn:Lane' }, { diff --git a/test/spec/features/modeling/lanes/AddLaneSpec.js b/test/spec/features/modeling/lanes/AddLaneSpec.js index b7229034..1b5d128d 100644 --- a/test/spec/features/modeling/lanes/AddLaneSpec.js +++ b/test/spec/features/modeling/lanes/AddLaneSpec.js @@ -20,6 +20,8 @@ import { query as domQuery } from 'min-dom'; var DEFAULT_LANE_HEIGHT = 120; +var testModules = [ coreModule, modelingModule ]; + function getBounds(element) { return pick(element, [ 'x', 'y', 'width', 'height' ]); @@ -32,9 +34,10 @@ describe('features/modeling - add Lane', function() { var diagramXML = require('./lanes.bpmn'); - var testModules = [ coreModule, modelingModule ]; + beforeEach(bootstrapModeler(diagramXML, { + modules: testModules + })); - beforeEach(bootstrapModeler(diagramXML, { modules: testModules })); it('should add after Lane', inject(function(elementRegistry, modeling) { @@ -191,9 +194,9 @@ describe('features/modeling - add Lane', function() { var diagramXML = require('./participant-no-lane.bpmn'); - var testModules = [ coreModule, modelingModule ]; - - beforeEach(bootstrapModeler(diagramXML, { modules: testModules })); + beforeEach(bootstrapModeler(diagramXML, { + modules: testModules + })); it('should add after Participant', inject(function(elementRegistry, modeling) { @@ -267,6 +270,47 @@ describe('features/modeling - add Lane', function() { }); + describe('flow node handling', function() { + + var diagramXML = require('./lanes.bpmn'); + + beforeEach(bootstrapModeler(diagramXML, { + modules: testModules + })); + + + it('should move flow nodes and sequence flows', inject(function(elementRegistry, modeling) { + + // given + var laneShape = elementRegistry.get('Nested_Lane_B'), + task_Boundary = elementRegistry.get('Task_Boundary'), + boundary = elementRegistry.get('Boundary'), + sequenceFlow = elementRegistry.get('SequenceFlow'), + sequenceFlow_From_Boundary = elementRegistry.get('SequenceFlow_From_Boundary'); + + // when + var newLane = modeling.addLane(laneShape, 'top'); + + // then + expect(task_Boundary).to.have.position({ x: 264, y: -57 }); + expect(boundary).to.have.position({ x: 311, y: 5 }); + + expect(sequenceFlow_From_Boundary).to.have.waypoints([ + { x: 329, y: 161 - newLane.height }, + { x: 329, y: 188 - newLane.height }, + { x: 482, y: 188 - newLane.height }, + { x: 482, y: 143 - newLane.height } + ]); + + expect(sequenceFlow).to.have.waypoints([ + { x: 364, y: 103 - newLane.height }, + { x: 432, y: 103 - newLane.height } + ]); + })); + + }); + + describe('Internet Explorer', function() { var diagramXML = require('./participant-single-lane.bpmn'); From 368f9e14b53713ec7464950aec815a080a89259b Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Mon, 9 Mar 2020 12:02:51 +0100 Subject: [PATCH 3/7] fix(modeling): make AddLaneHandler recognize top-level labels Related to https://github.com/camunda/camunda-modeler/issues/1715 --- lib/features/modeling/cmd/AddLaneHandler.js | 6 ++ .../features/modeling/lanes/AddLaneSpec.js | 71 +++++++++++---- .../modeling/lanes/lanes-flow-nodes.bpmn | 88 +++++++++++++++++++ 3 files changed, 149 insertions(+), 16 deletions(-) create mode 100644 test/spec/features/modeling/lanes/lanes-flow-nodes.bpmn diff --git a/lib/features/modeling/cmd/AddLaneHandler.js b/lib/features/modeling/cmd/AddLaneHandler.js index 711de3d7..bc9a6836 100644 --- a/lib/features/modeling/cmd/AddLaneHandler.js +++ b/lib/features/modeling/cmd/AddLaneHandler.js @@ -18,6 +18,7 @@ import { * above or below an existing one. * * @param {Modeling} modeling + * @param {SpaceTool} spaceTool */ export default function AddLaneHandler(modeling, spaceTool) { this._modeling = modeling; @@ -61,6 +62,11 @@ AddLaneHandler.prototype.preExecute = function(context) { eachElement(lanesRoot, function(element) { allAffected.push(element); + // handle element labels in the diagram root + if (element.label) { + allAffected.push(element.label); + } + if (element === shape) { return []; } diff --git a/test/spec/features/modeling/lanes/AddLaneSpec.js b/test/spec/features/modeling/lanes/AddLaneSpec.js index 1b5d128d..baf480da 100644 --- a/test/spec/features/modeling/lanes/AddLaneSpec.js +++ b/test/spec/features/modeling/lanes/AddLaneSpec.js @@ -272,42 +272,81 @@ describe('features/modeling - add Lane', function() { describe('flow node handling', function() { - var diagramXML = require('./lanes.bpmn'); + var diagramXML = require('./lanes-flow-nodes.bpmn'); beforeEach(bootstrapModeler(diagramXML, { modules: testModules })); + function addLaneAbove(laneId) { - it('should move flow nodes and sequence flows', inject(function(elementRegistry, modeling) { + return getBpmnJS().invoke(function(elementRegistry, modeling) { + var existingLane = elementRegistry.get(laneId); + + expect(existingLane).to.exist; + + return modeling.addLane(existingLane, 'top'); + }); + } + + + it('should move flow nodes', inject(function(elementRegistry, modeling) { // given - var laneShape = elementRegistry.get('Nested_Lane_B'), - task_Boundary = elementRegistry.get('Task_Boundary'), - boundary = elementRegistry.get('Boundary'), - sequenceFlow = elementRegistry.get('SequenceFlow'), + var task_Boundary = elementRegistry.get('Task_Boundary'), + boundary = elementRegistry.get('Boundary'); + + // when + addLaneAbove('Nested_Lane_B'); + + // then + expect(task_Boundary).to.have.position({ x: 344, y: -7 }); + expect(boundary).to.have.position({ x: 391, y: 55 }); + })); + + + it('should move sequence flows', inject(function(elementRegistry, modeling) { + + // given + var sequenceFlow = elementRegistry.get('SequenceFlow'), sequenceFlow_From_Boundary = elementRegistry.get('SequenceFlow_From_Boundary'); // when - var newLane = modeling.addLane(laneShape, 'top'); + addLaneAbove('Nested_Lane_B'); // then - expect(task_Boundary).to.have.position({ x: 264, y: -57 }); - expect(boundary).to.have.position({ x: 311, y: 5 }); - expect(sequenceFlow_From_Boundary).to.have.waypoints([ - { x: 329, y: 161 - newLane.height }, - { x: 329, y: 188 - newLane.height }, - { x: 482, y: 188 - newLane.height }, - { x: 482, y: 143 - newLane.height } + { x: 409, y: 91 }, + { x: 409, y: 118 }, + { x: 562, y: 118 }, + { x: 562, y: 73 } ]); expect(sequenceFlow).to.have.waypoints([ - { x: 364, y: 103 - newLane.height }, - { x: 432, y: 103 - newLane.height } + { x: 444, y: 33 }, + { x: 512, y: 33 } ]); })); + + it('should move external labels', inject(function(elementRegistry, modeling) { + + // given + var event = elementRegistry.get('Event'), + label = event.label; + + // TODO(nikku): consolidate import + editing behavior => not consistent right now + + // when + // force move label to trigger label editing + update parent behavior + modeling.moveElements([ label ], { x: 0, y: 0 }); + + addLaneAbove('Nested_Lane_B'); + + // then + expect(label.y).to.eql(58); + })); + }); diff --git a/test/spec/features/modeling/lanes/lanes-flow-nodes.bpmn b/test/spec/features/modeling/lanes/lanes-flow-nodes.bpmn new file mode 100644 index 00000000..dce2514e --- /dev/null +++ b/test/spec/features/modeling/lanes/lanes-flow-nodes.bpmn @@ -0,0 +1,88 @@ + + + + + + + + + Task_Boundary + Task + Event + Boundary + + + + Task_Boundary + Task + Event + Boundary + + + + + + SequenceFlow + + + SequenceFlow_From_Boundary + SequenceFlow + + + + SequenceFlow_From_Boundary + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From e81116f36845fdb5864d157e4ca12cc3af01dd12 Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Mon, 9 Mar 2020 15:11:41 +0100 Subject: [PATCH 4/7] chore(CHANGELOG): update to v6.3.2 --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 85205592..0ed6d914 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ All notable changes to [bpmn-js](https://github.com/bpmn-io/bpmn-js) are documen ___Note:__ Yet to be released changes appear here._ +## 6.3.2 + +* `FIX`: correctly move flows when adding lane ([#1287](https://github.com/bpmn-io/bpmn-js/pull/1287)) +* `FIX`: restore semantic IDs for non flow nodes ([#1285](https://github.com/bpmn-io/bpmn-js/issues/1285)) + ## 6.3.1 * `FIX`: prevent editor crash in some strict execution environments ([#1283](https://github.com/bpmn-io/bpmn-js/pull/1283)) From 96cd55a2c6e1d0bfc6e10961f55858c045e1d014 Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Mon, 9 Mar 2020 15:15:21 +0100 Subject: [PATCH 5/7] 6.3.2 --- package-lock.json | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index e9a1c530..acd4df45 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "bpmn-js", - "version": "6.3.1", + "version": "6.3.2", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index f3ce3270..40f87f91 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "bpmn-js", - "version": "6.3.1", + "version": "6.3.2", "description": "A bpmn 2.0 toolkit and web modeler", "scripts": { "all": "run-s lint test distro test:distro", From e4e789bd3eecc47597d97d6c8f1e71b941dee066 Mon Sep 17 00:00:00 2001 From: Philipp Fromme Date: Tue, 10 Mar 2020 14:35:39 +0100 Subject: [PATCH 6/7] fix(label-editing): resize empty text annotations on complete Related to bpmn-io/diagram-js-direct-editing#12 --- .../label-editing/cmd/UpdateLabelHandler.js | 5 ----- test/spec/features/modeling/UpdateLabelSpec.js | 17 ++++++++++++++++- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/lib/features/label-editing/cmd/UpdateLabelHandler.js b/lib/features/label-editing/cmd/UpdateLabelHandler.js index 04f2a382..6d12a2e9 100644 --- a/lib/features/label-editing/cmd/UpdateLabelHandler.js +++ b/lib/features/label-editing/cmd/UpdateLabelHandler.js @@ -104,11 +104,6 @@ export default function UpdateLabelHandler(modeling, textRenderer) { var text = getLabel(label); - // don't resize without text - if (!text) { - return; - } - // resize element based on label _or_ pre-defined bounds if (typeof newBounds === 'undefined') { newBounds = textRenderer.getExternalLabelBounds(label, text); diff --git a/test/spec/features/modeling/UpdateLabelSpec.js b/test/spec/features/modeling/UpdateLabelSpec.js index ac2e8bd9..6f8259d4 100644 --- a/test/spec/features/modeling/UpdateLabelSpec.js +++ b/test/spec/features/modeling/UpdateLabelSpec.js @@ -20,7 +20,7 @@ describe('features/modeling - update label', function() { it('should change name of start event', inject( - function(modeling, elementRegistry, eventBus) { + function(modeling, elementRegistry) { // given var startEvent_1 = elementRegistry.get('StartEvent_1'); @@ -190,4 +190,19 @@ describe('features/modeling - update label', function() { } )); + + it('should resize empty text annotation', inject(function(modeling, elementRegistry) { + + // given + var element = elementRegistry.get('TextAnnotation_1'); + + var newBounds = { x: 100, y: 100, width: 100, height: 30 }; + + // when + modeling.updateLabel(element, null, newBounds); + + // then + expect(element).to.have.bounds(newBounds); + })); + }); \ No newline at end of file From a767fd957701db4292917db4df3a9adf1e3da832 Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Thu, 12 Mar 2020 11:29:03 +0100 Subject: [PATCH 7/7] test(project): remove legacy CI hacks --- test/config/karma.distro.js | 33 +++++----------------------- test/config/karma.unit.js | 43 +++++++++---------------------------- 2 files changed, 15 insertions(+), 61 deletions(-) diff --git a/test/config/karma.distro.js b/test/config/karma.distro.js index fc2582f2..bed3d370 100644 --- a/test/config/karma.distro.js +++ b/test/config/karma.distro.js @@ -2,28 +2,16 @@ // configures browsers to run test against // any of [ 'ChromeHeadless', 'Chrome', 'Firefox', 'IE', 'PhantomJS' ] -var browsers = - (process.env.TEST_BROWSERS || 'PhantomJS') - .replace(/^\s+|\s+$/, '') - .split(/\s*,\s*/g) - .map(function(browser) { - if (browser === 'ChromeHeadless') { - process.env.CHROME_BIN = require('puppeteer').executablePath(); - - // workaround https://github.com/GoogleChrome/puppeteer/issues/290 - if (process.platform === 'linux') { - return 'ChromeHeadless_Linux'; - } - } - - return browser; - }); +var browsers = (process.env.TEST_BROWSERS || 'PhantomJS').split(','); +// use puppeteer provided Chrome for testing +process.env.CHROME_BIN = require('puppeteer').executablePath(); var VARIANT = process.env.VARIANT; var NODE_ENV = process.env.NODE_ENV; + module.exports = function(karma) { karma.set({ @@ -46,18 +34,7 @@ module.exports = function(karma) { reporters: [ 'progress' ], - customLaunchers: { - ChromeHeadless_Linux: { - base: 'ChromeHeadless', - flags: [ - '--no-sandbox', - '--disable-setuid-sandbox' - ], - debug: true - } - }, - - browsers: browsers, + browsers, browserNoActivityTimeout: 30000, diff --git a/test/config/karma.unit.js b/test/config/karma.unit.js index d8a99b82..d4f66c7d 100644 --- a/test/config/karma.unit.js +++ b/test/config/karma.unit.js @@ -1,31 +1,19 @@ -var coverage = process.env.COVERAGE; +/* global process */ var path = require('path'); -var basePath = '../../'; - -var absoluteBasePath = path.resolve(path.join(__dirname, basePath)); - -/* global process */ +var coverage = process.env.COVERAGE; // configures browsers to run test against // any of [ 'ChromeHeadless', 'Chrome', 'Firefox', 'IE', 'PhantomJS' ] -var browsers = - (process.env.TEST_BROWSERS || 'PhantomJS') - .replace(/^\s+|\s+$/, '') - .split(/\s*,\s*/g) - .map(function(browser) { - if (browser === 'ChromeHeadless') { - process.env.CHROME_BIN = require('puppeteer').executablePath(); +var browsers = (process.env.TEST_BROWSERS || 'PhantomJS').split(','); - // workaround https://github.com/GoogleChrome/puppeteer/issues/290 - if (process.platform === 'linux') { - return 'ChromeHeadless_Linux'; - } - } +// use puppeteer provided Chrome for testing +process.env.CHROME_BIN = require('puppeteer').executablePath(); - return browser; - }); +var basePath = '../..'; + +var absoluteBasePath = path.resolve(path.join(__dirname, basePath)); var suite = coverage ? 'test/coverageBundle.js' : 'test/testBundle.js'; @@ -33,7 +21,7 @@ var suite = coverage ? 'test/coverageBundle.js' : 'test/testBundle.js'; module.exports = function(karma) { karma.set({ - basePath: basePath, + basePath, frameworks: [ 'mocha', @@ -50,24 +38,13 @@ module.exports = function(karma) { reporters: [ 'progress' ].concat(coverage ? 'coverage' : []), - customLaunchers: { - ChromeHeadless_Linux: { - base: 'ChromeHeadless', - flags: [ - '--no-sandbox', - '--disable-setuid-sandbox' - ], - debug: true - } - }, - coverageReporter: { reporters: [ { type: 'lcov', subdir: '.' } ] }, - browsers: browsers, + browsers, browserNoActivityTimeout: 30000,