From 5e1ce3f456d1f604114bfac8408ea647c19fe1d4 Mon Sep 17 00:00:00 2001
From: Beka Westberg <bwestberg@google.com>
Date: Wed, 12 Oct 2022 21:49:14 +0000
Subject: [PATCH 1/2] fix: parent blocks not bumping neighbours

---
 core/block_svg.ts           | 62 ++++++++++++++++++-------------------
 core/rendered_connection.ts |  2 +-
 2 files changed, 31 insertions(+), 33 deletions(-)

diff --git a/core/block_svg.ts b/core/block_svg.ts
index b4bdd5dfaa6..9ba24f4b43c 100644
--- a/core/block_svg.ts
+++ b/core/block_svg.ts
@@ -1535,45 +1535,43 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg,
   }
 
   /**
-   * Bump unconnected blocks out of alignment.  Two blocks which aren't actually
-   * connected should not coincidentally line up on screen.
+   * Bumps unconnected blocks out of alignment.
+   *
+   * Two blocks which aren't actually connected should not coincidentally line
+   * up on screen, because that creates confusion for end-users.
    */
   override bumpNeighbours() {
-    if (this.isDeadOrDying()) {
-      return;
-    }
-    if (this.workspace.isDragging()) {
+    this.getRootBlock().bumpNeighboursInternal();
+  }
+
+  /**
+   * Bumps unconnected blocks out of alignment.
+   */
+  private bumpNeighboursInternal() {
+    const root = this.getRootBlock();
+    if (this.isDeadOrDying() || this.workspace.isDragging() ||
+        root.isInFlyout) {
       return;
     }
-    const rootBlock = this.getRootBlock();
-    if (rootBlock.isInFlyout) {
-      return;
+
+    function neighbourIsInStack(neighbour: RenderedConnection) {
+      return neighbour.getSourceBlock().getRootBlock() === root;
     }
-    // Don't move blocks around in a flyout.
-    // Loop through every connection on this block.
-    const myConnections = this.getConnections_(false);
-    for (let i = 0, connection; connection = myConnections[i]; i++) {
-      const renderedConn = (connection);
-      // Spider down from this block bumping all sub-blocks.
-      if (renderedConn.isConnected() && renderedConn.isSuperior()) {
-        renderedConn.targetBlock()!.bumpNeighbours();
+
+    for (const conn of this.getConnections_(false)) {
+      if (conn.isSuperior()) {
+        // Recurse down the block stack.
+        conn.targetBlock()?.bumpNeighboursInternal();
       }
 
-      const neighbours = connection.neighbours(config.snapRadius);
-      for (let j = 0, otherConnection; otherConnection = neighbours[j]; j++) {
-        const renderedOther = otherConnection as RenderedConnection;
-        // If both connections are connected, that's probably fine.  But if
-        // either one of them is unconnected, then there could be confusion.
-        if (!renderedConn.isConnected() || !renderedOther.isConnected()) {
-          // Only bump blocks if they are from different tree structures.
-          if (renderedOther.getSourceBlock().getRootBlock() !== rootBlock) {
-            // Always bump the inferior block.
-            if (renderedConn.isSuperior()) {
-              renderedOther.bumpAwayFrom(renderedConn);
-            } else {
-              renderedConn.bumpAwayFrom(renderedOther);
-            }
-          }
+      for (const neighbour of conn.neighbours(config.snapRadius)) {
+        if (neighbourIsInStack(neighbour)) continue;
+        if (conn.isConnected() && neighbour.isConnected()) continue;
+
+        if (conn.isSuperior()) {
+          neighbour.bumpAwayFrom(conn);
+        } else {
+          conn.bumpAwayFrom(neighbour);
         }
       }
     }
diff --git a/core/rendered_connection.ts b/core/rendered_connection.ts
index 7330ff9da80..82af1b1b197 100644
--- a/core/rendered_connection.ts
+++ b/core/rendered_connection.ts
@@ -499,7 +499,7 @@ export class RenderedConnection extends Connection {
    * @returns List of connections.
    * @internal
    */
-  override neighbours(maxLimit: number): Connection[] {
+  override neighbours(maxLimit: number): RenderedConnection[] {
     return this.dbOpposite_.getNeighbours(this, maxLimit);
   }
 

From c9c2619ef4187c92afc3c91d9784bd7353b643e6 Mon Sep 17 00:00:00 2001
From: Beka Westberg <bwestberg@google.com>
Date: Thu, 13 Oct 2022 17:24:03 +0000
Subject: [PATCH 2/2] chore: add more comments

---
 core/block_svg.ts | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/core/block_svg.ts b/core/block_svg.ts
index 9ba24f4b43c..a65bb40225c 100644
--- a/core/block_svg.ts
+++ b/core/block_svg.ts
@@ -1565,9 +1565,12 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg,
       }
 
       for (const neighbour of conn.neighbours(config.snapRadius)) {
+        // Don't bump away from things that are in our stack.
         if (neighbourIsInStack(neighbour)) continue;
+        // If both connections are connected, that's fine.
         if (conn.isConnected() && neighbour.isConnected()) continue;
 
+        // Always bump the inferior connection.
         if (conn.isSuperior()) {
           neighbour.bumpAwayFrom(conn);
         } else {