Skip to content

fix: delegate delete behavior to core #536

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
maribethb merged 2 commits into from
May 20, 2025
Merged

fix: delegate delete behavior to core #536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
74724ca
fix: delegate delete behavior to core
maribethb May 20, 2025
60481e2
chore: correct tsdoc
maribethb May 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
185 changes: 29 additions & 156 deletions src/actions/delete.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,200 +4,73 @@
* SPDX-License-Identifier: Apache-2.0
*/

import {
ContextMenuRegistry,
Gesture,
Msg,
ShortcutRegistry,
utils as BlocklyUtils,
LineCursor,
} from 'blockly';
import {ContextMenuRegistry, Msg, ShortcutItems} from 'blockly';
import {getShortActionShortcut} from '../shortcut_formatting';
import * as Constants from '../constants';
import type {WorkspaceSvg} from 'blockly';
import {Navigation} from '../navigation';

const KeyCodes = BlocklyUtils.KeyCodes;

/**
* Action to delete the block the cursor is currently on.
* Registers itself as both a keyboard shortcut and a context menu item.
*/
export class DeleteAction {
/**
* Saved context menu item, which is re-registered when this action
* is uninstalled.
*/
private oldContextMenuItem: ContextMenuRegistry.RegistryItem | null = null;

/**
* Saved delete shortcut, which is re-registered when this action
* is uninstalled.
* Saved context menu item display text function, which is restored
* when this action is uninstalled.
*/
private oldDeleteShortcut: ShortcutRegistry.KeyboardShortcut | null = null;
private oldDisplayText:
| ((scope: ContextMenuRegistry.Scope) => string | HTMLElement)
| string
| HTMLElement
| undefined = undefined;

/**
* Registration name for the keyboard shortcut.
* Saved context menu item, which has its display text restored when
* this action is uninstalled.
*/
private deleteShortcutName = Constants.SHORTCUT_NAMES.DELETE;
private oldContextMenuItem: ContextMenuRegistry.RegistryItem | null = null;

constructor(private navigation: Navigation) {}
constructor() {}

/**
* Install this action as both a keyboard shortcut and a context menu item.
*/
install() {
this.registerShortcut();
this.registerContextMenuAction();
}

/**
* Uninstall this action as both a keyboard shortcut and a context menu item.
* Reinstall the original context menu action if possible.
* Reinstall the original context menu display text if possible.
*/
uninstall() {
ContextMenuRegistry.registry.unregister('blockDeleteFromContextMenu');
if (this.oldContextMenuItem) {
ContextMenuRegistry.registry.register(this.oldContextMenuItem);
}
ShortcutRegistry.registry.unregister(this.deleteShortcutName);
if (this.oldDeleteShortcut) {
ShortcutRegistry.registry.register(this.oldDeleteShortcut);
if (this.oldContextMenuItem && this.oldDisplayText) {
this.oldContextMenuItem.displayText = this.oldDisplayText;
}
}

/**
* Create and register the keyboard shortcut for this action.
*/
private registerShortcut() {
this.oldDeleteShortcut = ShortcutRegistry.registry.getRegistry()['delete'];

if (!this.oldDeleteShortcut) return;

// Unregister the original shortcut.
ShortcutRegistry.registry.unregister(this.oldDeleteShortcut.name);

const deleteShortcut: ShortcutRegistry.KeyboardShortcut = {
name: this.deleteShortcutName,
preconditionFn: this.deletePrecondition.bind(this),
callback: this.deleteCallback.bind(this),
keyCodes: [KeyCodes.DELETE, KeyCodes.BACKSPACE],
allowCollision: true,
};

ShortcutRegistry.registry.register(deleteShortcut);
}

/**
* Register the delete block action as a context menu item on blocks.
* This function mixes together the keyboard and context menu preconditions
* but only calls the keyboard callback.
* Updates the text of the context menu delete action to include
* the keyboard shortcut.
*/
private registerContextMenuAction() {
this.oldContextMenuItem =
ContextMenuRegistry.registry.getItem('blockDelete');

if (!this.oldContextMenuItem) return;

// Unregister the original item.
ContextMenuRegistry.registry.unregister(this.oldContextMenuItem.id);

const deleteItem: ContextMenuRegistry.RegistryItem = {
displayText: (scope) => {
const shortcut = getShortActionShortcut(this.deleteShortcutName);
if (!this.oldContextMenuItem) {
return Msg['DELETE_BLOCK'].replace('%1', shortcut);
}
this.oldDisplayText = this.oldContextMenuItem.displayText;

type DisplayTextFn = (p1: ContextMenuRegistry.Scope) => string;
// Use the original item's text, which is dynamic based on the number
// of blocks that will be deleted.
const oldDisplayText = this.oldContextMenuItem
.displayText as DisplayTextFn;
return oldDisplayText(scope) + ` (${shortcut})`;
},
preconditionFn: (scope, menuOpenEvent: Event) => {
const ws = scope.block?.workspace;
const displayText = (scope: ContextMenuRegistry.Scope) => {
const shortcut = getShortActionShortcut(ShortcutItems.names.DELETE);

// Run the original precondition code, from the context menu option.
// If the item would be hidden or disabled, respect it.
const originalPreconditionResult =
this.oldContextMenuItem?.preconditionFn?.(scope, menuOpenEvent) ??
'enabled';
if (!ws || originalPreconditionResult !== 'enabled') {
return originalPreconditionResult;
}
// Use the original item's text, which is dynamic based on the number
// of blocks that will be deleted.
if (typeof this.oldDisplayText === 'function') {
return this.oldDisplayText(scope) + ` (${shortcut})`;
} else if (typeof this.oldDisplayText === 'string') {
return this.oldDisplayText + ` (${shortcut})`;
}

// Return enabled if the keyboard shortcut precondition is allowed,
// and disabled if the context menu precondition is met but the keyboard
// shortcut precondition is not met.
return this.deletePrecondition(ws) ? 'enabled' : 'disabled';
},
callback: (scope) => {
const ws = scope.block?.workspace;
if (!ws) return;

// Delete the block(s), and put the cursor back in a sane location.
return this.deleteCallback(ws, null);
},
scopeType: ContextMenuRegistry.ScopeType.BLOCK,
id: 'blockDeleteFromContextMenu',
weight: 11,
return Msg['DELETE_BLOCK'].replace('%1', shortcut);
};

ContextMenuRegistry.registry.register(deleteItem);
}

/**
* Precondition function for deleting a block from keyboard
* navigation. This precondition is shared between keyboard shortcuts
* and context menu items.
*
* @param workspace The `WorkspaceSvg` where the shortcut was
* invoked.
* @returns True iff `deleteCallback` function should be called.
*/
private deletePrecondition(workspace: WorkspaceSvg) {
const sourceBlock = workspace.getCursor()?.getSourceBlock();
return (
!workspace.isDragging() &&
this.navigation.canCurrentlyEdit(workspace) &&
!!sourceBlock?.isDeletable()
);
}

/**
* Callback function for deleting a block from keyboard
* navigation. This callback is shared between keyboard shortcuts
* and context menu items.
*
* @param workspace The `WorkspaceSvg` where the shortcut was
* invoked.
* @param e The originating event for a keyboard shortcut, or null
* if called from a context menu.
* @returns True if this function successfully handled deletion.
*/
private deleteCallback(workspace: WorkspaceSvg, e: Event | null) {
const cursor = workspace.getCursor();
if (!cursor) return false;

const sourceBlock = cursor.getSourceBlock();
if (!sourceBlock) return false;
// Delete or backspace.
// There is an event if this is triggered from a keyboard shortcut,
// but not if it's triggered from a context menu.
if (e) {
// Stop the browser from going back to the previous page.
// Do this first to prevent an error in the delete code from resulting
// in data loss.
e.preventDefault();
}
// Don't delete while dragging. Jeez.
if (Gesture.inProgress()) false;

if (cursor instanceof LineCursor) cursor.preDelete(sourceBlock);
sourceBlock.checkAndDelete();
if (cursor instanceof LineCursor) cursor.postDelete();
return true;
this.oldContextMenuItem.displayText = displayText;
}
}
1 change: 0 additions & 1 deletion src/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ export enum SHORTCUT_NAMES {
COPY = 'keyboard_nav_copy',
CUT = 'keyboard_nav_cut',
PASTE = 'keyboard_nav_paste',
DELETE = 'keyboard_nav_delete',
MOVE_WS_CURSOR_UP = 'workspace_up',
MOVE_WS_CURSOR_DOWN = 'workspace_down',
MOVE_WS_CURSOR_LEFT = 'workspace_left',
Expand Down
61 changes: 4 additions & 57 deletions src/navigation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,6 @@ import {
registrationType as cursorRegistrationType,
} from './flyout_cursor';

/**
* The default coordinate to use when focusing on the workspace and no
* blocks are present. In pixel coordinates, but will be converted to
* workspace coordinates when used to position the cursor.
*/
const DEFAULT_WS_COORDINATE: Blockly.utils.Coordinate =
new Blockly.utils.Coordinate(100, 100);

/**
* The default coordinate to use when moving the cursor to the workspace
* after a block has been deleted. In pixel coordinates, but will be
* converted to workspace coordinates when used to position the cursor.
*/
const WS_COORDINATE_ON_DELETE: Blockly.utils.Coordinate =
new Blockly.utils.Coordinate(100, 100);

/**
* Class that holds all methods necessary for keyboard navigation to work.
*/
Expand Down Expand Up @@ -181,18 +165,10 @@ export class Navigation {
if (!workspace || !workspace.keyboardAccessibilityMode) {
return;
}
switch (e.type) {
case Blockly.Events.DELETE:
this.handleBlockDeleteByDrag(
workspace,
e as Blockly.Events.BlockDelete,
);
break;
case Blockly.Events.BLOCK_CHANGE:
if ((e as Blockly.Events.BlockChange).element === 'mutation') {
this.handleBlockMutation(workspace, e as Blockly.Events.BlockChange);
}
break;
if (e.type === Blockly.Events.BLOCK_CHANGE) {
if ((e as Blockly.Events.BlockChange).element === 'mutation') {
this.handleBlockMutation(workspace, e as Blockly.Events.BlockChange);
}
}
}

Expand Down Expand Up @@ -289,31 +265,6 @@ export class Navigation {
}
}

/**
* Moves the cursor to the workspace when its parent block is deleted by
* being dragged to the flyout or to the trashcan.
*
* @param workspace The workspace the block was on.
* @param e The event emitted when a block is deleted.
*/
handleBlockDeleteByDrag(
workspace: Blockly.WorkspaceSvg,
e: Blockly.Events.BlockDelete,
) {
const deletedBlockId = e.blockId;
const ids = e.ids ?? [];
const cursor = workspace.getCursor();
if (!cursor) return;

// Make sure the cursor is on a block.
const sourceBlock = cursor.getSourceBlock();
if (!sourceBlock) return;

if (sourceBlock.id === deletedBlockId || ids.includes(sourceBlock.id)) {
cursor.setCurNode(workspace);
}
}

/**
* Handles when a user clicks on a block in the flyout by moving the cursor
* to that stack of blocks and setting the state of navigation to the flyout.
Expand Down Expand Up @@ -397,10 +348,6 @@ export class Navigation {
// disposed (which can happen when blocks are reloaded).
return false;
}
const wsCoordinates = new Blockly.utils.Coordinate(
DEFAULT_WS_COORDINATE.x / workspace.scale,
DEFAULT_WS_COORDINATE.y / workspace.scale,
);
if (topBlocks.length > 0) {
cursor.setCurNode(
topBlocks[prefer === 'first' ? 0 : topBlocks.length - 1],
Expand Down
2 changes: 1 addition & 1 deletion src/navigation_controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export class NavigationController {
shortcutDialog: ShortcutDialog = new ShortcutDialog();

/** Context menu and keyboard action for deletion. */
deleteAction: DeleteAction = new DeleteAction(this.navigation);
deleteAction: DeleteAction = new DeleteAction();

/** Context menu and keyboard action for deletion. */
editAction: EditAction = new EditAction(this.navigation);
Expand Down
Loading
Loading