diff --git a/src/actions/enter.ts b/src/actions/enter.ts index 72f6601b..d8016483 100644 --- a/src/actions/enter.ts +++ b/src/actions/enter.ts @@ -128,30 +128,28 @@ export class EnterAction { */ private insertFromFlyout(workspace: WorkspaceSvg) { workspace.setResizesEnabled(false); - Events.setGroup(true); + // Create a new event group or append to the existing group. + const existingGroup = Events.getGroup(); + if (!existingGroup) { + Events.setGroup(true); + } const stationaryNode = this.navigation.getStationaryNode(workspace); const newBlock = this.createNewBlock(workspace); if (!newBlock) return; - if (stationaryNode) { - if (!this.navigation.tryToConnectBlock(stationaryNode, newBlock)) { - console.warn( - 'Something went wrong while inserting a block from the flyout.', - ); - } - } - + const insertStartPoint = stationaryNode + ? this.navigation.findInsertStartPoint(stationaryNode, newBlock) + : null; if (workspace.getTopBlocks().includes(newBlock)) { this.positionNewTopLevelBlock(workspace, newBlock); } - Events.setGroup(false); workspace.setResizesEnabled(true); this.navigation.focusWorkspace(workspace); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion workspace.getCursor()?.setCurNode(ASTNode.createBlockNode(newBlock)!); - this.mover.startMove(workspace); + this.mover.startMove(workspace, newBlock, insertStartPoint); const isStartBlock = !newBlock.outputConnection && diff --git a/src/actions/move.ts b/src/actions/move.ts index d9c0e413..2734c89e 100644 --- a/src/actions/move.ts +++ b/src/actions/move.ts @@ -5,6 +5,7 @@ */ import { + BlockSvg, ContextMenuRegistry, ShortcutRegistry, utils, @@ -28,8 +29,16 @@ export class MoveActions { // Begin and end move. { name: 'Start move', - preconditionFn: (workspace) => this.mover.canMove(workspace), - callback: (workspace) => this.mover.startMove(workspace), + preconditionFn: (workspace) => { + const startBlock = this.getCurrentBlock(workspace); + return !!startBlock && this.mover.canMove(workspace, startBlock); + }, + callback: (workspace) => { + const startBlock = this.getCurrentBlock(workspace); + return ( + !!startBlock && this.mover.startMove(workspace, startBlock, null) + ); + }, keyCodes: [KeyCodes.M], }, { @@ -131,12 +140,19 @@ export class MoveActions { const workspace = scope.block?.workspace as WorkspaceSvg | null; if (!workspace || menuOpenEvent instanceof PointerEvent) return 'hidden'; - return this.mover.canMove(workspace) ? 'enabled' : 'disabled'; + + const startBlock = this.getCurrentBlock(workspace); + return !!startBlock && this.mover.canMove(workspace, startBlock) + ? 'enabled' + : 'disabled'; }, callback: (scope) => { const workspace = scope.block?.workspace as WorkspaceSvg | null; if (!workspace) return false; - this.mover.startMove(workspace); + const startBlock = this.getCurrentBlock(workspace); + return ( + !!startBlock && this.mover.startMove(workspace, startBlock, null) + ); }, scopeType: ContextMenuRegistry.ScopeType.BLOCK, id: 'move', @@ -168,4 +184,30 @@ export class MoveActions { ContextMenuRegistry.registry.unregister(menuItem.id); } } + + /** + * Get the source block for the cursor location, or undefined if no + * source block can be found. + * If the cursor is on a shadow block, walks up the tree until it finds + * a non-shadow block to drag. + * + * @param workspace The workspace to inspect for a cursor. + * @returns The source block, or undefined if no appropriate block + * could be found. + */ + getCurrentBlock(workspace: WorkspaceSvg): BlockSvg | undefined { + const curNode = workspace?.getCursor()?.getCurNode(); + let block = curNode?.getSourceBlock(); + if (!block) return undefined; + while (block?.isShadow()) { + block = block.getParent(); + if (!block) { + throw new Error( + 'Tried to drag a shadow block with no parent. ' + + 'Shadow blocks should always have parents.', + ); + } + } + return block as BlockSvg; + } } diff --git a/src/actions/mover.ts b/src/actions/mover.ts index 8c87bcad..922585cd 100644 --- a/src/actions/mover.ts +++ b/src/actions/mover.ts @@ -4,11 +4,17 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type {BlockSvg, IDragger, IDragStrategy, Gesture} from 'blockly'; +import type { + BlockSvg, + IDragger, + IDragStrategy, + Gesture, + RenderedConnection, +} from 'blockly'; import { ASTNode, - common, Connection, + dragging, registry, utils, WorkspaceSvg, @@ -63,11 +69,10 @@ export class Mover { * currently has focus on the given workspace. * * @param workspace The workspace to move on. + * @param block The block to try to drag. * @returns True iff we can begin a move. */ - canMove(workspace: WorkspaceSvg) { - const block = this.getCurrentBlock(workspace); - + canMove(workspace: WorkspaceSvg, block: BlockSvg) { return !!( this.navigation.getState(workspace) === Constants.STATE.WORKSPACE && this.navigation.canCurrentlyEdit(workspace) && @@ -96,19 +101,18 @@ export class Mover { * Should only be called if canMove has returned true. * * @param workspace The workspace we might be moving on. + * @param block The block to start dragging. + * @param insertStartPoint Where to insert the block, or null if the block + * already existed. * @returns True iff a move has successfully begun. */ - startMove(workspace: WorkspaceSvg) { - const cursor = workspace?.getCursor(); - const block = this.getCurrentBlock(workspace); - if (!cursor || !block) throw new Error('precondition failure'); - - // Select and focus block. - common.setSelected(block); - cursor.setCurNode(ASTNode.createBlockNode(block)); - + startMove( + workspace: WorkspaceSvg, + block: BlockSvg, + insertStartPoint: RenderedConnection | null, + ) { this.patchWorkspace(workspace); - this.patchDragStrategy(block); + this.patchDragStrategy(block, insertStartPoint); // Begin dragging block. const DraggerClass = registry.getClassFromOptions( registry.Type.BLOCK_DRAGGER, @@ -167,19 +171,34 @@ export class Mover { 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. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (info.dragger as any).shouldReturnToStart = () => true; - const blockSvg = info.block; + const dragStrategy = info.block.getDragStrategy() as KeyboardDragStrategy; + this.patchDragger( + info.dragger as dragging.Dragger, + dragStrategy.isNewBlock, + ); // Explicitly call `hidePreview` because it is not called in revertDrag. - // @ts-expect-error Access to private property dragStrategy. - blockSvg.dragStrategy.connectionPreviewer.hidePreview(); + // @ts-expect-error Access to private property connectionPreviewer. + dragStrategy.connectionPreviewer.hidePreview(); + + // Save the position so we can put the cursor in a reasonable spot. + // @ts-expect-error Access to private property connectionCandidate. + const target = dragStrategy.connectionCandidate?.neighbour; + + // Prevent the stragegy connecting the block so we just delete one block. + // @ts-expect-error Access to private property connectionCandidate. + dragStrategy.connectionCandidate = null; + info.dragger.onDragEnd( info.fakePointerEvent('pointerup'), - new utils.Coordinate(0, 0), + info.startLocation, ); + if (dragStrategy.isNewBlock && target) { + const newNode = ASTNode.createConnectionNode(target); + if (newNode) workspace.getCursor()?.setCurNode(newNode); + } + this.unpatchWorkspace(workspace); this.unpatchDragStrategy(info.block); this.moves.delete(workspace); @@ -233,32 +252,6 @@ export class Mover { return true; } - /** - * Get the source block for the cursor location, or undefined if no - * source block can be found. - * If the cursor is on a shadow block, walks up the tree until it finds - * a non-shadow block to drag. - * - * @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 curNode = workspace?.getCursor()?.getCurNode(); - let block = curNode?.getSourceBlock(); - if (!block) return undefined; - while (block && block.isShadow()) { - block = block.getParent(); - if (!block) { - throw new Error( - 'Tried to drag a shadow block with no parent. ' + - 'Shadow blocks should always have parents.', - ); - } - } - return block as BlockSvg; - } - /** * Monkeypatch over workspace functions to consider keyboard drags as * well as mouse/pointer drags. @@ -297,15 +290,21 @@ export class Mover { (workspace as any).getGesture = this.oldGetGesture; } } + /** * Monkeypatch: replace the block's drag strategy and cache the old value. * * @param block The block to patch. + * @param insertStartPoint Where to insert the block, or null if the block + * already existed. */ - private patchDragStrategy(block: BlockSvg) { + private patchDragStrategy( + block: BlockSvg, + insertStartPoint: RenderedConnection | null, + ) { // @ts-expect-error block.dragStrategy is private. this.oldDragStrategy = block.dragStrategy; - block.setDragStrategy(new KeyboardDragStrategy(block)); + block.setDragStrategy(new KeyboardDragStrategy(block, insertStartPoint)); } /** @@ -328,7 +327,7 @@ export class Mover { * the workspace's viewport. */ private scrollCurrentBlockIntoView(workspace: WorkspaceSvg, padding = 10) { - const blockToView = this.getCurrentBlock(workspace); + const blockToView = this.moves.get(workspace)?.block; if (blockToView) { workspace.scrollBoundsIntoView( blockToView.getBoundingRectangleWithoutChildren(), @@ -336,6 +335,26 @@ export class Mover { ); } } + + /** + * Monkeypatch: override either wouldDeleteDraggable or shouldReturnToStart, + * based on whether this was an insertion of a new block or a movement of + * an existing block. + * + * @param dragger The dragger to patch. + * @param isNewBlock Whether the moving block was created for this action. + */ + private patchDragger(dragger: dragging.Dragger, isNewBlock: boolean) { + if (isNewBlock) { + // Monkey patch dragger to trigger delete. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (dragger as any).wouldDeleteDraggable = () => true; + } else { + // Monkey patch dragger to trigger call to draggable.revertDrag. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (dragger as any).shouldReturnToStart = () => true; + } + } } /** diff --git a/src/keyboard_drag_strategy.ts b/src/keyboard_drag_strategy.ts index 9134c349..dd15c9a2 100644 --- a/src/keyboard_drag_strategy.ts +++ b/src/keyboard_drag_strategy.ts @@ -9,6 +9,7 @@ import { BlockSvg, ConnectionType, RenderedConnection, + LineCursor, dragging, utils, } from 'blockly'; @@ -35,7 +36,22 @@ export class KeyboardDragStrategy extends dragging.BlockDragStrategy { /** Where a constrained movement should start when traversing the tree. */ private searchNode: ASTNode | null = null; + private cursor: LineCursor | null; + + isNewBlock: boolean; + + constructor( + private block: BlockSvg, + private insertStartPoint: RenderedConnection | null, + ) { + super(block); + this.isNewBlock = !!this.insertStartPoint; + this.cursor = block.workspace.getCursor(); + } + override startDrag(e?: PointerEvent) { + if (!this.cursor) throw new Error('precondition failure'); + this.cursor.setCurNode(ASTNode.createBlockNode(this.block)); super.startDrag(e); // Set position of the dragging block, so that it doesn't pop // to the top left of the workspace. @@ -63,7 +79,6 @@ export class KeyboardDragStrategy extends dragging.BlockDragStrategy { if (this.isConstrainedMovement()) { // Position the moving block down and slightly to the right of the // target connection. - // @ts-expect-error block is private. this.block.moveDuringDrag( new utils.Coordinate(neighbour.x + 10, neighbour.y + 10), ); @@ -148,8 +163,7 @@ export class KeyboardDragStrategy extends dragging.BlockDragStrategy { draggingBlock: BlockSvg, localConns: RenderedConnection[], ): ConnectionCandidate | null { - const cursor = draggingBlock.workspace.getCursor(); - if (!cursor) return null; + if (!this.cursor) throw new Error('precondition failure'); // Helper function for traversal. function isConnection(node: ASTNode | null): boolean { @@ -162,9 +176,9 @@ export class KeyboardDragStrategy extends dragging.BlockDragStrategy { const dir = this.currentDragDirection; while (potential && !candidateConnection) { if (dir === Direction.Up || dir === Direction.Left) { - potential = cursor.getPreviousNode(potential, isConnection, true); + potential = this.cursor.getPreviousNode(potential, isConnection, true); } else if (dir === Direction.Down || dir === Direction.Right) { - potential = cursor.getNextNode(potential, isConnection, true); + potential = this.cursor.getNextNode(potential, isConnection, true); } localConns.forEach((conn: RenderedConnection) => { @@ -225,7 +239,6 @@ export class KeyboardDragStrategy extends dragging.BlockDragStrategy { // @ts-expect-error connectionCandidate is private const candidate = this.connectionCandidate as ConnectionCandidate; if (!candidate || !previewer) return; - // @ts-expect-error block is private const block = this.block; // This is essentially a copy of the second half of updateConnectionPreview @@ -269,21 +282,19 @@ export class KeyboardDragStrategy extends dragging.BlockDragStrategy { */ private createInitialCandidate(): ConnectionCandidate | null { // @ts-expect-error startParentConn is private. - const neighbour = this.startParentConn; + const neighbour = this.insertStartPoint ?? this.startParentConn; if (neighbour) { this.searchNode = ASTNode.createConnectionNode(neighbour); switch (neighbour.type) { case ConnectionType.INPUT_VALUE: return { neighbour: neighbour, - // @ts-expect-error block is private. local: this.block.outputConnection, distance: 0, }; case ConnectionType.NEXT_STATEMENT: return { neighbour: neighbour, - // @ts-expect-error block is private. local: this.block.previousConnection, distance: 0, }; diff --git a/src/navigation.ts b/src/navigation.ts index c9b1f83b..5fdf38de 100644 --- a/src/navigation.ts +++ b/src/navigation.ts @@ -670,17 +670,19 @@ export class Navigation { * @returns True if the key was handled; false if something went * wrong. */ - tryToConnectBlock( + findInsertStartPoint( stationaryNode: Blockly.ASTNode, movingBlock: Blockly.BlockSvg, - ): boolean { + ): Blockly.RenderedConnection | null { const stationaryType = stationaryNode.getType(); const stationaryLoc = stationaryNode.getLocation(); + const movingHasOutput = !!movingBlock.outputConnection; if (stationaryNode.getType() === Blockly.ASTNode.types.FIELD) { + // Can't connect a block to a field, so try going up to the source block. const sourceBlock = stationaryNode.getSourceBlock(); - if (!sourceBlock) return false; - return this.tryToConnectBlock( + if (!sourceBlock) return null; + return this.findInsertStartPoint( Blockly.ASTNode.createBlockNode(sourceBlock), movingBlock, ); @@ -691,12 +693,12 @@ export class Navigation { // Move to the block if we're trying to insert a statement block into // a value connection. if ( - !movingBlock.outputConnection && + !movingHasOutput && stationaryAsConnection.type === Blockly.ConnectionType.INPUT_VALUE ) { const sourceBlock = stationaryNode.getSourceBlock(); - if (!sourceBlock) return false; - return this.tryToConnectBlock( + if (!sourceBlock) return null; + return this.findInsertStartPoint( Blockly.ASTNode.createBlockNode(sourceBlock), movingBlock, ); @@ -704,14 +706,14 @@ export class Navigation { // Connect the moving block to the stationary connection using // the most plausible connection on the moving block. - return this.insertBlock(movingBlock, stationaryAsConnection); + return stationaryAsConnection; } else if (stationaryType === Blockly.ASTNode.types.WORKSPACE) { - return this.moveBlockToWorkspace(movingBlock, stationaryNode); + return null; } else if (stationaryType === Blockly.ASTNode.types.BLOCK) { const stationaryBlock = stationaryLoc as Blockly.BlockSvg; // 1. Connect blocks to first compatible input - const inputType = movingBlock.outputConnection + const inputType = movingHasOutput ? Blockly.inputs.inputTypes.VALUE : Blockly.inputs.inputTypes.STATEMENT; const compatibleInputs = stationaryBlock.inputList.filter( @@ -726,36 +728,60 @@ export class Navigation { connection = connection.targetBlock()!.nextConnection!; } } - return this.insertBlock( - movingBlock, - connection as Blockly.RenderedConnection, - ); + return connection as Blockly.RenderedConnection; } // 2. Connect statement blocks to next connection. - if (stationaryBlock.nextConnection && !movingBlock.outputConnection) { - return this.insertBlock(movingBlock, stationaryBlock.nextConnection); + if (stationaryBlock.nextConnection && !movingHasOutput) { + return stationaryBlock.nextConnection; } // 3. Output connection. This will wrap around or displace. if (stationaryBlock.outputConnection) { - // Move to parent if we're trying to insert a statement block. - if ( - !movingBlock.outputConnection && + // Try to wrap. + const target = stationaryBlock.outputConnection.targetConnection; + if (movingHasOutput && target) { + const sourceNode = Blockly.ASTNode.createConnectionNode(target); + if (sourceNode) { + return this.findInsertStartPoint(sourceNode, movingBlock); + } + } else if ( + !movingHasOutput && stationaryNode.getType() === Blockly.ASTNode.types.BLOCK ) { + // Move to parent if we're trying to insert a statement block. const parent = stationaryNode.getSourceBlock()?.getParent(); - if (!parent) return false; - return this.tryToConnectBlock( + if (!parent) return null; + return this.findInsertStartPoint( Blockly.ASTNode.createBlockNode(parent), movingBlock, ); } - return this.insertBlock(movingBlock, stationaryBlock.outputConnection); + return stationaryBlock.outputConnection; } } - this.warn(`Unexpected case in tryToConnectBlock ${stationaryType}.`); - return false; + this.warn(`Unexpected case in findInsertStartPoint ${stationaryType}.`); + return null; + } + + /** + * Tries to intelligently connect the blocks or connections + * represented by the given nodes, based on node types and locations. + * + * @param stationaryNode The first node to connect. + * @param movingBlock The block we're moving. + * @returns True if the connection was successful, false otherwise. + */ + tryToConnectBlock( + stationaryNode: Blockly.ASTNode, + movingBlock: Blockly.BlockSvg, + ): boolean { + const destConnection = this.findInsertStartPoint( + stationaryNode, + movingBlock, + ); + if (!destConnection) return false; + return this.insertBlock(movingBlock, destConnection); } /**