fix(modeling): do not layout hidden labels

closes #587

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) {
+  ///// update di information on label creation
-      return;
-    }
 
-    if (label.hidden) {
+  this.executed([ 'label.create' ], function(event) {
-      var position = getExternalLabelMid(connection);
 
-      var delta = {
+    var element = event.context.shape,
-        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,
         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);

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) {

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
+    describe('connection labels', function() {
-      var startEventShape = elementRegistry.get('StartEvent_1'),
-          taskShape = elementRegistry.get('Task_1');
 
-      var sequenceFlowConnection = modeling.createConnection(startEventShape, taskShape, {
+      it('should center position hidden on waypoint change', inject(function(elementRegistry, modeling) {
-        type: 'bpmn:SequenceFlow'
-      }, startEventShape.parent);
 
-      // when
+        // given
-      modeling.updateWaypoints(sequenceFlowConnection, [
+        var startEventShape = elementRegistry.get('StartEvent_1'),
-        sequenceFlowConnection.waypoints[0],
+            taskShape = elementRegistry.get('Task_1');
-        {
-          x: sequenceFlowConnection.waypoints[0].x,
-          y: 200
-        },
-        {
-          x: sequenceFlowConnection.waypoints[1].x,
-          y: 200
-        },
-        sequenceFlowConnection.waypoints[1]
-      ]);
 
-      // then
+        var sequenceFlowConnection = modeling.createConnection(startEventShape, taskShape, {
-      var expected = {
+          type: 'bpmn:SequenceFlow'
-        x: LabelUtil.getExternalLabelMid(sequenceFlowConnection).x - sequenceFlowConnection.label.width / 2,
+        }, startEventShape.parent);
-        y: LabelUtil.getExternalLabelMid(sequenceFlowConnection).y - sequenceFlowConnection.label.height / 2
-      };
 
-      expect({
+        // when
-        x: sequenceFlowConnection.label.x,
+        modeling.updateWaypoints(sequenceFlowConnection, [
-        y: sequenceFlowConnection.label.y
+          sequenceFlowConnection.waypoints[0],
-      }).to.eql(expected);
+          {
-    }));
+            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
+        expect({
-      var startEventShape = elementRegistry.get('StartEvent_1'),
+          x: sequenceFlowConnection.label.x,
-          taskShape = elementRegistry.get('Task_1');
+          y: sequenceFlowConnection.label.y
+        }).to.eql(expected);
+      }));
 
-      var sequenceFlowConnection = modeling.createConnection(startEventShape, taskShape, {
-        type: 'bpmn:SequenceFlow'
-      }, startEventShape.parent);
 
-      var labelPosition = {
+      it('should center position hidden on source move', inject(function(elementRegistry, modeling) {
-        x: sequenceFlowConnection.label.x,
-        y: sequenceFlowConnection.label.y
-      };
 
-      // when
+        // given
-      sequenceFlowConnection.label.hidden = false;
+        var startEventShape = elementRegistry.get('StartEvent_1'),
+            taskShape = elementRegistry.get('Task_1');
 
-      modeling.updateWaypoints(sequenceFlowConnection, [
+        var sequenceFlowConnection = modeling.createConnection(startEventShape, taskShape, {
-        sequenceFlowConnection.waypoints[0],
+          type: 'bpmn:SequenceFlow'
-        {
+        }, startEventShape.parent);
-          x: sequenceFlowConnection.waypoints[0].x,
-          y: 200
-        },
-        {
-          x: sequenceFlowConnection.waypoints[1].x,
-          y: 200
-        },
-        sequenceFlowConnection.waypoints[1]
-      ]);
 
-      // then
+        // when
-      expect({
+        modeling.moveElements([ startEventShape ], { x: 50, y: 0 });
-        x: sequenceFlowConnection.label.x,
+
-        y: sequenceFlowConnection.label.y
+        // then
-      }).to.eql(labelPosition);
+        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);
+      }));
+
+    });
 
   });