From ec49e825c993e56cee97b0af9a5df0d40dd06288 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Wed, 17 Dec 2025 13:52:33 -0800 Subject: [PATCH 1/5] fix: Make left and right arrow keys only navigate within the current block/comment --- core/comments/comment_editor.ts | 21 +++++++++++ core/comments/rendered_workspace_comment.ts | 2 + core/icons/comment_icon.ts | 3 ++ core/keyboard_nav/line_cursor.ts | 41 ++++++++++++++++++++- core/keyboard_nav/marker.ts | 14 +++++++ 5 files changed, 80 insertions(+), 1 deletion(-) diff --git a/core/comments/comment_editor.ts b/core/comments/comment_editor.ts index a5ce260a985..e542dca84a8 100644 --- a/core/comments/comment_editor.ts +++ b/core/comments/comment_editor.ts @@ -4,7 +4,9 @@ * SPDX-License-Identifier: Apache-2.0 */ +import type {BlockSvg} from '../block_svg.js'; import * as browserEvents from '../browser_events.js'; +import type {RenderedWorkspaceComment} from '../comments/rendered_workspace_comment.js'; import {getFocusManager} from '../focus_manager.js'; import {IFocusableNode} from '../interfaces/i_focusable_node.js'; import {IFocusableTree} from '../interfaces/i_focusable_tree.js'; @@ -40,6 +42,8 @@ export class CommentEditor implements IFocusableNode { /** The current text of the comment. Updates on text area change. */ private text: string = ''; + private parent?: BlockSvg | RenderedWorkspaceComment; + constructor( public workspace: WorkspaceSvg, commentId?: string, @@ -207,4 +211,21 @@ export class CommentEditor implements IFocusableNode { if (this.id) return true; return false; } + + /** + * Sets the parent object that owns this comment editor. + * + * @param newParent The parent of this comment editor. + * @internal + */ + setParent(newParent: BlockSvg | RenderedWorkspaceComment) { + this.parent = newParent; + } + + /** + * Returns the parent object that owns this comment editor, if any. + */ + getParent() { + return this.parent; + } } diff --git a/core/comments/rendered_workspace_comment.ts b/core/comments/rendered_workspace_comment.ts index 2903bff4bce..e188deca9a8 100644 --- a/core/comments/rendered_workspace_comment.ts +++ b/core/comments/rendered_workspace_comment.ts @@ -31,6 +31,7 @@ import {Rect} from '../utils/rect.js'; import {Size} from '../utils/size.js'; import * as svgMath from '../utils/svg_math.js'; import {WorkspaceSvg} from '../workspace_svg.js'; +import type {CommentEditor} from './comment_editor.js'; import {CommentView} from './comment_view.js'; import {WorkspaceComment} from './workspace_comment.js'; @@ -60,6 +61,7 @@ export class RenderedWorkspaceComment this.workspace = workspace; this.view = new CommentView(workspace, this.id); + (this.view.getEditorFocusableNode() as CommentEditor).setParent(this); // Set the size to the default size as defined in the superclass. this.view.setSize(this.getSize()); this.view.setEditable(this.isEditable()); diff --git a/core/icons/comment_icon.ts b/core/icons/comment_icon.ts index 1b2e47149e4..5beda080b1b 100644 --- a/core/icons/comment_icon.ts +++ b/core/icons/comment_icon.ts @@ -381,6 +381,9 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable { this.getBubbleOwnerRect(), this, ); + this.textInputBubble + .getEditor() + .setParent(this.getSourceBlock() as BlockSvg); this.textInputBubble.setText(this.getText()); this.textInputBubble.setSize(this.bubbleSize, true); if (this.bubbleLocation) { diff --git a/core/keyboard_nav/line_cursor.ts b/core/keyboard_nav/line_cursor.ts index c0d76fa5aae..f62d8dc0a32 100644 --- a/core/keyboard_nav/line_cursor.ts +++ b/core/keyboard_nav/line_cursor.ts @@ -345,7 +345,28 @@ export class LineCursor extends Marker { switch (direction) { case NavigationDirection.IN: case NavigationDirection.OUT: - return () => true; + return (candidate: IFocusableNode | null) => { + const candidateBlock = this.getSourceBlockFromNode(candidate); + const currentBlock = this.getSourceBlock(); + + // Preventing escaping the current block/comment/etc by: + // Disallow moving from a node with a block to a non-block node (other than a block comment editor) + // Disallow moving from a non-block node to a block node + // Disallow moving to the workspace + if ( + (currentBlock && !candidateBlock) || + (!currentBlock && candidateBlock) || + candidate === this.workspace + ) { + return false; + } + + if (!candidateBlock || !currentBlock) return true; + + const currentParents = this.getOutputParents(currentBlock); + const candidateParents = this.getOutputParents(candidateBlock); + return candidateParents.intersection(currentParents).size > 0; + }; case NavigationDirection.NEXT: case NavigationDirection.PREVIOUS: return (candidate: IFocusableNode | null) => { @@ -452,6 +473,24 @@ export class LineCursor extends Marker { return parents; } + /** + * Returns a set of all of the parent blocks of the given block. + * + * @param block The block to retrieve the parents of. + * @returns A set of the parents of the given block. + */ + private getOutputParents(block: BlockSvg): Set { + const parents = new Set(); + parents.add(block); + let parent = block.outputConnection?.targetBlock(); + while (parent) { + parents.add(parent); + parent = parent.outputConnection?.targetBlock(); + } + + return parents; + } + /** * Prepare for the deletion of a block by making a list of nodes we * could move the cursor to afterwards and save it to diff --git a/core/keyboard_nav/marker.ts b/core/keyboard_nav/marker.ts index 0cd066c163c..cb34e5e0877 100644 --- a/core/keyboard_nav/marker.ts +++ b/core/keyboard_nav/marker.ts @@ -13,6 +13,8 @@ // Former goog.module ID: Blockly.Marker import {BlockSvg} from '../block_svg.js'; +import {TextInputBubble} from '../bubbles/textinput_bubble.js'; +import {CommentEditor} from '../comments/comment_editor.js'; import {Field} from '../field.js'; import {Icon} from '../icons/icon.js'; import type {IFocusableNode} from '../interfaces/i_focusable_node.js'; @@ -69,6 +71,18 @@ export class Marker { return node.getSourceBlock(); } else if (node instanceof Icon) { return node.getSourceBlock() as BlockSvg; + } else if (node instanceof TextInputBubble) { + const owner = node.getOwner(); + if (owner instanceof BlockSvg) { + return owner; + } else if (owner) { + return this.getSourceBlockFromNode(owner); + } + } else if (node instanceof CommentEditor) { + const parent = node.getParent(); + if (parent instanceof BlockSvg) { + return parent; + } } return null; From 107378c67bb1d4645c2fed5083778876f1882713 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Wed, 17 Dec 2025 13:52:45 -0800 Subject: [PATCH 2/5] fix: Fix tests --- tests/mocha/cursor_test.js | 119 +------------------------------------ 1 file changed, 3 insertions(+), 116 deletions(-) diff --git a/tests/mocha/cursor_test.js b/tests/mocha/cursor_test.js index 2273ec4b381..fba8edc2f50 100644 --- a/tests/mocha/cursor_test.js +++ b/tests/mocha/cursor_test.js @@ -11,7 +11,7 @@ import { sharedTestTeardown, } from './test_helpers/setup_teardown.js'; -suite('Cursor', function () { +suite.only('Cursor', function () { suite('Movement', function () { setup(function () { sharedTestSetup.call(this); @@ -163,126 +163,13 @@ suite('Cursor', function () { assert.equal(curNode, this.blocks.E); }); - test('Out - From first connection loop to last next connection', function () { + test('Out - From previous connection loop to last input connection on block', function () { const prevConnection = this.blocks.A.previousConnection; const prevConnectionNode = prevConnection; this.cursor.setCurNode(prevConnectionNode); this.cursor.out(); const curNode = this.cursor.getCurNode(); - assert.equal(curNode, this.blocks.D.nextConnection); - }); - }); - - suite('Multiple statement inputs', function () { - setup(function () { - sharedTestSetup.call(this); - Blockly.defineBlocksWithJsonArray([ - { - 'type': 'multi_statement_input', - 'message0': '%1 %2', - 'args0': [ - { - 'type': 'input_statement', - 'name': 'FIRST', - }, - { - 'type': 'input_statement', - 'name': 'SECOND', - }, - ], - }, - { - 'type': 'simple_statement', - 'message0': '%1', - 'args0': [ - { - 'type': 'field_input', - 'name': 'NAME', - 'text': 'default', - }, - ], - 'previousStatement': null, - 'nextStatement': null, - }, - ]); - this.workspace = Blockly.inject('blocklyDiv', {}); - this.cursor = this.workspace.getCursor(); - - this.multiStatement1 = createRenderedBlock( - this.workspace, - 'multi_statement_input', - ); - this.multiStatement2 = createRenderedBlock( - this.workspace, - 'multi_statement_input', - ); - this.firstStatement = createRenderedBlock( - this.workspace, - 'simple_statement', - ); - this.secondStatement = createRenderedBlock( - this.workspace, - 'simple_statement', - ); - this.thirdStatement = createRenderedBlock( - this.workspace, - 'simple_statement', - ); - this.fourthStatement = createRenderedBlock( - this.workspace, - 'simple_statement', - ); - this.multiStatement1 - .getInput('FIRST') - .connection.connect(this.firstStatement.previousConnection); - this.firstStatement.nextConnection.connect( - this.secondStatement.previousConnection, - ); - this.multiStatement1 - .getInput('SECOND') - .connection.connect(this.thirdStatement.previousConnection); - this.multiStatement2 - .getInput('FIRST') - .connection.connect(this.fourthStatement.previousConnection); - }); - - teardown(function () { - sharedTestTeardown.call(this); - }); - - test('In - from field in nested statement block to next nested statement block', function () { - this.cursor.setCurNode(this.secondStatement.getField('NAME')); - this.cursor.in(); - // Skip over the next connection - this.cursor.in(); - const curNode = this.cursor.getCurNode(); - assert.equal(curNode, this.thirdStatement); - }); - test('In - from field in nested statement block to next stack', function () { - this.cursor.setCurNode(this.thirdStatement.getField('NAME')); - this.cursor.in(); - // Skip over the next connection - this.cursor.in(); - const curNode = this.cursor.getCurNode(); - assert.equal(curNode, this.multiStatement2); - }); - - test('Out - from nested statement block to last field of previous nested statement block', function () { - this.cursor.setCurNode(this.thirdStatement); - this.cursor.out(); - // Skip over the previous next connection - this.cursor.out(); - const curNode = this.cursor.getCurNode(); - assert.equal(curNode, this.secondStatement.getField('NAME')); - }); - - test('Out - from root block to last field of last nested statement block in previous stack', function () { - this.cursor.setCurNode(this.multiStatement2); - this.cursor.out(); - // Skip over the previous next connection - this.cursor.out(); - const curNode = this.cursor.getCurNode(); - assert.equal(curNode, this.thirdStatement.getField('NAME')); + assert.equal(curNode, this.blocks.A.getInput('NAME4').connection); }); }); From c62fd2d4480a0033d29543889464562a118f5086 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Wed, 17 Dec 2025 13:57:11 -0800 Subject: [PATCH 3/5] fix: Remove .only from test suite --- tests/mocha/cursor_test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/mocha/cursor_test.js b/tests/mocha/cursor_test.js index fba8edc2f50..5e2164ce8fd 100644 --- a/tests/mocha/cursor_test.js +++ b/tests/mocha/cursor_test.js @@ -11,7 +11,7 @@ import { sharedTestTeardown, } from './test_helpers/setup_teardown.js'; -suite.only('Cursor', function () { +suite('Cursor', function () { suite('Movement', function () { setup(function () { sharedTestSetup.call(this); From c26b874d40b0dd7f360cd6d750e0b6bd16b7ca6e Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Fri, 19 Dec 2025 09:33:49 -0800 Subject: [PATCH 4/5] test: Reenable statement block navigation tests with modifications --- tests/mocha/cursor_test.js | 114 +++++++++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/tests/mocha/cursor_test.js b/tests/mocha/cursor_test.js index 5e2164ce8fd..87e9423ce76 100644 --- a/tests/mocha/cursor_test.js +++ b/tests/mocha/cursor_test.js @@ -173,6 +173,120 @@ suite('Cursor', function () { }); }); + suite('Multiple statement inputs', function () { + setup(function () { + sharedTestSetup.call(this); + Blockly.defineBlocksWithJsonArray([ + { + 'type': 'multi_statement_input', + 'message0': '%1 %2', + 'args0': [ + { + 'type': 'input_statement', + 'name': 'FIRST', + }, + { + 'type': 'input_statement', + 'name': 'SECOND', + }, + ], + }, + { + 'type': 'simple_statement', + 'message0': '%1', + 'args0': [ + { + 'type': 'field_input', + 'name': 'NAME', + 'text': 'default', + }, + ], + 'previousStatement': null, + 'nextStatement': null, + }, + ]); + this.workspace = Blockly.inject('blocklyDiv', {}); + this.cursor = this.workspace.getCursor(); + + this.multiStatement1 = createRenderedBlock( + this.workspace, + 'multi_statement_input', + ); + this.multiStatement2 = createRenderedBlock( + this.workspace, + 'multi_statement_input', + ); + this.firstStatement = createRenderedBlock( + this.workspace, + 'simple_statement', + ); + this.secondStatement = createRenderedBlock( + this.workspace, + 'simple_statement', + ); + this.thirdStatement = createRenderedBlock( + this.workspace, + 'simple_statement', + ); + this.fourthStatement = createRenderedBlock( + this.workspace, + 'simple_statement', + ); + this.multiStatement1 + .getInput('FIRST') + .connection.connect(this.firstStatement.previousConnection); + this.firstStatement.nextConnection.connect( + this.secondStatement.previousConnection, + ); + this.multiStatement1 + .getInput('SECOND') + .connection.connect(this.thirdStatement.previousConnection); + this.multiStatement2 + .getInput('FIRST') + .connection.connect(this.fourthStatement.previousConnection); + }); + + teardown(function () { + sharedTestTeardown.call(this); + }); + + test('In - from field in nested statement block to next nested statement block', function () { + this.cursor.setCurNode(this.secondStatement.getField('NAME')); + this.cursor.next(); + // Skip over the next connection + this.cursor.next(); + const curNode = this.cursor.getCurNode(); + assert.equal(curNode, this.thirdStatement); + }); + + test('In - from field in nested statement block to next stack', function () { + this.cursor.setCurNode(this.thirdStatement.getField('NAME')); + this.cursor.next(); + // Skip over the next connection + this.cursor.next(); + const curNode = this.cursor.getCurNode(); + assert.equal(curNode, this.multiStatement2); + }); + + test('Out - from nested statement block to previous nested statement block', function () { + this.cursor.setCurNode(this.thirdStatement); + this.cursor.prev(); + // Skip over the previous next connection + this.cursor.prev(); + const curNode = this.cursor.getCurNode(); + assert.equal(curNode, this.secondStatement); + }); + + test('Out - from root block to last nested statement block in previous stack', function () { + this.cursor.setCurNode(this.multiStatement2); + this.cursor.prev(); + // Skip over the previous next connection + this.cursor.prev(); + const curNode = this.cursor.getCurNode(); + assert.equal(curNode, this.thirdStatement); + }); + }); + suite('Searching', function () { setup(function () { sharedTestSetup.call(this); From b7c528610185bb1a5796fd94ff6bb970276cea67 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Fri, 19 Dec 2025 09:42:05 -0800 Subject: [PATCH 5/5] chore: Clarify comments --- core/keyboard_nav/line_cursor.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/core/keyboard_nav/line_cursor.ts b/core/keyboard_nav/line_cursor.ts index f62d8dc0a32..e0f13e0540a 100644 --- a/core/keyboard_nav/line_cursor.ts +++ b/core/keyboard_nav/line_cursor.ts @@ -365,6 +365,12 @@ export class LineCursor extends Marker { const currentParents = this.getOutputParents(currentBlock); const candidateParents = this.getOutputParents(candidateBlock); + // If we're navigating from a block (or nested element) to a block + // (or nested element), ensure that we're not crossing a statement + // block boundary (i.e. moving to a next or previous block vertically) + // by verifying that the two blocks in question are either the same + // or have a common parent accessible only by traversing output + // connections, meaning that they are part of the same row. return candidateParents.intersection(currentParents).size > 0; }; case NavigationDirection.NEXT: @@ -474,10 +480,11 @@ export class LineCursor extends Marker { } /** - * Returns a set of all of the parent blocks of the given block. + * Returns a set of all of the parent blocks connected to an output of the + * given block or one of its parents. Also includes the given block. * - * @param block The block to retrieve the parents of. - * @returns A set of the parents of the given block. + * @param block The block to retrieve the output-connected parents of. + * @returns A set of the output-connected parents of the given block. */ private getOutputParents(block: BlockSvg): Set { const parents = new Set();