fix(labels): correct positioning + hide if empty

This commit improves the label handling by

* showing the label only if it exists
* updating the status accordingly
* positioning the label correctly on import

Related to #113

lib/core/BpmnImporter.js

@@ -125,7 +125,7 @@ BpmnImporter.prototype.addLabel = function (semantic, element) {
     id: semantic.id + '_label',
     labelTarget: element,
     type: 'label',
-    hidden: element.hidden,
+    hidden: element.hidden || !semantic.name,
     x: bounds.x,
     y: bounds.y,
     width: bounds.width,

lib/draw/BpmnRenderer.js

@@ -3,11 +3,11 @@
 var _ = require('lodash');
 
 var DefaultRenderer = require('diagram-js/lib/draw/Renderer');
-var LabelUtil = require('diagram-js/lib/util/LabelUtil');
+var TextUtil = require('diagram-js/lib/util/TextUtil');
 
 var DiUtil = require('../util/Di');
 
-var flattenPoints = DefaultRenderer.flattenPoints;
+var createLine = DefaultRenderer.createLine;
 
 
 function BpmnRenderer(events, styles, pathMap) {
@@ -22,7 +22,7 @@ function BpmnRenderer(events, styles, pathMap) {
     fontSize: '12px'
   };
 
-  var labelUtil = new LabelUtil({
+  var labelUtil = new TextUtil({
     style: LABEL_STYLE,
     size: { width: 100 }
   });
@@ -188,15 +188,13 @@ function BpmnRenderer(events, styles, pathMap) {
   }
 
   function drawLine(p, waypoints, attrs) {
-    var points = flattenPoints(waypoints);
-
     attrs = computeStyle(attrs, [ 'no-fill' ], {
       stroke: 'black',
       strokeWidth: 2,
       fill: 'none'
     });
 
-    return p.polyline(points).attr(attrs);
+    return createLine(waypoints, attrs).appendTo(p);
   }
 
   function drawPath(p, d, attrs) {
@@ -837,8 +835,8 @@ function BpmnRenderer(events, styles, pathMap) {
 
       if (expandedPool) {
         drawLine(p, [
-          {x: 30, y: 0},
+          { x: 30, y: 0 },
-          {x: 30, y: element.height}
+          { x: 30, y: element.height }
         ]);
         var text = getSemantic(element).name;
         renderLaneLabel(p, text, element);

lib/features/label-editing/LabelEditingProvider.js

@@ -48,7 +48,7 @@ LabelEditingProvider.prototype.activate = function(element) {
   var semantic = element.businessObject,
       di = semantic.di;
 
-  var text = LabelUtil.getLabel(semantic);
+  var text = LabelUtil.getLabel(element);
 
   if (text === undefined) {
     return;

lib/features/label-editing/LabelUtil.js

@@ -13,8 +13,9 @@ function getLabelAttr(semantic) {
   }
 }
 
-module.exports.getLabel = function(semantic) {
+module.exports.getLabel = function(element) {
-  var attr = getLabelAttr(semantic);
+  var semantic = element.businessObject,
+      attr = getLabelAttr(semantic);
 
   if (attr) {
     return semantic[attr] || '';
@@ -22,10 +23,18 @@ module.exports.getLabel = function(semantic) {
 };
 
 
-module.exports.setLabel = function(semantic, label) {
+module.exports.setLabel = function(element, text) {
-  var attr = getLabelAttr(semantic);
+  var semantic = element.businessObject,
+      attr = getLabelAttr(semantic);
 
   if (attr) {
-    semantic[attr] = label;
+    semantic[attr] = text;
   }
+
+  var label = element.label || element;
+
+  // hide label if not set
+  label.hidden = !text;
+
+  return label;
 };

lib/features/label-editing/cmd/UpdateLabelHandler.js

@@ -11,16 +11,13 @@ var LabelUtil = require('../LabelUtil');
 function UpdateTextHandler(eventBus) {
 
   function setText(element, text) {
+    var label = LabelUtil.setLabel(element, text);
 
-    var semantic = element.businessObject;
+    eventBus.fire('element.changed', { element: label });
-
-    LabelUtil.setLabel(semantic, text);
-
-    eventBus.fire('element.changed', { element: element.label || element });
   }
 
   function execute(ctx) {
-    ctx.oldLabel = LabelUtil.getLabel(ctx.element.businessObject);
+    ctx.oldLabel = LabelUtil.getLabel(ctx.element);
     setText(ctx.element, ctx.newLabel);
   }
 

lib/util/Label.js

@@ -94,6 +94,6 @@ module.exports.getExternalLabelBounds = function(semantic, element) {
 
   return _.extend({
     x: mid.x - size.width / 2,
-    y: mid.y
+    y: mid.y - size.height / 2
   }, size);
 };

test/spec/features/label-editing/LabelEditingProviderSpec.js

@@ -136,8 +136,7 @@ describe('features - label-editing', function() {
     it('should update via command stack', function() {
 
       // given
-      var diagramElement = elementRegistry.getById('user-task'),
+      var diagramElement = elementRegistry.getById('user-task');
-          semantic = diagramElement.businessObject;
 
       var listenerCalled;
 
@@ -157,10 +156,9 @@ describe('features - label-editing', function() {
     it('should undo via command stack', inject(function(commandStack) {
 
       // given
-      var diagramElement = elementRegistry.getById('user-task'),
+      var diagramElement = elementRegistry.getById('user-task');
-          semantic = diagramElement.businessObject;
 
-      var oldLabel = LabelUtil.getLabel(semantic);
+      var oldLabel = LabelUtil.getLabel(diagramElement);
 
       // when
       directEditActivate(diagramElement);
@@ -169,7 +167,7 @@ describe('features - label-editing', function() {
       commandStack.undo();
 
       // then
-      var label = LabelUtil.getLabel(semantic);
+      var label = LabelUtil.getLabel(diagramElement);
       expect(label).toBe(oldLabel);
     }));
 
@@ -181,8 +179,7 @@ describe('features - label-editing', function() {
     it('on shape change', function() {
 
       // given
-      var diagramElement = elementRegistry.getById('user-task'),
+      var diagramElement = elementRegistry.getById('user-task');
-          semantic = diagramElement.businessObject;
 
       var listenerCalled;
 
@@ -204,8 +201,7 @@ describe('features - label-editing', function() {
     it('on connection on change', function() {
 
       // given
-      var diagramElement = elementRegistry.getById('sequence-flow-no'),
+      var diagramElement = elementRegistry.getById('sequence-flow-no');
-          semantic = diagramElement.businessObject;
 
       var listenerCalled;
 
@@ -232,10 +228,9 @@ describe('features - label-editing', function() {
 
       return inject(function(elementRegistry, eventBus, directEditing) {
 
-        var diagramElement = elementRegistry.getById(elementId),
+        var diagramElement = elementRegistry.getById(elementId);
-            semantic = diagramElement.businessObject;
 
-        var label = LabelUtil.getLabel(semantic);
+        var label = LabelUtil.getLabel(diagramElement);
 
 
         // when
@@ -252,7 +247,7 @@ describe('features - label-editing', function() {
 
         // then
         // expect update to have happened
-        label = LabelUtil.getLabel(semantic);
+        label = LabelUtil.getLabel(diagramElement);
         expect(label).toBe('B');
 
 
@@ -261,7 +256,7 @@ describe('features - label-editing', function() {
         directEditCancel('C');
 
         // expect no label update to have happened
-        label = LabelUtil.getLabel(semantic);
+        label = LabelUtil.getLabel(diagramElement);
         expect(label).toBe('B');
       });
     }