From 41bc01a14e59a8c00ff1e2e627c79272fb395346 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Wed, 23 Apr 2025 21:14:06 +0000 Subject: [PATCH 1/8] feat: Make RenderedConnection focusable. Note that this doesn't yet contain all of the changes needed in order to ensure that this works correctly. --- core/connection.ts | 4 ++ core/rendered_connection.ts | 94 +++++++++++++++++++++++++++++++++++-- 2 files changed, 95 insertions(+), 3 deletions(-) diff --git a/core/connection.ts b/core/connection.ts index 039d8822c01..d28099d1d69 100644 --- a/core/connection.ts +++ b/core/connection.ts @@ -20,6 +20,7 @@ import type {Input} from './inputs/input.js'; import type {IASTNodeLocationWithBlock} from './interfaces/i_ast_node_location_with_block.js'; import type {IConnectionChecker} from './interfaces/i_connection_checker.js'; import * as blocks from './serialization/blocks.js'; +import { idGenerator } from './utils.js'; import * as Xml from './xml.js'; /** @@ -55,6 +56,8 @@ export class Connection implements IASTNodeLocationWithBlock { /** DOM representation of a shadow block, or null if none. */ private shadowDom: Element | null = null; + id: string; + /** * Horizontal location of this connection. * @@ -80,6 +83,7 @@ export class Connection implements IASTNodeLocationWithBlock { public type: number, ) { this.sourceBlock_ = source; + this.id = `${source.id}_connection_${idGenerator.getNextUniqueId()}`; } /** diff --git a/core/rendered_connection.ts b/core/rendered_connection.ts index 168e59744d2..de6b6df2bad 100644 --- a/core/rendered_connection.ts +++ b/core/rendered_connection.ts @@ -17,6 +17,8 @@ import * as common from './common.js'; import {config} from './config.js'; import {Connection} from './connection.js'; import type {ConnectionDB} from './connection_db.js'; +import type {IFocusableNode} from './interfaces/i_focusable_node.js'; +import type {IFocusableTree} from './interfaces/i_focusable_tree.js'; import {ConnectionType} from './connection_type.js'; import * as ContextMenu from './contextmenu.js'; import {ContextMenuRegistry} from './contextmenu_registry.js'; @@ -25,6 +27,11 @@ import {IContextMenu} from './interfaces/i_contextmenu.js'; import {hasBubble} from './interfaces/i_has_bubble.js'; import * as internalConstants from './internal_constants.js'; import {Coordinate} from './utils/coordinate.js'; +import {WorkspaceSvg} from './workspace_svg.js'; +import * as dom from './utils/dom.js'; +import {Svg} from './utils/svg.js'; +import * as svgPaths from './utils/svg_paths.js'; +import type {ConstantProvider, PuzzleTab} from './renderers/common/constants.js'; import * as svgMath from './utils/svg_math.js'; /** Maximum randomness in workspace units for bumping a block. */ @@ -33,7 +40,9 @@ const BUMP_RANDOMNESS = 10; /** * Class for a connection between blocks that may be rendered on screen. */ -export class RenderedConnection extends Connection implements IContextMenu { +export class RenderedConnection + extends Connection + implements IContextMenu, IFocusableNode { // TODO(b/109816955): remove '!', see go/strict-prop-init-fix. sourceBlock_!: BlockSvg; private readonly db: ConnectionDB; @@ -41,6 +50,9 @@ export class RenderedConnection extends Connection implements IContextMenu { private readonly offsetInBlock: Coordinate; private trackedState: TrackedState; private highlighted: boolean = false; + private constants: ConstantProvider; + private svgGroup: SVGElement; + private svgPath: SVGElement; /** Connection this connection connects to. Null if not connected. */ override targetConnection: RenderedConnection | null = null; @@ -70,6 +82,43 @@ export class RenderedConnection extends Connection implements IContextMenu { /** Describes the state of this connection's tracked-ness. */ this.trackedState = RenderedConnection.TrackedState.WILL_TRACK; + + this.constants = (source.workspace as WorkspaceSvg).getRenderer().getConstants(); + + this.svgGroup = dom.createSvgElement( + Svg.G, + { + 'class': 'blocklyCursor', + 'width': this.constants.CURSOR_WS_WIDTH, + 'height': this.constants.WS_CURSOR_HEIGHT, + } + ); + + this.svgPath = dom.createSvgElement( + Svg.PATH, + {'transform': ''}, + this.svgGroup, + ); + + // TODO: Ensure this auto-moves with the block. + const x = this.getOffsetInBlock().x; + const y = this.getOffsetInBlock().y; + + const path = + svgPaths.moveTo(0, 0) + + 'c 0,10 -8,-8 -8,7.5 s 8,-2.5 8,7.5'; + // TODO: It seems that constants isn't yet initialized at this point. + // (this.constants.shapeFor(this) as PuzzleTab).pathDown; + this.svgPath.setAttribute('d', path); + this.svgPath.setAttribute( + 'transform', + 'translate(' + + x + + ',' + + y + + ')' + + (this.sourceBlock_.workspace.RTL ? ' scale(-1 1)' : ''), + ); } /** @@ -320,13 +369,21 @@ export class RenderedConnection extends Connection implements IContextMenu { /** Add highlighting around this connection. */ highlight() { this.highlighted = true; - this.getSourceBlock().queueRender(); + const highlightSvg = this.findHighlightSvg(); + // if (highlightSvg) { + // highlightSvg.style.display = ''; + // } + // this.getSourceBlock().queueRender(); } /** Remove the highlighting around this connection. */ unhighlight() { this.highlighted = false; - this.getSourceBlock().queueRender(); + const highlightSvg = this.findHighlightSvg(); + // if (highlightSvg) { + // highlightSvg.style.display = 'none'; + // } + // this.getSourceBlock().queueRender(); } /** Returns true if this connection is highlighted, false otherwise. */ @@ -593,6 +650,12 @@ export class RenderedConnection extends Connection implements IContextMenu { return this; } + private findHighlightSvg(): SVGElement | null { + // This cast is valid as TypeScript's definition is wrong. See: + // https://github.com/microsoft/TypeScript/issues/60996. + return document.getElementById(this.id) as unknown | null as SVGElement | null; + } + /** * Handles showing the context menu when it is opened on a connection. * Note that typically the context menu can't be opened with the mouse @@ -626,6 +689,31 @@ export class RenderedConnection extends Connection implements IContextMenu { ContextMenu.show(e, menuOptions, block.RTL, workspace, location); } + + /** See IFocusableNode.getFocusableElement. */ + getFocusableElement(): HTMLElement | SVGElement { + const highlightSvg = this.findHighlightSvg(); + if (highlightSvg) { + highlightSvg.setAttribute('aria-label', 'Connection'); + return highlightSvg; + } + throw new Error('No highlight SVG found corresponding to this connection.'); + } + + /** See IFocusableNode.getFocusableTree. */ + getFocusableTree(): IFocusableTree { + return this.getSourceBlock().workspace as WorkspaceSvg; + } + + /** See IFocusableNode.onNodeFocus. */ + onNodeFocus(): void { + this.highlight(); + } + + /** See IFocusableNode.onNodeBlur. */ + onNodeBlur(): void { + this.unhighlight(); + } } export namespace RenderedConnection { From 7f14372f67336568e89ec147d05c5f36c1f7964d Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Thu, 24 Apr 2025 20:40:38 +0000 Subject: [PATCH 2/8] feat: add remaining connection support There's a lot of clean-up and simplification work needed yet. --- core/css.ts | 8 +++---- core/renderers/common/constants.ts | 6 +++--- core/renderers/common/drawer.ts | 20 ++++++++++------- core/renderers/common/i_path_object.ts | 2 +- core/renderers/common/path_object.ts | 30 ++++++++++++++++++-------- core/renderers/zelos/constants.ts | 6 +++--- core/renderers/zelos/drawer.ts | 7 +++--- core/renderers/zelos/path_object.ts | 30 +++++++++++++------------- core/workspace_svg.ts | 10 +++++++++ 9 files changed, 72 insertions(+), 47 deletions(-) diff --git a/core/css.ts b/core/css.ts index 6ca262f3b25..ebdc613c050 100644 --- a/core/css.ts +++ b/core/css.ts @@ -147,8 +147,8 @@ let content = ` .blocklyHighlightedConnectionPath { fill: none; - stroke: #fc3; - stroke-width: 4px; + // stroke: #fc3; + // stroke-width: 4px; } .blocklyPathLight { @@ -463,8 +463,8 @@ input[type=number] { } .blocklyMenuSeparator { - background-color: #ccc; - height: 1px; + background-color: #ccc; + height: 1px; border: 0; margin-left: 4px; margin-right: 4px; diff --git a/core/renderers/common/constants.ts b/core/renderers/common/constants.ts index c5a7a759c5c..c8f770cc407 100644 --- a/core/renderers/common/constants.ts +++ b/core/renderers/common/constants.ts @@ -1203,9 +1203,9 @@ export class ConstantProvider { `}`, // Connection highlight. - `${selector} .blocklyHighlightedConnectionPath {`, - `stroke: #fc3;`, - `}`, + // `${selector} .blocklyHighlightedConnectionPath {`, + // `stroke: #fc3;`, + // `}`, // Replaceable highlight. `${selector} .blocklyReplaceable .blocklyPath {`, diff --git a/core/renderers/common/drawer.ts b/core/renderers/common/drawer.ts index 09320710c51..f805451a725 100644 --- a/core/renderers/common/drawer.ts +++ b/core/renderers/common/drawer.ts @@ -435,19 +435,23 @@ export class Drawer { for (const elem of row.elements) { if (!(elem instanceof Connection)) continue; - if (elem.highlighted) { - this.drawConnectionHighlightPath(elem); - } else { - this.block_.pathObject.removeConnectionHighlight?.( - elem.connectionModel, - ); + const highlightSvg = this.drawConnectionHighlightPath(elem); + if (highlightSvg) { + // highlightSvg.style.display = elem.highlighted ? '' : 'none'; } + // if (elem.highlighted) { + // this.drawConnectionHighlightPath(elem); + // } else { + // this.block_.pathObject.removeConnectionHighlight?.( + // elem.connectionModel, + // ); + // } } } } /** Returns a path to highlight the given connection. */ - drawConnectionHighlightPath(measurable: Connection) { + drawConnectionHighlightPath(measurable: Connection): SVGElement | undefined { const conn = measurable.connectionModel; let path = ''; if ( @@ -459,7 +463,7 @@ export class Drawer { path = this.getStatementConnectionHighlightPath(measurable); } const block = conn.getSourceBlock(); - block.pathObject.addConnectionHighlight?.( + return block.pathObject.addConnectionHighlight?.( conn, path, conn.getOffsetInBlock(), diff --git a/core/renderers/common/i_path_object.ts b/core/renderers/common/i_path_object.ts index 699f1d92edb..776ba0067ea 100644 --- a/core/renderers/common/i_path_object.ts +++ b/core/renderers/common/i_path_object.ts @@ -113,7 +113,7 @@ export interface IPathObject { connectionPath: string, offset: Coordinate, rtl: boolean, - ): void; + ): SVGElement; /** * Apply the stored colours to the block's path, taking into account whether diff --git a/core/renderers/common/path_object.ts b/core/renderers/common/path_object.ts index 72cf2a594ce..d216db1a221 100644 --- a/core/renderers/common/path_object.ts +++ b/core/renderers/common/path_object.ts @@ -153,13 +153,17 @@ export class PathObject implements IPathObject { * removed. */ protected setClass_(className: string, add: boolean) { + this.setClassOnElem_(this.svgRoot, className, add); + } + + private setClassOnElem_(root: SVGElement, className: string, add: boolean) { if (!className) { return; } if (add) { - dom.addClass(this.svgRoot, className); + dom.addClass(root, className); } else { - dom.removeClass(this.svgRoot, className); + dom.removeClass(root, className); } } @@ -209,7 +213,7 @@ export class PathObject implements IPathObject { * @param enable True if selection is enabled, false otherwise. */ updateSelected(enable: boolean) { - this.setClass_('blocklySelected', enable); + this.setClassOnElem_(this.svgPath, 'blocklySelected', enable); } /** @@ -268,25 +272,33 @@ export class PathObject implements IPathObject { connectionPath: string, offset: Coordinate, rtl: boolean, - ) { - if (this.connectionHighlights.has(connection)) { - if (this.currentHighlightMatchesNew(connection, connectionPath, offset)) { - return; - } - this.removeConnectionHighlight(connection); + ): SVGElement { + const previousHighlight = this.connectionHighlights.get(connection); + if (previousHighlight) { + // TODO: Fix the highlight seemingly being recreated every time it's focused. + // if (this.currentHighlightMatchesNew(connection, connectionPath, offset)) { + return previousHighlight; + // } + // this.removeConnectionHighlight(connection); } const highlight = dom.createSvgElement( Svg.PATH, { + 'id': connection.id, 'class': 'blocklyHighlightedConnectionPath', + // 'style': 'display: none;', + 'tabindex': '-1', 'd': connectionPath, 'transform': `translate(${offset.x}, ${offset.y})` + (rtl ? ' scale(-1 1)' : ''), }, this.svgRoot, ); + // TODO: Do this in a cleaner way. One possibility: create the path without 'd' or 'transform' in RenderedConnection, then just update it here (and keep registrations). + (highlight as any).renderedConnection = connection; this.connectionHighlights.set(connection, highlight); + return highlight; } private currentHighlightMatchesNew( diff --git a/core/renderers/zelos/constants.ts b/core/renderers/zelos/constants.ts index 8cd36e02589..d7be09b93de 100644 --- a/core/renderers/zelos/constants.ts +++ b/core/renderers/zelos/constants.ts @@ -885,9 +885,9 @@ export class ConstantProvider extends BaseConstantProvider { `}`, // Connection highlight. - `${selector} .blocklyHighlightedConnectionPath {`, - `stroke: ${this.SELECTED_GLOW_COLOUR};`, - `}`, + // `${selector} .blocklyHighlightedConnectionPath {`, + // `stroke: ${this.SELECTED_GLOW_COLOUR};`, + // `}`, // Disabled outline paths. `${selector} .blocklyDisabledPattern > .blocklyOutlinePath {`, diff --git a/core/renderers/zelos/drawer.ts b/core/renderers/zelos/drawer.ts index 5cc52c0cbb2..9e08fd5d193 100644 --- a/core/renderers/zelos/drawer.ts +++ b/core/renderers/zelos/drawer.ts @@ -234,15 +234,14 @@ export class Drawer extends BaseDrawer { } /** Returns a path to highlight the given connection. */ - drawConnectionHighlightPath(measurable: Connection) { + override drawConnectionHighlightPath(measurable: Connection): SVGElement | undefined { const conn = measurable.connectionModel; if ( conn.type === ConnectionType.NEXT_STATEMENT || conn.type === ConnectionType.PREVIOUS_STATEMENT || (conn.type === ConnectionType.OUTPUT_VALUE && !measurable.isDynamicShape) ) { - super.drawConnectionHighlightPath(measurable); - return; + return super.drawConnectionHighlightPath(measurable); } let path = ''; @@ -261,7 +260,7 @@ export class Drawer extends BaseDrawer { (output.shape as DynamicShape).pathDown(output.height); } const block = conn.getSourceBlock(); - block.pathObject.addConnectionHighlight?.( + return block.pathObject.addConnectionHighlight?.( conn, path, conn.getOffsetInBlock(), diff --git a/core/renderers/zelos/path_object.ts b/core/renderers/zelos/path_object.ts index f40426483a7..091c8719f71 100644 --- a/core/renderers/zelos/path_object.ts +++ b/core/renderers/zelos/path_object.ts @@ -85,21 +85,21 @@ export class PathObject extends BasePathObject { } } - override updateSelected(enable: boolean) { - this.setClass_('blocklySelected', enable); - if (enable) { - if (!this.svgPathSelected) { - this.svgPathSelected = this.svgPath.cloneNode(true) as SVGElement; - this.svgPathSelected.classList.add('blocklyPathSelected'); - this.svgRoot.appendChild(this.svgPathSelected); - } - } else { - if (this.svgPathSelected) { - this.svgRoot.removeChild(this.svgPathSelected); - this.svgPathSelected = null; - } - } - } + // override updateSelected(enable: boolean) { + // this.setClass_('blocklySelected', enable); + // if (enable) { + // if (!this.svgPathSelected) { + // this.svgPathSelected = this.svgPath.cloneNode(true) as SVGElement; + // this.svgPathSelected.classList.add('blocklyPathSelected'); + // this.svgRoot.appendChild(this.svgPathSelected); + // } + // } else { + // if (this.svgPathSelected) { + // this.svgRoot.removeChild(this.svgPathSelected); + // this.svgPathSelected = null; + // } + // } + // } override updateReplacementFade(enable: boolean) { this.setClass_('blocklyReplaceable', enable); diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index b5a5b245a0a..983f62b8597 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -2681,6 +2681,7 @@ export class WorkspaceSvg /** See IFocusableTree.lookUpFocusableNode. */ lookUpFocusableNode(id: string): IFocusableNode | null { const fieldIndicatorIndex = id.indexOf('_field_'); + const connectionIndicatorIndex = id.indexOf('_connection_'); if (fieldIndicatorIndex !== -1) { const blockId = id.substring(0, fieldIndicatorIndex); const block = this.getBlockById(blockId); @@ -2690,6 +2691,15 @@ export class WorkspaceSvg } } return null; + } else if (connectionIndicatorIndex !== -1) { + const blockId = id.substring(0, connectionIndicatorIndex); + const block = this.getBlockById(blockId); + if (block) { + for (const connection of block.getConnections_(true)) { + if (connection.id === id) return connection; + } + } + return null; } return this.getBlockById(id) as IFocusableNode; } From 59198db06a4520afd8fdaece1d23924a6d51b8d9 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Thu, 24 Apr 2025 20:47:04 +0000 Subject: [PATCH 3/8] chore: lint fixes. --- core/connection.ts | 2 +- core/rendered_connection.ts | 47 ++++++++++++++-------------- core/renderers/common/path_object.ts | 2 +- core/renderers/zelos/drawer.ts | 4 ++- 4 files changed, 29 insertions(+), 26 deletions(-) diff --git a/core/connection.ts b/core/connection.ts index d28099d1d69..5730fff996d 100644 --- a/core/connection.ts +++ b/core/connection.ts @@ -20,7 +20,7 @@ import type {Input} from './inputs/input.js'; import type {IASTNodeLocationWithBlock} from './interfaces/i_ast_node_location_with_block.js'; import type {IConnectionChecker} from './interfaces/i_connection_checker.js'; import * as blocks from './serialization/blocks.js'; -import { idGenerator } from './utils.js'; +import {idGenerator} from './utils.js'; import * as Xml from './xml.js'; /** diff --git a/core/rendered_connection.ts b/core/rendered_connection.ts index de6b6df2bad..5355033ea12 100644 --- a/core/rendered_connection.ts +++ b/core/rendered_connection.ts @@ -17,22 +17,22 @@ import * as common from './common.js'; import {config} from './config.js'; import {Connection} from './connection.js'; import type {ConnectionDB} from './connection_db.js'; -import type {IFocusableNode} from './interfaces/i_focusable_node.js'; -import type {IFocusableTree} from './interfaces/i_focusable_tree.js'; import {ConnectionType} from './connection_type.js'; import * as ContextMenu from './contextmenu.js'; import {ContextMenuRegistry} from './contextmenu_registry.js'; import * as eventUtils from './events/utils.js'; import {IContextMenu} from './interfaces/i_contextmenu.js'; +import type {IFocusableNode} from './interfaces/i_focusable_node.js'; +import type {IFocusableTree} from './interfaces/i_focusable_tree.js'; import {hasBubble} from './interfaces/i_has_bubble.js'; import * as internalConstants from './internal_constants.js'; +import type {ConstantProvider} from './renderers/common/constants.js'; import {Coordinate} from './utils/coordinate.js'; -import {WorkspaceSvg} from './workspace_svg.js'; import * as dom from './utils/dom.js'; import {Svg} from './utils/svg.js'; -import * as svgPaths from './utils/svg_paths.js'; -import type {ConstantProvider, PuzzleTab} from './renderers/common/constants.js'; import * as svgMath from './utils/svg_math.js'; +import * as svgPaths from './utils/svg_paths.js'; +import {WorkspaceSvg} from './workspace_svg.js'; /** Maximum randomness in workspace units for bumping a block. */ const BUMP_RANDOMNESS = 10; @@ -42,7 +42,8 @@ const BUMP_RANDOMNESS = 10; */ export class RenderedConnection extends Connection - implements IContextMenu, IFocusableNode { + implements IContextMenu, IFocusableNode +{ // TODO(b/109816955): remove '!', see go/strict-prop-init-fix. sourceBlock_!: BlockSvg; private readonly db: ConnectionDB; @@ -83,16 +84,15 @@ export class RenderedConnection /** Describes the state of this connection's tracked-ness. */ this.trackedState = RenderedConnection.TrackedState.WILL_TRACK; - this.constants = (source.workspace as WorkspaceSvg).getRenderer().getConstants(); + this.constants = (source.workspace as WorkspaceSvg) + .getRenderer() + .getConstants(); - this.svgGroup = dom.createSvgElement( - Svg.G, - { - 'class': 'blocklyCursor', - 'width': this.constants.CURSOR_WS_WIDTH, - 'height': this.constants.WS_CURSOR_HEIGHT, - } - ); + this.svgGroup = dom.createSvgElement(Svg.G, { + 'class': 'blocklyCursor', + 'width': this.constants.CURSOR_WS_WIDTH, + 'height': this.constants.WS_CURSOR_HEIGHT, + }); this.svgPath = dom.createSvgElement( Svg.PATH, @@ -105,10 +105,9 @@ export class RenderedConnection const y = this.getOffsetInBlock().y; const path = - svgPaths.moveTo(0, 0) + - 'c 0,10 -8,-8 -8,7.5 s 8,-2.5 8,7.5'; - // TODO: It seems that constants isn't yet initialized at this point. - // (this.constants.shapeFor(this) as PuzzleTab).pathDown; + svgPaths.moveTo(0, 0) + 'c 0,10 -8,-8 -8,7.5 s 8,-2.5 8,7.5'; + // TODO: It seems that constants isn't yet initialized at this point. + // (this.constants.shapeFor(this) as PuzzleTab).pathDown; this.svgPath.setAttribute('d', path); this.svgPath.setAttribute( 'transform', @@ -369,7 +368,7 @@ export class RenderedConnection /** Add highlighting around this connection. */ highlight() { this.highlighted = true; - const highlightSvg = this.findHighlightSvg(); + const _highlightSvg = this.findHighlightSvg(); // if (highlightSvg) { // highlightSvg.style.display = ''; // } @@ -379,9 +378,9 @@ export class RenderedConnection /** Remove the highlighting around this connection. */ unhighlight() { this.highlighted = false; - const highlightSvg = this.findHighlightSvg(); + const _highlightSvg = this.findHighlightSvg(); // if (highlightSvg) { - // highlightSvg.style.display = 'none'; + // highlightSvg.style.display = 'none'; // } // this.getSourceBlock().queueRender(); } @@ -653,7 +652,9 @@ export class RenderedConnection private findHighlightSvg(): SVGElement | null { // This cast is valid as TypeScript's definition is wrong. See: // https://github.com/microsoft/TypeScript/issues/60996. - return document.getElementById(this.id) as unknown | null as SVGElement | null; + return document.getElementById(this.id) as + | unknown + | null as SVGElement | null; } /** diff --git a/core/renderers/common/path_object.ts b/core/renderers/common/path_object.ts index d216db1a221..f722170b0d1 100644 --- a/core/renderers/common/path_object.ts +++ b/core/renderers/common/path_object.ts @@ -277,7 +277,7 @@ export class PathObject implements IPathObject { if (previousHighlight) { // TODO: Fix the highlight seemingly being recreated every time it's focused. // if (this.currentHighlightMatchesNew(connection, connectionPath, offset)) { - return previousHighlight; + return previousHighlight; // } // this.removeConnectionHighlight(connection); } diff --git a/core/renderers/zelos/drawer.ts b/core/renderers/zelos/drawer.ts index 9e08fd5d193..b38711eb6c3 100644 --- a/core/renderers/zelos/drawer.ts +++ b/core/renderers/zelos/drawer.ts @@ -234,7 +234,9 @@ export class Drawer extends BaseDrawer { } /** Returns a path to highlight the given connection. */ - override drawConnectionHighlightPath(measurable: Connection): SVGElement | undefined { + override drawConnectionHighlightPath( + measurable: Connection, + ): SVGElement | undefined { const conn = measurable.connectionModel; if ( conn.type === ConnectionType.NEXT_STATEMENT || From 8fc5c05ff549e24e81dfb97e5edb093ebc76cda6 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Tue, 29 Apr 2025 20:06:25 +0000 Subject: [PATCH 4/8] chore: remove connections rendering changes. These will be done in a later change, instead, to expedite merging the actual focus integration bits. --- core/css.ts | 8 +-- core/rendered_connection.ts | 75 +++----------------------- core/renderers/common/constants.ts | 6 +-- core/renderers/common/drawer.ts | 20 +++---- core/renderers/common/i_path_object.ts | 2 +- core/renderers/common/path_object.ts | 30 ++++------- core/renderers/zelos/constants.ts | 6 +-- core/renderers/zelos/drawer.ts | 9 ++-- core/renderers/zelos/path_object.ts | 30 +++++------ 9 files changed, 56 insertions(+), 130 deletions(-) diff --git a/core/css.ts b/core/css.ts index ebdc613c050..6ca262f3b25 100644 --- a/core/css.ts +++ b/core/css.ts @@ -147,8 +147,8 @@ let content = ` .blocklyHighlightedConnectionPath { fill: none; - // stroke: #fc3; - // stroke-width: 4px; + stroke: #fc3; + stroke-width: 4px; } .blocklyPathLight { @@ -463,8 +463,8 @@ input[type=number] { } .blocklyMenuSeparator { - background-color: #ccc; - height: 1px; + background-color: #ccc; + height: 1px; border: 0; margin-left: 4px; margin-right: 4px; diff --git a/core/rendered_connection.ts b/core/rendered_connection.ts index 5355033ea12..406b539b4e9 100644 --- a/core/rendered_connection.ts +++ b/core/rendered_connection.ts @@ -26,12 +26,8 @@ import type {IFocusableNode} from './interfaces/i_focusable_node.js'; import type {IFocusableTree} from './interfaces/i_focusable_tree.js'; import {hasBubble} from './interfaces/i_has_bubble.js'; import * as internalConstants from './internal_constants.js'; -import type {ConstantProvider} from './renderers/common/constants.js'; import {Coordinate} from './utils/coordinate.js'; -import * as dom from './utils/dom.js'; -import {Svg} from './utils/svg.js'; import * as svgMath from './utils/svg_math.js'; -import * as svgPaths from './utils/svg_paths.js'; import {WorkspaceSvg} from './workspace_svg.js'; /** Maximum randomness in workspace units for bumping a block. */ @@ -51,9 +47,6 @@ export class RenderedConnection private readonly offsetInBlock: Coordinate; private trackedState: TrackedState; private highlighted: boolean = false; - private constants: ConstantProvider; - private svgGroup: SVGElement; - private svgPath: SVGElement; /** Connection this connection connects to. Null if not connected. */ override targetConnection: RenderedConnection | null = null; @@ -83,41 +76,6 @@ export class RenderedConnection /** Describes the state of this connection's tracked-ness. */ this.trackedState = RenderedConnection.TrackedState.WILL_TRACK; - - this.constants = (source.workspace as WorkspaceSvg) - .getRenderer() - .getConstants(); - - this.svgGroup = dom.createSvgElement(Svg.G, { - 'class': 'blocklyCursor', - 'width': this.constants.CURSOR_WS_WIDTH, - 'height': this.constants.WS_CURSOR_HEIGHT, - }); - - this.svgPath = dom.createSvgElement( - Svg.PATH, - {'transform': ''}, - this.svgGroup, - ); - - // TODO: Ensure this auto-moves with the block. - const x = this.getOffsetInBlock().x; - const y = this.getOffsetInBlock().y; - - const path = - svgPaths.moveTo(0, 0) + 'c 0,10 -8,-8 -8,7.5 s 8,-2.5 8,7.5'; - // TODO: It seems that constants isn't yet initialized at this point. - // (this.constants.shapeFor(this) as PuzzleTab).pathDown; - this.svgPath.setAttribute('d', path); - this.svgPath.setAttribute( - 'transform', - 'translate(' + - x + - ',' + - y + - ')' + - (this.sourceBlock_.workspace.RTL ? ' scale(-1 1)' : ''), - ); } /** @@ -368,21 +326,13 @@ export class RenderedConnection /** Add highlighting around this connection. */ highlight() { this.highlighted = true; - const _highlightSvg = this.findHighlightSvg(); - // if (highlightSvg) { - // highlightSvg.style.display = ''; - // } - // this.getSourceBlock().queueRender(); + this.getSourceBlock().queueRender(); } /** Remove the highlighting around this connection. */ unhighlight() { this.highlighted = false; - const _highlightSvg = this.findHighlightSvg(); - // if (highlightSvg) { - // highlightSvg.style.display = 'none'; - // } - // this.getSourceBlock().queueRender(); + this.getSourceBlock().queueRender(); } /** Returns true if this connection is highlighted, false otherwise. */ @@ -649,14 +599,6 @@ export class RenderedConnection return this; } - private findHighlightSvg(): SVGElement | null { - // This cast is valid as TypeScript's definition is wrong. See: - // https://github.com/microsoft/TypeScript/issues/60996. - return document.getElementById(this.id) as - | unknown - | null as SVGElement | null; - } - /** * Handles showing the context menu when it is opened on a connection. * Note that typically the context menu can't be opened with the mouse @@ -693,11 +635,12 @@ export class RenderedConnection /** See IFocusableNode.getFocusableElement. */ getFocusableElement(): HTMLElement | SVGElement { - const highlightSvg = this.findHighlightSvg(); - if (highlightSvg) { - highlightSvg.setAttribute('aria-label', 'Connection'); - return highlightSvg; - } + // This cast is valid as TypeScript's type definition is wrong. See: + // https://github.com/microsoft/TypeScript/issues/60996. + const highlightSvg = document.getElementById(this.id) as + | unknown + | null as SVGElement | null; + if (highlightSvg) return highlightSvg; throw new Error('No highlight SVG found corresponding to this connection.'); } @@ -708,12 +651,10 @@ export class RenderedConnection /** See IFocusableNode.onNodeFocus. */ onNodeFocus(): void { - this.highlight(); } /** See IFocusableNode.onNodeBlur. */ onNodeBlur(): void { - this.unhighlight(); } } diff --git a/core/renderers/common/constants.ts b/core/renderers/common/constants.ts index c8f770cc407..c5a7a759c5c 100644 --- a/core/renderers/common/constants.ts +++ b/core/renderers/common/constants.ts @@ -1203,9 +1203,9 @@ export class ConstantProvider { `}`, // Connection highlight. - // `${selector} .blocklyHighlightedConnectionPath {`, - // `stroke: #fc3;`, - // `}`, + `${selector} .blocklyHighlightedConnectionPath {`, + `stroke: #fc3;`, + `}`, // Replaceable highlight. `${selector} .blocklyReplaceable .blocklyPath {`, diff --git a/core/renderers/common/drawer.ts b/core/renderers/common/drawer.ts index f805451a725..09320710c51 100644 --- a/core/renderers/common/drawer.ts +++ b/core/renderers/common/drawer.ts @@ -435,23 +435,19 @@ export class Drawer { for (const elem of row.elements) { if (!(elem instanceof Connection)) continue; - const highlightSvg = this.drawConnectionHighlightPath(elem); - if (highlightSvg) { - // highlightSvg.style.display = elem.highlighted ? '' : 'none'; + if (elem.highlighted) { + this.drawConnectionHighlightPath(elem); + } else { + this.block_.pathObject.removeConnectionHighlight?.( + elem.connectionModel, + ); } - // if (elem.highlighted) { - // this.drawConnectionHighlightPath(elem); - // } else { - // this.block_.pathObject.removeConnectionHighlight?.( - // elem.connectionModel, - // ); - // } } } } /** Returns a path to highlight the given connection. */ - drawConnectionHighlightPath(measurable: Connection): SVGElement | undefined { + drawConnectionHighlightPath(measurable: Connection) { const conn = measurable.connectionModel; let path = ''; if ( @@ -463,7 +459,7 @@ export class Drawer { path = this.getStatementConnectionHighlightPath(measurable); } const block = conn.getSourceBlock(); - return block.pathObject.addConnectionHighlight?.( + block.pathObject.addConnectionHighlight?.( conn, path, conn.getOffsetInBlock(), diff --git a/core/renderers/common/i_path_object.ts b/core/renderers/common/i_path_object.ts index 776ba0067ea..699f1d92edb 100644 --- a/core/renderers/common/i_path_object.ts +++ b/core/renderers/common/i_path_object.ts @@ -113,7 +113,7 @@ export interface IPathObject { connectionPath: string, offset: Coordinate, rtl: boolean, - ): SVGElement; + ): void; /** * Apply the stored colours to the block's path, taking into account whether diff --git a/core/renderers/common/path_object.ts b/core/renderers/common/path_object.ts index f722170b0d1..5bd234b0d4d 100644 --- a/core/renderers/common/path_object.ts +++ b/core/renderers/common/path_object.ts @@ -62,7 +62,7 @@ export class PathObject implements IPathObject { /** The primary path of the block. */ this.svgPath = dom.createSvgElement( Svg.PATH, - {'class': 'blocklyPath', 'tabindex': '-1'}, + {'class': 'blocklyPath'}, this.svgRoot, ); @@ -153,17 +153,13 @@ export class PathObject implements IPathObject { * removed. */ protected setClass_(className: string, add: boolean) { - this.setClassOnElem_(this.svgRoot, className, add); - } - - private setClassOnElem_(root: SVGElement, className: string, add: boolean) { if (!className) { return; } if (add) { - dom.addClass(root, className); + dom.addClass(this.svgRoot, className); } else { - dom.removeClass(root, className); + dom.removeClass(this.svgRoot, className); } } @@ -213,7 +209,7 @@ export class PathObject implements IPathObject { * @param enable True if selection is enabled, false otherwise. */ updateSelected(enable: boolean) { - this.setClassOnElem_(this.svgPath, 'blocklySelected', enable); + this.setClass_('blocklySelected', enable); } /** @@ -272,14 +268,12 @@ export class PathObject implements IPathObject { connectionPath: string, offset: Coordinate, rtl: boolean, - ): SVGElement { - const previousHighlight = this.connectionHighlights.get(connection); - if (previousHighlight) { - // TODO: Fix the highlight seemingly being recreated every time it's focused. - // if (this.currentHighlightMatchesNew(connection, connectionPath, offset)) { - return previousHighlight; - // } - // this.removeConnectionHighlight(connection); + ) { + if (this.connectionHighlights.has(connection)) { + if (this.currentHighlightMatchesNew(connection, connectionPath, offset)) { + return; + } + this.removeConnectionHighlight(connection); } const highlight = dom.createSvgElement( @@ -287,7 +281,6 @@ export class PathObject implements IPathObject { { 'id': connection.id, 'class': 'blocklyHighlightedConnectionPath', - // 'style': 'display: none;', 'tabindex': '-1', 'd': connectionPath, 'transform': @@ -295,10 +288,7 @@ export class PathObject implements IPathObject { }, this.svgRoot, ); - // TODO: Do this in a cleaner way. One possibility: create the path without 'd' or 'transform' in RenderedConnection, then just update it here (and keep registrations). - (highlight as any).renderedConnection = connection; this.connectionHighlights.set(connection, highlight); - return highlight; } private currentHighlightMatchesNew( diff --git a/core/renderers/zelos/constants.ts b/core/renderers/zelos/constants.ts index d7be09b93de..8cd36e02589 100644 --- a/core/renderers/zelos/constants.ts +++ b/core/renderers/zelos/constants.ts @@ -885,9 +885,9 @@ export class ConstantProvider extends BaseConstantProvider { `}`, // Connection highlight. - // `${selector} .blocklyHighlightedConnectionPath {`, - // `stroke: ${this.SELECTED_GLOW_COLOUR};`, - // `}`, + `${selector} .blocklyHighlightedConnectionPath {`, + `stroke: ${this.SELECTED_GLOW_COLOUR};`, + `}`, // Disabled outline paths. `${selector} .blocklyDisabledPattern > .blocklyOutlinePath {`, diff --git a/core/renderers/zelos/drawer.ts b/core/renderers/zelos/drawer.ts index b38711eb6c3..5cc52c0cbb2 100644 --- a/core/renderers/zelos/drawer.ts +++ b/core/renderers/zelos/drawer.ts @@ -234,16 +234,15 @@ export class Drawer extends BaseDrawer { } /** Returns a path to highlight the given connection. */ - override drawConnectionHighlightPath( - measurable: Connection, - ): SVGElement | undefined { + drawConnectionHighlightPath(measurable: Connection) { const conn = measurable.connectionModel; if ( conn.type === ConnectionType.NEXT_STATEMENT || conn.type === ConnectionType.PREVIOUS_STATEMENT || (conn.type === ConnectionType.OUTPUT_VALUE && !measurable.isDynamicShape) ) { - return super.drawConnectionHighlightPath(measurable); + super.drawConnectionHighlightPath(measurable); + return; } let path = ''; @@ -262,7 +261,7 @@ export class Drawer extends BaseDrawer { (output.shape as DynamicShape).pathDown(output.height); } const block = conn.getSourceBlock(); - return block.pathObject.addConnectionHighlight?.( + block.pathObject.addConnectionHighlight?.( conn, path, conn.getOffsetInBlock(), diff --git a/core/renderers/zelos/path_object.ts b/core/renderers/zelos/path_object.ts index 091c8719f71..f40426483a7 100644 --- a/core/renderers/zelos/path_object.ts +++ b/core/renderers/zelos/path_object.ts @@ -85,21 +85,21 @@ export class PathObject extends BasePathObject { } } - // override updateSelected(enable: boolean) { - // this.setClass_('blocklySelected', enable); - // if (enable) { - // if (!this.svgPathSelected) { - // this.svgPathSelected = this.svgPath.cloneNode(true) as SVGElement; - // this.svgPathSelected.classList.add('blocklyPathSelected'); - // this.svgRoot.appendChild(this.svgPathSelected); - // } - // } else { - // if (this.svgPathSelected) { - // this.svgRoot.removeChild(this.svgPathSelected); - // this.svgPathSelected = null; - // } - // } - // } + override updateSelected(enable: boolean) { + this.setClass_('blocklySelected', enable); + if (enable) { + if (!this.svgPathSelected) { + this.svgPathSelected = this.svgPath.cloneNode(true) as SVGElement; + this.svgPathSelected.classList.add('blocklyPathSelected'); + this.svgRoot.appendChild(this.svgPathSelected); + } + } else { + if (this.svgPathSelected) { + this.svgRoot.removeChild(this.svgPathSelected); + this.svgPathSelected = null; + } + } + } override updateReplacementFade(enable: boolean) { this.setClass_('blocklyReplaceable', enable); From 8f65a3b55d2d5bf30e9ac4deda8e18c681078bd5 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Wed, 30 Apr 2025 00:18:56 +0000 Subject: [PATCH 5/8] fix: Undo break for Block focusability. --- core/renderers/common/path_object.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/renderers/common/path_object.ts b/core/renderers/common/path_object.ts index 5bd234b0d4d..fe7908b383f 100644 --- a/core/renderers/common/path_object.ts +++ b/core/renderers/common/path_object.ts @@ -62,7 +62,7 @@ export class PathObject implements IPathObject { /** The primary path of the block. */ this.svgPath = dom.createSvgElement( Svg.PATH, - {'class': 'blocklyPath'}, + {'class': 'blocklyPath', 'tabindex': '-1'}, this.svgRoot, ); From 151d8f433c6bfaf3fa06b418dbb4af596545feeb Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Wed, 30 Apr 2025 01:23:04 +0000 Subject: [PATCH 6/8] fix: Make RenderedConnection display correctly. This essentially makes the underlying highlight path permanent so that only its transform/d attributes need to be updated, plus its display property. --- core/rendered_connection.ts | 35 ++++++++++++++++++++------ core/renderers/common/drawer.ts | 13 ++++------ core/renderers/common/i_path_object.ts | 2 +- core/renderers/common/path_object.ts | 34 +++++++++++-------------- core/renderers/zelos/drawer.ts | 9 ++++--- 5 files changed, 53 insertions(+), 40 deletions(-) diff --git a/core/rendered_connection.ts b/core/rendered_connection.ts index 406b539b4e9..7ada1b9f6b7 100644 --- a/core/rendered_connection.ts +++ b/core/rendered_connection.ts @@ -326,13 +326,28 @@ export class RenderedConnection /** Add highlighting around this connection. */ highlight() { this.highlighted = true; - this.getSourceBlock().queueRender(); + + // Note that this needs to be done synchronously (vs. queuing a render pass) + // since only a displayed element can be focused, and this focusable node is + // implemented to make itself visible immediately prior to receiving DOM + // focus. It's expected that the connection's position should already be + // correct by this point (otherwise it will be corrected in a subsequent + // draw pass). + const highlightSvg = this.findHighlightSvg(); + if (highlightSvg) { + highlightSvg.style.display = ''; + } } /** Remove the highlighting around this connection. */ unhighlight() { this.highlighted = false; - this.getSourceBlock().queueRender(); + + // Note that this is done synchronously for parity with highlight(). + const highlightSvg = this.findHighlightSvg(); + if (highlightSvg) { + highlightSvg.style.display = 'none'; + } } /** Returns true if this connection is highlighted, false otherwise. */ @@ -635,11 +650,7 @@ export class RenderedConnection /** See IFocusableNode.getFocusableElement. */ getFocusableElement(): HTMLElement | SVGElement { - // This cast is valid as TypeScript's type definition is wrong. See: - // https://github.com/microsoft/TypeScript/issues/60996. - const highlightSvg = document.getElementById(this.id) as - | unknown - | null as SVGElement | null; + const highlightSvg = this.findHighlightSvg(); if (highlightSvg) return highlightSvg; throw new Error('No highlight SVG found corresponding to this connection.'); } @@ -651,10 +662,20 @@ export class RenderedConnection /** See IFocusableNode.onNodeFocus. */ onNodeFocus(): void { + this.highlight(); } /** See IFocusableNode.onNodeBlur. */ onNodeBlur(): void { + this.unhighlight(); + } + + private findHighlightSvg(): SVGElement | null { + // This cast is valid as TypeScript's definition is wrong. See: + // https://github.com/microsoft/TypeScript/issues/60996. + return document.getElementById(this.id) as + | unknown + | null as SVGElement | null; } } diff --git a/core/renderers/common/drawer.ts b/core/renderers/common/drawer.ts index 09320710c51..7046406adc7 100644 --- a/core/renderers/common/drawer.ts +++ b/core/renderers/common/drawer.ts @@ -435,19 +435,16 @@ export class Drawer { for (const elem of row.elements) { if (!(elem instanceof Connection)) continue; - if (elem.highlighted) { - this.drawConnectionHighlightPath(elem); - } else { - this.block_.pathObject.removeConnectionHighlight?.( - elem.connectionModel, - ); + const highlightSvg = this.drawConnectionHighlightPath(elem); + if (highlightSvg) { + highlightSvg.style.display = elem.highlighted ? '' : 'none'; } } } } /** Returns a path to highlight the given connection. */ - drawConnectionHighlightPath(measurable: Connection) { + drawConnectionHighlightPath(measurable: Connection): SVGElement | undefined { const conn = measurable.connectionModel; let path = ''; if ( @@ -459,7 +456,7 @@ export class Drawer { path = this.getStatementConnectionHighlightPath(measurable); } const block = conn.getSourceBlock(); - block.pathObject.addConnectionHighlight?.( + return block.pathObject.addConnectionHighlight?.( conn, path, conn.getOffsetInBlock(), diff --git a/core/renderers/common/i_path_object.ts b/core/renderers/common/i_path_object.ts index 699f1d92edb..776ba0067ea 100644 --- a/core/renderers/common/i_path_object.ts +++ b/core/renderers/common/i_path_object.ts @@ -113,7 +113,7 @@ export interface IPathObject { connectionPath: string, offset: Coordinate, rtl: boolean, - ): void; + ): SVGElement; /** * Apply the stored colours to the block's path, taking into account whether diff --git a/core/renderers/common/path_object.ts b/core/renderers/common/path_object.ts index fe7908b383f..ed2bb7dda75 100644 --- a/core/renderers/common/path_object.ts +++ b/core/renderers/common/path_object.ts @@ -268,12 +268,17 @@ export class PathObject implements IPathObject { connectionPath: string, offset: Coordinate, rtl: boolean, - ) { - if (this.connectionHighlights.has(connection)) { - if (this.currentHighlightMatchesNew(connection, connectionPath, offset)) { - return; - } - this.removeConnectionHighlight(connection); + ): SVGElement { + const transformation = + `translate(${offset.x}, ${offset.y})` + (rtl ? ' scale(-1 1)' : ''); + + const previousHighlight = this.connectionHighlights.get(connection); + if (previousHighlight) { + // Since a connection already exists, make sure that its path and + // transform are correct. + previousHighlight.setAttribute('d', connectionPath); + previousHighlight.setAttribute('transform', transformation); + return previousHighlight; } const highlight = dom.createSvgElement( @@ -281,26 +286,15 @@ export class PathObject implements IPathObject { { 'id': connection.id, 'class': 'blocklyHighlightedConnectionPath', + 'style': 'display: none;', 'tabindex': '-1', 'd': connectionPath, - 'transform': - `translate(${offset.x}, ${offset.y})` + (rtl ? ' scale(-1 1)' : ''), + 'transform': transformation, }, this.svgRoot, ); this.connectionHighlights.set(connection, highlight); - } - - private currentHighlightMatchesNew( - connection: RenderedConnection, - newPath: string, - newOffset: Coordinate, - ): boolean { - const currPath = this.connectionHighlights - .get(connection) - ?.getAttribute('d'); - const currOffset = this.highlightOffsets.get(connection); - return currPath === newPath && Coordinate.equals(currOffset, newOffset); + return highlight; } /** diff --git a/core/renderers/zelos/drawer.ts b/core/renderers/zelos/drawer.ts index 5cc52c0cbb2..b38711eb6c3 100644 --- a/core/renderers/zelos/drawer.ts +++ b/core/renderers/zelos/drawer.ts @@ -234,15 +234,16 @@ export class Drawer extends BaseDrawer { } /** Returns a path to highlight the given connection. */ - drawConnectionHighlightPath(measurable: Connection) { + override drawConnectionHighlightPath( + measurable: Connection, + ): SVGElement | undefined { const conn = measurable.connectionModel; if ( conn.type === ConnectionType.NEXT_STATEMENT || conn.type === ConnectionType.PREVIOUS_STATEMENT || (conn.type === ConnectionType.OUTPUT_VALUE && !measurable.isDynamicShape) ) { - super.drawConnectionHighlightPath(measurable); - return; + return super.drawConnectionHighlightPath(measurable); } let path = ''; @@ -261,7 +262,7 @@ export class Drawer extends BaseDrawer { (output.shape as DynamicShape).pathDown(output.height); } const block = conn.getSourceBlock(); - block.pathObject.addConnectionHighlight?.( + return block.pathObject.addConnectionHighlight?.( conn, path, conn.getOffsetInBlock(), From d146f61141b9b637cd0ee22e4ddfe4b123bad2b6 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Wed, 30 Apr 2025 02:14:23 +0000 Subject: [PATCH 7/8] chore: Add field docs. This addresses a self-review comment. --- core/connection.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/core/connection.ts b/core/connection.ts index 5730fff996d..aed90e7c78c 100644 --- a/core/connection.ts +++ b/core/connection.ts @@ -56,6 +56,7 @@ export class Connection implements IASTNodeLocationWithBlock { /** DOM representation of a shadow block, or null if none. */ private shadowDom: Element | null = null; + /** The unique ID of this connection. */ id: string; /** From 0cbcc3144a33bf1f9be9fc00e1fa828292f4c436 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Wed, 30 Apr 2025 16:39:03 -0700 Subject: [PATCH 8/8] feat: Make connections focusable (#8928) ## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves Fixes #8930 Fixes part of #8771 ### Proposed Changes This PR introduces support for connections to be focusable (and thus navigable with keyboard navigation when paired with downstream changes to `LineCursor` and the keyboard navigation plugin). This is a largely isolated change in how it fundamentally works: - `RenderedConnection` has been updated to be an `IFocusableNode` using a new unique ID maintained by `Connection` and automatically enabling/disabling the connection highlight based on whether it's focused (per keyboard navigation). - The way that rendering works here has changed: rather than recreating the connection's highlight SVG each time, it's only created once and updated thereafter to ensure that it correctly fits block resizes or movements. Visibility of the highlight is controlled entirely through display visibility and can now be done synchronously (which was a requirement for focusability as only displayed elements can be focused). - This employs the same type of ID schema strategy as fields in #8923. ### Reason for Changes This is part of an ongoing effort to ensure key components of Blockly are focusable so that they can be keyboard-navigable (with other needed changes yet both in Core Blockly and the keyboard navigation plugin). ### Test Coverage No new tests have been added. It's certainly possible to add unit tests for the focusable configurations being introduced in this PR, but it may not be highly beneficial. It's largely assumed that the individual implementations should work due to a highly tested FocusManager, and it may be the case that the interactions of the components working together is far more important to verify (that is, the end user flows). The latter is planned to be tackled as part of #8915. ### Documentation No documentation changes should be needed here. ### Additional Information This includes changes that have been pulled from #8875. --- core/connection.ts | 5 +++ core/rendered_connection.ts | 57 ++++++++++++++++++++++++-- core/renderers/common/drawer.ts | 13 +++--- core/renderers/common/i_path_object.ts | 2 +- core/renderers/common/path_object.ts | 36 ++++++++-------- core/renderers/zelos/drawer.ts | 9 ++-- core/workspace_svg.ts | 10 +++++ 7 files changed, 96 insertions(+), 36 deletions(-) diff --git a/core/connection.ts b/core/connection.ts index 039d8822c01..aed90e7c78c 100644 --- a/core/connection.ts +++ b/core/connection.ts @@ -20,6 +20,7 @@ import type {Input} from './inputs/input.js'; import type {IASTNodeLocationWithBlock} from './interfaces/i_ast_node_location_with_block.js'; import type {IConnectionChecker} from './interfaces/i_connection_checker.js'; import * as blocks from './serialization/blocks.js'; +import {idGenerator} from './utils.js'; import * as Xml from './xml.js'; /** @@ -55,6 +56,9 @@ export class Connection implements IASTNodeLocationWithBlock { /** DOM representation of a shadow block, or null if none. */ private shadowDom: Element | null = null; + /** The unique ID of this connection. */ + id: string; + /** * Horizontal location of this connection. * @@ -80,6 +84,7 @@ export class Connection implements IASTNodeLocationWithBlock { public type: number, ) { this.sourceBlock_ = source; + this.id = `${source.id}_connection_${idGenerator.getNextUniqueId()}`; } /** diff --git a/core/rendered_connection.ts b/core/rendered_connection.ts index 168e59744d2..7ada1b9f6b7 100644 --- a/core/rendered_connection.ts +++ b/core/rendered_connection.ts @@ -22,10 +22,13 @@ import * as ContextMenu from './contextmenu.js'; import {ContextMenuRegistry} from './contextmenu_registry.js'; import * as eventUtils from './events/utils.js'; import {IContextMenu} from './interfaces/i_contextmenu.js'; +import type {IFocusableNode} from './interfaces/i_focusable_node.js'; +import type {IFocusableTree} from './interfaces/i_focusable_tree.js'; import {hasBubble} from './interfaces/i_has_bubble.js'; import * as internalConstants from './internal_constants.js'; import {Coordinate} from './utils/coordinate.js'; import * as svgMath from './utils/svg_math.js'; +import {WorkspaceSvg} from './workspace_svg.js'; /** Maximum randomness in workspace units for bumping a block. */ const BUMP_RANDOMNESS = 10; @@ -33,7 +36,10 @@ const BUMP_RANDOMNESS = 10; /** * Class for a connection between blocks that may be rendered on screen. */ -export class RenderedConnection extends Connection implements IContextMenu { +export class RenderedConnection + extends Connection + implements IContextMenu, IFocusableNode +{ // TODO(b/109816955): remove '!', see go/strict-prop-init-fix. sourceBlock_!: BlockSvg; private readonly db: ConnectionDB; @@ -320,13 +326,28 @@ export class RenderedConnection extends Connection implements IContextMenu { /** Add highlighting around this connection. */ highlight() { this.highlighted = true; - this.getSourceBlock().queueRender(); + + // Note that this needs to be done synchronously (vs. queuing a render pass) + // since only a displayed element can be focused, and this focusable node is + // implemented to make itself visible immediately prior to receiving DOM + // focus. It's expected that the connection's position should already be + // correct by this point (otherwise it will be corrected in a subsequent + // draw pass). + const highlightSvg = this.findHighlightSvg(); + if (highlightSvg) { + highlightSvg.style.display = ''; + } } /** Remove the highlighting around this connection. */ unhighlight() { this.highlighted = false; - this.getSourceBlock().queueRender(); + + // Note that this is done synchronously for parity with highlight(). + const highlightSvg = this.findHighlightSvg(); + if (highlightSvg) { + highlightSvg.style.display = 'none'; + } } /** Returns true if this connection is highlighted, false otherwise. */ @@ -626,6 +647,36 @@ export class RenderedConnection extends Connection implements IContextMenu { ContextMenu.show(e, menuOptions, block.RTL, workspace, location); } + + /** See IFocusableNode.getFocusableElement. */ + getFocusableElement(): HTMLElement | SVGElement { + const highlightSvg = this.findHighlightSvg(); + if (highlightSvg) return highlightSvg; + throw new Error('No highlight SVG found corresponding to this connection.'); + } + + /** See IFocusableNode.getFocusableTree. */ + getFocusableTree(): IFocusableTree { + return this.getSourceBlock().workspace as WorkspaceSvg; + } + + /** See IFocusableNode.onNodeFocus. */ + onNodeFocus(): void { + this.highlight(); + } + + /** See IFocusableNode.onNodeBlur. */ + onNodeBlur(): void { + this.unhighlight(); + } + + private findHighlightSvg(): SVGElement | null { + // This cast is valid as TypeScript's definition is wrong. See: + // https://github.com/microsoft/TypeScript/issues/60996. + return document.getElementById(this.id) as + | unknown + | null as SVGElement | null; + } } export namespace RenderedConnection { diff --git a/core/renderers/common/drawer.ts b/core/renderers/common/drawer.ts index 09320710c51..7046406adc7 100644 --- a/core/renderers/common/drawer.ts +++ b/core/renderers/common/drawer.ts @@ -435,19 +435,16 @@ export class Drawer { for (const elem of row.elements) { if (!(elem instanceof Connection)) continue; - if (elem.highlighted) { - this.drawConnectionHighlightPath(elem); - } else { - this.block_.pathObject.removeConnectionHighlight?.( - elem.connectionModel, - ); + const highlightSvg = this.drawConnectionHighlightPath(elem); + if (highlightSvg) { + highlightSvg.style.display = elem.highlighted ? '' : 'none'; } } } } /** Returns a path to highlight the given connection. */ - drawConnectionHighlightPath(measurable: Connection) { + drawConnectionHighlightPath(measurable: Connection): SVGElement | undefined { const conn = measurable.connectionModel; let path = ''; if ( @@ -459,7 +456,7 @@ export class Drawer { path = this.getStatementConnectionHighlightPath(measurable); } const block = conn.getSourceBlock(); - block.pathObject.addConnectionHighlight?.( + return block.pathObject.addConnectionHighlight?.( conn, path, conn.getOffsetInBlock(), diff --git a/core/renderers/common/i_path_object.ts b/core/renderers/common/i_path_object.ts index 699f1d92edb..776ba0067ea 100644 --- a/core/renderers/common/i_path_object.ts +++ b/core/renderers/common/i_path_object.ts @@ -113,7 +113,7 @@ export interface IPathObject { connectionPath: string, offset: Coordinate, rtl: boolean, - ): void; + ): SVGElement; /** * Apply the stored colours to the block's path, taking into account whether diff --git a/core/renderers/common/path_object.ts b/core/renderers/common/path_object.ts index 72cf2a594ce..ed2bb7dda75 100644 --- a/core/renderers/common/path_object.ts +++ b/core/renderers/common/path_object.ts @@ -268,37 +268,33 @@ export class PathObject implements IPathObject { connectionPath: string, offset: Coordinate, rtl: boolean, - ) { - if (this.connectionHighlights.has(connection)) { - if (this.currentHighlightMatchesNew(connection, connectionPath, offset)) { - return; - } - this.removeConnectionHighlight(connection); + ): SVGElement { + const transformation = + `translate(${offset.x}, ${offset.y})` + (rtl ? ' scale(-1 1)' : ''); + + const previousHighlight = this.connectionHighlights.get(connection); + if (previousHighlight) { + // Since a connection already exists, make sure that its path and + // transform are correct. + previousHighlight.setAttribute('d', connectionPath); + previousHighlight.setAttribute('transform', transformation); + return previousHighlight; } const highlight = dom.createSvgElement( Svg.PATH, { + 'id': connection.id, 'class': 'blocklyHighlightedConnectionPath', + 'style': 'display: none;', + 'tabindex': '-1', 'd': connectionPath, - 'transform': - `translate(${offset.x}, ${offset.y})` + (rtl ? ' scale(-1 1)' : ''), + 'transform': transformation, }, this.svgRoot, ); this.connectionHighlights.set(connection, highlight); - } - - private currentHighlightMatchesNew( - connection: RenderedConnection, - newPath: string, - newOffset: Coordinate, - ): boolean { - const currPath = this.connectionHighlights - .get(connection) - ?.getAttribute('d'); - const currOffset = this.highlightOffsets.get(connection); - return currPath === newPath && Coordinate.equals(currOffset, newOffset); + return highlight; } /** diff --git a/core/renderers/zelos/drawer.ts b/core/renderers/zelos/drawer.ts index 5cc52c0cbb2..b38711eb6c3 100644 --- a/core/renderers/zelos/drawer.ts +++ b/core/renderers/zelos/drawer.ts @@ -234,15 +234,16 @@ export class Drawer extends BaseDrawer { } /** Returns a path to highlight the given connection. */ - drawConnectionHighlightPath(measurable: Connection) { + override drawConnectionHighlightPath( + measurable: Connection, + ): SVGElement | undefined { const conn = measurable.connectionModel; if ( conn.type === ConnectionType.NEXT_STATEMENT || conn.type === ConnectionType.PREVIOUS_STATEMENT || (conn.type === ConnectionType.OUTPUT_VALUE && !measurable.isDynamicShape) ) { - super.drawConnectionHighlightPath(measurable); - return; + return super.drawConnectionHighlightPath(measurable); } let path = ''; @@ -261,7 +262,7 @@ export class Drawer extends BaseDrawer { (output.shape as DynamicShape).pathDown(output.height); } const block = conn.getSourceBlock(); - block.pathObject.addConnectionHighlight?.( + return block.pathObject.addConnectionHighlight?.( conn, path, conn.getOffsetInBlock(), diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index ee526bf8b6b..51992fcf0af 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -2710,6 +2710,7 @@ export class WorkspaceSvg } const fieldIndicatorIndex = id.indexOf('_field_'); + const connectionIndicatorIndex = id.indexOf('_connection_'); if (fieldIndicatorIndex !== -1) { const blockId = id.substring(0, fieldIndicatorIndex); const block = this.getBlockById(blockId); @@ -2719,6 +2720,15 @@ export class WorkspaceSvg } } return null; + } else if (connectionIndicatorIndex !== -1) { + const blockId = id.substring(0, connectionIndicatorIndex); + const block = this.getBlockById(blockId); + if (block) { + for (const connection of block.getConnections_(true)) { + if (connection.id === id) return connection; + } + } + return null; } return this.getBlockById(id) as IFocusableNode;