Update connect to work with multiple connecting blocks (#2905)

* Update connect to work with multiple connecting blocks

* Make logic for connect more readable

core/keyboard_nav/navigation.js

@@ -484,34 +484,107 @@ Blockly.navigation.modify = function() {
 };
 
 /**
- * Connect the moving connection to the targetConnection.  Disconnect the moving
- * connection if necessary, and and position the blocks so that the target
- * connection does not move.
- * @param {Blockly.RenderedConnection} movingConnection The connection to move.
- * @param {Blockly.RenderedConnection} targetConnection The connection that
- *     stays stationary as the movingConnection attaches to it.
- * @return {boolean} Whether the connection was successful.
+ * If the two blocks are compatible move the moving connection to the target
+ * connection and connect them.
+ * @param {Blockly.Connection} movingConnection The connection that is being
+ *     moved.
+ * @param {Blockly.Connection} destConnection The connection to be moved to.
+ * @return {boolean} True if the connections were connected, false otherwise.
+ * @private
+ */
+Blockly.navigation.moveAndConnect_ = function(movingConnection, destConnection) {
+  var movingBlock = movingConnection.getSourceBlock();
+
+  if (destConnection.canConnectWithReason_(movingConnection) ==
+      Blockly.Connection.CAN_CONNECT) {
+    if (destConnection.type == Blockly.PREVIOUS_STATEMENT ||
+        destConnection.type == Blockly.OUTPUT_VALUE) {
+      movingBlock.positionNearConnection(movingConnection, destConnection);
+    }
+    destConnection.connect(movingConnection);
+    return true;
+  }
+  return false;
+};
+
+/**
+ * If the given connection is superior find the inferior connection on the
+ * source block.
+ * @param {Blockly.Connection} connection The connection trying to be connected.
+ * @return {Blockly.Connection} The inferior connection or null if none exists.
+ * @private
+ */
+Blockly.navigation.getInferiorConnection_ = function(connection) {
+  var block = connection.getSourceBlock();
+  if (!connection.isSuperior()) {
+    return connection;
+  } else if (block.previousConnection) {
+    return block.previousConnection;
+  } else if (block.outputConnection) {
+    return block.outputConnection;
+  } else {
+    return null;
+  }
+};
+
+/**
+ * If the given connection is inferior tries to find a superior connection to
+ * connect to.
+ * @param {Blockly.Connection} connection The connection trying to be connected.
+ * @return {Blockly.Connection} The superior connection or null if none exists.
+ * @private
+ */
+Blockly.navigation.getSuperiorConnection_ = function(connection) {
+  if (connection.isSuperior()) {
+    return connection;
+  } else if (connection.targetConnection) {
+    return connection.targetConnection;
+  }
+  return null;
+};
+
+/**
+ * Tries to connect the  given connections.
+ *
+ * If the given connections are not compatible try finding compatible connections
+ * on the source blocks of the given connections.
+ *
+ * @param {Blockly.Connection} movingConnection The connection that is being
+ *     moved.
+ * @param {Blockly.Connection} destConnection The connection to be moved to.
+ * @return {boolean} True if the two connections or their target connections
+ *     were connected, false otherwise.
  * @package
  */
-Blockly.navigation.connect = function(movingConnection, targetConnection) {
-  if (movingConnection) {
-    var movingBlock = movingConnection.getSourceBlock();
-    if (targetConnection.type == Blockly.PREVIOUS_STATEMENT ||
-        targetConnection.type == Blockly.OUTPUT_VALUE) {
-      movingBlock.positionNearConnection(movingConnection, targetConnection);
-    }
+Blockly.navigation.connect = function(movingConnection, destConnection) {
+  if (!movingConnection || !destConnection) {
+    return false;
+  }
+
+  var movingInferior = Blockly.navigation.getInferiorConnection_(movingConnection);
+  var destSuperior = Blockly.navigation.getSuperiorConnection_(destConnection);
+
+  var movingSuperior = Blockly.navigation.getSuperiorConnection_(movingConnection);
+  var destInferior = Blockly.navigation.getInferiorConnection_(destConnection);
+
+  if (movingInferior && destSuperior &&
+      Blockly.navigation.moveAndConnect_(movingInferior, destSuperior)) {
+    return true;
+  // Try swapping the inferior and superior connections on the blocks.
+  } else if (movingSuperior && destInferior &&
+      Blockly.navigation.moveAndConnect_(movingSuperior, destInferior)) {
+    return true;
+  // If nothing else worked try connecting the given connections and report
+  // any errors.
+  } else {
     try {
-      targetConnection.connect(movingConnection);
-      return true;
+      destConnection.connect(movingConnection);
     }
     catch (e) {
-      // TODO: Is there anything else useful to do at this catch?
-      // Perhaps position the block near the target connection?
       Blockly.navigation.warn('Connection failed with error: ' + e);
       return false;
     }
   }
-  return false;
 };
 
 /**

tests/mocha/navigation_modify_test.js

@@ -55,8 +55,10 @@ suite('Insert/Modify', function() {
         Blockly.navigation.cursor_.setLocation(
             Blockly.ASTNode.createConnectionNode(
                 this.stack_block_2.nextConnection));
-        chai.assert.isFalse(Blockly.navigation.modify());
-        chai.assert.isNull(this.stack_block_1.getNextBlock());
+        // Connect method will try to find a way to connect blocks with
+        // incompatible types.
+        chai.assert.isTrue(Blockly.navigation.modify());
+        chai.assert.equal(this.stack_block_1.getNextBlock(), this.stack_block_2);
       });
       test('Cursor on really incompatible connection', function() {
         Blockly.navigation.cursor_.setLocation(
@@ -134,7 +136,11 @@ suite('Insert/Modify', function() {
         Blockly.navigation.cursor_.setLocation(
             Blockly.ASTNode.createConnectionNode(
                 this.row_block_2.inputList[0].connection));
-        chai.assert.isFalse(Blockly.navigation.modify());
+        // Connect method will try to find a way to connect blocks with
+        // incompatible types.
+        chai.assert.isTrue(Blockly.navigation.modify());
+        chai.assert.equal(this.row_block_1.inputList[0].connection.targetBlock(),
+            this.row_block_2);
       });
       test('Cursor on really incompatible connection', function() {
         Blockly.navigation.cursor_.setLocation(

tests/mocha/navigation_test.js

@@ -444,6 +444,72 @@ suite('Navigation', function() {
     });
   });
 
+  suite('Connect Blocks', function() {
+    setup(function() {
+      Blockly.defineBlocksWithJsonArray([{
+        "type": "basic_block",
+        "message0": "",
+        "previousStatement": null,
+        "nextStatement": null,
+      }]);
+
+      var toolbox = document.getElementById('toolbox-categories');
+      this.workspace = Blockly.inject('blocklyDiv', {toolbox: toolbox});
+
+      var basicBlock = this.workspace.newBlock('basic_block');
+      var basicBlock2 = this.workspace.newBlock('basic_block');
+      var basicBlock3 = this.workspace.newBlock('basic_block');
+      var basicBlock4 = this.workspace.newBlock('basic_block');
+
+      this.basicBlock = basicBlock;
+      this.basicBlock2 = basicBlock2;
+      this.basicBlock3 = basicBlock3;
+      this.basicBlock4 = basicBlock4;
+
+      this.basicBlock.nextConnection.connect(this.basicBlock2.previousConnection);
+
+      this.basicBlock3.nextConnection.connect(this.basicBlock4.previousConnection);
+    });
+
+    teardown(function() {
+      delete Blockly.Blocks['basic_block'];
+      this.workspace.dispose();
+      Blockly.navigation.currentCategory_ = null;
+    });
+
+    test('Connect cursor on previous into stack', function() {
+      var markedLocation = this.basicBlock2.previousConnection;
+      var cursorLocation = this.basicBlock3.previousConnection;
+
+      Blockly.navigation.connect(cursorLocation, markedLocation);
+
+      chai.assert.equal(this.basicBlock.nextConnection.targetBlock(), this.basicBlock3);
+      chai.assert.equal(this.basicBlock2.previousConnection.targetBlock(), this.basicBlock4);
+    });
+
+    test('Connect marker on previous into stack', function() {
+      var markedLocation = this.basicBlock3.previousConnection;
+      var cursorLocation = this.basicBlock2.previousConnection;
+
+      Blockly.navigation.connect(cursorLocation, markedLocation);
+
+      chai.assert.equal(this.basicBlock.nextConnection.targetBlock(), this.basicBlock3);
+      chai.assert.equal(this.basicBlock2.previousConnection.targetBlock(), this.basicBlock4);
+
+    });
+
+    test('Connect cursor on next into stack', function() {
+      var markedLocation = this.basicBlock2.previousConnection;
+      var cursorLocation = this.basicBlock4.nextConnection;
+
+      Blockly.navigation.connect(cursorLocation, markedLocation);
+
+      chai.assert.equal(this.basicBlock.nextConnection.targetBlock(), this.basicBlock4);
+      chai.assert.equal(this.basicBlock3.nextConnection.targetConnection, null);
+    });
+  });
+
+
   suite('Test cursor move on block delete', function() {
     setup(function() {
       Blockly.defineBlocksWithJsonArray([{