From 79e8378db00e4ce7dc8a296d98eaad094cea304f Mon Sep 17 00:00:00 2001 From: Philipp Fromme Date: Tue, 7 Mar 2017 09:30:48 +0100 Subject: [PATCH] fix(label-support): fix positioning of label created on shape create * make sure label shape is created with correct size * make sure test varifies behaviour Closes camunda/camunda-modeler#535 --- .../modeling/behavior/LabelBehavior.js | 67 ++++++++++++++++--- .../features/modeling/LabelLayoutingSpec.js | 14 ++-- .../modeling/behavior/LabelBehaviorSpec.js | 7 +- test/spec/features/replace/BpmnReplaceSpec.js | 8 +-- 4 files changed, 71 insertions(+), 25 deletions(-) diff --git a/lib/features/modeling/behavior/LabelBehavior.js b/lib/features/modeling/behavior/LabelBehavior.js index 3e3bc912..892aa6c4 100644 --- a/lib/features/modeling/behavior/LabelBehavior.js +++ b/lib/features/modeling/behavior/LabelBehavior.js @@ -15,6 +15,13 @@ var hasExternalLabel = LabelUtil.hasExternalLabel, var CommandInterceptor = require('diagram-js/lib/command/CommandInterceptor'); +var TextUtil = require('diagram-js/lib/util/Text'); + +var DEFAULT_LABEL_DIMENSIONS = { + width: 90, + height: 20 +}; + /** * A component that makes sure that external labels are added @@ -29,6 +36,8 @@ function LabelSupport(eventBus, modeling, bpmnFactory) { CommandInterceptor.call(this, eventBus); + var textUtil = new TextUtil(); + ///// create external labels on shape creation @@ -38,17 +47,26 @@ function LabelSupport(eventBus, modeling, bpmnFactory) { var element = context.shape || context.connection, businessObject = element.businessObject; - var position; - - if (hasExternalLabel(businessObject)) { - position = getExternalLabelMid(element); - - modeling.createLabel(element, position, { - id: businessObject.id + '_label', - hidden: !businessObject.name, - businessObject: businessObject - }); + if (!hasExternalLabel(businessObject)) { + return; } + + var labelCenter = getExternalLabelMid(element); + + // we don't care about x and y + var labelDimensions = getLayoutedBounds( + DEFAULT_LABEL_DIMENSIONS, + businessObject.name || '', + textUtil + ); + + modeling.createLabel(element, labelCenter, { + id: businessObject.id + '_label', + hidden: !businessObject.name, + businessObject: businessObject, + width: labelDimensions.width, + height: labelDimensions.height + }); }); @@ -173,3 +191,32 @@ inherits(LabelSupport, CommandInterceptor); LabelSupport.$inject = [ 'eventBus', 'modeling', 'bpmnFactory' ]; module.exports = LabelSupport; + + +// TODO(nikku): repeating code (search for ) + +var EXTERNAL_LABEL_STYLE = { + fontFamily: 'Arial, sans-serif', + fontSize: '11px' +}; + +function getLayoutedBounds(bounds, text, textUtil) { + + var layoutedLabelDimensions = textUtil.getDimensions(text, { + box: { + width: 90, + height: 30, + x: bounds.width / 2 + bounds.x, + y: bounds.height / 2 + bounds.y + }, + style: EXTERNAL_LABEL_STYLE + }); + + // resize label shape to fit label text + return { + x: Math.round(bounds.x + bounds.width / 2 - layoutedLabelDimensions.width / 2), + y: Math.round(bounds.y), + width: Math.ceil(layoutedLabelDimensions.width), + height: Math.ceil(layoutedLabelDimensions.height) + }; +} diff --git a/test/spec/features/modeling/LabelLayoutingSpec.js b/test/spec/features/modeling/LabelLayoutingSpec.js index 0023ba0f..08291533 100644 --- a/test/spec/features/modeling/LabelLayoutingSpec.js +++ b/test/spec/features/modeling/LabelLayoutingSpec.js @@ -24,7 +24,7 @@ var testModules = [ describe('modeling - label layouting', function() { - describe('should create label', function() { + describe('should position created label', function() { var diagramXML = require('./LabelLayouting.initial.bpmn'); @@ -33,7 +33,7 @@ describe('modeling - label layouting', function() { })); - it('horizontal', inject(function(modeling, elementRegistry) { // only + it('horizontal', inject(function(modeling, elementRegistry) { // given var element1 = elementRegistry.get('StartEvent_1'), @@ -43,10 +43,8 @@ describe('modeling - label layouting', function() { var connection = modeling.connect(element1, element2); // then - // we don't need to resize the label since it's invisible - // therefore it has the standard width of 90px - expect(connection.label.x).to.be.equal(427); - expect(connection.label.y).to.be.equal(332); + expect(connection.label.x).to.be.equal(472); + expect(connection.label.y).to.be.within(335, 336); })); @@ -60,8 +58,8 @@ describe('modeling - label layouting', function() { var connection = modeling.connect(element1, element2); // then - expect(connection.label.x).to.be.equal(292); - expect(connection.label.y).to.be.equal(219.5); + expect(connection.label.x).to.be.equal(337); + expect(connection.label.y).to.be.within(222, 224); })); }); diff --git a/test/spec/features/modeling/behavior/LabelBehaviorSpec.js b/test/spec/features/modeling/behavior/LabelBehaviorSpec.js index 5ab26b22..8e4785af 100644 --- a/test/spec/features/modeling/behavior/LabelBehaviorSpec.js +++ b/test/spec/features/modeling/behavior/LabelBehaviorSpec.js @@ -4,8 +4,6 @@ require('../../../../TestHelper'); /* global bootstrapModeler, inject */ -var assign = require('lodash/object/assign'); - var modelingModule = require('../../../../../lib/features/modeling'), coreModule = require('../../../../../lib/core'); @@ -81,7 +79,10 @@ describe('behavior - LabelBehavior', function() { expect(label).to.exist; expect(elementRegistry.get(label.id)).to.exist; - expect(label).to.have.bounds(assign({ x: 262, y: 138, width: 90, height: 20 })); + expect(label.x).to.equal(307); + expect(label.y).to.be.within(141, 142); + expect(label.width).to.be.equal(0); + expect(label.height).to.be.within(12, 14); })); diff --git a/test/spec/features/replace/BpmnReplaceSpec.js b/test/spec/features/replace/BpmnReplaceSpec.js index b3ee41e4..6b192d07 100644 --- a/test/spec/features/replace/BpmnReplaceSpec.js +++ b/test/spec/features/replace/BpmnReplaceSpec.js @@ -263,15 +263,15 @@ describe('features/replace - bpmn replace', function() { it('should keep label position', inject(function(elementRegistry, bpmnReplace, modeling) { // given - var exclusiveGateway = elementRegistry.get('ExclusiveGateway_1'); - var label = elementRegistry.get('ExclusiveGateway_1_label'); + var startEvent = elementRegistry.get('StartEvent_1'); + var label = elementRegistry.get('StartEvent_1_label'); var newElementData = { - type: 'bpmn:InclusiveGateway' + type: 'bpmn:EndEvent' }; // when - var newElement = bpmnReplace.replaceElement(exclusiveGateway, newElementData); + var newElement = bpmnReplace.replaceElement(startEvent, newElementData); // then expect(newElement.label.x).to.equal(label.x);