Skip to content

Commit 0cbcc31

Browse files
authored
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.
1 parent f68081b commit 0cbcc31

File tree

7 files changed

+96
-36
lines changed

7 files changed

+96
-36
lines changed

core/connection.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import type {Input} from './inputs/input.js';
2020
import type {IASTNodeLocationWithBlock} from './interfaces/i_ast_node_location_with_block.js';
2121
import type {IConnectionChecker} from './interfaces/i_connection_checker.js';
2222
import * as blocks from './serialization/blocks.js';
23+
import {idGenerator} from './utils.js';
2324
import * as Xml from './xml.js';
2425

2526
/**
@@ -55,6 +56,9 @@ export class Connection implements IASTNodeLocationWithBlock {
5556
/** DOM representation of a shadow block, or null if none. */
5657
private shadowDom: Element | null = null;
5758

59+
/** The unique ID of this connection. */
60+
id: string;
61+
5862
/**
5963
* Horizontal location of this connection.
6064
*
@@ -80,6 +84,7 @@ export class Connection implements IASTNodeLocationWithBlock {
8084
public type: number,
8185
) {
8286
this.sourceBlock_ = source;
87+
this.id = `${source.id}_connection_${idGenerator.getNextUniqueId()}`;
8388
}
8489

8590
/**

core/rendered_connection.ts

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,24 @@ import * as ContextMenu from './contextmenu.js';
2222
import {ContextMenuRegistry} from './contextmenu_registry.js';
2323
import * as eventUtils from './events/utils.js';
2424
import {IContextMenu} from './interfaces/i_contextmenu.js';
25+
import type {IFocusableNode} from './interfaces/i_focusable_node.js';
26+
import type {IFocusableTree} from './interfaces/i_focusable_tree.js';
2527
import {hasBubble} from './interfaces/i_has_bubble.js';
2628
import * as internalConstants from './internal_constants.js';
2729
import {Coordinate} from './utils/coordinate.js';
2830
import * as svgMath from './utils/svg_math.js';
31+
import {WorkspaceSvg} from './workspace_svg.js';
2932

3033
/** Maximum randomness in workspace units for bumping a block. */
3134
const BUMP_RANDOMNESS = 10;
3235

3336
/**
3437
* Class for a connection between blocks that may be rendered on screen.
3538
*/
36-
export class RenderedConnection extends Connection implements IContextMenu {
39+
export class RenderedConnection
40+
extends Connection
41+
implements IContextMenu, IFocusableNode
42+
{
3743
// TODO(b/109816955): remove '!', see go/strict-prop-init-fix.
3844
sourceBlock_!: BlockSvg;
3945
private readonly db: ConnectionDB;
@@ -320,13 +326,28 @@ export class RenderedConnection extends Connection implements IContextMenu {
320326
/** Add highlighting around this connection. */
321327
highlight() {
322328
this.highlighted = true;
323-
this.getSourceBlock().queueRender();
329+
330+
// Note that this needs to be done synchronously (vs. queuing a render pass)
331+
// since only a displayed element can be focused, and this focusable node is
332+
// implemented to make itself visible immediately prior to receiving DOM
333+
// focus. It's expected that the connection's position should already be
334+
// correct by this point (otherwise it will be corrected in a subsequent
335+
// draw pass).
336+
const highlightSvg = this.findHighlightSvg();
337+
if (highlightSvg) {
338+
highlightSvg.style.display = '';
339+
}
324340
}
325341

326342
/** Remove the highlighting around this connection. */
327343
unhighlight() {
328344
this.highlighted = false;
329-
this.getSourceBlock().queueRender();
345+
346+
// Note that this is done synchronously for parity with highlight().
347+
const highlightSvg = this.findHighlightSvg();
348+
if (highlightSvg) {
349+
highlightSvg.style.display = 'none';
350+
}
330351
}
331352

332353
/** Returns true if this connection is highlighted, false otherwise. */
@@ -626,6 +647,36 @@ export class RenderedConnection extends Connection implements IContextMenu {
626647

627648
ContextMenu.show(e, menuOptions, block.RTL, workspace, location);
628649
}
650+
651+
/** See IFocusableNode.getFocusableElement. */
652+
getFocusableElement(): HTMLElement | SVGElement {
653+
const highlightSvg = this.findHighlightSvg();
654+
if (highlightSvg) return highlightSvg;
655+
throw new Error('No highlight SVG found corresponding to this connection.');
656+
}
657+
658+
/** See IFocusableNode.getFocusableTree. */
659+
getFocusableTree(): IFocusableTree {
660+
return this.getSourceBlock().workspace as WorkspaceSvg;
661+
}
662+
663+
/** See IFocusableNode.onNodeFocus. */
664+
onNodeFocus(): void {
665+
this.highlight();
666+
}
667+
668+
/** See IFocusableNode.onNodeBlur. */
669+
onNodeBlur(): void {
670+
this.unhighlight();
671+
}
672+
673+
private findHighlightSvg(): SVGElement | null {
674+
// This cast is valid as TypeScript's definition is wrong. See:
675+
// https://github.com/microsoft/TypeScript/issues/60996.
676+
return document.getElementById(this.id) as
677+
| unknown
678+
| null as SVGElement | null;
679+
}
629680
}
630681

631682
export namespace RenderedConnection {

core/renderers/common/drawer.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -435,19 +435,16 @@ export class Drawer {
435435
for (const elem of row.elements) {
436436
if (!(elem instanceof Connection)) continue;
437437

438-
if (elem.highlighted) {
439-
this.drawConnectionHighlightPath(elem);
440-
} else {
441-
this.block_.pathObject.removeConnectionHighlight?.(
442-
elem.connectionModel,
443-
);
438+
const highlightSvg = this.drawConnectionHighlightPath(elem);
439+
if (highlightSvg) {
440+
highlightSvg.style.display = elem.highlighted ? '' : 'none';
444441
}
445442
}
446443
}
447444
}
448445

449446
/** Returns a path to highlight the given connection. */
450-
drawConnectionHighlightPath(measurable: Connection) {
447+
drawConnectionHighlightPath(measurable: Connection): SVGElement | undefined {
451448
const conn = measurable.connectionModel;
452449
let path = '';
453450
if (
@@ -459,7 +456,7 @@ export class Drawer {
459456
path = this.getStatementConnectionHighlightPath(measurable);
460457
}
461458
const block = conn.getSourceBlock();
462-
block.pathObject.addConnectionHighlight?.(
459+
return block.pathObject.addConnectionHighlight?.(
463460
conn,
464461
path,
465462
conn.getOffsetInBlock(),

core/renderers/common/i_path_object.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ export interface IPathObject {
113113
connectionPath: string,
114114
offset: Coordinate,
115115
rtl: boolean,
116-
): void;
116+
): SVGElement;
117117

118118
/**
119119
* Apply the stored colours to the block's path, taking into account whether

core/renderers/common/path_object.ts

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -268,37 +268,33 @@ export class PathObject implements IPathObject {
268268
connectionPath: string,
269269
offset: Coordinate,
270270
rtl: boolean,
271-
) {
272-
if (this.connectionHighlights.has(connection)) {
273-
if (this.currentHighlightMatchesNew(connection, connectionPath, offset)) {
274-
return;
275-
}
276-
this.removeConnectionHighlight(connection);
271+
): SVGElement {
272+
const transformation =
273+
`translate(${offset.x}, ${offset.y})` + (rtl ? ' scale(-1 1)' : '');
274+
275+
const previousHighlight = this.connectionHighlights.get(connection);
276+
if (previousHighlight) {
277+
// Since a connection already exists, make sure that its path and
278+
// transform are correct.
279+
previousHighlight.setAttribute('d', connectionPath);
280+
previousHighlight.setAttribute('transform', transformation);
281+
return previousHighlight;
277282
}
278283

279284
const highlight = dom.createSvgElement(
280285
Svg.PATH,
281286
{
287+
'id': connection.id,
282288
'class': 'blocklyHighlightedConnectionPath',
289+
'style': 'display: none;',
290+
'tabindex': '-1',
283291
'd': connectionPath,
284-
'transform':
285-
`translate(${offset.x}, ${offset.y})` + (rtl ? ' scale(-1 1)' : ''),
292+
'transform': transformation,
286293
},
287294
this.svgRoot,
288295
);
289296
this.connectionHighlights.set(connection, highlight);
290-
}
291-
292-
private currentHighlightMatchesNew(
293-
connection: RenderedConnection,
294-
newPath: string,
295-
newOffset: Coordinate,
296-
): boolean {
297-
const currPath = this.connectionHighlights
298-
.get(connection)
299-
?.getAttribute('d');
300-
const currOffset = this.highlightOffsets.get(connection);
301-
return currPath === newPath && Coordinate.equals(currOffset, newOffset);
297+
return highlight;
302298
}
303299

304300
/**

core/renderers/zelos/drawer.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -234,15 +234,16 @@ export class Drawer extends BaseDrawer {
234234
}
235235

236236
/** Returns a path to highlight the given connection. */
237-
drawConnectionHighlightPath(measurable: Connection) {
237+
override drawConnectionHighlightPath(
238+
measurable: Connection,
239+
): SVGElement | undefined {
238240
const conn = measurable.connectionModel;
239241
if (
240242
conn.type === ConnectionType.NEXT_STATEMENT ||
241243
conn.type === ConnectionType.PREVIOUS_STATEMENT ||
242244
(conn.type === ConnectionType.OUTPUT_VALUE && !measurable.isDynamicShape)
243245
) {
244-
super.drawConnectionHighlightPath(measurable);
245-
return;
246+
return super.drawConnectionHighlightPath(measurable);
246247
}
247248

248249
let path = '';
@@ -261,7 +262,7 @@ export class Drawer extends BaseDrawer {
261262
(output.shape as DynamicShape).pathDown(output.height);
262263
}
263264
const block = conn.getSourceBlock();
264-
block.pathObject.addConnectionHighlight?.(
265+
return block.pathObject.addConnectionHighlight?.(
265266
conn,
266267
path,
267268
conn.getOffsetInBlock(),

core/workspace_svg.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2710,6 +2710,7 @@ export class WorkspaceSvg
27102710
}
27112711

27122712
const fieldIndicatorIndex = id.indexOf('_field_');
2713+
const connectionIndicatorIndex = id.indexOf('_connection_');
27132714
if (fieldIndicatorIndex !== -1) {
27142715
const blockId = id.substring(0, fieldIndicatorIndex);
27152716
const block = this.getBlockById(blockId);
@@ -2719,6 +2720,15 @@ export class WorkspaceSvg
27192720
}
27202721
}
27212722
return null;
2723+
} else if (connectionIndicatorIndex !== -1) {
2724+
const blockId = id.substring(0, connectionIndicatorIndex);
2725+
const block = this.getBlockById(blockId);
2726+
if (block) {
2727+
for (const connection of block.getConnections_(true)) {
2728+
if (connection.id === id) return connection;
2729+
}
2730+
}
2731+
return null;
27222732
}
27232733

27242734
return this.getBlockById(id) as IFocusableNode;

0 commit comments

Comments
 (0)