From 3fd749205f201b96baabfbd18db54db5fc8fbe1d Mon Sep 17 00:00:00 2001
From: Beka Westberg <bwestberg@google.com>
Date: Fri, 17 May 2024 20:22:53 +0000
Subject: [PATCH] fix: dragging blocks by shadows to delete (#8138)

* Revert "fix: dragging shadow blocks (#7992)"

This reverts commit c0e6e6745f7bb07d40be5012b23f0fa6f1ec3cec.

* fix: dragging by shadow not being deletable

* fix: unselecting shadows

* fix: revert changes to select and unselect
---
 core/block.ts                        | 11 +++++++
 core/dragging/block_drag_strategy.ts | 49 ++--------------------------
 core/gesture.ts                      |  2 +-
 3 files changed, 14 insertions(+), 48 deletions(-)

diff --git a/core/block.ts b/core/block.ts
index 52191d63c..fa00c3ab4 100644
--- a/core/block.ts
+++ b/core/block.ts
@@ -675,6 +675,17 @@ export class Block implements IASTNodeLocation {
     return block;
   }
 
+  /**
+   * Returns this block if it is a shadow block, or the first non-shadow parent.
+   *
+   * @internal
+   */
+  getFirstNonShadowBlock(): this {
+    if (!this.isShadow()) return this;
+    // We can assert the parent is non-null because shadows must have parents.
+    return this.getParent()!.getFirstNonShadowBlock();
+  }
+
   /**
    * Find all the blocks that are directly nested inside this one.
    * Includes value and statement inputs, as well as any following statement.
diff --git a/core/dragging/block_drag_strategy.ts b/core/dragging/block_drag_strategy.ts
index fadba28fb..fb913f882 100644
--- a/core/dragging/block_drag_strategy.ts
+++ b/core/dragging/block_drag_strategy.ts
@@ -55,24 +55,15 @@ export class BlockDragStrategy implements IDragStrategy {
 
   private dragging = false;
 
-  /**
-   * If this is a shadow block, the offset between this block and the parent
-   * block, to add to the drag location. In workspace units.
-   */
-  private dragOffset = new Coordinate(0, 0);
-
   constructor(private block: BlockSvg) {
     this.workspace = block.workspace;
   }
 
   /** Returns true if the block is currently movable. False otherwise. */
   isMovable(): boolean {
-    if (this.block.isShadow()) {
-      return this.block.getParent()?.isMovable() ?? false;
-    }
-
     return (
       this.block.isOwnMovable() &&
+      !this.block.isShadow() &&
       !this.block.isDeadOrDying() &&
       !this.workspace.options.readOnly &&
       // We never drag blocks in the flyout, only create new blocks that are
@@ -86,11 +77,6 @@ export class BlockDragStrategy implements IDragStrategy {
    * from any parent blocks.
    */
   startDrag(e?: PointerEvent): void {
-    if (this.block.isShadow()) {
-      this.startDraggingShadow(e);
-      return;
-    }
-
     this.dragging = true;
     if (!eventUtils.getGroup()) {
       eventUtils.setGroup(true);
@@ -120,22 +106,6 @@ export class BlockDragStrategy implements IDragStrategy {
     this.workspace.getLayerManager()?.moveToDragLayer(this.block);
   }
 
-  /** Starts a drag on a shadow, recording the drag offset. */
-  private startDraggingShadow(e?: PointerEvent) {
-    const parent = this.block.getParent();
-    if (!parent) {
-      throw new Error(
-        'Tried to drag a shadow block with no parent. ' +
-          'Shadow blocks should always have parents.',
-      );
-    }
-    this.dragOffset = Coordinate.difference(
-      parent.getRelativeToSurfaceXY(),
-      this.block.getRelativeToSurfaceXY(),
-    );
-    parent.startDrag(e);
-  }
-
   /**
    * Whether or not we should disconnect the block when a drag is started.
    *
@@ -204,11 +174,6 @@ export class BlockDragStrategy implements IDragStrategy {
 
   /** Moves the block and updates any connection previews. */
   drag(newLoc: Coordinate): void {
-    if (this.block.isShadow()) {
-      this.block.getParent()?.drag(Coordinate.sum(newLoc, this.dragOffset));
-      return;
-    }
-
     this.block.moveDuringDrag(newLoc);
     this.updateConnectionPreview(
       this.block,
@@ -352,12 +317,7 @@ export class BlockDragStrategy implements IDragStrategy {
    * Cleans up any state at the end of the drag. Applies any pending
    * connections.
    */
-  endDrag(e?: PointerEvent): void {
-    if (this.block.isShadow()) {
-      this.block.getParent()?.endDrag(e);
-      return;
-    }
-
+  endDrag(): void {
     this.fireDragEndEvent();
     this.fireMoveEvent();
 
@@ -413,11 +373,6 @@ export class BlockDragStrategy implements IDragStrategy {
    * including reconnecting connections.
    */
   revertDrag(): void {
-    if (this.block.isShadow()) {
-      this.block.getParent()?.revertDrag();
-      return;
-    }
-
     this.startChildConn?.connect(this.block.nextConnection);
     if (this.startParentConn) {
       switch (this.startParentConn.type) {
diff --git a/core/gesture.ts b/core/gesture.ts
index 7970ed006..46938d24f 100644
--- a/core/gesture.ts
+++ b/core/gesture.ts
@@ -1015,7 +1015,7 @@ export class Gesture {
     // If the gesture already went through a bubble, don't set the start block.
     if (!this.startBlock && !this.startBubble) {
       this.startBlock = block;
-      common.setSelected(this.startBlock);
+      common.setSelected(this.startBlock.getFirstNonShadowBlock());
       if (block.isInFlyout && block !== block.getRootBlock()) {
         this.setTargetBlock(block.getRootBlock());
       } else {