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..e0f13e0540a 100644 --- a/core/keyboard_nav/line_cursor.ts +++ b/core/keyboard_nav/line_cursor.ts @@ -345,7 +345,34 @@ 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); + // 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: case NavigationDirection.PREVIOUS: return (candidate: IFocusableNode | null) => { @@ -452,6 +479,25 @@ export class LineCursor extends Marker { return parents; } + /** + * 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 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(); + 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; diff --git a/tests/mocha/cursor_test.js b/tests/mocha/cursor_test.js index 2273ec4b381..87e9423ce76 100644 --- a/tests/mocha/cursor_test.js +++ b/tests/mocha/cursor_test.js @@ -163,13 +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); + assert.equal(curNode, this.blocks.A.getInput('NAME4').connection); }); }); @@ -252,37 +252,38 @@ suite('Cursor', function () { test('In - from field in nested statement block to next nested statement block', function () { this.cursor.setCurNode(this.secondStatement.getField('NAME')); - this.cursor.in(); + this.cursor.next(); // Skip over the next connection - this.cursor.in(); + 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.in(); + this.cursor.next(); // Skip over the next connection - this.cursor.in(); + this.cursor.next(); 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 () { + test('Out - from nested statement block to previous nested statement block', function () { this.cursor.setCurNode(this.thirdStatement); - this.cursor.out(); + this.cursor.prev(); // Skip over the previous next connection - this.cursor.out(); + this.cursor.prev(); const curNode = this.cursor.getCurNode(); - assert.equal(curNode, this.secondStatement.getField('NAME')); + assert.equal(curNode, this.secondStatement); }); - test('Out - from root block to last field of last nested statement block in previous stack', function () { + test('Out - from root block to last nested statement block in previous stack', function () { this.cursor.setCurNode(this.multiStatement2); - this.cursor.out(); + this.cursor.prev(); // Skip over the previous next connection - this.cursor.out(); + this.cursor.prev(); const curNode = this.cursor.getCurNode(); - assert.equal(curNode, this.thirdStatement.getField('NAME')); + assert.equal(curNode, this.thirdStatement); }); });