From ca362725eef3d8f455ec9479425e6760a4de1198 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Thu, 3 Apr 2025 12:15:17 -0700 Subject: [PATCH] refactor!: Backport LineCursor to core. (#8834) * refactor: Backport LineCursor to core. * fix: Fix instantiation of LineCursor. * fix: Fix tests. * chore: Assauge the linter. * chore: Fix some typos. * feat: Make padding configurable for scrollBoundsIntoView. * chore: Merge in the latest changes from keyboard-experimentation. * refactor: Clarify name and docs for findSiblingOrParentSibling(). * fix: Improve scrollBoundsIntoView() behavior. * fix: Export CursorOptions. * refactor: Further clarify second parameter of setCurNode(). * fix: Revert change that could prevent scrolling bounds into view. --- core/blockly.ts | 5 +- core/field.ts | 4 +- core/field_input.ts | 2 +- core/keyboard_nav/cursor.ts | 137 ----- core/keyboard_nav/line_cursor.ts | 761 ++++++++++++++++++++++++++++ core/marker_manager.ts | 8 +- core/registry.ts | 4 +- core/renderers/zelos/path_object.ts | 3 +- core/workspace_svg.ts | 71 ++- tests/mocha/cursor_test.js | 33 +- 10 files changed, 861 insertions(+), 167 deletions(-) delete mode 100644 core/keyboard_nav/cursor.ts create mode 100644 core/keyboard_nav/line_cursor.ts diff --git a/core/blockly.ts b/core/blockly.ts index 4c50c5ed5..e14a89e74 100644 --- a/core/blockly.ts +++ b/core/blockly.ts @@ -169,7 +169,7 @@ import {IVariableMap} from './interfaces/i_variable_map.js'; import {IVariableModel, IVariableState} from './interfaces/i_variable_model.js'; import * as internalConstants from './internal_constants.js'; import {ASTNode} from './keyboard_nav/ast_node.js'; -import {Cursor} from './keyboard_nav/cursor.js'; +import {CursorOptions, LineCursor} from './keyboard_nav/line_cursor.js'; import {Marker} from './keyboard_nav/marker.js'; import type {LayerManager} from './layer_manager.js'; import * as layers from './layers.js'; @@ -444,11 +444,12 @@ export { ContextMenuItems, ContextMenuRegistry, Css, - Cursor, + CursorOptions, DeleteArea, DragTarget, Events, Extensions, + LineCursor, Procedures, ShortcutItems, Themes, diff --git a/core/field.ts b/core/field.ts index 9dbd5a5cf..725a2867d 100644 --- a/core/field.ts +++ b/core/field.ts @@ -336,8 +336,10 @@ export abstract class Field * intend because the behavior was kind of hacked in. If you are thinking * about overriding this function, post on the forum with your intended * behavior to see if there's another approach. + * + * @internal */ - protected isFullBlockField(): boolean { + isFullBlockField(): boolean { return !this.borderRect_; } diff --git a/core/field_input.ts b/core/field_input.ts index ed97544dc..2cdd80565 100644 --- a/core/field_input.ts +++ b/core/field_input.ts @@ -152,7 +152,7 @@ export abstract class FieldInput extends Field< } } - protected override isFullBlockField(): boolean { + override isFullBlockField(): boolean { const block = this.getSourceBlock(); if (!block) throw new UnattachedFieldError(); diff --git a/core/keyboard_nav/cursor.ts b/core/keyboard_nav/cursor.ts deleted file mode 100644 index 92279da56..000000000 --- a/core/keyboard_nav/cursor.ts +++ /dev/null @@ -1,137 +0,0 @@ -/** - * @license - * Copyright 2019 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -/** - * The class representing a cursor. - * Used primarily for keyboard navigation. - * - * @class - */ -// Former goog.module ID: Blockly.Cursor - -import * as registry from '../registry.js'; -import {ASTNode} from './ast_node.js'; -import {Marker} from './marker.js'; - -/** - * Class for a cursor. - * A cursor controls how a user navigates the Blockly AST. - */ -export class Cursor extends Marker { - override type = 'cursor'; - - constructor() { - super(); - } - - /** - * Find the next connection, field, or block. - * - * @returns The next element, or null if the current node is not set or there - * is no next value. - */ - next(): ASTNode | null { - const curNode = this.getCurNode(); - if (!curNode) { - return null; - } - - let newNode = curNode.next(); - while ( - newNode && - newNode.next() && - (newNode.getType() === ASTNode.types.NEXT || - newNode.getType() === ASTNode.types.BLOCK) - ) { - newNode = newNode.next(); - } - - if (newNode) { - this.setCurNode(newNode); - } - return newNode; - } - - /** - * Find the in connection or field. - * - * @returns The in element, or null if the current node is not set or there is - * no in value. - */ - in(): ASTNode | null { - let curNode: ASTNode | null = this.getCurNode(); - if (!curNode) { - return null; - } - // If we are on a previous or output connection, go to the block level - // before performing next operation. - if ( - curNode.getType() === ASTNode.types.PREVIOUS || - curNode.getType() === ASTNode.types.OUTPUT - ) { - curNode = curNode.next(); - } - const newNode = curNode?.in() ?? null; - - if (newNode) { - this.setCurNode(newNode); - } - return newNode; - } - - /** - * Find the previous connection, field, or block. - * - * @returns The previous element, or null if the current node is not set or - * there is no previous value. - */ - prev(): ASTNode | null { - const curNode = this.getCurNode(); - if (!curNode) { - return null; - } - let newNode = curNode.prev(); - - while ( - newNode && - newNode.prev() && - (newNode.getType() === ASTNode.types.NEXT || - newNode.getType() === ASTNode.types.BLOCK) - ) { - newNode = newNode.prev(); - } - - if (newNode) { - this.setCurNode(newNode); - } - return newNode; - } - - /** - * Find the out connection, field, or block. - * - * @returns The out element, or null if the current node is not set or there - * is no out value. - */ - out(): ASTNode | null { - const curNode = this.getCurNode(); - if (!curNode) { - return null; - } - let newNode = curNode.out(); - - if (newNode && newNode.getType() === ASTNode.types.BLOCK) { - newNode = newNode.prev() || newNode; - } - - if (newNode) { - this.setCurNode(newNode); - } - return newNode; - } -} - -registry.register(registry.Type.CURSOR, registry.DEFAULT, Cursor); diff --git a/core/keyboard_nav/line_cursor.ts b/core/keyboard_nav/line_cursor.ts new file mode 100644 index 000000000..8f3ac19b4 --- /dev/null +++ b/core/keyboard_nav/line_cursor.ts @@ -0,0 +1,761 @@ +/** + * @license + * Copyright 2020 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * @fileoverview The class representing a line cursor. + * A line cursor tries to traverse the blocks and connections on a block as if + * they were lines of code in a text editor. Previous and next traverse previous + * connections, next connections and blocks, while in and out traverse input + * connections and fields. + * @author aschmiedt@google.com (Abby Schmiedt) + */ + +import type {Block} from '../block.js'; +import {BlockSvg} from '../block_svg.js'; +import * as common from '../common.js'; +import type {Connection} from '../connection.js'; +import {ConnectionType} from '../connection_type.js'; +import type {Abstract} from '../events/events_abstract.js'; +import {Click, ClickTarget} from '../events/events_click.js'; +import {EventType} from '../events/type.js'; +import * as eventUtils from '../events/utils.js'; +import type {Field} from '../field.js'; +import * as registry from '../registry.js'; +import type {MarkerSvg} from '../renderers/common/marker_svg.js'; +import type {PathObject} from '../renderers/zelos/path_object.js'; +import {Renderer} from '../renderers/zelos/renderer.js'; +import * as dom from '../utils/dom.js'; +import type {WorkspaceSvg} from '../workspace_svg.js'; +import {ASTNode} from './ast_node.js'; +import {Marker} from './marker.js'; + +/** Options object for LineCursor instances. */ +export interface CursorOptions { + /** + * Can the cursor visit all stack connections (next/previous), or + * (if false) only unconnected next connections? + */ + stackConnections: boolean; +} + +/** Default options for LineCursor instances. */ +const defaultOptions: CursorOptions = { + stackConnections: true, +}; + +/** + * Class for a line cursor. + */ +export class LineCursor extends Marker { + override type = 'cursor'; + + /** Options for this line cursor. */ + private readonly options: CursorOptions; + + /** Locations to try moving the cursor to after a deletion. */ + private potentialNodes: ASTNode[] | null = null; + + /** Whether the renderer is zelos-style. */ + private isZelos = false; + + /** + * @param workspace The workspace this cursor belongs to. + * @param options Cursor options. + */ + constructor( + private readonly workspace: WorkspaceSvg, + options?: Partial, + ) { + super(); + // Bind changeListener to facilitate future disposal. + this.changeListener = this.changeListener.bind(this); + this.workspace.addChangeListener(this.changeListener); + // Regularise options and apply defaults. + this.options = {...defaultOptions, ...options}; + + this.isZelos = workspace.getRenderer() instanceof Renderer; + } + + /** + * Clean up this cursor. + */ + dispose() { + this.workspace.removeChangeListener(this.changeListener); + super.dispose(); + } + + /** + * Moves the cursor to the next previous connection, next connection or block + * in the pre order traversal. Finds the next node in the pre order traversal. + * + * @returns The next node, or null if the current node is + * not set or there is no next value. + */ + next(): ASTNode | null { + const curNode = this.getCurNode(); + if (!curNode) { + return null; + } + const newNode = this.getNextNode(curNode, this.validLineNode.bind(this)); + + if (newNode) { + this.setCurNode(newNode); + } + return newNode; + } + + /** + * Moves the cursor to the next input connection or field + * in the pre order traversal. + * + * @returns The next node, or null if the current node is + * not set or there is no next value. + */ + in(): ASTNode | null { + const curNode = this.getCurNode(); + if (!curNode) { + return null; + } + const newNode = this.getNextNode(curNode, this.validInLineNode.bind(this)); + + if (newNode) { + this.setCurNode(newNode); + } + return newNode; + } + /** + * Moves the cursor to the previous next connection or previous connection in + * the pre order traversal. + * + * @returns The previous node, or null if the current node + * is not set or there is no previous value. + */ + prev(): ASTNode | null { + const curNode = this.getCurNode(); + if (!curNode) { + return null; + } + const newNode = this.getPreviousNode( + curNode, + this.validLineNode.bind(this), + ); + + if (newNode) { + this.setCurNode(newNode); + } + return newNode; + } + + /** + * Moves the cursor to the previous input connection or field in the pre order + * traversal. + * + * @returns The previous node, or null if the current node + * is not set or there is no previous value. + */ + out(): ASTNode | null { + const curNode = this.getCurNode(); + if (!curNode) { + return null; + } + const newNode = this.getPreviousNode( + curNode, + this.validInLineNode.bind(this), + ); + + if (newNode) { + this.setCurNode(newNode); + } + return newNode; + } + + /** + * Returns true iff the node to which we would navigate if in() were + * called, which will be a validInLineNode, is also a validLineNode + * - in effect, if the LineCursor is at the end of the 'current + * line' of the program. + */ + atEndOfLine(): boolean { + const curNode = this.getCurNode(); + if (!curNode) return false; + const rightNode = this.getNextNode( + curNode, + this.validInLineNode.bind(this), + ); + return this.validLineNode(rightNode); + } + + /** + * Returns true iff the given node represents the "beginning of a + * new line of code" (and thus can be visited by pressing the + * up/down arrow keys). Specifically, if the node is for: + * + * - Any block that is not a value block. + * - A top-level value block (one that is unconnected). + * - An unconnected next statement input. + * - An unconnected 'next' connection - the "blank line at the end". + * This is to facilitate connecting additional blocks to a + * stack/substack. + * + * If options.stackConnections is true (the default) then allow the + * cursor to visit all (useful) stack connection by additionally + * returning true for: + * + * - Any next statement input + * - Any 'next' connection. + * - An unconnected previous statement input. + * + * @param node The AST node to check. + * @returns True if the node should be visited, false otherwise. + */ + protected validLineNode(node: ASTNode | null): boolean { + if (!node) return false; + const location = node.getLocation(); + const type = node && node.getType(); + switch (type) { + case ASTNode.types.BLOCK: + return !(location as Block).outputConnection?.isConnected(); + case ASTNode.types.INPUT: { + const connection = location as Connection; + return ( + connection.type === ConnectionType.NEXT_STATEMENT && + (this.options.stackConnections || !connection.isConnected()) + ); + } + case ASTNode.types.NEXT: + return ( + this.options.stackConnections || + !(location as Connection).isConnected() + ); + case ASTNode.types.PREVIOUS: + return ( + this.options.stackConnections && + !(location as Connection).isConnected() + ); + default: + return false; + } + } + + /** + * Returns true iff the given node can be visited by the cursor when + * using the left/right arrow keys. Specifically, if the node is + * any node for which valideLineNode would return true, plus: + * + * - Any block. + * - Any field that is not a full block field. + * - Any unconnected next or input connection. This is to + * facilitate connecting additional blocks. + * + * @param node The AST node to check whether it is valid. + * @returns True if the node should be visited, false otherwise. + */ + protected validInLineNode(node: ASTNode | null): boolean { + if (!node) return false; + if (this.validLineNode(node)) return true; + const location = node.getLocation(); + const type = node && node.getType(); + switch (type) { + case ASTNode.types.BLOCK: + return true; + case ASTNode.types.INPUT: + return !(location as Connection).isConnected(); + case ASTNode.types.FIELD: { + const field = node.getLocation() as Field; + return !( + field.getSourceBlock()?.isSimpleReporter() && field.isFullBlockField() + ); + } + default: + return false; + } + } + + /** + * Returns true iff the given node can be visited by the cursor. + * Specifically, if the node is any for which validInLineNode would + * return true, or if it is a workspace node. + * + * @param node The AST node to check whether it is valid. + * @returns True if the node should be visited, false otherwise. + */ + protected validNode(node: ASTNode | null): boolean { + return ( + !!node && + (this.validInLineNode(node) || node.getType() === ASTNode.types.WORKSPACE) + ); + } + + /** + * Uses pre order traversal to navigate the Blockly AST. This will allow + * a user to easily navigate the entire Blockly AST without having to go in + * and out levels on the tree. + * + * @param node The current position in the AST. + * @param isValid A function true/false depending on whether the given node + * should be traversed. + * @returns The next node in the traversal. + */ + private getNextNode( + node: ASTNode | null, + isValid: (p1: ASTNode | null) => boolean, + ): ASTNode | null { + if (!node) { + return null; + } + const newNode = node.in() || node.next(); + if (isValid(newNode)) { + return newNode; + } else if (newNode) { + return this.getNextNode(newNode, isValid); + } + const siblingOrParentSibling = this.findSiblingOrParentSibling(node.out()); + if (isValid(siblingOrParentSibling)) { + return siblingOrParentSibling; + } else if (siblingOrParentSibling) { + return this.getNextNode(siblingOrParentSibling, isValid); + } + return null; + } + + /** + * Reverses the pre order traversal in order to find the previous node. This + * will allow a user to easily navigate the entire Blockly AST without having + * to go in and out levels on the tree. + * + * @param node The current position in the AST. + * @param isValid A function true/false depending on whether the given node + * should be traversed. + * @returns The previous node in the traversal or null if no previous node + * exists. + */ + private getPreviousNode( + node: ASTNode | null, + isValid: (p1: ASTNode | null) => boolean, + ): ASTNode | null { + if (!node) { + return null; + } + let newNode: ASTNode | null = node.prev(); + + if (newNode) { + newNode = this.getRightMostChild(newNode); + } else { + newNode = node.out(); + } + if (isValid(newNode)) { + return newNode; + } else if (newNode) { + return this.getPreviousNode(newNode, isValid); + } + return null; + } + + /** + * From the given node find either the next valid sibling or the parent's + * next sibling. + * + * @param node The current position in the AST. + * @returns The next sibling node, the parent's next sibling, or null. + */ + private findSiblingOrParentSibling(node: ASTNode | null): ASTNode | null { + if (!node) { + return null; + } + const nextNode = node.next(); + if (nextNode) { + return nextNode; + } + return this.findSiblingOrParentSibling(node.out()); + } + + /** + * Get the right most child of a node. + * + * @param node The node to find the right most child of. + * @returns The right most child of the given node, or the node if no child + * exists. + */ + private getRightMostChild(node: ASTNode): ASTNode | null { + let newNode = node.in(); + if (!newNode) { + return node; + } + for ( + let nextNode: ASTNode | null = newNode; + nextNode; + nextNode = newNode.next() + ) { + newNode = nextNode; + } + return this.getRightMostChild(newNode); + } + + /** + * Prepare for the deletion of a block by making a list of nodes we + * could move the cursor to afterwards and save it to + * this.potentialNodes. + * + * After the deletion has occurred, call postDelete to move it to + * the first valid node on that list. + * + * The locations to try (in order of preference) are: + * + * - The current location. + * - The connection to which the deleted block is attached. + * - The block connected to the next connection of the deleted block. + * - The parent block of the deleted block. + * - A location on the workspace beneath the deleted block. + * + * N.B.: When block is deleted, all of the blocks conneccted to that + * block's inputs are also deleted, but not blocks connected to its + * next connection. + * + * @param deletedBlock The block that is being deleted. + */ + preDelete(deletedBlock: Block) { + const curNode = this.getCurNode(); + + const nodes: ASTNode[] = curNode ? [curNode] : []; + // The connection to which the deleted block is attached. + const parentConnection = + deletedBlock.previousConnection?.targetConnection ?? + deletedBlock.outputConnection?.targetConnection; + if (parentConnection) { + const parentNode = ASTNode.createConnectionNode(parentConnection); + if (parentNode) nodes.push(parentNode); + } + // The block connected to the next connection of the deleted block. + const nextBlock = deletedBlock.getNextBlock(); + if (nextBlock) { + const nextNode = ASTNode.createBlockNode(nextBlock); + if (nextNode) nodes.push(nextNode); + } + // The parent block of the deleted block. + const parentBlock = deletedBlock.getParent(); + if (parentBlock) { + const parentNode = ASTNode.createBlockNode(parentBlock); + if (parentNode) nodes.push(parentNode); + } + // A location on the workspace beneath the deleted block. + // Move to the workspace. + const curBlock = curNode?.getSourceBlock(); + if (curBlock) { + const workspaceNode = ASTNode.createWorkspaceNode( + this.workspace, + curBlock.getRelativeToSurfaceXY(), + ); + if (workspaceNode) nodes.push(workspaceNode); + } + this.potentialNodes = nodes; + } + + /** + * Move the cursor to the first valid location in + * this.potentialNodes, following a block deletion. + */ + postDelete() { + const nodes = this.potentialNodes; + this.potentialNodes = null; + if (!nodes) throw new Error('must call preDelete first'); + for (const node of nodes) { + if (this.validNode(node) && !node.getSourceBlock()?.disposed) { + this.setCurNode(node); + return; + } + } + throw new Error('no valid nodes in this.potentialNodes'); + } + + /** + * Get the current location of the cursor. + * + * Overrides normal Marker getCurNode to update the current node from the + * selected block. This typically happens via the selection listener but that + * is not called immediately when `Gesture` calls + * `Blockly.common.setSelected`. In particular the listener runs after showing + * the context menu. + * + * @returns The current field, connection, or block the cursor is on. + */ + override getCurNode(): ASTNode | null { + this.updateCurNodeFromSelection(); + return super.getCurNode(); + } + + /** + * Sets the object in charge of drawing the marker. + * + * We want to customize drawing, so rather than directly setting the given + * object, we instead set a wrapper proxy object that passes through all + * method calls and property accesses except for draw(), which it delegates + * to the drawMarker() method in this class. + * + * @param drawer The object ~in charge of drawing the marker. + */ + override setDrawer(drawer: MarkerSvg) { + const altDraw = function ( + this: LineCursor, + oldNode: ASTNode | null, + curNode: ASTNode | null, + ) { + // Pass the unproxied, raw drawer object so that drawMarker can call its + // `draw()` method without triggering infinite recursion. + this.drawMarker(oldNode, curNode, drawer); + }.bind(this); + + super.setDrawer( + new Proxy(drawer, { + get(target: typeof drawer, prop: keyof typeof drawer) { + if (prop === 'draw') { + return altDraw; + } + + return target[prop]; + }, + }), + ); + } + + /** + * Set the location of the cursor and draw it. + * + * Overrides normal Marker setCurNode logic to call + * this.drawMarker() instead of this.drawer.draw() directly. + * + * @param newNode The new location of the cursor. + * @param updateSelection If true (the default) we'll update the selection + * too. + */ + override setCurNode(newNode: ASTNode | null, updateSelection = true) { + if (updateSelection) { + this.updateSelectionFromNode(newNode); + } + + super.setCurNode(newNode); + + // Try to scroll cursor into view. + if (newNode?.getType() === ASTNode.types.BLOCK) { + const block = newNode.getLocation() as BlockSvg; + block.workspace.scrollBoundsIntoView( + block.getBoundingRectangleWithoutChildren(), + ); + } + } + + /** + * Draw this cursor's marker. + * + * This is a wrapper around this.drawer.draw (usually implemented by + * MarkerSvg.prototype.draw) that will, if newNode is a BLOCK node, + * instead call `setSelected` to select it (if it's a regular block) + * or `addSelect` (if it's a shadow block, since shadow blocks can't + * be selected) instead of using the normal drawer logic. + * + * TODO(#142): The selection and fake-selection code was originally + * a hack added for testing on October 28 2024, because the default + * drawer (MarkerSvg) behaviour in Zelos was to draw a box around + * the block and all attached child blocks, which was confusing when + * navigating stacks. + * + * Since then we have decided that we probably _do_ in most cases + * want navigating to a block to select the block, but more + * particularly that we want navigation to move _focus_. Replace + * this selection hack with non-hacky changing of focus once that's + * possible. + * + * @param oldNode The previous node. + * @param curNode The current node. + * @param realDrawer The object ~in charge of drawing the marker. + */ + private drawMarker( + oldNode: ASTNode | null, + curNode: ASTNode | null, + realDrawer: MarkerSvg, + ) { + // If old node was a block, unselect it or remove fake selection. + if (oldNode?.getType() === ASTNode.types.BLOCK) { + const block = oldNode.getLocation() as BlockSvg; + if (!block.isShadow()) { + // Selection should already be in sync. + } else { + block.removeSelect(); + } + } + + if (this.isZelos && oldNode && this.isValueInputConnection(oldNode)) { + this.hideAtInput(oldNode); + } + + const curNodeType = curNode?.getType(); + const isZelosInputConnection = + this.isZelos && curNode && this.isValueInputConnection(curNode); + + // If drawing can't be handled locally, just use the drawer. + if (curNodeType !== ASTNode.types.BLOCK && !isZelosInputConnection) { + realDrawer.draw(oldNode, curNode); + return; + } + + // Hide any visible marker SVG and instead do some manual rendering. + realDrawer.hide(); + + if (isZelosInputConnection) { + this.showAtInput(curNode); + } else if (curNode && curNodeType === ASTNode.types.BLOCK) { + const block = curNode.getLocation() as BlockSvg; + if (!block.isShadow()) { + // Selection should already be in sync. + } else { + block.addSelect(); + block.getParent()?.removeSelect(); + } + } + + // Call MarkerSvg.prototype.fireMarkerEvent like + // MarkerSvg.prototype.draw would (even though it's private). + (realDrawer as any)?.fireMarkerEvent?.(oldNode, curNode); + } + + /** + * Check whether the node represents a value input connection. + * + * @param node The node to check + * @returns True if the node represents a value input connection. + */ + private isValueInputConnection(node: ASTNode) { + if (node?.getType() !== ASTNode.types.INPUT) return false; + const connection = node.getLocation() as Connection; + return connection.type === ConnectionType.INPUT_VALUE; + } + + /** + * Hide the cursor rendering at the given input node. + * + * @param node The input node to hide. + */ + private hideAtInput(node: ASTNode) { + const inputConnection = node.getLocation() as Connection; + const sourceBlock = inputConnection.getSourceBlock() as BlockSvg; + const input = inputConnection.getParentInput(); + if (input) { + const pathObject = sourceBlock.pathObject as PathObject; + const outlinePath = pathObject.getOutlinePath(input.name); + dom.removeClass(outlinePath, 'inputActiveFocus'); + } + } + + /** + * Show the cursor rendering at the given input node. + * + * @param node The input node to show. + */ + private showAtInput(node: ASTNode) { + const inputConnection = node.getLocation() as Connection; + const sourceBlock = inputConnection.getSourceBlock() as BlockSvg; + const input = inputConnection.getParentInput(); + if (input) { + const pathObject = sourceBlock.pathObject as PathObject; + const outlinePath = pathObject.getOutlinePath(input.name); + dom.addClass(outlinePath, 'inputActiveFocus'); + } + } + + /** + * Event listener that syncs the cursor location to the selected block on + * SELECTED events. + * + * This does not run early enough in all cases so `getCurNode()` also updates + * the node from the selection. + * + * @param event The `Selected` event. + */ + private changeListener(event: Abstract) { + switch (event.type) { + case EventType.SELECTED: + this.updateCurNodeFromSelection(); + break; + case EventType.CLICK: { + const click = event as Click; + if ( + click.workspaceId === this.workspace.id && + click.targetType === ClickTarget.WORKSPACE + ) { + this.setCurNode(null); + } + } + } + } + + /** + * Updates the current node to match the selection. + * + * Clears the current node if it's on a block but the selection is null. + * Sets the node to a block if selected for our workspace. + * For shadow blocks selections the parent is used by default (unless we're + * already on the shadow block via keyboard) as that's where the visual + * selection is. + */ + private updateCurNodeFromSelection() { + const curNode = super.getCurNode(); + const selected = common.getSelected(); + + if (selected === null && curNode?.getType() === ASTNode.types.BLOCK) { + this.setCurNode(null, false); + return; + } + if (selected?.workspace !== this.workspace) { + return; + } + if (selected instanceof BlockSvg) { + let block: BlockSvg | null = selected; + if (selected.isShadow()) { + // OK if the current node is on the parent OR the shadow block. + // The former happens for clicks, the latter for keyboard nav. + if ( + curNode && + (curNode.getLocation() === block || + curNode.getLocation() === block.getParent()) + ) { + return; + } + block = block.getParent(); + } + if (block) { + this.setCurNode(ASTNode.createBlockNode(block), false); + } + } + } + + /** + * Updates the selection from the node. + * + * Clears the selection for non-block nodes. + * Clears the selection for shadow blocks as the selection is drawn on + * the parent but the cursor will be drawn on the shadow block itself. + * We need to take care not to later clear the current node due to that null + * selection, so we track the latest selection we're in sync with. + * + * @param newNode The new node. + */ + private updateSelectionFromNode(newNode: ASTNode | null) { + if (newNode?.getType() === ASTNode.types.BLOCK) { + if (common.getSelected() !== newNode.getLocation()) { + eventUtils.disable(); + common.setSelected(newNode.getLocation() as BlockSvg); + eventUtils.enable(); + } + } else { + if (common.getSelected()) { + eventUtils.disable(); + common.setSelected(null); + eventUtils.enable(); + } + } + } +} + +registry.register(registry.Type.CURSOR, registry.DEFAULT, LineCursor); diff --git a/core/marker_manager.ts b/core/marker_manager.ts index 51183242d..a8d1d20c2 100644 --- a/core/marker_manager.ts +++ b/core/marker_manager.ts @@ -11,7 +11,7 @@ */ // Former goog.module ID: Blockly.MarkerManager -import type {Cursor} from './keyboard_nav/cursor.js'; +import type {LineCursor} from './keyboard_nav/line_cursor.js'; import type {Marker} from './keyboard_nav/marker.js'; import type {WorkspaceSvg} from './workspace_svg.js'; @@ -23,7 +23,7 @@ export class MarkerManager { static readonly LOCAL_MARKER = 'local_marker_1'; /** The cursor. */ - private cursor: Cursor | null = null; + private cursor: LineCursor | null = null; /** The cursor's SVG element. */ private cursorSvg: SVGElement | null = null; @@ -83,7 +83,7 @@ export class MarkerManager { * * @returns The cursor for this workspace. */ - getCursor(): Cursor | null { + getCursor(): LineCursor | null { return this.cursor; } @@ -104,7 +104,7 @@ export class MarkerManager { * * @param cursor The cursor used to move around this workspace. */ - setCursor(cursor: Cursor) { + setCursor(cursor: LineCursor) { this.cursor?.getDrawer()?.dispose(); this.cursor = cursor; if (this.cursor) { diff --git a/core/registry.ts b/core/registry.ts index e026514f1..2b00b775d 100644 --- a/core/registry.ts +++ b/core/registry.ts @@ -26,7 +26,7 @@ import type { IVariableModelStatic, IVariableState, } from './interfaces/i_variable_model.js'; -import type {Cursor} from './keyboard_nav/cursor.js'; +import type {LineCursor} from './keyboard_nav/line_cursor.js'; import type {Options} from './options.js'; import type {Renderer} from './renderers/common/renderer.js'; import type {Theme} from './theme.js'; @@ -78,7 +78,7 @@ export class Type<_T> { 'connectionPreviewer', ); - static CURSOR = new Type('cursor'); + static CURSOR = new Type('cursor'); static EVENT = new Type('event'); diff --git a/core/renderers/zelos/path_object.ts b/core/renderers/zelos/path_object.ts index cf6fff7e8..f40426483 100644 --- a/core/renderers/zelos/path_object.ts +++ b/core/renderers/zelos/path_object.ts @@ -161,10 +161,11 @@ export class PathObject extends BasePathObject { /** * Create's an outline path for the specified input. * + * @internal * @param name The input name. * @returns The SVG outline path. */ - private getOutlinePath(name: string): SVGElement { + getOutlinePath(name: string): SVGElement { if (!this.outlines.has(name)) { this.outlines.set( name, diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index 78506115e..68a1bd939 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -49,7 +49,7 @@ import type { IVariableModel, IVariableState, } from './interfaces/i_variable_model.js'; -import type {Cursor} from './keyboard_nav/cursor.js'; +import type {LineCursor} from './keyboard_nav/line_cursor.js'; import type {Marker} from './keyboard_nav/marker.js'; import {LayerManager} from './layer_manager.js'; import {MarkerManager} from './marker_manager.js'; @@ -487,7 +487,7 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg { * * @returns The cursor for the workspace. */ - getCursor(): Cursor | null { + getCursor(): LineCursor | null { if (this.markerManager) { return this.markerManager.getCursor(); } @@ -828,7 +828,7 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg { this.options, ); - if (CursorClass) this.markerManager.setCursor(new CursorClass()); + if (CursorClass) this.markerManager.setCursor(new CursorClass(this)); const isParentWorkspace = this.options.parentWorkspace === null; this.renderer.createDom( @@ -2541,6 +2541,71 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg { this.removeClass('blocklyReadOnly'); } } + + /** + * Scrolls the provided bounds into view. + * + * In the case of small workspaces/large bounds, this function prioritizes + * getting the top left corner of the bounds into view. It also adds some + * padding around the bounds to allow the element to be comfortably in view. + * + * @internal + * @param bounds A rectangle to scroll into view, as best as possible. + * @param padding Amount of spacing to put between the bounds and the edge of + * the workspace's viewport. + */ + scrollBoundsIntoView(bounds: Rect, padding = 10) { + if (Gesture.inProgress()) { + // This can cause jumps during a drag and is only suited for keyboard nav. + return; + } + const scale = this.getScale(); + + const rawViewport = this.getMetricsManager().getViewMetrics(true); + const viewport = new Rect( + rawViewport.top, + rawViewport.top + rawViewport.height, + rawViewport.left, + rawViewport.left + rawViewport.width, + ); + + if ( + bounds.left >= viewport.left && + bounds.top >= viewport.top && + bounds.right <= viewport.right && + bounds.bottom <= viewport.bottom + ) { + // Do nothing if the block is fully inside the viewport. + return; + } + + // Add some padding to the bounds so the element is scrolled comfortably + // into view. + bounds = bounds.clone(); + bounds.top -= padding; + bounds.bottom += padding; + bounds.left -= padding; + bounds.right += padding; + + let deltaX = 0; + let deltaY = 0; + + if (bounds.left < viewport.left) { + deltaX = viewport.left - bounds.left; + } else if (bounds.right > viewport.right) { + deltaX = viewport.right - bounds.right; + } + + if (bounds.top < viewport.top) { + deltaY = viewport.top - bounds.top; + } else if (bounds.bottom > viewport.bottom) { + deltaY = viewport.bottom - bounds.bottom; + } + + deltaX *= scale; + deltaY *= scale; + this.scroll(this.scrollX + deltaX, this.scrollY + deltaY); + } } /** diff --git a/tests/mocha/cursor_test.js b/tests/mocha/cursor_test.js index bb5026d7a..3242edd2a 100644 --- a/tests/mocha/cursor_test.js +++ b/tests/mocha/cursor_test.js @@ -21,21 +21,21 @@ suite('Cursor', function () { 'args0': [ { 'type': 'field_input', - 'name': 'NAME', + 'name': 'NAME1', 'text': 'default', }, { 'type': 'field_input', - 'name': 'NAME', + 'name': 'NAME2', 'text': 'default', }, { 'type': 'input_value', - 'name': 'NAME', + 'name': 'NAME3', }, { 'type': 'input_statement', - 'name': 'NAME', + 'name': 'NAME4', }, ], 'previousStatement': null, @@ -84,23 +84,24 @@ suite('Cursor', function () { sharedTestTeardown.call(this); }); - test('Next - From a Previous skip over next connection and block', function () { + test('Next - From a Previous connection go to the next block', function () { const prevNode = ASTNode.createConnectionNode( this.blocks.A.previousConnection, ); this.cursor.setCurNode(prevNode); this.cursor.next(); const curNode = this.cursor.getCurNode(); - assert.equal(curNode.getLocation(), this.blocks.B.previousConnection); + assert.equal(curNode.getLocation(), this.blocks.A); }); - test('Next - From last block in a stack go to next connection', function () { - const prevNode = ASTNode.createConnectionNode( - this.blocks.B.previousConnection, - ); + test('Next - From a block go to its statement input', function () { + const prevNode = ASTNode.createBlockNode(this.blocks.B); this.cursor.setCurNode(prevNode); this.cursor.next(); const curNode = this.cursor.getCurNode(); - assert.equal(curNode.getLocation(), this.blocks.B.nextConnection); + assert.equal( + curNode.getLocation(), + this.blocks.B.getInput('NAME4').connection, + ); }); test('In - From output connection', function () { @@ -111,24 +112,24 @@ suite('Cursor', function () { this.cursor.setCurNode(outputNode); this.cursor.in(); const curNode = this.cursor.getCurNode(); - assert.equal(curNode.getLocation(), fieldBlock.inputList[0].fieldRow[0]); + assert.equal(curNode.getLocation(), fieldBlock); }); - test('Prev - From previous connection skip over next connection', function () { + test('Prev - From previous connection does not skip over next connection', function () { const prevConnection = this.blocks.B.previousConnection; const prevConnectionNode = ASTNode.createConnectionNode(prevConnection); this.cursor.setCurNode(prevConnectionNode); this.cursor.prev(); const curNode = this.cursor.getCurNode(); - assert.equal(curNode.getLocation(), this.blocks.A.previousConnection); + assert.equal(curNode.getLocation(), this.blocks.A.nextConnection); }); - test('Out - From field skip over block node', function () { + test('Out - From field does not skip over block node', function () { const field = this.blocks.E.inputList[0].fieldRow[0]; const fieldNode = ASTNode.createFieldNode(field); this.cursor.setCurNode(fieldNode); this.cursor.out(); const curNode = this.cursor.getCurNode(); - assert.equal(curNode.getLocation(), this.blocks.E.outputConnection); + assert.equal(curNode.getLocation(), this.blocks.E); }); });