Skip to content

feat: Insert blocks at focus point#9806

Merged
gonfunko merged 6 commits into
v13from
smart-insert
May 5, 2026
Merged

feat: Insert blocks at focus point#9806
gonfunko merged 6 commits into
v13from
smart-insert

Conversation

@gonfunko
Copy link
Copy Markdown
Contributor

@gonfunko gonfunko commented May 1, 2026

The basics

The details

Resolves

Fixes #9758

Proposed Changes

This PR reintroduces support for inserting blocks from the flyout at or near the currently focused node in the workspace. If a connection is focused, and the chosen block can connect to it, it will be initially moved to/proposed for connection at that point; otherwise, the focused node's parent block will be identified, and, assuming there is one, the inserted block will be proposed for connection to the first compatible connection on that block. A final attempt will be made to connect the inserted block to the focused block's parent block's input connection, after which the inserted block will just be initially proposed as a top-level block.

@gonfunko gonfunko requested a review from a team as a code owner May 1, 2026 22:51
@gonfunko gonfunko requested a review from mikeharv May 1, 2026 22:51
@github-actions github-actions Bot added the PR: feature Adds a feature label May 1, 2026
Copy link
Copy Markdown
Contributor

@mikeharv mikeharv left a comment

Choose a reason for hiding this comment

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

Pulled and tested. Seems to work great!

Comment thread packages/blockly/core/dragging/block_drag_strategy.ts Outdated

// 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.

Comment thread packages/blockly/tests/mocha/keyboard_movement_test.js
Comment thread packages/blockly/tests/mocha/keyboard_movement_test.js
Comment thread packages/blockly/tests/mocha/keyboard_movement_test.js Outdated
Comment thread packages/blockly/tests/mocha/keyboard_movement_test.js
Comment thread packages/blockly/core/dragging/block_drag_strategy.ts
@gonfunko gonfunko requested a review from mikeharv May 5, 2026 17:23
@gonfunko gonfunko merged commit a56b221 into v13 May 5, 2026
5 checks passed
@gonfunko gonfunko deleted the smart-insert branch May 5, 2026 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: feature Adds a feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants