From 25ba1b555e2847ef051ca11a0e49c28bb0a1ab59 Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Wed, 22 Apr 2026 13:38:15 -0400 Subject: [PATCH 1/2] fix: dont activate base delete areas for keyboard moves --- packages/blockly/core/delete_area.ts | 6 ++++- packages/blockly/core/toolbox/toolbox.ts | 29 ++++++------------------ packages/blockly/core/trashcan.ts | 7 ++++++ 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/packages/blockly/core/delete_area.ts b/packages/blockly/core/delete_area.ts index 405084db9b1..c53acb195fb 100644 --- a/packages/blockly/core/delete_area.ts +++ b/packages/blockly/core/delete_area.ts @@ -17,6 +17,7 @@ import {DragTarget} from './drag_target.js'; import {isDeletable} from './interfaces/i_deletable.js'; import type {IDeleteArea} from './interfaces/i_delete_area.js'; import type {IDraggable} from './interfaces/i_draggable.js'; +import {KeyboardMover} from './keyboard_nav/keyboard_mover.js'; /** * Abstract class for a component that can delete a block or bubble that is @@ -56,7 +57,10 @@ export class DeleteArea extends DragTarget implements IDeleteArea { * area. */ wouldDelete(element: IDraggable): boolean { - if (element instanceof BlockSvg) { + // don't delete things if we're doing a keyboard move + if (KeyboardMover.mover.isMoving()) { + this.updateWouldDelete_(false); + } else if (element instanceof BlockSvg) { const block = element; const couldDeleteBlock = !block.getParent() && block.isDeletable(); this.updateWouldDelete_(couldDeleteBlock); diff --git a/packages/blockly/core/toolbox/toolbox.ts b/packages/blockly/core/toolbox/toolbox.ts index 67bb0fc84b4..c77f32a86a8 100644 --- a/packages/blockly/core/toolbox/toolbox.ts +++ b/packages/blockly/core/toolbox/toolbox.ts @@ -11,7 +11,6 @@ */ // Former goog.module ID: Blockly.Toolbox -import {BlockSvg} from '../block_svg.js'; import * as browserEvents from '../browser_events.js'; import * as common from '../common.js'; import {ComponentManager} from '../component_manager.js'; @@ -26,7 +25,6 @@ import { isCollapsibleToolboxItem, type ICollapsibleToolboxItem, } from '../interfaces/i_collapsible_toolbox_item.js'; -import {isDeletable} from '../interfaces/i_deletable.js'; import type {IDraggable} from '../interfaces/i_draggable.js'; import type {IFlyout} from '../interfaces/i_flyout.js'; import type {IFocusableNode} from '../interfaces/i_focusable_node.js'; @@ -37,6 +35,7 @@ import {isSelectableToolboxItem} from '../interfaces/i_selectable_toolbox_item.j import type {IStyleable} from '../interfaces/i_styleable.js'; import type {IToolbox} from '../interfaces/i_toolbox.js'; import type {IToolboxItem} from '../interfaces/i_toolbox_item.js'; +import {KeyboardMover} from '../keyboard_nav/keyboard_mover.js'; import {ToolboxNavigator} from '../keyboard_nav/navigators/toolbox_navigator.js'; import * as registry from '../registry.js'; import type {KeyboardShortcut} from '../shortcut_registry.js'; @@ -508,32 +507,14 @@ export class Toolbox } } - /** - * Returns whether the provided block or bubble would be deleted if dropped on - * this area. - * This method should check if the element is deletable and is always called - * before onDragEnter/onDragOver/onDragExit. - * - * @param element The block or bubble currently being dragged. - * @returns Whether the element provided would be deleted if dropped on this - * area. - */ - override wouldDelete(element: IDraggable): boolean { - if (element instanceof BlockSvg) { - const block = element; - this.updateWouldDelete_(!block.getParent() && block.isDeletable()); - } else { - this.updateWouldDelete_(isDeletable(element) && element.isDeletable()); - } - return this.wouldDelete_; - } - /** * Handles when a cursor with a block or bubble enters this drag target. * * @param _dragElement The block or bubble currently being dragged. */ override onDragEnter(_dragElement: IDraggable) { + // don't trigger for keyboard moves + if (KeyboardMover.mover.isMoving()) return; this.updateCursorDeleteStyle_(true); } @@ -543,6 +524,8 @@ export class Toolbox * @param _dragElement The block or bubble currently being dragged. */ override onDragExit(_dragElement: IDraggable) { + // don't trigger for keyboard moves + if (KeyboardMover.mover.isMoving()) return; this.updateCursorDeleteStyle_(false); } @@ -553,6 +536,8 @@ export class Toolbox * @param _dragElement The block or bubble currently being dragged. */ override onDrop(_dragElement: IDraggable) { + // don't trigger for keyboard moves + if (KeyboardMover.mover.isMoving()) return; this.updateCursorDeleteStyle_(false); } diff --git a/packages/blockly/core/trashcan.ts b/packages/blockly/core/trashcan.ts index 08211bac9dd..3f750923995 100644 --- a/packages/blockly/core/trashcan.ts +++ b/packages/blockly/core/trashcan.ts @@ -24,6 +24,7 @@ import type {IAutoHideable} from './interfaces/i_autohideable.js'; import type {IDraggable} from './interfaces/i_draggable.js'; import type {IFlyout} from './interfaces/i_flyout.js'; import type {IPositionable} from './interfaces/i_positionable.js'; +import {KeyboardMover} from './keyboard_nav/keyboard_mover.js'; import type {UiMetrics} from './metrics_manager.js'; import * as uiPosition from './positionable_helpers.js'; import * as registry from './registry.js'; @@ -426,6 +427,8 @@ export class Trashcan * @param _dragElement The block or bubble currently being dragged. */ override onDragOver(_dragElement: IDraggable) { + // don't trigger for keyboard moves + if (KeyboardMover.mover.isMoving()) return; this.setLidOpen(this.wouldDelete_); } @@ -435,6 +438,8 @@ export class Trashcan * @param _dragElement The block or bubble currently being dragged. */ override onDragExit(_dragElement: IDraggable) { + // don't trigger for keyboard moves + if (KeyboardMover.mover.isMoving()) return; this.setLidOpen(false); } @@ -445,6 +450,8 @@ export class Trashcan * @param _dragElement The block or bubble currently being dragged. */ override onDrop(_dragElement: IDraggable) { + // don't trigger for keyboard moves + if (KeyboardMover.mover.isMoving()) return; setTimeout(this.setLidOpen.bind(this, false), 100); } From 2fe0c1d040dc582883eac1472ae6d51ca0bdb3aa Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Wed, 22 Apr 2026 16:21:46 -0400 Subject: [PATCH 2/2] fix: add tests --- packages/blockly/tests/mocha/toolbox_test.js | 29 ++++++++++++++++++- packages/blockly/tests/mocha/trashcan_test.js | 23 +++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/packages/blockly/tests/mocha/toolbox_test.js b/packages/blockly/tests/mocha/toolbox_test.js index 5886fe7f6a7..c02f09f4ddb 100644 --- a/packages/blockly/tests/mocha/toolbox_test.js +++ b/packages/blockly/tests/mocha/toolbox_test.js @@ -5,7 +5,10 @@ */ import {assert} from '../../node_modules/chai/index.js'; -import {defineStackBlock} from './test_helpers/block_definitions.js'; +import { + defineBasicBlockWithField, + defineStackBlock, +} from './test_helpers/block_definitions.js'; import { sharedTestSetup, sharedTestTeardown, @@ -812,4 +815,28 @@ suite('Toolbox', function () { ); }); }); + suite('delete area', function () { + test('Keyboard drag - wouldDelete returns false', function () { + // Create a deletable block + defineBasicBlockWithField(); + const block = this.toolbox.getWorkspace().newBlock('test_field_block'); + block.initSvg(); + block.render(); + + // Stub KeyboardMover.mover.isMoving() to return true + const isMovingStub = sinon + .stub(Blockly.KeyboardMover.mover, 'isMoving') + .returns(true); + + try { + const result = this.toolbox.wouldDelete(block); + assert.isFalse( + result, + 'wouldDelete should return false during keyboard move', + ); + } finally { + isMovingStub.restore(); + } + }); + }); }); diff --git a/packages/blockly/tests/mocha/trashcan_test.js b/packages/blockly/tests/mocha/trashcan_test.js index d96e00f3a21..62486bb5f3e 100644 --- a/packages/blockly/tests/mocha/trashcan_test.js +++ b/packages/blockly/tests/mocha/trashcan_test.js @@ -373,4 +373,27 @@ suite('Trashcan', function () { this.workspace.options.maxTrashcanContents = Infinity; }); }); + suite('delete area', function () { + test('Keyboard drag - wouldDelete returns false', function () { + // Create a deletable block + const block = this.workspace.newBlock('test_field_block'); + block.initSvg(); + block.render(); + + // Stub KeyboardMover.mover.isMoving() to return true + const isMovingStub = sinon + .stub(Blockly.KeyboardMover.mover, 'isMoving') + .returns(true); + + try { + const result = this.trashcan.wouldDelete(block); + assert.isFalse( + result, + 'wouldDelete should return false during keyboard move', + ); + } finally { + isMovingStub.restore(); + } + }); + }); });