fix: dragging and disposing of shadows (#8172)

* Revert "fix: dragging blocks by shadows to delete (#8138)"

This reverts commit 3fd749205f.

* fix: dragging shadows

core/block.ts

@@ -675,17 +675,6 @@ 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.

core/dragging/block_drag_strategy.ts

@@ -55,15 +55,24 @@ 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
@@ -77,6 +86,11 @@ 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);
@@ -106,6 +120,22 @@ 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.
    *
@@ -174,6 +204,11 @@ 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,
@@ -317,7 +352,12 @@ export class BlockDragStrategy implements IDragStrategy {
    * Cleans up any state at the end of the drag. Applies any pending
    * connections.
    */
-  endDrag(): void {
+  endDrag(e?: PointerEvent): void {
+    if (this.block.isShadow()) {
+      this.block.getParent()?.endDrag(e);
+      return;
+    }
+
     this.fireDragEndEvent();
     this.fireMoveEvent();
 
@@ -373,6 +413,11 @@ 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) {

core/dragging/dragger.ts

@@ -42,13 +42,12 @@ export class Dragger implements IDragger {
    */
   onDrag(e: PointerEvent, totalDelta: Coordinate) {
     this.moveDraggable(e, totalDelta);
+    const root = this.getRoot(this.draggable);
 
     // Must check `wouldDelete` before calling other hooks on drag targets
     // since we have documented that we would do so.
-    if (isDeletable(this.draggable)) {
-      this.draggable.setDeleteStyle(
-        this.wouldDeleteDraggable(e, this.draggable),
-      );
+    if (isDeletable(root)) {
+      root.setDeleteStyle(this.wouldDeleteDraggable(e, root));
     }
     this.updateDragTarget(e);
   }
@@ -56,11 +55,12 @@ export class Dragger implements IDragger {
   /** Updates the drag target under the pointer (if there is one). */
   protected updateDragTarget(e: PointerEvent) {
     const newDragTarget = this.workspace.getDragTarget(e);
+    const root = this.getRoot(this.draggable);
     if (this.dragTarget !== newDragTarget) {
-      this.dragTarget?.onDragExit(this.draggable);
-      newDragTarget?.onDragEnter(this.draggable);
+      this.dragTarget?.onDragExit(root);
+      newDragTarget?.onDragEnter(root);
     }
-    newDragTarget?.onDragOver(this.draggable);
+    newDragTarget?.onDragOver(root);
     this.dragTarget = newDragTarget;
   }
 
@@ -80,7 +80,7 @@ export class Dragger implements IDragger {
    */
   protected wouldDeleteDraggable(
     e: PointerEvent,
-    draggable: IDraggable & IDeletable,
+    rootDraggable: IDraggable & IDeletable,
   ) {
     const dragTarget = this.workspace.getDragTarget(e);
     if (!dragTarget) return false;
@@ -92,50 +92,56 @@ export class Dragger implements IDragger {
     );
     if (!isDeleteArea) return false;
 
-    return (dragTarget as IDeleteArea).wouldDelete(draggable);
+    return (dragTarget as IDeleteArea).wouldDelete(rootDraggable);
   }
 
   /** Handles any drag cleanup. */
   onDragEnd(e: PointerEvent) {
     const origGroup = eventUtils.getGroup();
     const dragTarget = this.workspace.getDragTarget(e);
+    const root = this.getRoot(this.draggable);
+
     if (dragTarget) {
-      this.dragTarget?.onDrop(this.draggable);
+      this.dragTarget?.onDrop(root);
     }
 
-    if (this.shouldReturnToStart(e, this.draggable)) {
+    if (this.shouldReturnToStart(e, root)) {
       this.draggable.revertDrag();
     }
 
-    const wouldDelete =
-      isDeletable(this.draggable) &&
-      this.wouldDeleteDraggable(e, this.draggable);
+    const wouldDelete = isDeletable(root) && this.wouldDeleteDraggable(e, root);
 
     // TODO(#8148): use a generalized API instead of an instanceof check.
     if (wouldDelete && this.draggable instanceof BlockSvg) {
-      blockAnimations.disposeUiEffect(this.draggable);
+      blockAnimations.disposeUiEffect(this.draggable.getRootBlock());
     }
 
     this.draggable.endDrag(e);
 
-    if (wouldDelete && isDeletable(this.draggable)) {
+    if (wouldDelete && isDeletable(root)) {
       // We want to make sure the delete gets grouped with any possible
       // move event.
       const newGroup = eventUtils.getGroup();
       eventUtils.setGroup(origGroup);
-      this.draggable.dispose();
+      root.dispose();
       eventUtils.setGroup(newGroup);
     }
   }
 
+  // We need to special case blocks for now so that we look at the root block
+  // instead of the one actually being dragged in most cases.
+  private getRoot(draggable: IDraggable): IDraggable {
+    return draggable instanceof BlockSvg ? draggable.getRootBlock() : draggable;
+  }
+
   /**
    * Returns true if we should return the draggable to its original location
    * at the end of the drag.
    */
-  protected shouldReturnToStart(e: PointerEvent, draggable: IDraggable) {
+  protected shouldReturnToStart(e: PointerEvent, rootDraggable: IDraggable) {
     const dragTarget = this.workspace.getDragTarget(e);
     if (!dragTarget) return false;
-    return dragTarget.shouldPreventMove(draggable);
+    return dragTarget.shouldPreventMove(rootDraggable);
   }
 
   protected pixelsToWorkspaceUnits(pixelCoord: Coordinate): Coordinate {

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.getFirstNonShadowBlock());
+      common.setSelected(this.startBlock);
       if (block.isInFlyout && block !== block.getRootBlock()) {
         this.setTargetBlock(block.getRootBlock());
       } else {