Skip to content

Commit 10739f9

Browse files
authored
fix: dont activate base delete areas for keyboard moves (#9748)
* fix: dont activate base delete areas for keyboard moves * fix: add tests
1 parent 44d8554 commit 10739f9

5 files changed

Lines changed: 70 additions & 24 deletions

File tree

packages/blockly/core/delete_area.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {DragTarget} from './drag_target.js';
1717
import {isDeletable} from './interfaces/i_deletable.js';
1818
import type {IDeleteArea} from './interfaces/i_delete_area.js';
1919
import type {IDraggable} from './interfaces/i_draggable.js';
20+
import {KeyboardMover} from './keyboard_nav/keyboard_mover.js';
2021

2122
/**
2223
* 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 {
5657
* area.
5758
*/
5859
wouldDelete(element: IDraggable): boolean {
59-
if (element instanceof BlockSvg) {
60+
// don't delete things if we're doing a keyboard move
61+
if (KeyboardMover.mover.isMoving()) {
62+
this.updateWouldDelete_(false);
63+
} else if (element instanceof BlockSvg) {
6064
const block = element;
6165
const couldDeleteBlock = !block.getParent() && block.isDeletable();
6266
this.updateWouldDelete_(couldDeleteBlock);

packages/blockly/core/toolbox/toolbox.ts

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
*/
1212
// Former goog.module ID: Blockly.Toolbox
1313

14-
import {BlockSvg} from '../block_svg.js';
1514
import * as browserEvents from '../browser_events.js';
1615
import * as common from '../common.js';
1716
import {ComponentManager} from '../component_manager.js';
@@ -26,7 +25,6 @@ import {
2625
isCollapsibleToolboxItem,
2726
type ICollapsibleToolboxItem,
2827
} from '../interfaces/i_collapsible_toolbox_item.js';
29-
import {isDeletable} from '../interfaces/i_deletable.js';
3028
import type {IDraggable} from '../interfaces/i_draggable.js';
3129
import type {IFlyout} from '../interfaces/i_flyout.js';
3230
import type {IFocusableNode} from '../interfaces/i_focusable_node.js';
@@ -37,6 +35,7 @@ import {isSelectableToolboxItem} from '../interfaces/i_selectable_toolbox_item.j
3735
import type {IStyleable} from '../interfaces/i_styleable.js';
3836
import type {IToolbox} from '../interfaces/i_toolbox.js';
3937
import type {IToolboxItem} from '../interfaces/i_toolbox_item.js';
38+
import {KeyboardMover} from '../keyboard_nav/keyboard_mover.js';
4039
import {ToolboxNavigator} from '../keyboard_nav/navigators/toolbox_navigator.js';
4140
import * as registry from '../registry.js';
4241
import type {KeyboardShortcut} from '../shortcut_registry.js';
@@ -508,32 +507,14 @@ export class Toolbox
508507
}
509508
}
510509

511-
/**
512-
* Returns whether the provided block or bubble would be deleted if dropped on
513-
* this area.
514-
* This method should check if the element is deletable and is always called
515-
* before onDragEnter/onDragOver/onDragExit.
516-
*
517-
* @param element The block or bubble currently being dragged.
518-
* @returns Whether the element provided would be deleted if dropped on this
519-
* area.
520-
*/
521-
override wouldDelete(element: IDraggable): boolean {
522-
if (element instanceof BlockSvg) {
523-
const block = element;
524-
this.updateWouldDelete_(!block.getParent() && block.isDeletable());
525-
} else {
526-
this.updateWouldDelete_(isDeletable(element) && element.isDeletable());
527-
}
528-
return this.wouldDelete_;
529-
}
530-
531510
/**
532511
* Handles when a cursor with a block or bubble enters this drag target.
533512
*
534513
* @param _dragElement The block or bubble currently being dragged.
535514
*/
536515
override onDragEnter(_dragElement: IDraggable) {
516+
// don't trigger for keyboard moves
517+
if (KeyboardMover.mover.isMoving()) return;
537518
this.updateCursorDeleteStyle_(true);
538519
}
539520

@@ -543,6 +524,8 @@ export class Toolbox
543524
* @param _dragElement The block or bubble currently being dragged.
544525
*/
545526
override onDragExit(_dragElement: IDraggable) {
527+
// don't trigger for keyboard moves
528+
if (KeyboardMover.mover.isMoving()) return;
546529
this.updateCursorDeleteStyle_(false);
547530
}
548531

@@ -553,6 +536,8 @@ export class Toolbox
553536
* @param _dragElement The block or bubble currently being dragged.
554537
*/
555538
override onDrop(_dragElement: IDraggable) {
539+
// don't trigger for keyboard moves
540+
if (KeyboardMover.mover.isMoving()) return;
556541
this.updateCursorDeleteStyle_(false);
557542
}
558543

packages/blockly/core/trashcan.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import type {IAutoHideable} from './interfaces/i_autohideable.js';
2424
import type {IDraggable} from './interfaces/i_draggable.js';
2525
import type {IFlyout} from './interfaces/i_flyout.js';
2626
import type {IPositionable} from './interfaces/i_positionable.js';
27+
import {KeyboardMover} from './keyboard_nav/keyboard_mover.js';
2728
import type {UiMetrics} from './metrics_manager.js';
2829
import * as uiPosition from './positionable_helpers.js';
2930
import * as registry from './registry.js';
@@ -426,6 +427,8 @@ export class Trashcan
426427
* @param _dragElement The block or bubble currently being dragged.
427428
*/
428429
override onDragOver(_dragElement: IDraggable) {
430+
// don't trigger for keyboard moves
431+
if (KeyboardMover.mover.isMoving()) return;
429432
this.setLidOpen(this.wouldDelete_);
430433
}
431434

@@ -435,6 +438,8 @@ export class Trashcan
435438
* @param _dragElement The block or bubble currently being dragged.
436439
*/
437440
override onDragExit(_dragElement: IDraggable) {
441+
// don't trigger for keyboard moves
442+
if (KeyboardMover.mover.isMoving()) return;
438443
this.setLidOpen(false);
439444
}
440445

@@ -445,6 +450,8 @@ export class Trashcan
445450
* @param _dragElement The block or bubble currently being dragged.
446451
*/
447452
override onDrop(_dragElement: IDraggable) {
453+
// don't trigger for keyboard moves
454+
if (KeyboardMover.mover.isMoving()) return;
448455
setTimeout(this.setLidOpen.bind(this, false), 100);
449456
}
450457

packages/blockly/tests/mocha/toolbox_test.js

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@
55
*/
66

77
import {assert} from '../../node_modules/chai/index.js';
8-
import {defineStackBlock} from './test_helpers/block_definitions.js';
8+
import {
9+
defineBasicBlockWithField,
10+
defineStackBlock,
11+
} from './test_helpers/block_definitions.js';
912
import {
1013
sharedTestSetup,
1114
sharedTestTeardown,
@@ -812,4 +815,28 @@ suite('Toolbox', function () {
812815
);
813816
});
814817
});
818+
suite('delete area', function () {
819+
test('Keyboard drag - wouldDelete returns false', function () {
820+
// Create a deletable block
821+
defineBasicBlockWithField();
822+
const block = this.toolbox.getWorkspace().newBlock('test_field_block');
823+
block.initSvg();
824+
block.render();
825+
826+
// Stub KeyboardMover.mover.isMoving() to return true
827+
const isMovingStub = sinon
828+
.stub(Blockly.KeyboardMover.mover, 'isMoving')
829+
.returns(true);
830+
831+
try {
832+
const result = this.toolbox.wouldDelete(block);
833+
assert.isFalse(
834+
result,
835+
'wouldDelete should return false during keyboard move',
836+
);
837+
} finally {
838+
isMovingStub.restore();
839+
}
840+
});
841+
});
815842
});

packages/blockly/tests/mocha/trashcan_test.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,4 +373,27 @@ suite('Trashcan', function () {
373373
this.workspace.options.maxTrashcanContents = Infinity;
374374
});
375375
});
376+
suite('delete area', function () {
377+
test('Keyboard drag - wouldDelete returns false', function () {
378+
// Create a deletable block
379+
const block = this.workspace.newBlock('test_field_block');
380+
block.initSvg();
381+
block.render();
382+
383+
// Stub KeyboardMover.mover.isMoving() to return true
384+
const isMovingStub = sinon
385+
.stub(Blockly.KeyboardMover.mover, 'isMoving')
386+
.returns(true);
387+
388+
try {
389+
const result = this.trashcan.wouldDelete(block);
390+
assert.isFalse(
391+
result,
392+
'wouldDelete should return false during keyboard move',
393+
);
394+
} finally {
395+
isMovingStub.restore();
396+
}
397+
});
398+
});
376399
});

0 commit comments

Comments
 (0)