fix(modeling): mark elements as changed during label update

Closes #484

lib/features/label-editing/LabelUtil.js

@@ -27,7 +27,7 @@ module.exports.getLabel = function(element) {
 };
 
 
-module.exports.setLabel = function(element, text) {
+module.exports.setLabel = function(element, text, isExternal) {
   var semantic = element.businessObject,
       attr = getLabelAttr(semantic);
 
@@ -35,10 +35,10 @@ module.exports.setLabel = function(element, text) {
     semantic[attr] = text;
   }
 
-  var label = element.label || element;
+  // show external label if not empty
+  if (isExternal) {
+    element.hidden = !text;
+  }
 
-  // show label
+  return element;
-  label.hidden = false;
-
-  return label;
 };

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

@@ -5,15 +5,27 @@ var LabelUtil = require('../LabelUtil');
 
 /**
  * A handler that updates the text of a BPMN element.
- *
- * @param {EventBus} eventBus
  */
-function UpdateTextHandler(eventBus) {
+function UpdateLabelHandler() {
 
+  /**
+   * Set the label and return the changed elements.
+   *
+   * Element parameter can be label itself or connection (i.e. sequence flow).
+   *
+   * @param {djs.model.Base} element
+   * @param {String} text
+   */
   function setText(element, text) {
-    var label = LabelUtil.setLabel(element, text);
 
-    eventBus.fire('element.changed', { element: label });
+    // external label if present
+    var label = element.label || element;
+
+    var labelTarget = element.labelTarget || element;
+
+    LabelUtil.setLabel(label, text, labelTarget !== label);
+
+    return [ label, labelTarget ];
   }
 
   function execute(ctx) {
@@ -25,20 +37,10 @@ function UpdateTextHandler(eventBus) {
     return setText(ctx.element, ctx.oldLabel);
   }
 
-
-  function canExecute(ctx) {
-    return true;
-  }
-
   // API
 
   this.execute = execute;
   this.revert = revert;
-
-  this.canExecute = canExecute;
 }
 
-
+module.exports = UpdateLabelHandler;
-UpdateTextHandler.$inject = [ 'eventBus' ];
-
-module.exports = UpdateTextHandler;

lib/features/modeling/cmd/UpdateCanvasRootHandler.js

@@ -41,6 +41,9 @@ UpdateCanvasRootHandler.prototype.execute = function(context) {
   newRootBusinessObject.di = diPlane;
 
   context.oldRoot = oldRoot;
+
+  // TODO(nikku): return changed elements?
+  // return [ newRoot, oldRoot ];
 };
 
 
@@ -70,4 +73,7 @@ UpdateCanvasRootHandler.prototype.revert = function(context) {
 
   diPlane.bpmnElement = oldRootBusinessObject;
   oldRootBusinessObject.di = diPlane;
+
+  // TODO(nikku): return changed elements?
+  // return [ newRoot, oldRoot ];
 };

lib/features/modeling/cmd/UpdateFlowNodeRefsHandler.js

@@ -153,6 +153,9 @@ UpdateFlowNodeRefsHandler.prototype.execute = function(context) {
       Collections.add(newLane.get(FLOW_NODE_REFS_ATTR), flowNode);
     });
   });
+
+  // TODO(nikku): return changed elements
+  // return [ ... ];
 };
 
 
@@ -177,4 +180,7 @@ UpdateFlowNodeRefsHandler.prototype.revert = function(context) {
       Collections.add(oldLane.get(FLOW_NODE_REFS_ATTR), flowNode);
     });
   });
+
+  // TODO(nikku): return changed elements
+  // return [ ... ];
 };

test/fixtures/bpmn/features/modeling/update-label.bpmn

@@ -0,0 +1,24 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<bpmn:definitions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:bpmn="http://www.omg.org/spec/BPMN/20100524/MODEL" xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI" xmlns:dc="http://www.omg.org/spec/DD/20100524/DC" xmlns:di="http://www.omg.org/spec/DD/20100524/DI" id="Definitions_1" targetNamespace="http://bpmn.io/schema/bpmn">
+  <bpmn:process id="Process_1" isExecutable="false">
+    <bpmn:startEvent id="StartEvent_1" name="Foo" />
+    <bpmn:startEvent id="StartEvent_2" />
+    <bpmn:task id="Task_1" />
+  </bpmn:process>
+  <bpmndi:BPMNDiagram id="BPMNDiagram_1">
+    <bpmndi:BPMNPlane id="BPMNPlane_1" bpmnElement="Process_1">
+      <bpmndi:BPMNShape id="_BPMNShape_StartEvent_2" bpmnElement="StartEvent_1">
+        <dc:Bounds x="173" y="102" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="StartEvent_1peicoz_di" bpmnElement="StartEvent_2">
+        <dc:Bounds x="255" y="102" width="36" height="36" />
+        <bpmndi:BPMNLabel>
+          <dc:Bounds x="228" y="138" width="90" height="20" />
+        </bpmndi:BPMNLabel>
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="Task_16agoun_di" bpmnElement="Task_1">
+        <dc:Bounds x="339" y="80" width="100" height="80" />
+      </bpmndi:BPMNShape>
+    </bpmndi:BPMNPlane>
+  </bpmndi:BPMNDiagram>
+</bpmn:definitions>

test/spec/features/modeling/UpdateLabelSpec.js

@@ -0,0 +1,129 @@
+'use strict';
+
+/* global bootstrapModeler, inject */
+
+var modelingModule = require('../../../../lib/features/modeling'),
+    coreModule = require('../../../../lib/core');
+
+
+describe('features/modeling - update label', function() {
+
+  var diagramXML = require('../../../fixtures/bpmn/features/modeling/update-label.bpmn');
+
+  var testModules = [ coreModule, modelingModule ];
+
+  beforeEach(bootstrapModeler(diagramXML, { modules: testModules }));
+
+
+  it('should change name of start event', inject(function(modeling, elementRegistry, eventBus) {
+
+    // given
+    var startEvent_1 = elementRegistry.get('StartEvent_1');
+
+    // when
+    modeling.updateLabel(startEvent_1, 'bar');
+
+    // then
+    expect(startEvent_1.businessObject.name).to.equal('bar');
+    expect(startEvent_1.label.hidden).to.be.false;
+  }));
+
+
+  it('should change name of start event with hidden label', inject(function(modeling, elementRegistry) {
+
+    // given
+    var startEvent_2 = elementRegistry.get('StartEvent_2');
+
+    // when
+    modeling.updateLabel(startEvent_2, 'bar');
+
+    // then
+    expect(startEvent_2.businessObject.name).to.equal('bar');
+    expect(startEvent_2.label.hidden).to.be.false;
+  }));
+
+
+  it('should hide label when setting empty string', inject(function(modeling, elementRegistry) {
+
+    // given
+    var startEvent_1 = elementRegistry.get('StartEvent_1');
+
+    // when
+    modeling.updateLabel(startEvent_1, '');
+
+    // then
+    expect(startEvent_1.businessObject.name).to.equal('');
+    expect(startEvent_1.label.hidden).to.be.true;
+  }));
+
+
+  it('should change name of start event when editing label', inject(function(modeling, elementRegistry) {
+
+    // given
+    var startEvent_1 = elementRegistry.get('StartEvent_1');
+    var startEvent_1_label = elementRegistry.get('StartEvent_1_label');
+
+    // when
+    modeling.updateLabel(startEvent_1_label, 'bar');
+
+    // then
+    expect(startEvent_1.businessObject.name).to.equal('bar');
+    expect(startEvent_1.label.hidden).to.be.false;
+  }));
+
+
+  it('should change name of task', inject(function(modeling, elementRegistry) {
+
+    // given
+    var task_1 = elementRegistry.get('Task_1');
+
+    // when
+    modeling.updateLabel(task_1, 'foo');
+
+    // then
+    expect(task_1.businessObject.name).to.equal('foo');
+    expect(task_1.label).to.be.undefined;
+  }));
+
+
+  it('should propertly fire events.changed after event name change',
+    inject(function(modeling, elementRegistry, eventBus) {
+
+    // given
+    var startEvent_1 = elementRegistry.get('StartEvent_1');
+
+    var changedEvent;
+
+    eventBus.on('elements.changed', function(event) {
+      changedEvent = event;
+    });
+
+    // when
+    modeling.updateLabel(startEvent_1, 'foo');
+
+    // then
+    expect(changedEvent.elements).to.include(startEvent_1);
+  }));
+
+
+  it('should propertly fire events.changed after event label change',
+    inject(function(modeling, elementRegistry, eventBus) {
+
+    // given
+    var startEvent_1 = elementRegistry.get('StartEvent_1');
+    var startEvent_1_label = elementRegistry.get('StartEvent_1_label');
+
+    var changedEvent;
+
+    eventBus.on('elements.changed', function(event) {
+      changedEvent = event;
+    });
+
+    // when
+    modeling.updateLabel(startEvent_1_label, 'foo');
+
+    // then
+    expect(changedEvent.elements).to.include(startEvent_1);
+  }));
+
+});