fix(replace): pass hints when moving children on replace

Related to bpmn-io/bpmn-js#1231

lib/features/modeling/cmd/ReplaceShapeHandler.js

@@ -40,7 +40,7 @@ ReplaceShapeHandler.prototype.preExecute = function(context) {
 
   var oldShape = context.oldShape,
       newData = context.newData,
-      hints = context.hints,
+      hints = context.hints || {},
       newShape;
 
   function canReconnect(type, source, target, connection) {
@@ -78,7 +78,7 @@ ReplaceShapeHandler.prototype.preExecute = function(context) {
   if (hints.moveChildren !== false) {
     children = oldShape.children.slice();
 
-    modeling.moveElements(children, { x: 0, y: 0 }, newShape);
+    modeling.moveElements(children, { x: 0, y: 0 }, newShape, hints);
   }
 
   // (4) reconnect connections to the new shape (where allowed)
@@ -93,7 +93,7 @@ ReplaceShapeHandler.prototype.preExecute = function(context) {
         allowed = canReconnect('connection.reconnectEnd', source, newShape, connection);
 
     if (allowed) {
-      self.reconnectEnd(connection, newShape, docking, { layoutConnection: hints.layoutConnection });
+      self.reconnectEnd(connection, newShape, docking, hints);
     }
   });
 
@@ -104,7 +104,7 @@ ReplaceShapeHandler.prototype.preExecute = function(context) {
         allowed = canReconnect('connection.reconnectStart', newShape, target, connection);
 
     if (allowed) {
-      self.reconnectStart(connection, newShape, docking, { layoutConnection: hints.layoutConnection });
+      self.reconnectStart(connection, newShape, docking, hints);
     }
 
   });

test/spec/features/auto-resize/AutoResizeSpec.js

@@ -340,6 +340,7 @@ describe('features/auto-resize', function() {
 
   });
 
+
   describe('collapsed shape', function() {
 
     var rootShape,
@@ -647,23 +648,19 @@ describe('features/auto-resize', function() {
     describe('hints', function() {
 
       it('should NOT resize on autoResize=false hint',
-        inject(function(autoResize, eventBus, replace) {
+        inject(function(autoResize, replace) {
 
           // given
           var autoResizeSpy = sinon.spy(autoResize, '_expand');
 
-          eventBus.on('commandStack.shape.replace.preExecute', function(event) {
-            event.context.hints = { autoResize: false };
-          });
-
           var replacement = {
             id: 'replacement',
             width: 300,
             height: 300
           };
 
           // when
-          replace.replaceElement(replacedShape, replacement);
+          replace.replaceElement(replacedShape, replacement, { autoResize: false });
 
           // then
           expect(autoResizeSpy).to.not.be.called;

test/spec/features/modeling/LayoutConnectionSpec.js

@@ -1,10 +1,14 @@
+/* global sinon */
+
 import {
   bootstrapDiagram,
   inject
 } from 'test/TestHelper';
 
 import customModelingModule from './custom';
 
+var spy = sinon.spy;
+
 
 describe('features/modeling - layout connection', function() {
 
@@ -215,4 +219,99 @@ describe('features/modeling - layout connection', function() {
 
   });
 
+
+  describe('integration', function() {
+
+    var rootShape, sourceShape, targetShape, connection;
+
+    beforeEach(inject(function(elementFactory, canvas) {
+
+      rootShape = elementFactory.createRoot({
+        id: 'root'
+      });
+
+      canvas.setRootElement(rootShape);
+
+      sourceShape = elementFactory.createShape({
+        id: 'source',
+        x: 0, y: 0, width: 100, height: 100
+      });
+
+      canvas.addShape(sourceShape);
+
+      targetShape = elementFactory.createShape({
+        id: 'target',
+        x: 200, y: 0, width: 100, height: 100
+      });
+
+      canvas.addShape(targetShape);
+
+
+      connection = elementFactory.createConnection({
+        id: 'connection',
+        waypoints: [
+          { x: 100, y: 50 },
+          { x: 200, y: 50 }
+        ],
+        source: sourceShape,
+        target: targetShape
+      });
+
+      canvas.addConnection(connection);
+    }));
+
+
+    it('should lay out connection on source replace', inject(function(modeling) {
+
+      // given
+      var layoutConnectionSpy = spy(modeling, 'layoutConnection');
+
+      // when
+      modeling.replaceShape(sourceShape, { x: 0, y: 0, width: 100, height: 100 });
+
+      // then
+      expect(layoutConnectionSpy).to.have.been.called;
+    }));
+
+
+    it('should lay out connection on target replace', inject(function(modeling) {
+
+      // given
+      var layoutConnectionSpy = spy(modeling, 'layoutConnection');
+
+      // when
+      modeling.replaceShape(targetShape, { x: 200, y: 0, width: 100, height: 100 });
+
+      // then
+      expect(layoutConnectionSpy).to.have.been.called;
+    }));
+
+
+    it('should NOT lay out connection on source replace', inject(function(modeling) {
+
+      // given
+      var layoutConnectionSpy = spy(modeling, 'layoutConnection');
+
+      // when
+      modeling.replaceShape(sourceShape, { x: 0, y: 0, width: 100, height: 100 }, { layoutConnection: false });
+
+      // then
+      expect(layoutConnectionSpy).not.to.have.been.called;
+    }));
+
+
+    it('should NOT lay out connection on target replace', inject(function(modeling) {
+
+      // given
+      var layoutConnectionSpy = spy(modeling, 'layoutConnection');
+
+      // when
+      modeling.replaceShape(targetShape, { x: 200, y: 0, width: 100, height: 100 }, { layoutConnection: false });
+
+      // then
+      expect(layoutConnectionSpy).not.to.have.been.called;
+    }));
+
+  });
+
 });

test/spec/features/modeling/ReplaceShapeSpec.js

@@ -1,10 +1,15 @@
+/* global sinon */
+
 import {
   bootstrapDiagram,
   inject
 } from 'test/TestHelper';
 
 import modelingModule from 'lib/features/modeling';
 
+var spy = sinon.spy;
+
+
 describe('features/modeling - replace shape', function() {
 
   beforeEach(bootstrapDiagram({ modules: [ modelingModule ] }));
@@ -42,7 +47,7 @@ describe('features/modeling - replace shape', function() {
     }));
 
 
-    it('should move children per default', inject(function(elementFactory, modeling) {
+    it('should move children by default', inject(function(elementFactory, modeling) {
 
       // when
       var newShapeData = { x: 120, y: 120, width: 200, height: 200 };
@@ -77,82 +82,65 @@ describe('features/modeling - replace shape', function() {
 
   });
 
-  describe('different size', function() {
+  describe('hints', function() {
 
-    var root, parent, child1, child2, connection;
-
-    beforeEach(inject(function(elementFactory, canvas, modeling) {
+    var root, parent, shape1, shape2, connection;
 
+    beforeEach(inject(function(canvas, elementFactory) {
       root = elementFactory.createRoot({
         id: 'root'
       });
 
       canvas.setRootElement(root);
 
-      parent = elementFactory.createShape({
-        id: 'parent',
-        x: 20, y: 20, width: 400, height: 400
+      shape1 = elementFactory.createShape({
+        id: 'shape1',
+        x: 0, y: 0, width: 100, height: 100
       });
 
-      child1 = elementFactory.createShape({
-        id: 'child1',
-        x: 360, y: 30, width: 50, height: 50
-      });
+      canvas.addShape(shape1, root);
 
-      child2 = elementFactory.createShape({
-        id: 'child2',
-        x: 30, y: 30, width: 100, height: 100
+      shape2 = elementFactory.createShape({
+        id: 'shape2',
+        x: 200, y: 0, width: 100, height: 100
       });
 
+      canvas.addShape(shape2, root);
+
       connection = elementFactory.createConnection({
         id: 'connection',
-        source: child1,
-        target: child2,
+        source: shape1,
+        target: shape2,
         waypoints: [
-          { x: 360, y: 30 },
-          { x: 30, y: 30 }
+          { x: 100, y: 50 },
+          { x: 200, y: 50 }
         ]
       });
 
-      canvas.addShape(parent, root);
-      canvas.addShape(child1, parent);
-      canvas.addShape(child2, parent);
       canvas.addConnection(connection, parent);
-
-      modeling.layoutConnection(connection);
     }));
 
 
-    it('should relayout connection when replacing elements with different size',
-      inject(function(modeling) {
+    it('should pass hints', inject(function(modeling) {
 
-        // given
-        var newShapeData = { x: 130, y: 130, width: 200, height: 200 };
+      // given
+      var newShapeData = { x: 50, y: 50, width: 100, height: 100 },
+          hints = { foo: 'foo' };
 
-        // when
-        modeling.replaceShape(child2, newShapeData);
+      var moveElementsSpy = spy(modeling, 'moveElements'),
+          reconnectStartSpy = spy(modeling, 'reconnectStart');
 
-        // then
-        expect(connection.waypoints[0]).to.be.eql({ x: 385, y: 55 });
-        expect(connection.waypoints[1]).to.be.eql({ x: 130, y: 130 });
-      })
-    );
-
-
-    it('should NOT relayout connection when layoutConnection=false',
-      inject(function(modeling) {
+      // when
+      modeling.replaceShape(shape1, newShapeData, hints);
 
-        // given
-        var newShapeData = { x: 130, y: 130, width: 200, height: 200 };
+      // then
+      expect(moveElementsSpy).to.have.been.called;
+      expect(moveElementsSpy.firstCall.args[ 3 ]).to.eql(hints);
 
-        // when
-        modeling.replaceShape(child2, newShapeData, { layoutConnection: false });
+      expect(reconnectStartSpy).to.have.been.called;
+      expect(reconnectStartSpy.firstCall.args[ 3 ]).to.eql(hints);
+    }));
 
-        // then
-        expect(connection.waypoints[0]).to.be.eql({ x: 385, y: 55 });
-        expect(connection.waypoints[1]).to.be.eql({ x: 80, y: 80 });
-      })
-    );
   });
 
 });