fix: delete-area hit testing at non-default zoom (#10095) (#10096)

* chore: sync package.json

* fix: delete-area hit testing at non-default zoom (#10095)

* chore: revert moving recordDragTargets call

* Revert "chore: sync package.json"

This reverts commit 3366b79.

Author: mikeharv

packages/blockly/core/dragging/dragger.ts

@@ -16,7 +16,6 @@ import type {IDragger} from '../interfaces/i_dragger.js';
 import {isFocusableNode} from '../interfaces/i_focusable_node.js';
 import * as registry from '../registry.js';
 import {Coordinate} from '../utils/coordinate.js';
-import {screenToWsCoordinates} from '../utils/svg_math.js';
 
 export class Dragger implements IDragger {
   protected startLoc: Coordinate;
@@ -50,7 +49,10 @@ export class Dragger implements IDragger {
     const pointerEvent = e instanceof PointerEvent ? e : null;
     if (!pointerEvent) return;
 
-    const coordinate = this.pointerToWorkspaceCoordinate(pointerEvent);
+    const coordinate = new Coordinate(
+      pointerEvent.clientX,
+      pointerEvent.clientY,
+    );
     // Must check `wouldDelete` before calling other hooks on drag targets
     // since we have documented that we would do so.
     if (isDeletable(this.draggable)) {
@@ -122,7 +124,10 @@ export class Dragger implements IDragger {
       return;
     }
 
-    const coordinate = this.pointerToWorkspaceCoordinate(pointerEvent);
+    const coordinate = new Coordinate(
+      pointerEvent.clientX,
+      pointerEvent.clientY,
+    );
     const dragTarget = this.draggable.workspace.getDragTarget(coordinate);
 
     if (dragTarget) {
@@ -182,17 +187,6 @@ export class Dragger implements IDragger {
     return dragTarget.shouldPreventMove(rootDraggable);
   }
 
-  /**
-   * Returns the workspace coordinate for a pointer position, for delete-area
-   * hit testing.
-   */
-  private pointerToWorkspaceCoordinate(e: PointerEvent): Coordinate {
-    return screenToWsCoordinates(
-      this.draggable.workspace,
-      new Coordinate(e.clientX, e.clientY),
-    );
-  }
-
   protected pixelsToWorkspaceUnits(pixelCoord: Coordinate): Coordinate {
     const result = new Coordinate(
       pixelCoord.x / this.draggable.workspace.scale,

packages/blockly/core/workspace_svg.ts

@@ -1517,17 +1517,16 @@ export class WorkspaceSvg
   /* eslint-enable */
 
   /**
-   * Returns the drag target the pointer event is over.
+   * Returns the drag target at the given point.
    *
-   * @param e Pointer move event or a workspace coordinate.
-   * @returns Null if not over a drag target, or the drag target the event is
-   *     over.
+   * @param point A pointer event, or a point in client/viewport coordinates.
+   * @returns Null if not over a drag target, or the drag target at that point.
    */
-  getDragTarget(e: PointerEvent | Coordinate): IDragTarget | null {
+  getDragTarget(point: PointerEvent | Coordinate): IDragTarget | null {
     const coordinate =
-      e instanceof Coordinate
-        ? svgMath.wsToScreenCoordinates(this, e)
-        : new Coordinate(e.clientX, e.clientY);
+      point instanceof Coordinate
+        ? point
+        : new Coordinate(point.clientX, point.clientY);
     for (let i = 0, targetArea; (targetArea = this.dragTargetAreas[i]); i++) {
       if (targetArea.clientRect.contains(coordinate.x, coordinate.y)) {
         return targetArea.component;

packages/blockly/tests/mocha/dragger_test.js

@@ -32,10 +32,12 @@ suite('Dragger', function () {
    * @returns {{x: number, y: number}} Viewport coordinates at the block origin.
    */
   function blockOriginClient(block) {
-    const screenCoords = Blockly.utils.svgMath.wsToScreenCoordinates(
-      block.workspace,
-      block.getRelativeToSurfaceXY(),
-    );
+    const ws = block.workspace;
+    let point = block.getRelativeToSurfaceXY();
+    if (ws.isMutator) {
+      point = point.scale(ws.options.parentWorkspace.scale);
+    }
+    const screenCoords = Blockly.utils.svgMath.wsToScreenCoordinates(ws, point);
     return {x: screenCoords.x, y: screenCoords.y};
   }
 
@@ -64,13 +66,29 @@ suite('Dragger', function () {
     return block.getSvgRoot().classList.contains('blocklyDraggingDelete');
   }
 
+  /**
+   * @param {!Blockly.WorkspaceSvg} workspace The workspace with a trashcan.
+   * @returns {boolean} Whether the trashcan lid open style is applied.
+   */
+  function hasTrashLidOpen(workspace) {
+    return workspace.trashcan.svgGroup.classList.contains('blocklyTrashOpen');
+  }
+
+  /**
+   * @param {!Blockly.WorkspaceSvg} workspace The workspace to zoom.
+   * @param {number} scale The target zoom factor.
+   */
+  function setWorkspaceScale(workspace, scale) {
+    workspace.setScale(scale);
+  }
+
   /**
    * Simulates pressing on the block center and dragging to a viewport point.
    *
    * @param {!Blockly.BlockSvg} block The block to drag.
    * @param {{x: number, y: number}} pointerEnd The viewport point to drag to.
-   * @returns {{dragger: !Blockly.dragging.Dragger, dragEvent: !PointerEvent}}
-   *     The dragger and final pointer event from the simulated drag.
+   * @returns {{dragger: !Blockly.dragging.Dragger, dragEvent: !PointerEvent, block: !Blockly.BlockSvg}}
+   *     The dragger, final pointer event, and block being dragged.
    */
   function dragBlock(block, pointerEnd) {
     const start = blockCenterClient(block);
@@ -86,7 +104,7 @@ suite('Dragger', function () {
     dragger.onDragStart(dragStartEvent);
     dragger.onDrag(dragEvent, totalDelta);
 
-    return {dragger, dragEvent};
+    return {dragger, dragEvent, block: dragger.draggable};
   }
 
   setup(function () {
@@ -110,25 +128,45 @@ suite('Dragger', function () {
     sharedTestTeardown.call(this);
   });
 
-  [
-    {name: 'trashcan', rectKey: 'trashRect'},
-    {name: 'toolbox', rectKey: 'toolboxRect'},
-  ].forEach(({name, rectKey}) => {
-    test(`applies delete styling and deletes when dragged to ${name}`, function () {
-      const deleteRect = this[rectKey];
-      const {dragger, dragEvent} = dragBlock(
-        this.block,
-        rectCenterClient(deleteRect),
-      );
+  const zoomLevels = [
+    {name: 'default scale', scale: null},
+    {name: 'zoomed in', scale: 1.5},
+    {name: 'zoomed out', scale: 0.7},
+  ];
 
-      assert.isTrue(
-        deleteRect.contains(dragEvent.clientX, dragEvent.clientY),
-        `Expected cursor to be inside ${name} delete area`,
-      );
-      assert.isTrue(hasDeleteStyle(this.block));
+  zoomLevels.forEach(({name: zoomName, scale}) => {
+    [
+      {name: 'trashcan', rectKey: 'trashRect', checkLid: true},
+      {name: 'toolbox', rectKey: 'toolboxRect', checkLid: false},
+    ].forEach(({name, rectKey, checkLid}) => {
+      test(`applies delete styling and deletes when dragged to ${name} at ${zoomName}`, function () {
+        if (scale !== null) {
+          setWorkspaceScale(this.workspace, scale);
+          this.trashRect = this.workspace.trashcan.getClientRect();
+          this.toolboxRect = this.workspace.toolbox.getClientRect();
+        }
 
-      dragger.onDragEnd(dragEvent);
-      assert.isTrue(this.block.isDeadOrDying());
+        const deleteRect = this[rectKey];
+        const {dragger, dragEvent, block} = dragBlock(
+          this.block,
+          rectCenterClient(deleteRect),
+        );
+
+        assert.isTrue(
+          deleteRect.contains(dragEvent.clientX, dragEvent.clientY),
+          `Expected cursor to be inside ${name} delete area`,
+        );
+        assert.isTrue(hasDeleteStyle(block));
+        if (checkLid) {
+          assert.isTrue(
+            hasTrashLidOpen(this.workspace),
+            'Expected trashcan lid to be open',
+          );
+        }
+
+        dragger.onDragEnd(dragEvent);
+        assert.isTrue(block.isDeadOrDying());
+      });
     });
   });
 
@@ -140,12 +178,12 @@ suite('Dragger', function () {
       x: deleteAreaRect.right - 5,
       y: originBefore.y,
     };
-    const {dragger, dragEvent} = dragBlock(this.block, {
+    const {dragger, dragEvent, block} = dragBlock(this.block, {
       x: start.x + desiredOrigin.x - originBefore.x,
       y: start.y + desiredOrigin.y - originBefore.y,
     });
 
-    const originAfter = blockOriginClient(this.block);
+    const originAfter = blockOriginClient(block);
     assert.isTrue(
       deleteAreaRect.contains(originAfter.x, originAfter.y),
       'Expected block origin to overlap delete area',
@@ -154,9 +192,112 @@ suite('Dragger', function () {
       deleteAreaRect.contains(dragEvent.clientX, dragEvent.clientY),
       'Expected cursor to be outside delete area',
     );
-    assert.isFalse(hasDeleteStyle(this.block));
+    assert.isFalse(hasDeleteStyle(block));
 
     dragger.onDragEnd(dragEvent);
-    assert.isFalse(this.block.isDeadOrDying());
+    assert.isFalse(block.isDeadOrDying());
+  });
+
+  suite('Mutator', function () {
+    /**
+     * Opens a mutator on a controls_if block and returns the mutator workspace.
+     *
+     * @param {!Blockly.WorkspaceSvg} workspace The main workspace.
+     * @returns {!Promise<!Blockly.WorkspaceSvg>} The mutator workspace.
+     */
+    async function openMutator(workspace) {
+      const block = Blockly.serialization.blocks.append(
+        {
+          'type': 'controls_if',
+          'extraState': {
+            'elseIfCount': 0,
+          },
+        },
+        workspace,
+      );
+      block.initSvg();
+      block.render();
+      const icon = block.getIcon(Blockly.icons.MutatorIcon.TYPE);
+      await icon.setBubbleVisible(true);
+      return icon.getWorkspace();
+    }
+
+    test('deletes flyout block when pointer is over flyout delete area at zoomed scale', async function () {
+      for (let i = 0; i < 3; i++) {
+        this.workspace.zoomCenter(1);
+      }
+
+      const mutatorWorkspace = await openMutator(this.workspace);
+      this.clock.runAll();
+      mutatorWorkspace.recordDragTargets();
+
+      const flyout = mutatorWorkspace.getFlyout();
+      const flyoutRect = flyout.getClientRect();
+      assert.isNotNull(flyoutRect);
+
+      const flyoutBlock = flyout
+        .getWorkspace()
+        .getBlocksByType('controls_if_elseif')[0];
+      flyoutBlock.initSvg();
+      flyoutBlock.render();
+
+      const {dragger, dragEvent, block} = dragBlock(
+        flyoutBlock,
+        rectCenterClient(flyoutRect),
+      );
+
+      assert.isTrue(
+        flyoutRect.contains(dragEvent.clientX, dragEvent.clientY),
+        'Expected cursor to be inside flyout delete area',
+      );
+      assert.isTrue(hasDeleteStyle(block));
+
+      dragger.onDragEnd(dragEvent);
+      assert.isTrue(block.isDeadOrDying());
+    });
+
+    test('does not apply delete styling when only block origin overlaps flyout delete area at zoomed scale', async function () {
+      for (let i = 0; i < 3; i++) {
+        this.workspace.zoomCenter(1);
+      }
+
+      const mutatorWorkspace = await openMutator(this.workspace);
+      this.clock.runAll();
+      mutatorWorkspace.recordDragTargets();
+
+      const flyout = mutatorWorkspace.getFlyout();
+      const flyoutRect = flyout.getClientRect();
+      assert.isNotNull(flyoutRect);
+
+      const workspaceBlock = mutatorWorkspace.newBlock('controls_if_elseif');
+      workspaceBlock.initSvg();
+      workspaceBlock.render();
+      workspaceBlock.moveBy(200, 50);
+
+      const start = blockCenterClient(workspaceBlock);
+      const originBefore = blockOriginClient(workspaceBlock);
+      const desiredOrigin = {
+        x: flyoutRect.right - 5,
+        y: originBefore.y,
+      };
+      const {dragger, dragEvent, block} = dragBlock(workspaceBlock, {
+        x: start.x + desiredOrigin.x - originBefore.x,
+        y: start.y + desiredOrigin.y - originBefore.y,
+      });
+
+      const originAfter = blockOriginClient(block);
+      assert.isTrue(
+        flyoutRect.contains(originAfter.x, originAfter.y),
+        'Expected block origin to overlap flyout delete area',
+      );
+      assert.isFalse(
+        flyoutRect.contains(dragEvent.clientX, dragEvent.clientY),
+        'Expected cursor to be outside flyout delete area',
+      );
+      assert.isFalse(hasDeleteStyle(block));
+
+      dragger.onDragEnd(dragEvent);
+      assert.isFalse(block.isDeadOrDying());
+    });
   });
 });