fix: blocks being dragged behind toolbox (#7619)

* fix: add layer manager to fix dragging

* chore: fix block animations

* chore: add tests

* chore: format

core/block_dragger.ts

@@ -30,6 +30,7 @@ import * as dom from './utils/dom.js';
 import type {WorkspaceSvg} from './workspace_svg.js';
 import {hasBubble} from './interfaces/i_has_bubble.js';
 import * as deprecation from './utils/deprecation.js';
+import * as layers from './layers.js';
 
 /**
  * Class for a block dragger.  It moves blocks around the workspace when they
@@ -119,6 +120,7 @@ export class BlockDragger implements IBlockDragger {
       this.disconnectBlock_(healStack, currentDragDeltaXY);
     }
     this.draggingBlock_.setDragging(true);
+    this.workspace_.getLayerManager()?.moveToDragLayer(this.draggingBlock_);
   }
 
   /**
@@ -231,6 +233,9 @@ export class BlockDragger implements IBlockDragger {
     const deleted = this.maybeDeleteBlock_();
     if (!deleted) {
       // These are expensive and don't need to be done if we're deleting.
+      this.workspace_
+        .getLayerManager()
+        ?.moveOffDragLayer(this.draggingBlock_, layers.BLOCK);
       this.draggingBlock_.setDragging(false);
       if (delta) {
         // !preventMove

core/block_svg.ts

@@ -354,6 +354,12 @@ export class BlockSvg
    * @returns Object with .x and .y properties in workspace coordinates.
    */
   override getRelativeToSurfaceXY(): Coordinate {
+    const layerManger = this.workspace.getLayerManager();
+    if (!layerManger) {
+      throw new Error(
+        'Cannot calculate position because the workspace has not been appended',
+      );
+    }
     let x = 0;
     let y = 0;
 
@@ -365,7 +371,7 @@ export class BlockSvg
         x += xy.x;
         y += xy.y;
         element = element.parentNode as SVGElement;
-      } while (element && element !== this.workspace.getCanvas());
+      } while (element && !layerManger.hasLayer(element));
     }
     return new Coordinate(x, y);
   }

core/bubble_dragger.ts

@@ -20,6 +20,7 @@ import type {IDragTarget} from './interfaces/i_drag_target.js';
 import {Coordinate} from './utils/coordinate.js';
 import {WorkspaceCommentSvg} from './workspace_comment_svg.js';
 import type {WorkspaceSvg} from './workspace_svg.js';
+import * as layers from './layers.js';
 
 /**
  * Class for a bubble dragger.  It moves things on the bubble canvas around the
@@ -64,6 +65,8 @@ export class BubbleDragger {
       (this.bubble as AnyDuringMigration).setAutoLayout(false);
     }
 
+    this.workspace.getLayerManager()?.moveToDragLayer(this.bubble);
+
     this.bubble.setDragging && this.bubble.setDragging(true);
   }
 
@@ -163,6 +166,9 @@ export class BubbleDragger {
       // Put everything back onto the bubble canvas.
       if (this.bubble.setDragging) {
         this.bubble.setDragging(false);
+        this.workspace
+          .getLayerManager()
+          ?.moveOffDragLayer(this.bubble, layers.BUBBLE);
       }
       this.fireMoveEvent_();
     }

core/css.ts

@@ -496,4 +496,15 @@ input[type=number] {
   float: right;
   margin-right: -24px;
 }
+
+.blocklyBlockDragSurface {
+  position: absolute;
+  top: 0;
+  left: 0;
+  right: 0;
+  bottom: 0;
+  overflow: visible !important;
+  z-index: 80;
+  pointer-events: none;
+}
 `;

core/inject.ts

@@ -60,7 +60,7 @@ export function inject(
   containerElement!.appendChild(subContainer);
   const svg = createDom(subContainer, options);
 
-  const workspace = createMainWorkspace(svg, options);
+  const workspace = createMainWorkspace(subContainer, svg, options);
 
   init(workspace);
 
@@ -138,15 +138,20 @@ function createDom(container: Element, options: Options): SVGElement {
  * @param options Dictionary of options.
  * @returns Newly created main workspace.
  */
-function createMainWorkspace(svg: SVGElement, options: Options): WorkspaceSvg {
+function createMainWorkspace(
+  injectionDiv: Element,
+  svg: SVGElement,
+  options: Options,
+): WorkspaceSvg {
   options.parentWorkspace = null;
   const mainWorkspace = new WorkspaceSvg(options);
   const wsOptions = mainWorkspace.options;
   mainWorkspace.scale = wsOptions.zoomOptions.startScale;
-  svg.appendChild(mainWorkspace.createDom('blocklyMainBackground'));
+  svg.appendChild(
+    mainWorkspace.createDom('blocklyMainBackground', injectionDiv),
+  );
 
   // Set the theme name and renderer name onto the injection div.
-  const injectionDiv = mainWorkspace.getInjectionDiv();
   const rendererClassName = mainWorkspace.getRenderer().getClassName();
   if (rendererClassName) {
     dom.addClass(injectionDiv, rendererClassName);

core/interfaces/i_rendered_element.ts

@@ -0,0 +1,22 @@
+/**
+ * @license
+ * Copyright 2023 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/** @internal */
+export interface IRenderedElement {
+  /**
+   * @returns The root SVG element of htis rendered element.
+   */
+  getSvgRoot(): SVGElement;
+}
+
+/**
+ * @returns True if the given object is an IRenderedElement.
+ *
+ * @internal
+ */
+export function isRenderedElement(obj: any): obj is IRenderedElement {
+  return obj['getSvgRoot'] !== undefined;
+}

core/layer_manager.ts

@@ -0,0 +1,154 @@
+/**
+ * @license
+ * Copyright 2023 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import {WorkspaceSvg} from './workspace_svg.js';
+import * as dom from './utils/dom.js';
+import {Svg} from './utils/svg.js';
+import {IRenderedElement} from './interfaces/i_rendered_element.js';
+import * as layerNums from './layers.js';
+import {Coordinate} from './utils/coordinate.js';
+
+/** @internal */
+export class LayerManager {
+  /** The layer elements being dragged are appended to. */
+  private dragLayer: SVGGElement | undefined;
+  /** The layers elements not being dragged are appended to.  */
+  private layers = new Map<number, SVGGElement>();
+
+  /** @internal */
+  constructor(private workspace: WorkspaceSvg) {
+    const injectionDiv = workspace.getInjectionDiv();
+    // `getInjectionDiv` is actually nullable. We hit this if the workspace
+    // is part of a flyout and the workspace the flyout is attached to hasn't
+    // been appended yet.
+    if (injectionDiv) {
+      this.dragLayer = this.createDragLayer(injectionDiv);
+    }
+
+    // We construct these manually so we can add the css class for backwards
+    // compatibility.
+    const blockLayer = this.createLayer(layerNums.BLOCK);
+    dom.addClass(blockLayer, 'blocklyBlockCanvas');
+    const bubbleLayer = this.createLayer(layerNums.BUBBLE);
+    dom.addClass(bubbleLayer, 'blocklyBubbleCanvas');
+  }
+
+  private createDragLayer(injectionDiv: Element) {
+    const svg = dom.createSvgElement(Svg.SVG, {
+      'class': 'blocklyBlockDragSurface',
+      'xmlns': dom.SVG_NS,
+      'xmlns:html': dom.HTML_NS,
+      'xmlns:xlink': dom.XLINK_NS,
+      'version': '1.1',
+    });
+    injectionDiv.append(svg);
+    return dom.createSvgElement(Svg.G, {}, svg);
+  }
+
+  /**
+   * Translates layers when the workspace is dragged or zoomed.
+   *
+   * @internal
+   */
+  translateLayers(newCoord: Coordinate, newScale: number) {
+    const translation = `translate(${newCoord.x}, ${newCoord.y}) scale(${newScale})`;
+    this.dragLayer?.setAttribute('transform', translation);
+    for (const [_, layer] of this.layers) {
+      layer.setAttribute('transform', translation);
+    }
+  }
+
+  /**
+   * Moves the given element to the drag layer, which exists on top of all other
+   * layers, and the drag surface.
+   *
+   * @internal
+   */
+  moveToDragLayer(elem: IRenderedElement) {
+    this.dragLayer?.appendChild(elem.getSvgRoot());
+  }
+
+  /**
+   * Moves the given element off of the drag layer.
+   *
+   * @internal
+   */
+  moveOffDragLayer(elem: IRenderedElement, layerNum: number) {
+    this.append(elem, layerNum);
+  }
+
+  /**
+   * Appends the given element to a layer. If the layer does not exist, it is
+   * created.
+   *
+   * @internal
+   */
+  append(elem: IRenderedElement, layerNum: number) {
+    if (!this.layers.has(layerNum)) {
+      this.createLayer(layerNum);
+    }
+    this.layers.get(layerNum)?.appendChild(elem.getSvgRoot());
+  }
+
+  /**
+   * Creates a layer and inserts it at the proper place given the layer number.
+   *
+   * More positive layers exist later in the dom and are rendered ontop of
+   * less positive layers. Layers are added to the layer map as a side effect.
+   */
+  private createLayer(layerNum: number): SVGGElement {
+    const parent = this.workspace.getSvgGroup();
+    const layer = dom.createSvgElement(Svg.G, {});
+
+    let inserted = false;
+    const sortedLayers = [...this.layers].sort((a, b) => a[0] - b[0]);
+    for (const [num, sib] of sortedLayers) {
+      if (layerNum < num) {
+        parent.insertBefore(layer, sib);
+        inserted = true;
+        break;
+      }
+    }
+    if (!inserted) {
+      parent.appendChild(layer);
+    }
+    this.layers.set(layerNum, layer);
+    return layer;
+  }
+
+  /**
+   * Returns true if the given element is a layer managed by the layer manager.
+   * False otherwise.
+   *
+   * @internal
+   */
+  hasLayer(elem: SVGElement) {
+    return (
+      elem === this.dragLayer ||
+      new Set(this.layers.values()).has(elem as SVGGElement)
+    );
+  }
+
+  /**
+   * We need to be able to access this layer explicitly for backwards
+   * compatibility.
+   *
+   * @internal
+   */
+  getBlockLayer(): SVGGElement {
+    return this.layers.get(layerNums.BLOCK)!;
+  }
+
+  /**
+   * We need to be able to access this layer explicitly for backwards
+   * compatibility.
+   *
+   * @internal
+   */
+  getBubbleLayer(): SVGGElement {
+    return this.layers.get(layerNums.BUBBLE)!;
+  }
+}

core/layers.ts

@@ -0,0 +1,19 @@
+/**
+ * @license
+ * Copyright 2023 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/**
+ * The layer to place blocks on.
+ *
+ * @internal
+ */
+export const BLOCK = 50;
+
+/**
+ * The layer to place bubbles on.
+ *
+ * @internal
+ */
+export const BUBBLE = 100;

core/workspace_comment_svg.ts

@@ -315,22 +315,25 @@ export class WorkspaceCommentSvg
    * @internal
    */
   override getRelativeToSurfaceXY(): Coordinate {
+    const layerManger = this.workspace.getLayerManager();
+    if (!layerManger) {
+      throw new Error(
+        'Cannot calculate position because the workspace has not been appended',
+      );
+    }
+
     let x = 0;
     let y = 0;
 
-    let element: Node | null = this.getSvgRoot();
+    let element: SVGElement | null = this.getSvgRoot();
     if (element) {
       do {
         // Loop through this comment and every parent.
-        const xy = svgMath.getRelativeXY(element as Element);
+        const xy = svgMath.getRelativeXY(element);
         x += xy.x;
         y += xy.y;
-        element = element.parentNode;
-      } while (
-        element &&
-        element !== this.workspace.getBubbleCanvas() &&
-        element !== null
-      );
+        element = element.parentNode as SVGElement;
+      } while (element && !layerManger.hasLayer(element) && element !== null);
     }
     this.xy_ = new Coordinate(x, y);
     return this.xy_;

core/workspace_svg.ts

@@ -78,6 +78,7 @@ import {ZoomControls} from './zoom_controls.js';
 import {ContextMenuOption} from './contextmenu_registry.js';
 import * as renderManagement from './render_management.js';
 import * as deprecation from './utils/deprecation.js';
+import {LayerManager} from './layer_manager.js';
 
 /** Margin around the top/bottom/left/right after a zoomToFit call. */
 const ZOOM_TO_FIT_MARGIN = 20;
@@ -305,6 +306,7 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg {
   private dragTargetAreas: Array<{component: IDragTarget; clientRect: Rect}> =
     [];
   private readonly cachedParentSvgSize: Size;
+  private layerManager: LayerManager | null = null;
   // TODO(b/109816955): remove '!', see go/strict-prop-init-fix.
   svgGroup_!: SVGElement;
   // TODO(b/109816955): remove '!', see go/strict-prop-init-fix.
@@ -639,7 +641,11 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg {
       x += xy.x * scale;
       y += xy.y * scale;
       element = element.parentNode as SVGElement;
-    } while (element && element !== this.getParentSvg());
+    } while (
+      element &&
+      element !== this.getParentSvg() &&
+      element !== this.getInjectionDiv()
+    );
     return new Coordinate(x, y);
   }
 
@@ -709,7 +715,7 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg {
    * @internal
    */
   getBlockCanvas(): SVGElement | null {
-    return this.svgBlockCanvas_;
+    return this.getCanvas();
   }
 
   /**
@@ -728,7 +734,11 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg {
    *     'blocklyMutatorBackground'.
    * @returns The workspace's SVG group.
    */
-  createDom(opt_backgroundClass?: string): Element {
+  createDom(opt_backgroundClass?: string, injectionDiv?: Element): Element {
+    if (!this.injectionDiv) {
+      this.injectionDiv = injectionDiv ?? null;
+    }
+
     /**
      * <g class="blocklyWorkspace">
      *   <rect class="blocklyMainBackground" height="100%" width="100%"></rect>
@@ -760,16 +770,11 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg {
         );
       }
     }
-    this.svgBlockCanvas_ = dom.createSvgElement(
-      Svg.G,
-      {'class': 'blocklyBlockCanvas'},
-      this.svgGroup_,
-    );
-    this.svgBubbleCanvas_ = dom.createSvgElement(
-      Svg.G,
-      {'class': 'blocklyBubbleCanvas'},
-      this.svgGroup_,
-    );
+
+    this.layerManager = new LayerManager(this);
+    // Assign the canvases for backwards compatibility.
+    this.svgBlockCanvas_ = this.layerManager.getBlockLayer();
+    this.svgBubbleCanvas_ = this.layerManager.getBubbleLayer();
 
     if (!this.isFlyout) {
       browserEvents.conditionalBind(
@@ -901,7 +906,7 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg {
   addTrashcan() {
     this.trashcan = WorkspaceSvg.newTrashcan(this);
     const svgTrashcan = this.trashcan.createDom();
-    this.svgGroup_.insertBefore(svgTrashcan, this.svgBlockCanvas_);
+    this.svgGroup_.insertBefore(svgTrashcan, this.getCanvas());
   }
 
   /**
@@ -1074,13 +1079,22 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg {
   }
   /* eslint-enable indent */
 
+  /**
+   * @returns The layer manager for this workspace.
+   *
+   * @internal
+   */
+  getLayerManager(): LayerManager | null {
+    return this.layerManager;
+  }
+
   /**
    * Get the SVG element that forms the drawing surface.
    *
    * @returns SVG group element.
    */
   getCanvas(): SVGGElement {
-    return this.svgBlockCanvas_ as SVGGElement;
+    return this.layerManager!.getBlockLayer();
   }
 
   /**
@@ -1113,7 +1127,7 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg {
    * @returns SVG group element.
    */
   getBubbleCanvas(): SVGGElement {
-    return this.svgBubbleCanvas_ as SVGGElement;
+    return this.layerManager!.getBubbleLayer();
   }
 
   /**
@@ -1181,15 +1195,8 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg {
    *     the Blockly div.
    */
   translate(x: number, y: number) {
-    const translation =
-      'translate(' + x + ',' + y + ') ' + 'scale(' + this.scale + ')';
-    this.svgBlockCanvas_.setAttribute('transform', translation);
-    this.svgBubbleCanvas_.setAttribute('transform', translation);
-    // And update the grid if we're using one.
-    if (this.grid) {
-      this.grid.moveTo(x, y);
-    }
-
+    this.layerManager?.translateLayers(new Coordinate(x, y), this.scale);
+    this.grid?.moveTo(x, y);
     this.maybeFireViewportChangeEvent();
   }
 
@@ -2023,8 +2030,8 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg {
    * @internal
    */
   beginCanvasTransition() {
-    dom.addClass(this.svgBlockCanvas_, 'blocklyCanvasTransitioning');
-    dom.addClass(this.svgBubbleCanvas_, 'blocklyCanvasTransitioning');
+    dom.addClass(this.getCanvas(), 'blocklyCanvasTransitioning');
+    dom.addClass(this.getBubbleCanvas(), 'blocklyCanvasTransitioning');
   }
 
   /**
@@ -2033,8 +2040,8 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg {
    * @internal
    */
   endCanvasTransition() {
-    dom.removeClass(this.svgBlockCanvas_, 'blocklyCanvasTransitioning');
-    dom.removeClass(this.svgBubbleCanvas_, 'blocklyCanvasTransitioning');
+    dom.removeClass(this.getCanvas(), 'blocklyCanvasTransitioning');
+    dom.removeClass(this.getBubbleCanvas(), 'blocklyCanvasTransitioning');
   }
 
   /** Center the workspace. */