Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
95 changes: 85 additions & 10 deletions packages/blockly/core/dragging/block_drag_strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {ConnectionType} from '../connection_type.js';
import type {BlockMove} from '../events/events_block_move.js';
import {EventType} from '../events/type.js';
import * as eventUtils from '../events/utils.js';
import {FocusManager} from '../focus_manager.js';
import {showUnconstrainedMoveHint} from '../hints.js';
import type {IBubble} from '../interfaces/i_bubble.js';
import type {IConnectionPreviewer} from '../interfaces/i_connection_previewer.js';
Expand Down Expand Up @@ -243,29 +244,25 @@ export class BlockDragStrategy implements IDragStrategy {
}

this.block.setDragging(true);
this.workspace.getLayerManager()?.moveToDragLayer(this.block);
this.getVisibleBubbles(this.block).forEach((bubble) => {
this.workspace.getLayerManager()?.moveToDragLayer(bubble, false);
});

// For keyboard-driven moves, cache a list of valid connection points for
// use in constrained moved mode.
if (e instanceof KeyboardEvent) {
this.cacheAllConnectionPairs();

// Scooch the block to be offset from the connection preview indicator.
this.block.moveDuringDrag(this.startLoc);
const neighbour = this.updateConnectionPreview(
this.block,
new Coordinate(0, 0),
);
if (neighbour) {
let offset: Coordinate;
if (neighbour.type === ConnectionType.PREVIOUS_STATEMENT) {
const origin = this.block.getRelativeToSurfaceXY();
offset = new Coordinate(
origin.x + this.BLOCK_CONNECTION_OFFSET,
origin.y - this.BLOCK_CONNECTION_OFFSET,
neighbour.x,
neighbour.y -
this.block.getHeightWidth().height -
this.BLOCK_CONNECTION_OFFSET,
);
} else {
offset = new Coordinate(
Expand All @@ -275,8 +272,15 @@ export class BlockDragStrategy implements IDragStrategy {
}
this.block.moveDuringDrag(offset);
}
} else {
this.block.moveDuringDrag(this.startLoc);
}

this.workspace.getLayerManager()?.moveToDragLayer(this.block);
this.getVisibleBubbles(this.block).forEach((bubble) => {
this.workspace.getLayerManager()?.moveToDragLayer(bubble, false);
});

this.announceMove(true);
return this.block;
}
Expand Down Expand Up @@ -388,7 +392,7 @@ export class BlockDragStrategy implements IDragStrategy {
* the initial connection pair is also used as the first connection candidate.
*/
private storeInitialConnections(healStack: boolean) {
// Prioritze the block's parent connection (output or previous) if one exists.
// Prioritize the block's parent connection (output or previous) if one exists.
Comment thread
gonfunko marked this conversation as resolved.
let localParentConn: RenderedConnection | null = null;
let parentTargetConn: RenderedConnection | null = null;

Expand Down Expand Up @@ -536,7 +540,8 @@ export class BlockDragStrategy implements IDragStrategy {
delta: Coordinate,
): RenderedConnection | undefined {
const currCandidate = this.connectionCandidate;
const newCandidate = this.getConnectionCandidate(delta);
const newCandidate =
this.getInitialCandidate() ?? this.getConnectionCandidate(delta);

if (!newCandidate) {
// Position above or below the first/last block.
Expand Down Expand Up @@ -1049,4 +1054,74 @@ export class BlockDragStrategy implements IDragStrategy {

return connections as RenderedConnection[];
}

/**
* Returns a connection candidate to move the dragged block to at the start of
* a drag. If the passively focused node is a connection and the dragged block
* can connect to it, the connection will be returned. Otherwise, the first
* compatible connection on the passively focused node's block, if any, will
* be returned. Returns null if the workspace does not have passive focus.
*/
private getInitialCandidate(): ConnectionCandidate | null {
const passiveElement = this.workspace
.getSvgGroup()
.querySelector(`.${FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME}`);
if (
!passiveElement ||
!passiveElement.id ||
passiveElement === this.block.getFocusableElement()
) {
return null;
}
const passiveNode = this.workspace.lookUpFocusableNode(passiveElement.id);
if (!passiveNode) return null;

const passiveBlock = this.workspace
.getNavigator()
.getSourceBlockFromNode(passiveNode);
if (!passiveBlock) return null;

const passiveBlockConnections = passiveBlock.getConnections_(false);
let passiveConnection: RenderedConnection | null = null;

// If the passively focused node is a connection, return it if it is
// compatible with the dragged block.
if (passiveBlockConnections.includes(passiveNode as any)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we check whether passiveNode instanceof RenderedConnection instead of casting as any? At that point, I think we'd also be able to safely infer that it's one of passiveBlockConnection because they have same source block.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not the purpose for the cast, passiveBlockConnections is an array of RenderedConnection, so if it includes passiveNode, we know it is one; unfortunately without this, TS whinges that "Argument of type 'IFocusableNode' is not assignable to parameter of type 'RenderedConnection'.", stubbornly refusing to consider the possibility that the IFocusableNode passiveNode may in fact be a RenderedConnection (which implements IFocusableNode!). Since this is all verifiably safe I just casted to any to satisfy the compiler; doing an instanceof requires a full import of RenderedConnection, and IIRC that leads to an import cycle.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, got it! I think that's totally reasonable then.

passiveConnection = passiveNode as RenderedConnection;
const connectionChecker = this.block.workspace.connectionChecker;
const local = this.block.getConnections_(false).find((connection) => {
return connectionChecker.canConnect(
connection,
passiveConnection,
true,
Infinity,
);
});

if (local) {
return {local, neighbour: passiveConnection, distance: 0};
}
}

// Fall back to returning the first compatible connection on the passively
// focused block, if any.
const pair = this.allConnectionPairs.find(
(pair) => pair.neighbour.getSourceBlock() === passiveBlock,
);
if (pair) {
return this.pairToCandidate(pair);
}

// Fall back further to the parent connection of the passively focused
// block, if any.
const outputTarget = passiveBlock.outputConnection?.targetConnection;
const parentPair = this.allConnectionPairs.find(
(pair) => pair.neighbour === outputTarget,
);
if (parentPair) {
return this.pairToCandidate(parentPair);
}

return null;
}
}
182 changes: 181 additions & 1 deletion packages/blockly/tests/mocha/keyboard_movement_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,26 @@ import {createKeyDownEvent} from './test_helpers/user_input.js';
suite('Keyboard-driven movement', function () {
setup(function () {
sharedTestSetup.call(this);
const toolbox = document.getElementById('toolbox-simple');
const toolbox = {
// There are two kinds of toolboxes. The simpler one is a flyout toolbox.
kind: 'flyoutToolbox',
// The contents is the blocks and other items that exist in your toolbox.
contents: [
{
kind: 'block',
type: 'text_print',
},
{
kind: 'block',
type: 'logic_negate',
},
{
kind: 'block',
type: 'math_number',
},
],
};

this.workspace = Blockly.inject('blocklyDiv', {toolbox: toolbox});
Blockly.common.defineBlocks(p5blocks);
Blockly.KeyboardMover.mover.setMoveDistance(20);
Expand Down Expand Up @@ -73,6 +92,11 @@ suite('Keyboard-driven movement', function () {
workspace.getInjectionDiv().dispatchEvent(event);
}

function focusToolbox(workspace) {
const event = createKeyDownEvent(Blockly.utils.KeyCodes.T);
workspace.getInjectionDiv().dispatchEvent(event);
}

/**
* Create a new block from serialised state (parsed JSON) and
* optionally attach it to an existing block on the workspace.
Expand Down Expand Up @@ -554,6 +578,162 @@ suite('Keyboard-driven movement', function () {
toastSpy.restore();
});

test('initially moves the block to the previously-focused statement connection', function () {
const ifBlock = this.workspace.newBlock('controls_if');
ifBlock.initSvg();
ifBlock.render();

const statementConnection = ifBlock.getInput('DO0').connection;
Blockly.getFocusManager().focusNode(statementConnection);
focusToolbox(this.workspace);
startMove(this.workspace);

const printBlock = Blockly.getFocusManager().getFocusedNode();
const candidate = printBlock.getDragStrategy().connectionCandidate;

assert.equal(candidate.local, printBlock.previousConnection);
assert.equal(candidate.neighbour, statementConnection);

cancelMove(this.workspace);
});

test("initially moves the block to the previously-focused block's previous connection", function () {
const ifBlock = this.workspace.newBlock('controls_if');
ifBlock.initSvg();
ifBlock.render();

Blockly.getFocusManager().focusNode(ifBlock);
focusToolbox(this.workspace);
startMove(this.workspace);

const printBlock = Blockly.getFocusManager().getFocusedNode();
const candidate = printBlock.getDragStrategy().connectionCandidate;

assert.equal(candidate.local, printBlock.nextConnection);
assert.equal(candidate.neighbour, ifBlock.previousConnection);

cancelMove(this.workspace);
});

test('initially moves the block to the previously-focused input connection', function () {
const ifBlock = this.workspace.newBlock('controls_if');
ifBlock.initSvg();
ifBlock.render();

const inputConnection = ifBlock.getInput('IF0').connection;
Blockly.getFocusManager().focusNode(inputConnection);
Comment thread
gonfunko marked this conversation as resolved.
focusToolbox(this.workspace);
moveDown(this.workspace);
startMove(this.workspace);

const notBlock = Blockly.getFocusManager().getFocusedNode();
const candidate = notBlock.getDragStrategy().connectionCandidate;

assert.equal(candidate.local, notBlock.outputConnection);
assert.equal(candidate.neighbour, inputConnection);

cancelMove(this.workspace);
});

test('initially moves the block to the previously-focused not-first statement connection', function () {
const ifBlock = this.workspace.newBlock('controls_ifelse');
ifBlock.initSvg();
ifBlock.render();

const statementConnection = ifBlock.getInput('ELSE').connection;
Blockly.getFocusManager().focusNode(statementConnection);
focusToolbox(this.workspace);
startMove(this.workspace);

const printBlock = Blockly.getFocusManager().getFocusedNode();
const candidate = printBlock.getDragStrategy().connectionCandidate;

assert.equal(candidate.local, printBlock.previousConnection);
assert.equal(candidate.neighbour, statementConnection);

cancelMove(this.workspace);
});

test("initially moves the block to the previously-focused block's input connection", function () {
const ifBlock = this.workspace.newBlock('controls_if');
ifBlock.initSvg();
ifBlock.render();

Blockly.getFocusManager().focusNode(ifBlock);
focusToolbox(this.workspace);
moveDown(this.workspace);
startMove(this.workspace);

const notBlock = Blockly.getFocusManager().getFocusedNode();
const candidate = notBlock.getDragStrategy().connectionCandidate;

assert.equal(candidate.local, notBlock.outputConnection);
assert.equal(candidate.neighbour, ifBlock.getInput('IF0').connection);
Comment thread
gonfunko marked this conversation as resolved.

cancelMove(this.workspace);
});

test('initially moves the block to the previously-focused not-first input connection', function () {
const compare = this.workspace.newBlock('logic_compare');
compare.initSvg();
compare.render();

Blockly.getFocusManager().focusNode(compare.getInput('B').connection);
focusToolbox(this.workspace);
moveDown(this.workspace);
startMove(this.workspace);

const notBlock = Blockly.getFocusManager().getFocusedNode();
const candidate = notBlock.getDragStrategy().connectionCandidate;

assert.equal(candidate.local, notBlock.outputConnection);
assert.equal(candidate.neighbour, compare.getInput('B').connection);

cancelMove(this.workspace);
});

test("initially moves the block to the previously-focused block's parent input connection", function () {
const compare = this.workspace.newBlock('logic_compare');
compare.initSvg();
compare.render();

const boolean = this.workspace.newBlock('logic_boolean');
boolean.initSvg();
boolean.render();
boolean.outputConnection.connect(compare.getInput('A').connection);

Blockly.getFocusManager().focusNode(boolean);
focusToolbox(this.workspace);
moveDown(this.workspace);
startMove(this.workspace);

const notBlock = Blockly.getFocusManager().getFocusedNode();
const candidate = notBlock.getDragStrategy().connectionCandidate;

assert.equal(candidate.local, notBlock.outputConnection);
assert.equal(candidate.neighbour, compare.getInput('A').connection);

cancelMove(this.workspace);
});

test('initially moves the block to the workspace when the previously-focused block has no compatible connections', function () {
const repeat = this.workspace.newBlock('controls_repeat');
repeat.initSvg();
repeat.render();

Blockly.getFocusManager().focusNode(repeat);
focusToolbox(this.workspace);
moveDown(this.workspace);
startMove(this.workspace);

const notBlock = Blockly.getFocusManager().getFocusedNode();
const candidate = notBlock.getDragStrategy().connectionCandidate;

assert.isNull(candidate);

cancelMove(this.workspace);
});
Comment thread
gonfunko marked this conversation as resolved.

suite('Statement move tests', function () {
// Clear the workspace and load start blocks.
setup(function () {
Expand Down