From 6c77e236923e0575ad9e89ec24f82bca152d57a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20St=C3=BCmmel?= Date: Tue, 5 Jul 2016 15:54:14 +0200 Subject: [PATCH] fix(modeling): do not layout hidden labels closes #587 --- .../modeling/behavior/LabelBehavior.js | 170 ++++++++--------- .../features/modeling/LabelLayoutingSpec.js | 22 +++ .../modeling/behavior/LabelBehaviorSpec.js | 180 ++++++++++++------ 3 files changed, 225 insertions(+), 147 deletions(-) diff --git a/lib/features/modeling/behavior/LabelBehavior.js b/lib/features/modeling/behavior/LabelBehavior.js index 3e174d12..3e3bc912 100644 --- a/lib/features/modeling/behavior/LabelBehavior.js +++ b/lib/features/modeling/behavior/LabelBehavior.js @@ -29,7 +29,8 @@ function LabelSupport(eventBus, modeling, bpmnFactory) { CommandInterceptor.call(this, eventBus); - // create external labels on shape creation + + ///// create external labels on shape creation this.postExecute([ 'shape.create', 'connection.create' ], function(e) { var context = e.context; @@ -50,94 +51,12 @@ function LabelSupport(eventBus, modeling, bpmnFactory) { } }); - // update label position on waypoints change if still hidden - this.postExecute([ 'connection.updateWaypoints' ], function(e) { - var context = e.context, - connection = context.connection, - label = connection.label; - if (!label) { - return; - } + ///// update di information on label creation - if (label.hidden) { - var position = getExternalLabelMid(connection); + this.executed([ 'label.create' ], function(event) { - var delta = { - x: position.x - label.x - label.width / 2, - y: position.y - label.y - label.height / 2 - }; - - modeling.moveShape(label, delta); - } - }); - - - - this.postExecute([ - 'connection.layout', - 'connection.reconnectEnd', - 'connection.reconnectStart' - ], function(e) { - - var context = e.context, - oldWaypoints = context.oldWaypoints, - newWaypoints = context.newWaypoints || context.connection.waypoints, - label = context.connection.label, - hints = context.hints || {}, - delta; - - if (e.command != 'connection.layout') { - hints.startChanged = e.command == 'connection.reconnectStart' ? true : false; - hints.endChanged = e.command == 'connection.reconnectEnd' ? true : false; - } - - if (!label) { - return; - } - - delta = getLabelAdjustment(label, newWaypoints, oldWaypoints, hints); - - modeling.moveShape(label, delta); - }); - - this.postExecute([ - 'connection.updateWaypoints' - ], function(e) { - - var context = e.context, - oldWaypoints = context.oldWaypoints, - newWaypoints = context.newWaypoints, - label = context.connection.label, - hints = context.hints || {}, - delta; - - if (!label) { - return; - } - - delta = getLabelAdjustment(label, newWaypoints, oldWaypoints, hints); - - modeling.moveShape(label, delta); - }); - - this.postExecute([ 'shape.replace' ], function(e) { - var context = e.context, - newShape = context.newShape, - oldShape = context.oldShape; - - var businessObject = getBusinessObject(newShape); - - if (businessObject && hasExternalLabel(businessObject)) { - newShape.label.x = oldShape.label.x; - newShape.label.y = oldShape.label.y; - } - }); - - // update di information on label creation - this.executed([ 'label.create' ], function(e) { - - var element = e.context.shape, + var element = event.context.shape, businessObject, di; @@ -168,6 +87,85 @@ function LabelSupport(eventBus, modeling, bpmnFactory) { height: element.height }); }); + + + ///// update label position on connection change + + function getHiddenLabelAdjustment(event) { + + var context = event.context, + connection = context.connection, + label = connection.label; + + var labelPosition = getExternalLabelMid(connection); + + return { + x: labelPosition.x - label.x - label.width / 2, + y: labelPosition.y - label.y - label.height / 2 + }; + } + + function getVisibleLabelAdjustment(event) { + + var command = event.command, + context = event.context, + connection = context.connection, + label = connection.label, + hints = assign({}, context.hints), + newWaypoints = context.newWaypoints || connection.waypoints, + oldWaypoints = context.oldWaypoints; + + + if (typeof hints.startChanged === 'undefined') { + hints.startChanged = (command === 'connection.reconnectStart'); + } + + if (typeof hints.endChanged === 'undefined') { + hints.endChanged = (command === 'connection.reconnectEnd'); + } + + return getLabelAdjustment(label, newWaypoints, oldWaypoints, hints); + } + + this.postExecute([ + 'connection.layout', + 'connection.reconnectEnd', + 'connection.reconnectStart', + 'connection.updateWaypoints' + ], function(event) { + + var label = event.context.connection.label, + labelAdjustment; + + if (!label) { + return; + } + + if (label.hidden) { + labelAdjustment = getHiddenLabelAdjustment(event); + } else { + labelAdjustment = getVisibleLabelAdjustment(event); + } + + modeling.moveShape(label, labelAdjustment); + }); + + + ////// keep label position on shape replace + + this.postExecute([ 'shape.replace' ], function(event) { + var context = event.context, + newShape = context.newShape, + oldShape = context.oldShape; + + var businessObject = getBusinessObject(newShape); + + if (businessObject && hasExternalLabel(businessObject)) { + newShape.label.x = oldShape.label.x; + newShape.label.y = oldShape.label.y; + } + }); + } inherits(LabelSupport, CommandInterceptor); diff --git a/test/spec/features/modeling/LabelLayoutingSpec.js b/test/spec/features/modeling/LabelLayoutingSpec.js index f3c64e4f..b9a232a0 100644 --- a/test/spec/features/modeling/LabelLayoutingSpec.js +++ b/test/spec/features/modeling/LabelLayoutingSpec.js @@ -73,6 +73,27 @@ describe('modeling - label layouting', function() { describe('on segment move', function() { + it('label name not set -> move label to waypoints mid', inject(function(modeling, elementRegistry, connectionSegmentMove, dragging) { + + // given + var connection = elementRegistry.get('SequenceFlow_C'), + labelPosition = getLabelPosition(connection); + + connection.label.businessObject.name = false; + connection.label.hidden = true; + + // when + connectionSegmentMove.start(canvasEvent({ x: 0, y: 0 }), connection, 2); + + dragging.move(canvasEvent({ x: 0, y: 50 })); + + dragging.end(); + + // then + expectLabelMoved(connection, labelPosition, { x: -82, y: 18 }); // waypoints mid + + })); + it('left - no relayout', inject(function(elementRegistry, connectionSegmentMove, dragging) { // given @@ -181,6 +202,7 @@ describe('modeling - label layouting', function() { expectLabelMoved(connection, labelPosition, { x: -39, y: -85 }); })); + // TODO(@janstuemmel): solve by connectionSegmentMove refactoring it.skip('up - remove two bendpoints - redundant waypoints', inject(function(elementRegistry, connectionSegmentMove, dragging, bendpointMove) { diff --git a/test/spec/features/modeling/behavior/LabelBehaviorSpec.js b/test/spec/features/modeling/behavior/LabelBehaviorSpec.js index cc9a7255..00bd7b3b 100644 --- a/test/spec/features/modeling/behavior/LabelBehaviorSpec.js +++ b/test/spec/features/modeling/behavior/LabelBehaviorSpec.js @@ -138,79 +138,137 @@ describe('behavior - LabelBehavior', function() { expect(startEvent.di.label).to.have.position({ x: 156, y: 128 }); })); - it('should move connection label on waypoints update if still hidden', inject(function(elementRegistry, modeling) { - // given - var startEventShape = elementRegistry.get('StartEvent_1'), - taskShape = elementRegistry.get('Task_1'); + describe('connection labels', function() { - var sequenceFlowConnection = modeling.createConnection(startEventShape, taskShape, { - type: 'bpmn:SequenceFlow' - }, startEventShape.parent); + it('should center position hidden on waypoint change', inject(function(elementRegistry, modeling) { - // when - modeling.updateWaypoints(sequenceFlowConnection, [ - sequenceFlowConnection.waypoints[0], - { - x: sequenceFlowConnection.waypoints[0].x, - y: 200 - }, - { - x: sequenceFlowConnection.waypoints[1].x, - y: 200 - }, - sequenceFlowConnection.waypoints[1] - ]); + // given + var startEventShape = elementRegistry.get('StartEvent_1'), + taskShape = elementRegistry.get('Task_1'); - // then - var expected = { - x: LabelUtil.getExternalLabelMid(sequenceFlowConnection).x - sequenceFlowConnection.label.width / 2, - y: LabelUtil.getExternalLabelMid(sequenceFlowConnection).y - sequenceFlowConnection.label.height / 2 - }; + var sequenceFlowConnection = modeling.createConnection(startEventShape, taskShape, { + type: 'bpmn:SequenceFlow' + }, startEventShape.parent); - expect({ - x: sequenceFlowConnection.label.x, - y: sequenceFlowConnection.label.y - }).to.eql(expected); - })); + // when + modeling.updateWaypoints(sequenceFlowConnection, [ + sequenceFlowConnection.waypoints[0], + { + x: sequenceFlowConnection.waypoints[0].x, + y: 200 + }, + { + x: sequenceFlowConnection.waypoints[1].x, + y: 200 + }, + sequenceFlowConnection.waypoints[1] + ]); - it('should not move connection label on waypoints change if not hidden', inject(function(elementRegistry, modeling) { + // then + var expected = { + x: LabelUtil.getExternalLabelMid(sequenceFlowConnection).x - sequenceFlowConnection.label.width / 2, + y: LabelUtil.getExternalLabelMid(sequenceFlowConnection).y - sequenceFlowConnection.label.height / 2 + }; - // given - var startEventShape = elementRegistry.get('StartEvent_1'), - taskShape = elementRegistry.get('Task_1'); + expect({ + x: sequenceFlowConnection.label.x, + y: sequenceFlowConnection.label.y + }).to.eql(expected); + })); - var sequenceFlowConnection = modeling.createConnection(startEventShape, taskShape, { - type: 'bpmn:SequenceFlow' - }, startEventShape.parent); - var labelPosition = { - x: sequenceFlowConnection.label.x, - y: sequenceFlowConnection.label.y - }; + it('should center position hidden on source move', inject(function(elementRegistry, modeling) { - // when - sequenceFlowConnection.label.hidden = false; + // given + var startEventShape = elementRegistry.get('StartEvent_1'), + taskShape = elementRegistry.get('Task_1'); - modeling.updateWaypoints(sequenceFlowConnection, [ - sequenceFlowConnection.waypoints[0], - { - x: sequenceFlowConnection.waypoints[0].x, - y: 200 - }, - { - x: sequenceFlowConnection.waypoints[1].x, - y: 200 - }, - sequenceFlowConnection.waypoints[1] - ]); + var sequenceFlowConnection = modeling.createConnection(startEventShape, taskShape, { + type: 'bpmn:SequenceFlow' + }, startEventShape.parent); - // then - expect({ - x: sequenceFlowConnection.label.x, - y: sequenceFlowConnection.label.y - }).to.eql(labelPosition); - })); + // when + modeling.moveElements([ startEventShape ], { x: 50, y: 0 }); + + // then + var expected = { + x: LabelUtil.getExternalLabelMid(sequenceFlowConnection).x - sequenceFlowConnection.label.width / 2, + y: LabelUtil.getExternalLabelMid(sequenceFlowConnection).y - sequenceFlowConnection.label.height / 2 + }; + + expect({ + x: sequenceFlowConnection.label.x, + y: sequenceFlowConnection.label.y + }).to.eql(expected); + })); + + + it('should center position hidden on target move', inject(function(elementRegistry, modeling) { + + // given + var startEventShape = elementRegistry.get('StartEvent_1'), + taskShape = elementRegistry.get('Task_1'); + + var sequenceFlowConnection = modeling.createConnection(startEventShape, taskShape, { + type: 'bpmn:SequenceFlow' + }, startEventShape.parent); + + // when + modeling.moveElements([ taskShape ], { x: 50, y: 0 }); + + // then + var expected = { + x: LabelUtil.getExternalLabelMid(sequenceFlowConnection).x - sequenceFlowConnection.label.width / 2, + y: LabelUtil.getExternalLabelMid(sequenceFlowConnection).y - sequenceFlowConnection.label.height / 2 + }; + + expect({ + x: sequenceFlowConnection.label.x, + y: sequenceFlowConnection.label.y + }).to.eql(expected); + })); + + + it('should NOT center position visible', inject(function(elementRegistry, modeling) { + + // given + var startEventShape = elementRegistry.get('StartEvent_1'), + taskShape = elementRegistry.get('Task_1'); + + var sequenceFlowConnection = modeling.createConnection(startEventShape, taskShape, { + type: 'bpmn:SequenceFlow' + }, startEventShape.parent); + + var oldLabelPosition = { + x: sequenceFlowConnection.label.x, + y: sequenceFlowConnection.label.y + }; + + // when + sequenceFlowConnection.label.hidden = false; + + modeling.updateWaypoints(sequenceFlowConnection, [ + sequenceFlowConnection.waypoints[0], + { + x: sequenceFlowConnection.waypoints[0].x, + y: 200 + }, + { + x: sequenceFlowConnection.waypoints[1].x, + y: 200 + }, + sequenceFlowConnection.waypoints[1] + ]); + + // then + expect({ + x: sequenceFlowConnection.label.x, + y: sequenceFlowConnection.label.y + }).to.eql(oldLabelPosition); + })); + + }); });