Refactor connect logic (#4880)

* Refactor connect logic. * Fixup from rebase * Fix build * PR Comments
google · Jun 18, 2021 · 13bb9f5 · 13bb9f5
1 parent 2929110
commit 13bb9f5
Show file tree

Hide file tree

Showing 9 changed files with 443 additions and 502 deletions.
diff --git a/core/block.js b/core/block.js
@@ -575,20 +575,21 @@ Blockly.Block.prototype.getConnections_ = function(_all) {
 
 /**
  * Walks down a stack of blocks and finds the last next connection on the stack.
+ * @param {boolean} ignoreShadows If true,the last connection on a non-shadow
+ *     block will be returned. If false, this will follow shadows to find the
+ *     last connection.
  * @return {?Blockly.Connection} The last next connection on the stack, or null.
  * @package
  */
-Blockly.Block.prototype.lastConnectionInStack = function() {
+Blockly.Block.prototype.lastConnectionInStack = function(ignoreShadows) {
   var nextConnection = this.nextConnection;
   while (nextConnection) {
     var nextBlock = nextConnection.targetBlock();
-    if (!nextBlock) {
-      // Found a next connection with nothing on the other side.
+    if (!nextBlock || (ignoreShadows && nextBlock.isShadow())) {
       return nextConnection;
     }
     nextConnection = nextBlock.nextConnection;
   }
-  // Ran out of next connections.
   return null;
 };
 
@@ -607,7 +608,6 @@ Blockly.Block.prototype.bumpNeighbours = function() {
  * @return {?Blockly.Block} The block (if any) that holds the current block.
  */
 Blockly.Block.prototype.getParent = function() {
-  // Look at the DOM to see if we are nested in another block.
   return this.parentBlock_;
 };
 

diff --git a/core/block_svg.js b/core/block_svg.js
@@ -1475,14 +1475,18 @@ Blockly.BlockSvg.prototype.getConnections_ = function(all) {
 
 /**
  * Walks down a stack of blocks and finds the last next connection on the stack.
+ * @param {boolean} ignoreShadows If true,the last connection on a non-shadow
+ *     block will be returned. If false, this will follow shadows to find the
+ *     last connection.
  * @return {?Blockly.RenderedConnection} The last next connection on the stack,
  *     or null.
  * @package
  * @override
  */
-Blockly.BlockSvg.prototype.lastConnectionInStack = function() {
+Blockly.BlockSvg.prototype.lastConnectionInStack = function(ignoreShadows) {
   return /** @type {Blockly.RenderedConnection} */ (
-    Blockly.BlockSvg.superClass_.lastConnectionInStack.call(this));
+    Blockly.BlockSvg.superClass_
+        .lastConnectionInStack.call(this, ignoreShadows));
 };
 
 /**

diff --git a/core/connection.js b/core/connection.js
@@ -104,106 +104,58 @@ Blockly.Connection.prototype.y = 0;
  * @protected
  */
 Blockly.Connection.prototype.connect_ = function(childConnection) {
+  var INPUT = Blockly.connectionTypes.INPUT_VALUE;
   var parentConnection = this;
   var parentBlock = parentConnection.getSourceBlock();
   var childBlock = childConnection.getSourceBlock();
-  // Disconnect any existing parent on the child connection.
+
+  // Make sure the childConnection is available.
   if (childConnection.isConnected()) {
     childConnection.disconnect();
   }
+
+  // Make sure the parentConnection is available.
+  var orphan;
   if (parentConnection.isConnected()) {
-    // Other connection is already connected to something.
-    // Disconnect it and reattach it or bump it as needed.
-    var orphanBlock = parentConnection.targetBlock();
-    var shadowDom = parentConnection.getShadowDom();
-    // Temporarily set the shadow DOM to null so it does not respawn.
-    parentConnection.shadowDom_ = null;
-    // Displaced shadow blocks dissolve rather than reattaching or bumping.
-    if (orphanBlock.isShadow()) {
-      // Save the shadow block so that field values are preserved.
-      // This cast assumes that a block can not be both a shadow block and an insertion marker.
-      shadowDom = /** @type {!Element} */ (Blockly.Xml.blockToDom(orphanBlock));
-      orphanBlock.dispose(false);
-      orphanBlock = null;
-    } else if (parentConnection.type == Blockly.connectionTypes.INPUT_VALUE) {
-      // Value connections.
-      // If female block is already connected, disconnect and bump the male.
-      if (!orphanBlock.outputConnection) {
-        throw Error('Orphan block does not have an output connection.');
-      }
-      // Attempt to reattach the orphan at the end of the newly inserted
-      // block.  Since this block may be a row, walk down to the end
-      // or to the first (and only) shadow block.
-      var connection = Blockly.Connection.getConnectionForOrphanedOutput(
-          childBlock, orphanBlock);
-      if (connection) {
-        orphanBlock.outputConnection.connect(connection);
-        orphanBlock = null;
-      }
-    } else if (
-      parentConnection.type == Blockly.connectionTypes.NEXT_STATEMENT) {
-      // Statement connections.
-      // Statement blocks may be inserted into the middle of a stack.
-      // Split the stack.
-      if (!orphanBlock.previousConnection) {
-        throw Error('Orphan block does not have a previous connection.');
-      }
-      // Attempt to reattach the orphan at the bottom of the newly inserted
-      // block.  Since this block may be a stack, walk down to the end.
-      var newBlock = childBlock;
-      while (newBlock.nextConnection) {
-        var nextBlock = newBlock.getNextBlock();
-        if (nextBlock && !nextBlock.isShadow()) {
-          newBlock = nextBlock;
-        } else {
-          var checker = orphanBlock.workspace.connectionChecker;
-          if (checker.canConnect(
-              orphanBlock.previousConnection, newBlock.nextConnection, false)) {
-            newBlock.nextConnection.connect(orphanBlock.previousConnection);
-            orphanBlock = null;
-          }
-          break;
-        }
-      }
-    }
-    if (orphanBlock) {
-      // Unable to reattach orphan.
+    var shadowDom = parentConnection.getShadowDom(true);
+    parentConnection.shadowDom_ = null;  // Set to null so it doesn't respawn.
+    var target = parentConnection.targetBlock();
+    if (target.isShadow()) {
+      target.dispose(false);
+    } else {
       parentConnection.disconnect();
-      if (Blockly.Events.recordUndo) {
-        // Bump it off to the side after a moment.
-        var group = Blockly.Events.getGroup();
-        setTimeout(function() {
-          // Verify orphan hasn't been deleted or reconnected.
-          if (orphanBlock.workspace && !orphanBlock.getParent()) {
-            Blockly.Events.setGroup(group);
-            if (orphanBlock.outputConnection) {
-              orphanBlock.outputConnection.onFailedConnect(parentConnection);
-            } else if (orphanBlock.previousConnection) {
-              orphanBlock.previousConnection.onFailedConnect(parentConnection);
-            }
-            Blockly.Events.setGroup(false);
-          }
-        }, Blockly.BUMP_DELAY);
-      }
+      orphan = target;
     }
-    // Restore the shadow DOM.
     parentConnection.shadowDom_ = shadowDom;
   }
 
+  // Connect the new connection to the parent.
   var event;
   if (Blockly.Events.isEnabled()) {
     event = new (Blockly.Events.get(Blockly.Events.BLOCK_MOVE))(childBlock);
   }
-  // Establish the connections.
   Blockly.Connection.connectReciprocally_(parentConnection, childConnection);
-  // Demote the inferior block so that one is a child of the superior one.
   childBlock.setParent(parentBlock);
   if (event) {
     event.recordNew();
     Blockly.Events.fire(event);
   }
+
+  // Deal with the orphan if it exists.
+  if (orphan) {
+    var orphanConnection = parentConnection.type === INPUT ?
+        orphan.outputConnection : orphan.previousConnection;
+    var connection = Blockly.Connection.getConnectionForOrphanedConnection(
+        childBlock, /** @type {!Blockly.Connection} */ (orphanConnection));
+    if (connection) {
+      orphanConnection.connect(connection);
+    } else {
+      orphanConnection.onFailedConnect(parentConnection);
+    }
+  }
 };
 
+
 /**
  * Dispose of this connection and deal with connected blocks.
  * @package
@@ -318,7 +270,8 @@ Blockly.Connection.prototype.isConnectionAllowed = function(candidate) {
 };
 
 /**
- * Behavior after a connection attempt fails.
+ * Called when an attempted connection fails. NOP by default (i.e. for headless
+ * workspaces).
  * @param {!Blockly.Connection} _otherConnection Connection that this connection
  *     failed to connect to.
  * @package
@@ -409,9 +362,9 @@ Blockly.Connection.getSingleConnection_ = function(block, orphanBlock) {
  * @param {!Blockly.Block} orphanBlock The block that is looking for a home.
  * @return {?Blockly.Connection} The suitable connection point on the chain
  *     of blocks, or null.
- * @package
+ * @private
  */
-Blockly.Connection.getConnectionForOrphanedOutput =
+Blockly.Connection.getConnectionForOrphanedOutput_ =
     function(startBlock, orphanBlock) {
       var newBlock = startBlock;
       var connection;
@@ -425,6 +378,32 @@ Blockly.Connection.getConnectionForOrphanedOutput =
       return null;
     };
 
+/**
+ * Returns the connection (starting at the startBlock) which will accept
+ * the given connection. This includes compatible connection types and
+ * connection checks.
+ * @param {!Blockly.Block} startBlock The block on which to start the search.
+ * @param {!Blockly.Connection} orphanConnection The connection that is looking
+ *     for a home.
+ * @return {?Blockly.Connection} The suitable connection point on the chain of
+ *     blocks, or null.
+ */
+Blockly.Connection.getConnectionForOrphanedConnection =
+    function(startBlock, orphanConnection) {
+      if (orphanConnection.type === Blockly.connectionTypes.OUTPUT_VALUE) {
+        return Blockly.Connection.getConnectionForOrphanedOutput_(
+            startBlock, orphanConnection.getSourceBlock());
+      }
+      // Otherwise we're dealing with a stack.
+      var connection = startBlock.lastConnectionInStack(true);
+      var checker = orphanConnection.getConnectionChecker();
+      if (connection &&
+          checker.canConnect(orphanConnection, connection, false)) {
+        return connection;
+      }
+      return null;
+    };
+
 /**
  * Disconnect this connection.
  */
@@ -614,11 +593,18 @@ Blockly.Connection.prototype.setShadowDom = function(shadow) {
 };
 
 /**
- * Returns the XML representation of the connection's shadow block.
+ * Returns the xml representation of the connection's shadow block.
+ * @param {boolean=} returnCurrent If true, and the shadow block is currently
+ *     attached to this connection, this serializes the state of that block
+ *     and returns it (so that field values are correct). Otherwise the saved
+ *     shadowDom is just returned.
  * @return {?Element} Shadow DOM representation of a block or null.
  */
-Blockly.Connection.prototype.getShadowDom = function() {
-  return this.shadowDom_;
+Blockly.Connection.prototype.getShadowDom = function(returnCurrent) {
+  return (returnCurrent && this.targetBlock().isShadow()) ?
+      /** @type {!Element} */ (Blockly.Xml.blockToDom(
+          /** @type {!Blockly.Block} */ (this.targetBlock()))) :
+      this.shadowDom_;
 };
 
 /**

diff --git a/core/insertion_marker_manager.js b/core/insertion_marker_manager.js
@@ -331,7 +331,7 @@ Blockly.InsertionMarkerManager.prototype.createMarkerBlock_ = function(sourceBlo
 Blockly.InsertionMarkerManager.prototype.initAvailableConnections_ = function() {
   var available = this.topBlock_.getConnections_(false);
   // Also check the last connection on this stack
-  var lastOnStack = this.topBlock_.lastConnectionInStack();
+  var lastOnStack = this.topBlock_.lastConnectionInStack(true);
   if (lastOnStack && lastOnStack != this.topBlock_.nextConnection) {
     available.push(lastOnStack);
     this.lastOnStack_ = lastOnStack;

diff --git a/core/rendered_connection.js b/core/rendered_connection.js
@@ -449,14 +449,26 @@ Blockly.RenderedConnection.prototype.isConnectionAllowed = function(candidate,
 
 /**
  * Behavior after a connection attempt fails.
+ * Bumps this connection away from the other connection. Called when an
+ * attempted connection fails.
  * @param {!Blockly.Connection} otherConnection Connection that this connection
  *     failed to connect to.
  * @package
  */
-Blockly.RenderedConnection.prototype.onFailedConnect = function(
-    otherConnection) {
-  this.bumpAwayFrom(otherConnection);
-};
+Blockly.RenderedConnection.prototype.onFailedConnect =
+    function(otherConnection) {
+      var block = this.getSourceBlock();
+      if (Blockly.Events.recordUndo) {
+        var group = Blockly.Events.getGroup();
+        setTimeout(function() {
+          if (!block.isDisposed() && !block.getParent()) {
+            Blockly.Events.setGroup(group);
+            this.bumpAwayFrom(otherConnection);
+            Blockly.Events.setGroup(false);
+          }
+        }.bind(this), Blockly.BUMP_DELAY);
+      }
+    };
 
 
 /**

diff --git a/core/renderers/common/renderer.js b/core/renderers/common/renderer.js
@@ -246,23 +246,11 @@ Blockly.blockRendering.Renderer.prototype.shouldHighlightConnection =
  */
 Blockly.blockRendering.Renderer.prototype.orphanCanConnectAtEnd =
     function(topBlock, orphanBlock, localType) {
-      var orphanConnection = null;
-      var lastConnection = null;
-      if (localType == Blockly.connectionTypes.OUTPUT_VALUE) {
-        orphanConnection = orphanBlock.outputConnection;
-        lastConnection =
-            Blockly.Connection.getConnectionForOrphanedOutput(
-                /** @type {!Blockly.Block} **/ (topBlock), orphanBlock);
-      } else {
-        orphanConnection = orphanBlock.previousConnection;
-        lastConnection = topBlock.lastConnectionInStack();
-      }
-
-      if (!lastConnection) {
-        return false;
-      }
-      return orphanConnection.getConnectionChecker().canConnect(
-          lastConnection, orphanConnection, false);
+      var orphanConnection = localType === Blockly.connectionTypes.OUTPUT_VALUE ?
+          orphanBlock.outputConnection : orphanBlock.previousConnection;
+      return !!Blockly.Connection.getConnectionForOrphanedConnection(
+          /** @type {!Blockly.Block} **/ (topBlock),
+          /** @type {!Blockly.Connection} **/ (orphanConnection));
     };
 
 /**