From 3078745e806b2f26f8c480530217fc8d01cab9cb Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Tue, 11 Mar 2025 18:30:00 +0000 Subject: [PATCH 01/11] feat: Introduce Mover class, moving mode Introduce the Mover class, with shortcuts that will put a workspace into (and exit from) moving mode. While in moving mode, cursor navigation via the cursor keys is disabled. --- src/actions/mover.ts | 265 +++++++++++++++++++++++++++++++++++ src/navigation_controller.ts | 12 +- 2 files changed, 273 insertions(+), 4 deletions(-) create mode 100644 src/actions/mover.ts diff --git a/src/actions/mover.ts b/src/actions/mover.ts new file mode 100644 index 00000000..c2f16bc8 --- /dev/null +++ b/src/actions/mover.ts @@ -0,0 +1,265 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as Constants from '../constants'; +import {ASTNode, ShortcutRegistry, utils as BlocklyUtils} from 'blockly'; +import type {Block, WorkspaceSvg} from 'blockly'; +import {Navigation} from '../navigation'; + +const KeyCodes = BlocklyUtils.KeyCodes; +const createSerializedKey = ShortcutRegistry.registry.createSerializedKey.bind( + ShortcutRegistry.registry, +); + +/** + * Actions for moving blocks with keyboard shortcuts. + */ +export class Mover { + /** + * Function provided by the navigation controller to say whether editing + * is allowed. + */ + protected canCurrentlyEdit: (ws: WorkspaceSvg) => boolean; + + /** + * Map of moves in progress. + * + * An entry for a given workspace in this map means that the this + * Mover is moving a block on that workspace, and will disable + * normal cursor movement until the move is complete. + */ + protected moves: Map = new Map(); + + constructor( + protected navigation: Navigation, + canEdit: (ws: WorkspaceSvg) => boolean, + ) { + this.canCurrentlyEdit = canEdit; + } + + private shortcuts: ShortcutRegistry.KeyboardShortcut[] = [ + // Begin and end move. + { + name: 'Start move', + preconditionFn: (workspace) => this.canMove(workspace), + callback: (workspace) => this.startMove(workspace), + keyCodes: [KeyCodes.M], + allowCollision: true, // TODO: remove once #309 has been merged. + }, + { + name: 'Finish move', + preconditionFn: (workspace) => this.isMoving(workspace), + callback: (workspace) => this.finishMove(workspace), + keyCodes: [KeyCodes.ENTER], + allowCollision: true, + }, + { + name: 'Abort move', + preconditionFn: (workspace) => this.isMoving(workspace), + callback: (workspace) => this.abortMove(workspace), + keyCodes: [KeyCodes.ESC], + allowCollision: true, + }, + + // Constrained moves. + { + name: 'Move left, constrained', + preconditionFn: (workspace) => this.isMoving(workspace), + callback: (workspace) => this.moveConstrained(workspace /* , ...*/), + keyCodes: [KeyCodes.LEFT], + allowCollision: true, + }, + { + name: 'Move right unconstraind', + preconditionFn: (workspace) => this.isMoving(workspace), + callback: (workspace) => this.moveConstrained(workspace /* , ... */), + keyCodes: [KeyCodes.RIGHT], + allowCollision: true, + }, + { + name: 'Move up, constrained', + preconditionFn: (workspace) => this.isMoving(workspace), + callback: (workspace) => this.moveConstrained(workspace /* , ... */), + keyCodes: [KeyCodes.UP], + allowCollision: true, + }, + { + name: 'Move down constrained', + preconditionFn: (workspace) => this.isMoving(workspace), + callback: (workspace) => this.moveConstrained(workspace /* , ... */), + keyCodes: [KeyCodes.DOWN], + allowCollision: true, + }, + + // Unconstrained moves. + { + name: 'Move left, unconstrained', + preconditionFn: (workspace) => this.isMoving(workspace), + callback: (workspace) => this.moveUnconstrained(workspace, -1, 0), + keyCodes: [createSerializedKey(KeyCodes.LEFT, [KeyCodes.ALT])], + }, + { + name: 'Move right, unconstraind', + preconditionFn: (workspace) => this.isMoving(workspace), + callback: (workspace) => this.moveUnconstrained(workspace, 1, 0), + keyCodes: [createSerializedKey(KeyCodes.RIGHT, [KeyCodes.ALT])], + }, + { + name: 'Move up unconstrained', + preconditionFn: (workspace) => this.isMoving(workspace), + callback: (workspace) => this.moveUnconstrained(workspace, 0, -1), + keyCodes: [createSerializedKey(KeyCodes.UP, [KeyCodes.ALT])], + }, + { + name: 'Move down, unconstrained', + preconditionFn: (workspace) => this.isMoving(workspace), + callback: (workspace) => this.moveUnconstrained(workspace, 0, 1), + keyCodes: [createSerializedKey(KeyCodes.DOWN, [KeyCodes.ALT])], + }, + ]; + + /** + * Install the actions as both keyboard shortcuts and (where + * applicable) context menu items. + */ + install() { + for (const shortcut of this.shortcuts) { + ShortcutRegistry.registry.register(shortcut); + } + } + + /** + * Uninstall these actions. + */ + uninstall() { + for (const shortcut of this.shortcuts) { + ShortcutRegistry.registry.unregister(shortcut.name); + } + } + + /** + * Returns true iff we are able to begin moving a block on the given + * workspace. + * + * @param workspace The workspace to move on. + * @returns True iff we can beign a move. + */ + canMove(workspace: WorkspaceSvg) { + // TODO: also check if current block is movable. + return ( + this.navigation.getState(workspace) === Constants.STATE.WORKSPACE && + this.canCurrentlyEdit(workspace) && + !this.moves.has(workspace) + ); + } + + /** + * Returns true iff we are currently moving a block on the given + * workspace. + * + * @param workspace The workspace we might be moving on. + * @returns True iff we are moving. + */ + isMoving(workspace: WorkspaceSvg) { + return this.canCurrentlyEdit(workspace) && this.moves.has(workspace); + } + + /** + * Start moving the currently-focused item on workspace, if + * possible. + * + * Should only be called if canMove has returned true. + * + * @param workspace The workspace we might be moving on. + * @returns True iff a move has successfully begun. + */ + startMove(workspace: WorkspaceSvg) { + const cursor = workspace?.getCursor(); + const curNode = cursor?.getCurNode(); + const block = curNode?.getSourceBlock(); + if (!block || !block.isMovable()) return false; + + console.log('startMove'); + + this.moves.set(workspace, {}); + return true; + } + + /** + * Finish moving the currently-focused item on workspace. + * + * Should only be called if isMoving has returned true. + * + * @param workspace The workspace on which we are moving. + * @returns True iff move successfully finished. + */ + finishMove(workspace: WorkspaceSvg) { + if (!workspace) return false; + + console.log('finishMove'); + + this.moves.delete(workspace); + return true; + } + + /** + * Abort moving the currently-focused item on workspace. + * + * Should only be called if isMoving has returned true. + * + * @param workspace The workspace on which we are moving. + * @returns True iff move successfully aborted. + */ + abortMove(workspace: WorkspaceSvg) { + if (!workspace) return false; + + console.log('abortMove'); + + this.moves.delete(workspace); + return true; + } + + /** + * Action to move the item being moved in the given direction, + * constrained to valid attachment points (if any). + * + * @param workspace The workspace to move on. + * @returns True iff this action applies and has been performed. + */ + moveConstrained( + workspace: WorkspaceSvg, + /* ... */ + ) { + console.log('moveConstrained'); + // Not yet implemented. Absorb keystroke to avoid moving cursor. + return true; + } + + /** + * Action to move the item being moved in the given direction, + * without constraint. + * + * @param workspace The workspace to move on. + * @param xDirection -1 to move left. 1 to move right. + * @param yDirection -1 to move up. 1 to move down. + * @returns True iff this action applies and has been performed. + */ + moveUnconstrained( + workspace: WorkspaceSvg, + xDirection: number, + yDirection: number, + ): boolean { + console.log('moveUnconstrained'); + // Not yet implemented. Absorb keystroke to avoid moving cursor. + return true; + } +} + +/** + * Information about the currently in-progress move for a given + * Workspace. + */ +type MoveInfo = {}; diff --git a/src/navigation_controller.ts b/src/navigation_controller.ts index 2dc62479..5e53e73f 100644 --- a/src/navigation_controller.ts +++ b/src/navigation_controller.ts @@ -34,6 +34,7 @@ import {ExitAction} from './actions/exit'; import {EnterAction} from './actions/enter'; import {DisconnectAction} from './actions/disconnect'; import {ActionMenu} from './actions/action_menu'; +import {Mover} from './actions/mover'; const KeyCodes = BlocklyUtils.KeyCodes; @@ -96,6 +97,8 @@ export class NavigationController { this.canCurrentlyNavigate.bind(this), ); + mover = new Mover(this.navigation, this.canCurrentlyEdit.bind(this)); + /** * Original Toolbox.prototype.onShortcut method, saved by * addShortcutHandlers. @@ -340,6 +343,7 @@ export class NavigationController { this.actionMenu.install(); this.clipboard.install(); + this.mover.install(); this.shortcutDialog.install(); // Initialize the shortcut modal with available shortcuts. Needs @@ -352,10 +356,7 @@ export class NavigationController { * Removes all the keyboard navigation shortcuts. */ dispose() { - for (const shortcut of Object.values(this.shortcuts)) { - ShortcutRegistry.registry.unregister(shortcut.name); - } - + this.mover.install(); this.deleteAction.uninstall(); this.editAction.uninstall(); this.insertAction.uninstall(); @@ -368,6 +369,9 @@ export class NavigationController { this.actionMenu.uninstall(); this.shortcutDialog.uninstall(); + for (const shortcut of Object.values(this.shortcuts)) { + ShortcutRegistry.registry.unregister(shortcut.name); + } this.removeShortcutHandlers(); this.navigation.dispose(); } From c3c715f5679c0db788a6d3b28a44ac7481aed92f Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Tue, 11 Mar 2025 19:03:49 +0000 Subject: [PATCH 02/11] feat: Save block starting location and connections --- src/actions/mover.ts | 45 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/src/actions/mover.ts b/src/actions/mover.ts index c2f16bc8..9f120033 100644 --- a/src/actions/mover.ts +++ b/src/actions/mover.ts @@ -5,11 +5,11 @@ */ import * as Constants from '../constants'; -import {ASTNode, ShortcutRegistry, utils as BlocklyUtils} from 'blockly'; -import type {Block, WorkspaceSvg} from 'blockly'; +import {ASTNode, Connection, ShortcutRegistry, common, utils} from 'blockly'; +import type {Block, BlockSvg, WorkspaceSvg} from 'blockly'; import {Navigation} from '../navigation'; -const KeyCodes = BlocklyUtils.KeyCodes; +const KeyCodes = utils.KeyCodes; const createSerializedKey = ShortcutRegistry.registry.createSerializedKey.bind( ShortcutRegistry.registry, ); @@ -148,11 +148,15 @@ export class Mover { * @returns True iff we can beign a move. */ canMove(workspace: WorkspaceSvg) { - // TODO: also check if current block is movable. - return ( + const cursor = workspace?.getCursor(); + const curNode = cursor?.getCurNode(); + const block = curNode?.getSourceBlock(); + + return !!( this.navigation.getState(workspace) === Constants.STATE.WORKSPACE && this.canCurrentlyEdit(workspace) && - !this.moves.has(workspace) + !this.moves.has(workspace) && // No move in progress. + block?.isMovable() ); } @@ -179,12 +183,17 @@ export class Mover { startMove(workspace: WorkspaceSvg) { const cursor = workspace?.getCursor(); const curNode = cursor?.getCurNode(); - const block = curNode?.getSourceBlock(); - if (!block || !block.isMovable()) return false; + const block = curNode?.getSourceBlock() as BlockSvg | null; + if (!cursor || !block) throw new Error('precondition failure'); + // Select and focus block. + common.setSelected(block); + cursor.setCurNode(ASTNode.createBlockNode(block)!); + + // Additional implementation goes here. console.log('startMove'); - this.moves.set(workspace, {}); + this.moves.set(workspace, new MoveInfo(block)); return true; } @@ -198,7 +207,10 @@ export class Mover { */ finishMove(workspace: WorkspaceSvg) { if (!workspace) return false; + const info = this.moves.get(workspace); + if (!info) throw new Error('no move info for workspace'); + // Additional implementation goes here. console.log('finishMove'); this.moves.delete(workspace); @@ -215,7 +227,10 @@ export class Mover { */ abortMove(workspace: WorkspaceSvg) { if (!workspace) return false; + const info = this.moves.get(workspace); + if (!info) throw new Error('no move info for workspace'); + // Additional implementation goes here. console.log('abortMove'); this.moves.delete(workspace); @@ -262,4 +277,14 @@ export class Mover { * Information about the currently in-progress move for a given * Workspace. */ -type MoveInfo = {}; +export class MoveInfo { + public readonly parentNext: Connection | null; + public readonly parentInput: Connection | null; + public readonly startLocation: utils.Coordinate | null; + + constructor(public readonly block: Block) { + this.parentNext = block.previousConnection?.targetConnection ?? null; + this.parentInput = block.outputConnection?.targetConnection ?? null; + this.startLocation = block.getRelativeToSurfaceXY(); + } +} From 5bd5aef0ed888617172730fb33b1948b4374a9c6 Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Thu, 13 Mar 2025 13:29:45 +0000 Subject: [PATCH 03/11] feat: Experiment: unconstrained movement using Dragger Create an experimental implementation of unconstrained dragging using the existing Dragger class. Implement this on a new Mover subclass DragMover, to keep this experimental code (that will probably be thrown away) separate from the main Mover implementation. Known bugs: * Blocks lurch around somewhat wildly while being moved. * Moving top-level blocks works reasonably well, but attempting to move a non-top-level block will result in it being deleted unless the move is aborted. --- src/actions/drag_mover.ts | 179 +++++++++++++++++++++++++++++++++++ src/navigation_controller.ts | 2 +- 2 files changed, 180 insertions(+), 1 deletion(-) create mode 100644 src/actions/drag_mover.ts diff --git a/src/actions/drag_mover.ts b/src/actions/drag_mover.ts new file mode 100644 index 00000000..522b0d2c --- /dev/null +++ b/src/actions/drag_mover.ts @@ -0,0 +1,179 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as Constants from '../constants'; +import { + ASTNode, + Connection, + ShortcutRegistry, + common, + registry, + utils, +} from 'blockly'; +import type {Block, BlockSvg, IDragger, WorkspaceSvg} from 'blockly'; +import {Mover, MoveInfo} from './mover'; +import {Navigation} from '../navigation'; + +/** + * An experimental implementation of Mover that uses a dragger to + * perform unconstraind movment. + */ +export class DragMover extends Mover { + /** + * The distance to move an item, in workspace coordinates, when + * making an unconstrained move. + */ + UNCONSTRAINED_MOVE_DISTANCE = 20; + + /** + * Map of moves in progress. + * + * An entry for a given workspace in this map means that the this + * Mover is moving a block on that workspace, and will disable + * normal cursor movement until the move is complete. + */ + protected declare moves: Map; + + /** + * Start moving the currently-focused item on workspace, if + * possible. + * + * Should only be called if canMove has returned true. + * + * @param workspace The workspace we might be moving on. + * @returns True iff a move has successfully begun. + */ + override startMove(workspace: WorkspaceSvg) { + const cursor = workspace?.getCursor(); + const curNode = cursor?.getCurNode(); + const block = curNode?.getSourceBlock() as BlockSvg | null; + if (!cursor || !block) throw new Error('precondition failure'); + + // Select and focus block. + common.setSelected(block); + cursor.setCurNode(ASTNode.createBlockNode(block)!); + // Begin dragging block. + const DraggerClass = registry.getClassFromOptions( + registry.Type.BLOCK_DRAGGER, + workspace.options, + true, + ); + if (!DraggerClass) throw new Error('no Dragger registered'); + const dragger = new DraggerClass(block, workspace); + // Record that a move is in progress and start dragging. + this.moves.set(workspace, new DragMoveInfo(block, dragger)); + dragger.onDragStart(new PointerEvent('pointerdown')); + return true; + } + + /** + * Finish moving the currently-focused item on workspace. + * + * Should only be called if isMoving has returned true. + * + * @param workspace The workspace on which we are moving. + * @returns True iff move successfully finished. + */ + override finishMove(workspace: WorkspaceSvg) { + if (!workspace) return false; + const info = this.moves.get(workspace); + if (!info) throw new Error('no move info for workspace'); + + info.dragger.onDragEnd( + new PointerEvent('pointerup'), + new utils.Coordinate(0, 0), + ); + + this.moves.delete(workspace); + return true; + } + + /** + * Abort moving the currently-focused item on workspace. + * + * Should only be called if isMoving has returned true. + * + * @param workspace The workspace on which we are moving. + * @returns True iff move successfully aborted. + */ + override abortMove(workspace: WorkspaceSvg) { + if (!workspace) return false; + const info = this.moves.get(workspace); + if (!info) throw new Error('no move info for workspace'); + + // Monkey patch dragger to trigger call to draggable.revertDrag. + (info.dragger as any).shouldReturnToStart = () => true; + info.dragger.onDragEnd( + new PointerEvent('pointerup'), + new utils.Coordinate(0, 0), + ); + + this.moves.delete(workspace); + return true; + } + + /** + * Action to move the item being moved in the given direction, + * constrained to valid attachment points (if any). + * + * @param workspace The workspace to move on. + * @returns True iff this action applies and has been performed. + */ + override moveConstrained( + workspace: WorkspaceSvg, + /* ... */ + ) { + // Not yet implemented. Absorb keystroke to avoid moving cursor. + alert(`Constrained movement not implemented. + +Use alt+arrow (option+arrow on macOS) for unconstrained move. +Use enter to complete the move, or escape to abort.`); + return true; + } + + /** + * Action to move the item being moved in the given direction, + * without constraint. + * + * @param workspace The workspace to move on. + * @param xDirection -1 to move left. 1 to move right. + * @param yDirection -1 to move up. 1 to move down. + * @returns True iff this action applies and has been performed. + */ + override moveUnconstrained( + workspace: WorkspaceSvg, + xDirection: number, + yDirection: number, + ): boolean { + if (!workspace) return false; + const info = this.moves.get(workspace); + if (!info) throw new Error('no move info for workspace'); + + info.totalDelta.x += + xDirection * this.UNCONSTRAINED_MOVE_DISTANCE * workspace.scale; + info.totalDelta.y += + yDirection * this.UNCONSTRAINED_MOVE_DISTANCE * workspace.scale; + + info.dragger.onDrag(new PointerEvent('pointermove'), info.totalDelta); + return true; + } +} + +/** + * Information about the currently in-progress move for a given + * Workspace. + */ +class DragMoveInfo extends MoveInfo { + /** Total distance moved, in screen pixels */ + totalDelta = new utils.Coordinate(0, 0); + + constructor( + public readonly block: Block, + public readonly dragger: IDragger, + ) { + super(block); + } +} diff --git a/src/navigation_controller.ts b/src/navigation_controller.ts index 5e53e73f..3260708f 100644 --- a/src/navigation_controller.ts +++ b/src/navigation_controller.ts @@ -34,7 +34,7 @@ import {ExitAction} from './actions/exit'; import {EnterAction} from './actions/enter'; import {DisconnectAction} from './actions/disconnect'; import {ActionMenu} from './actions/action_menu'; -import {Mover} from './actions/mover'; +import {DragMover as Mover} from './actions/drag_mover'; const KeyCodes = BlocklyUtils.KeyCodes; From 14482fac860425728cff68100d9b36e1e1877008 Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Thu, 13 Mar 2025 14:55:42 +0000 Subject: [PATCH 04/11] feat: Add Move Block (M) context menu item --- src/actions/mover.ts | 59 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 48 insertions(+), 11 deletions(-) diff --git a/src/actions/mover.ts b/src/actions/mover.ts index 9f120033..6ecb2cbf 100644 --- a/src/actions/mover.ts +++ b/src/actions/mover.ts @@ -5,7 +5,14 @@ */ import * as Constants from '../constants'; -import {ASTNode, Connection, ShortcutRegistry, common, utils} from 'blockly'; +import { + ASTNode, + Connection, + ContextMenuRegistry, + ShortcutRegistry, + common, + utils, +} from 'blockly'; import type {Block, BlockSvg, WorkspaceSvg} from 'blockly'; import {Navigation} from '../navigation'; @@ -121,6 +128,25 @@ export class Mover { }, ]; + menuItems: ContextMenuRegistry.RegistryItem[] = [ + { + displayText: 'Move Block (M)', + preconditionFn: (scope) => { + const workspace = scope.block?.workspace as WorkspaceSvg | null; + if (!workspace) return 'hidden'; + return this.canMove(workspace, scope.block) ? 'enabled' : 'disabled'; + }, + callback: (scope) => { + const workspace = scope.block?.workspace as WorkspaceSvg | null; + if (!workspace) return false; + this.startMove(workspace, scope.block); + }, + scopeType: ContextMenuRegistry.ScopeType.BLOCK, + id: 'move', + weight: 8.5, + }, + ]; + /** * Install the actions as both keyboard shortcuts and (where * applicable) context menu items. @@ -129,6 +155,9 @@ export class Mover { for (const shortcut of this.shortcuts) { ShortcutRegistry.registry.register(shortcut); } + for (const menuItem of this.menuItems) { + ContextMenuRegistry.registry.register(menuItem); + } } /** @@ -138,19 +167,25 @@ export class Mover { for (const shortcut of this.shortcuts) { ShortcutRegistry.registry.unregister(shortcut.name); } + for (const menuItem of this.menuItems) { + ContextMenuRegistry.registry.unregister(menuItem.id); + } } /** - * Returns true iff we are able to begin moving a block on the given - * workspace. + * Returns true iff we are able to begin moving the given block (or, + * if no block supplied, the block wich currently has focus) on the + * given workspace. * * @param workspace The workspace to move on. * @returns True iff we can beign a move. */ - canMove(workspace: WorkspaceSvg) { - const cursor = workspace?.getCursor(); - const curNode = cursor?.getCurNode(); - const block = curNode?.getSourceBlock(); + canMove(workspace: WorkspaceSvg, block?: Block) { + if (!block) { + const cursor = workspace?.getCursor(); + const curNode = cursor?.getCurNode(); + block = curNode?.getSourceBlock() ?? undefined; + } return !!( this.navigation.getState(workspace) === Constants.STATE.WORKSPACE && @@ -180,14 +215,16 @@ export class Mover { * @param workspace The workspace we might be moving on. * @returns True iff a move has successfully begun. */ - startMove(workspace: WorkspaceSvg) { + startMove(workspace: WorkspaceSvg, block?: Block) { const cursor = workspace?.getCursor(); - const curNode = cursor?.getCurNode(); - const block = curNode?.getSourceBlock() as BlockSvg | null; + if (!block) { + const curNode = cursor?.getCurNode(); + block = curNode?.getSourceBlock() ?? undefined; + } if (!cursor || !block) throw new Error('precondition failure'); // Select and focus block. - common.setSelected(block); + common.setSelected(block as BlockSvg); cursor.setCurNode(ASTNode.createBlockNode(block)!); // Additional implementation goes here. From 06d0b81e84b96027742484d80e22e0968ad857e8 Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Thu, 13 Mar 2025 16:29:34 +0000 Subject: [PATCH 05/11] fix: Allow ctrl+arrows to also perform unconstrained moves Alt+arrows causes browser navigation on Windows. :-( --- src/actions/drag_mover.ts | 2 +- src/actions/mover.ts | 20 ++++++++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/actions/drag_mover.ts b/src/actions/drag_mover.ts index 522b0d2c..2786db55 100644 --- a/src/actions/drag_mover.ts +++ b/src/actions/drag_mover.ts @@ -129,7 +129,7 @@ export class DragMover extends Mover { // Not yet implemented. Absorb keystroke to avoid moving cursor. alert(`Constrained movement not implemented. -Use alt+arrow (option+arrow on macOS) for unconstrained move. +Use ctrl+arrow or alt+arrow (option+arrow on macOS) for unconstrained move. Use enter to complete the move, or escape to abort.`); return true; } diff --git a/src/actions/mover.ts b/src/actions/mover.ts index 6ecb2cbf..e54d6647 100644 --- a/src/actions/mover.ts +++ b/src/actions/mover.ts @@ -106,25 +106,37 @@ export class Mover { name: 'Move left, unconstrained', preconditionFn: (workspace) => this.isMoving(workspace), callback: (workspace) => this.moveUnconstrained(workspace, -1, 0), - keyCodes: [createSerializedKey(KeyCodes.LEFT, [KeyCodes.ALT])], + keyCodes: [ + createSerializedKey(KeyCodes.LEFT, [KeyCodes.ALT]), + createSerializedKey(KeyCodes.LEFT, [KeyCodes.CTRL]), + ], }, { name: 'Move right, unconstraind', preconditionFn: (workspace) => this.isMoving(workspace), callback: (workspace) => this.moveUnconstrained(workspace, 1, 0), - keyCodes: [createSerializedKey(KeyCodes.RIGHT, [KeyCodes.ALT])], + keyCodes: [ + createSerializedKey(KeyCodes.RIGHT, [KeyCodes.ALT]), + createSerializedKey(KeyCodes.RIGHT, [KeyCodes.CTRL]), + ], }, { name: 'Move up unconstrained', preconditionFn: (workspace) => this.isMoving(workspace), callback: (workspace) => this.moveUnconstrained(workspace, 0, -1), - keyCodes: [createSerializedKey(KeyCodes.UP, [KeyCodes.ALT])], + keyCodes: [ + createSerializedKey(KeyCodes.UP, [KeyCodes.ALT]), + createSerializedKey(KeyCodes.UP, [KeyCodes.CTRL]), + ], }, { name: 'Move down, unconstrained', preconditionFn: (workspace) => this.isMoving(workspace), callback: (workspace) => this.moveUnconstrained(workspace, 0, 1), - keyCodes: [createSerializedKey(KeyCodes.DOWN, [KeyCodes.ALT])], + keyCodes: [ + createSerializedKey(KeyCodes.DOWN, [KeyCodes.ALT]), + createSerializedKey(KeyCodes.DOWN, [KeyCodes.CTRL]), + ], }, ]; From e2e85410eba961f3d0ba9daf990845145b131bf6 Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Thu, 13 Mar 2025 17:02:19 +0000 Subject: [PATCH 06/11] fix: Use better fake PointerEvents to prevent random deletions There was a bug that would often cause blocks to be deleted. This was because the dragger thought the block was on the toolbox, which is a delete area. Fix this by giving the dragger much better fake PointerEvents, so that it knows where the block actually is - good enough that you can move the block to the trash can and it will be deleted! --- src/actions/drag_mover.ts | 34 ++++++++++++++++++++++++++++------ src/actions/mover.ts | 2 +- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/actions/drag_mover.ts b/src/actions/drag_mover.ts index 2786db55..9846dab4 100644 --- a/src/actions/drag_mover.ts +++ b/src/actions/drag_mover.ts @@ -9,11 +9,12 @@ import { ASTNode, Connection, ShortcutRegistry, + WorkspaceSvg, common, registry, utils, } from 'blockly'; -import type {Block, BlockSvg, IDragger, WorkspaceSvg} from 'blockly'; +import type {Block, BlockSvg, IDragger} from 'blockly'; import {Mover, MoveInfo} from './mover'; import {Navigation} from '../navigation'; @@ -64,8 +65,10 @@ export class DragMover extends Mover { if (!DraggerClass) throw new Error('no Dragger registered'); const dragger = new DraggerClass(block, workspace); // Record that a move is in progress and start dragging. - this.moves.set(workspace, new DragMoveInfo(block, dragger)); - dragger.onDragStart(new PointerEvent('pointerdown')); + const info = new DragMoveInfo(block, dragger); + this.moves.set(workspace, info); + // Begin drag. + dragger.onDragStart(info.fakePointerEvent('pointerdown')); return true; } @@ -83,7 +86,7 @@ export class DragMover extends Mover { if (!info) throw new Error('no move info for workspace'); info.dragger.onDragEnd( - new PointerEvent('pointerup'), + info.fakePointerEvent('pointerup'), new utils.Coordinate(0, 0), ); @@ -107,7 +110,7 @@ export class DragMover extends Mover { // Monkey patch dragger to trigger call to draggable.revertDrag. (info.dragger as any).shouldReturnToStart = () => true; info.dragger.onDragEnd( - new PointerEvent('pointerup'), + info.fakePointerEvent('pointerup'), new utils.Coordinate(0, 0), ); @@ -157,7 +160,7 @@ Use enter to complete the move, or escape to abort.`); info.totalDelta.y += yDirection * this.UNCONSTRAINED_MOVE_DISTANCE * workspace.scale; - info.dragger.onDrag(new PointerEvent('pointermove'), info.totalDelta); + info.dragger.onDrag(info.fakePointerEvent('pointermove'), info.totalDelta); return true; } } @@ -176,4 +179,23 @@ class DragMoveInfo extends MoveInfo { ) { super(block); } + + /** Create fake pointer event for dragging. */ + fakePointerEvent(type: string): PointerEvent { + const workspace = this.block.workspace; + if (!(workspace instanceof WorkspaceSvg)) throw new TypeError(); + + const blockCoords = utils.svgMath.wsToScreenCoordinates( + workspace, + new utils.Coordinate( + this.startLocation.x + this.totalDelta.x, + this.startLocation.y + this.totalDelta.y, + ), + ); + return new PointerEvent(type, { + clientX: blockCoords.x, + clientY: blockCoords.y, + }); + } + } diff --git a/src/actions/mover.ts b/src/actions/mover.ts index e54d6647..88c83560 100644 --- a/src/actions/mover.ts +++ b/src/actions/mover.ts @@ -329,7 +329,7 @@ export class Mover { export class MoveInfo { public readonly parentNext: Connection | null; public readonly parentInput: Connection | null; - public readonly startLocation: utils.Coordinate | null; + public readonly startLocation: utils.Coordinate; constructor(public readonly block: Block) { this.parentNext = block.previousConnection?.targetConnection ?? null; From 0582351b0a8a343a57f390c2fe2b7f36028361c1 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Fri, 28 Mar 2025 17:02:44 -0700 Subject: [PATCH 07/11] chore: address feedback from #317 --- src/actions/drag_mover.ts | 5 +-- src/actions/mover.ts | 61 +++++++++++++++++------------------- src/navigation_controller.ts | 2 +- 3 files changed, 31 insertions(+), 37 deletions(-) diff --git a/src/actions/drag_mover.ts b/src/actions/drag_mover.ts index 9846dab4..caaae3de 100644 --- a/src/actions/drag_mover.ts +++ b/src/actions/drag_mover.ts @@ -49,8 +49,7 @@ export class DragMover extends Mover { */ override startMove(workspace: WorkspaceSvg) { const cursor = workspace?.getCursor(); - const curNode = cursor?.getCurNode(); - const block = curNode?.getSourceBlock() as BlockSvg | null; + const block = this.getCurrentBlock(workspace); if (!cursor || !block) throw new Error('precondition failure'); // Select and focus block. @@ -81,7 +80,6 @@ export class DragMover extends Mover { * @returns True iff move successfully finished. */ override finishMove(workspace: WorkspaceSvg) { - if (!workspace) return false; const info = this.moves.get(workspace); if (!info) throw new Error('no move info for workspace'); @@ -103,7 +101,6 @@ export class DragMover extends Mover { * @returns True iff move successfully aborted. */ override abortMove(workspace: WorkspaceSvg) { - if (!workspace) return false; const info = this.moves.get(workspace); if (!info) throw new Error('no move info for workspace'); diff --git a/src/actions/mover.ts b/src/actions/mover.ts index 88c83560..f7ad14eb 100644 --- a/src/actions/mover.ts +++ b/src/actions/mover.ts @@ -25,11 +25,6 @@ const createSerializedKey = ShortcutRegistry.registry.createSerializedKey.bind( * Actions for moving blocks with keyboard shortcuts. */ export class Mover { - /** - * Function provided by the navigation controller to say whether editing - * is allowed. - */ - protected canCurrentlyEdit: (ws: WorkspaceSvg) => boolean; /** * Map of moves in progress. @@ -42,10 +37,8 @@ export class Mover { constructor( protected navigation: Navigation, - canEdit: (ws: WorkspaceSvg) => boolean, - ) { - this.canCurrentlyEdit = canEdit; - } + protected canEdit: (ws: WorkspaceSvg) => boolean, + ) {} private shortcuts: ShortcutRegistry.KeyboardShortcut[] = [ // Begin and end move. @@ -146,12 +139,12 @@ export class Mover { preconditionFn: (scope) => { const workspace = scope.block?.workspace as WorkspaceSvg | null; if (!workspace) return 'hidden'; - return this.canMove(workspace, scope.block) ? 'enabled' : 'disabled'; + return this.canMove(workspace) ? 'enabled' : 'disabled'; }, callback: (scope) => { const workspace = scope.block?.workspace as WorkspaceSvg | null; if (!workspace) return false; - this.startMove(workspace, scope.block); + this.startMove(workspace); }, scopeType: ContextMenuRegistry.ScopeType.BLOCK, id: 'move', @@ -185,23 +178,18 @@ export class Mover { } /** - * Returns true iff we are able to begin moving the given block (or, - * if no block supplied, the block wich currently has focus) on the - * given workspace. + * Returns true iff we are able to begin moving the block which + * currently has focus on the given workspace. * * @param workspace The workspace to move on. - * @returns True iff we can beign a move. + * @returns True iff we can begin a move. */ - canMove(workspace: WorkspaceSvg, block?: Block) { - if (!block) { - const cursor = workspace?.getCursor(); - const curNode = cursor?.getCurNode(); - block = curNode?.getSourceBlock() ?? undefined; - } + canMove(workspace: WorkspaceSvg) { + const block = this.getCurrentBlock(workspace); return !!( this.navigation.getState(workspace) === Constants.STATE.WORKSPACE && - this.canCurrentlyEdit(workspace) && + this.canEdit(workspace) && !this.moves.has(workspace) && // No move in progress. block?.isMovable() ); @@ -215,7 +203,7 @@ export class Mover { * @returns True iff we are moving. */ isMoving(workspace: WorkspaceSvg) { - return this.canCurrentlyEdit(workspace) && this.moves.has(workspace); + return this.canEdit(workspace) && this.moves.has(workspace); } /** @@ -227,17 +215,14 @@ export class Mover { * @param workspace The workspace we might be moving on. * @returns True iff a move has successfully begun. */ - startMove(workspace: WorkspaceSvg, block?: Block) { + startMove(workspace: WorkspaceSvg) { const cursor = workspace?.getCursor(); - if (!block) { - const curNode = cursor?.getCurNode(); - block = curNode?.getSourceBlock() ?? undefined; - } + const block = this.getCurrentBlock(workspace); if (!cursor || !block) throw new Error('precondition failure'); // Select and focus block. - common.setSelected(block as BlockSvg); - cursor.setCurNode(ASTNode.createBlockNode(block)!); + common.setSelected(block); + cursor.setCurNode(ASTNode.createBlockNode(block)); // Additional implementation goes here. console.log('startMove'); @@ -255,7 +240,6 @@ export class Mover { * @returns True iff move successfully finished. */ finishMove(workspace: WorkspaceSvg) { - if (!workspace) return false; const info = this.moves.get(workspace); if (!info) throw new Error('no move info for workspace'); @@ -275,7 +259,6 @@ export class Mover { * @returns True iff move successfully aborted. */ abortMove(workspace: WorkspaceSvg) { - if (!workspace) return false; const info = this.moves.get(workspace); if (!info) throw new Error('no move info for workspace'); @@ -320,6 +303,20 @@ export class Mover { // Not yet implemented. Absorb keystroke to avoid moving cursor. return true; } + + /** + * Get the source block for the cursor location, or undefined if no + * source block can be found. + * + * @param workspace The workspace to inspect for a cursor. + * @returns The source block, or undefined if no appropriate block + * could be found. + */ + protected getCurrentBlock(workspace: WorkspaceSvg): BlockSvg | undefined { + const cursor = workspace?.getCursor(); + const curNode = cursor?.getCurNode(); + return (curNode?.getSourceBlock() as BlockSvg) ?? undefined; + } } /** diff --git a/src/navigation_controller.ts b/src/navigation_controller.ts index 3260708f..546a4045 100644 --- a/src/navigation_controller.ts +++ b/src/navigation_controller.ts @@ -356,7 +356,7 @@ export class NavigationController { * Removes all the keyboard navigation shortcuts. */ dispose() { - this.mover.install(); + this.mover.uninstall(); this.deleteAction.uninstall(); this.editAction.uninstall(); this.insertAction.uninstall(); From 575aa322ba2785a314c40cd053c20736ea9cdfe4 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Fri, 28 Mar 2025 17:09:30 -0700 Subject: [PATCH 08/11] chore: lint and format --- src/actions/drag_mover.ts | 50 ++++++++++++++++++--------------------- src/actions/mover.ts | 9 ++++--- 2 files changed, 27 insertions(+), 32 deletions(-) diff --git a/src/actions/drag_mover.ts b/src/actions/drag_mover.ts index caaae3de..9607f016 100644 --- a/src/actions/drag_mover.ts +++ b/src/actions/drag_mover.ts @@ -4,31 +4,21 @@ * SPDX-License-Identifier: Apache-2.0 */ -import * as Constants from '../constants'; -import { - ASTNode, - Connection, - ShortcutRegistry, - WorkspaceSvg, - common, - registry, - utils, -} from 'blockly'; -import type {Block, BlockSvg, IDragger} from 'blockly'; +import {ASTNode, WorkspaceSvg, common, registry, utils} from 'blockly'; +import type {Block, IDragger} from 'blockly'; import {Mover, MoveInfo} from './mover'; -import {Navigation} from '../navigation'; + +/** + * The distance to move an item, in workspace coordinates, when + * making an unconstrained move. + */ +const UNCONSTRAINED_MOVE_DISTANCE = 20; /** * An experimental implementation of Mover that uses a dragger to - * perform unconstraind movment. + * perform unconstraind movement. */ export class DragMover extends Mover { - /** - * The distance to move an item, in workspace coordinates, when - * making an unconstrained move. - */ - UNCONSTRAINED_MOVE_DISTANCE = 20; - /** * Map of moves in progress. * @@ -54,7 +44,7 @@ export class DragMover extends Mover { // Select and focus block. common.setSelected(block); - cursor.setCurNode(ASTNode.createBlockNode(block)!); + cursor.setCurNode(ASTNode.createBlockNode(block)); // Begin dragging block. const DraggerClass = registry.getClassFromOptions( registry.Type.BLOCK_DRAGGER, @@ -105,6 +95,7 @@ export class DragMover extends Mover { if (!info) throw new Error('no move info for workspace'); // Monkey patch dragger to trigger call to draggable.revertDrag. + // eslint-disable-next-line @typescript-eslint/no-explicit-any (info.dragger as any).shouldReturnToStart = () => true; info.dragger.onDragEnd( info.fakePointerEvent('pointerup'), @@ -153,9 +144,9 @@ Use enter to complete the move, or escape to abort.`); if (!info) throw new Error('no move info for workspace'); info.totalDelta.x += - xDirection * this.UNCONSTRAINED_MOVE_DISTANCE * workspace.scale; + xDirection * UNCONSTRAINED_MOVE_DISTANCE * workspace.scale; info.totalDelta.y += - yDirection * this.UNCONSTRAINED_MOVE_DISTANCE * workspace.scale; + yDirection * UNCONSTRAINED_MOVE_DISTANCE * workspace.scale; info.dragger.onDrag(info.fakePointerEvent('pointermove'), info.totalDelta); return true; @@ -171,17 +162,23 @@ class DragMoveInfo extends MoveInfo { totalDelta = new utils.Coordinate(0, 0); constructor( - public readonly block: Block, - public readonly dragger: IDragger, + readonly block: Block, + readonly dragger: IDragger, ) { super(block); } - /** Create fake pointer event for dragging. */ + /** + * Create a fake pointer event for dragging. + * + * @param type Which type of pointer event to create. + * @returns A synthetic PointerEvent that can be consumed by Blockly's + * dragging code. + */ fakePointerEvent(type: string): PointerEvent { const workspace = this.block.workspace; if (!(workspace instanceof WorkspaceSvg)) throw new TypeError(); - + const blockCoords = utils.svgMath.wsToScreenCoordinates( workspace, new utils.Coordinate( @@ -194,5 +191,4 @@ class DragMoveInfo extends MoveInfo { clientY: blockCoords.y, }); } - } diff --git a/src/actions/mover.ts b/src/actions/mover.ts index f7ad14eb..8f5d32dd 100644 --- a/src/actions/mover.ts +++ b/src/actions/mover.ts @@ -25,7 +25,6 @@ const createSerializedKey = ShortcutRegistry.registry.createSerializedKey.bind( * Actions for moving blocks with keyboard shortcuts. */ export class Mover { - /** * Map of moves in progress. * @@ -324,11 +323,11 @@ export class Mover { * Workspace. */ export class MoveInfo { - public readonly parentNext: Connection | null; - public readonly parentInput: Connection | null; - public readonly startLocation: utils.Coordinate; + readonly parentNext: Connection | null; + readonly parentInput: Connection | null; + readonly startLocation: utils.Coordinate; - constructor(public readonly block: Block) { + constructor(readonly block: Block) { this.parentNext = block.previousConnection?.targetConnection ?? null; this.parentInput = block.outputConnection?.targetConnection ?? null; this.startLocation = block.getRelativeToSurfaceXY(); From 94f3330a7738b15014a6cb27ebc213ec72f0fa57 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Fri, 28 Mar 2025 17:14:32 -0700 Subject: [PATCH 09/11] fix: remove allowCollision for m --- src/actions/mover.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/actions/mover.ts b/src/actions/mover.ts index 8f5d32dd..f2143348 100644 --- a/src/actions/mover.ts +++ b/src/actions/mover.ts @@ -46,7 +46,6 @@ export class Mover { preconditionFn: (workspace) => this.canMove(workspace), callback: (workspace) => this.startMove(workspace), keyCodes: [KeyCodes.M], - allowCollision: true, // TODO: remove once #309 has been merged. }, { name: 'Finish move', From 31e86169e936019c9c754a9775fffc89c10b7361 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Mon, 31 Mar 2025 08:11:35 -0700 Subject: [PATCH 10/11] fix: #356 by hiding the connection preview before reverting a drag --- src/actions/drag_mover.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/actions/drag_mover.ts b/src/actions/drag_mover.ts index 9607f016..ffcb1e6e 100644 --- a/src/actions/drag_mover.ts +++ b/src/actions/drag_mover.ts @@ -4,7 +4,14 @@ * SPDX-License-Identifier: Apache-2.0 */ -import {ASTNode, WorkspaceSvg, common, registry, utils} from 'blockly'; +import { + ASTNode, + BlockSvg, + WorkspaceSvg, + common, + registry, + utils, +} from 'blockly'; import type {Block, IDragger} from 'blockly'; import {Mover, MoveInfo} from './mover'; @@ -97,6 +104,11 @@ export class DragMover extends Mover { // Monkey patch dragger to trigger call to draggable.revertDrag. // eslint-disable-next-line @typescript-eslint/no-explicit-any (info.dragger as any).shouldReturnToStart = () => true; + const blockSvg = info.block as BlockSvg; + + // Explicitly call `hidePreview` because it is not called in revertDrag. + // @ts-expect-error Access to private property dragStrategy. + blockSvg.dragStrategy.connectionPreviewer.hidePreview(); info.dragger.onDragEnd( info.fakePointerEvent('pointerup'), new utils.Coordinate(0, 0), From f159cdad1f32bf0b5638b7063a10653feef89a57 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Mon, 31 Mar 2025 09:15:59 -0700 Subject: [PATCH 11/11] fix: spelling --- src/actions/drag_mover.ts | 2 +- src/actions/mover.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/actions/drag_mover.ts b/src/actions/drag_mover.ts index ffcb1e6e..f0a34046 100644 --- a/src/actions/drag_mover.ts +++ b/src/actions/drag_mover.ts @@ -23,7 +23,7 @@ const UNCONSTRAINED_MOVE_DISTANCE = 20; /** * An experimental implementation of Mover that uses a dragger to - * perform unconstraind movement. + * perform unconstrained movement. */ export class DragMover extends Mover { /** diff --git a/src/actions/mover.ts b/src/actions/mover.ts index f2143348..63ec0b7f 100644 --- a/src/actions/mover.ts +++ b/src/actions/mover.ts @@ -71,7 +71,7 @@ export class Mover { allowCollision: true, }, { - name: 'Move right unconstraind', + name: 'Move right unconstrained', preconditionFn: (workspace) => this.isMoving(workspace), callback: (workspace) => this.moveConstrained(workspace /* , ... */), keyCodes: [KeyCodes.RIGHT], @@ -103,7 +103,7 @@ export class Mover { ], }, { - name: 'Move right, unconstraind', + name: 'Move right, unconstrained', preconditionFn: (workspace) => this.isMoving(workspace), callback: (workspace) => this.moveUnconstrained(workspace, 1, 0), keyCodes: [