Skip to content

Commit c75aea7

Browse files
committed
feat: Make WorkspaceSvg and BlockSvg focusable (RaspberryPiFoundation#8916)
## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves Fixes RaspberryPiFoundation#8913 Fixes RaspberryPiFoundation#8914 Fixes part of RaspberryPiFoundation#8771 ### Proposed Changes This updates `WorkspaceSvg` and `BlockSvg` to be focusable, that is, it makes the workspace a `IFocusableTree` and blocks `IFocusableNode`s. Some important details: - While this introduces focusable tree support for `Workspace` it doesn't include two other components that are obviously needed by the keyboard navigation plugin's playground: fields and connections. These will be introduced in subsequent PRs. - Blocks are set up to automatically synchronize their selection state with their focus state. This will eventually help to replace `LineCursor`'s responsibility for managing selection state itself. - The tabindex property for the workspace and its ARIA label have been moved down to the `.blocklyWorkspace` element itself rather than its wrapper. This helps address some tab stop issues that are already addressed in the plugin (via monkey patches), but also to ensure that the workspace's main SVG group interacts correctly with `FocusManager`. - `WorkspaceSvg` is being initially set up to default to its first top block when being focused for the first time. This is to match parity with the keyboard navigation plugin, however the latter also has functionality for defaulting to a position when no blocks are present. It's not clear how to actually support this under the new focus-based system (without adding an ephemeral element on which to focus), or if it's even necessary (since the workspace root can hold focus). ### 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 RaspberryPiFoundation#8915. ### Documentation No documentation changes should be needed here. ### Additional Information This includes changes that have been pulled from RaspberryPiFoundation#8875.
1 parent b9b40f4 commit c75aea7

File tree

4 files changed

+103
-9
lines changed

4 files changed

+103
-9
lines changed

core/block_svg.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ import {IContextMenu} from './interfaces/i_contextmenu.js';
4444
import type {ICopyable} from './interfaces/i_copyable.js';
4545
import {IDeletable} from './interfaces/i_deletable.js';
4646
import type {IDragStrategy, IDraggable} from './interfaces/i_draggable.js';
47+
import type {IFocusableNode} from './interfaces/i_focusable_node.js';
48+
import type {IFocusableTree} from './interfaces/i_focusable_tree.js';
4749
import {IIcon} from './interfaces/i_icon.js';
4850
import * as internalConstants from './internal_constants.js';
4951
import {MarkerManager} from './marker_manager.js';
@@ -76,7 +78,8 @@ export class BlockSvg
7678
IContextMenu,
7779
ICopyable<BlockCopyData>,
7880
IDraggable,
79-
IDeletable
81+
IDeletable,
82+
IFocusableNode
8083
{
8184
/**
8285
* Constant for identifying rows that are to be rendered inline.
@@ -210,6 +213,7 @@ export class BlockSvg
210213

211214
// Expose this block's ID on its top-level SVG group.
212215
this.svgGroup.setAttribute('data-id', this.id);
216+
svgPath.id = this.id;
213217

214218
this.doInit_();
215219
}
@@ -1827,4 +1831,26 @@ export class BlockSvg
18271831
);
18281832
}
18291833
}
1834+
1835+
/** See IFocusableNode.getFocusableElement. */
1836+
getFocusableElement(): HTMLElement | SVGElement {
1837+
return this.pathObject.svgPath;
1838+
}
1839+
1840+
/** See IFocusableNode.getFocusableTree. */
1841+
getFocusableTree(): IFocusableTree {
1842+
return this.workspace;
1843+
}
1844+
1845+
/** See IFocusableNode.onNodeFocus. */
1846+
onNodeFocus(): void {
1847+
common.setSelected(this);
1848+
}
1849+
1850+
/** See IFocusableNode.onNodeBlur. */
1851+
onNodeBlur(): void {
1852+
if (common.getSelected() === this) {
1853+
common.setSelected(null);
1854+
}
1855+
}
18301856
}

core/inject.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,11 @@ import * as common from './common.js';
1313
import * as Css from './css.js';
1414
import * as dropDownDiv from './dropdowndiv.js';
1515
import {Grid} from './grid.js';
16-
import {Msg} from './msg.js';
1716
import {Options} from './options.js';
1817
import {ScrollbarPair} from './scrollbar_pair.js';
1918
import {ShortcutRegistry} from './shortcut_registry.js';
2019
import * as Tooltip from './tooltip.js';
2120
import * as Touch from './touch.js';
22-
import * as aria from './utils/aria.js';
2321
import * as dom from './utils/dom.js';
2422
import {Svg} from './utils/svg.js';
2523
import * as WidgetDiv from './widgetdiv.js';
@@ -56,8 +54,6 @@ export function inject(
5654
if (opt_options?.rtl) {
5755
dom.addClass(subContainer, 'blocklyRTL');
5856
}
59-
subContainer.tabIndex = 0;
60-
aria.setState(subContainer, aria.State.LABEL, Msg['WORKSPACE_ARIA_LABEL']);
6157

6258
containerElement!.appendChild(subContainer);
6359
const svg = createDom(subContainer, options);
@@ -126,7 +122,6 @@ function createDom(container: HTMLElement, options: Options): SVGElement {
126122
'xmlns:xlink': dom.XLINK_NS,
127123
'version': '1.1',
128124
'class': 'blocklySvg',
129-
'tabindex': '0',
130125
},
131126
container,
132127
);

core/renderers/common/path_object.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ export class PathObject implements IPathObject {
6262
/** The primary path of the block. */
6363
this.svgPath = dom.createSvgElement(
6464
Svg.PATH,
65-
{'class': 'blocklyPath'},
65+
{'class': 'blocklyPath', 'tabindex': '-1'},
6666
this.svgRoot,
6767
);
6868

core/workspace_svg.ts

Lines changed: 75 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,16 @@ import {EventType} from './events/type.js';
3737
import * as eventUtils from './events/utils.js';
3838
import {Flyout} from './flyout_base.js';
3939
import type {FlyoutButton} from './flyout_button.js';
40+
import {getFocusManager} from './focus_manager.js';
4041
import {Gesture} from './gesture.js';
4142
import {Grid} from './grid.js';
4243
import type {IASTNodeLocationSvg} from './interfaces/i_ast_node_location_svg.js';
4344
import type {IBoundedElement} from './interfaces/i_bounded_element.js';
4445
import {IContextMenu} from './interfaces/i_contextmenu.js';
4546
import type {IDragTarget} from './interfaces/i_drag_target.js';
4647
import type {IFlyout} from './interfaces/i_flyout.js';
48+
import type {IFocusableNode} from './interfaces/i_focusable_node.js';
49+
import type {IFocusableTree} from './interfaces/i_focusable_tree.js';
4750
import type {IMetricsManager} from './interfaces/i_metrics_manager.js';
4851
import type {IToolbox} from './interfaces/i_toolbox.js';
4952
import type {
@@ -54,6 +57,7 @@ import type {LineCursor} from './keyboard_nav/line_cursor.js';
5457
import type {Marker} from './keyboard_nav/marker.js';
5558
import {LayerManager} from './layer_manager.js';
5659
import {MarkerManager} from './marker_manager.js';
60+
import {Msg} from './msg.js';
5761
import {Options} from './options.js';
5862
import * as Procedures from './procedures.js';
5963
import * as registry from './registry.js';
@@ -66,6 +70,7 @@ import {Classic} from './theme/classic.js';
6670
import {ThemeManager} from './theme_manager.js';
6771
import * as Tooltip from './tooltip.js';
6872
import type {Trashcan} from './trashcan.js';
73+
import * as aria from './utils/aria.js';
6974
import * as arrayUtils from './utils/array.js';
7075
import {Coordinate} from './utils/coordinate.js';
7176
import * as dom from './utils/dom.js';
@@ -93,7 +98,7 @@ const ZOOM_TO_FIT_MARGIN = 20;
9398
*/
9499
export class WorkspaceSvg
95100
extends Workspace
96-
implements IASTNodeLocationSvg, IContextMenu
101+
implements IASTNodeLocationSvg, IContextMenu, IFocusableNode, IFocusableTree
97102
{
98103
/**
99104
* A wrapper function called when a resize event occurs.
@@ -764,7 +769,19 @@ export class WorkspaceSvg
764769
* <g class="blocklyBubbleCanvas"></g>
765770
* </g>
766771
*/
767-
this.svgGroup_ = dom.createSvgElement(Svg.G, {'class': 'blocklyWorkspace'});
772+
this.svgGroup_ = dom.createSvgElement(Svg.G, {
773+
'class': 'blocklyWorkspace',
774+
// Only the top-level workspace should be tabbable.
775+
'tabindex': injectionDiv ? '0' : '-1',
776+
'id': this.id,
777+
});
778+
if (injectionDiv) {
779+
aria.setState(
780+
this.svgGroup_,
781+
aria.State.LABEL,
782+
Msg['WORKSPACE_ARIA_LABEL'],
783+
);
784+
}
768785

769786
// Note that a <g> alone does not receive mouse events--it must have a
770787
// valid target inside it. If no background class is specified, as in the
@@ -840,6 +857,9 @@ export class WorkspaceSvg
840857
this.getTheme(),
841858
isParentWorkspace ? this.getInjectionDiv() : undefined,
842859
);
860+
861+
getFocusManager().registerTree(this);
862+
843863
return this.svgGroup_;
844864
}
845865

@@ -924,6 +944,10 @@ export class WorkspaceSvg
924944
document.body.removeEventListener('wheel', this.dummyWheelListener);
925945
this.dummyWheelListener = null;
926946
}
947+
948+
if (getFocusManager().isRegistered(this)) {
949+
getFocusManager().unregisterTree(this);
950+
}
927951
}
928952

929953
/**
@@ -2631,6 +2655,55 @@ export class WorkspaceSvg
26312655
deltaY *= scale;
26322656
this.scroll(this.scrollX + deltaX, this.scrollY + deltaY);
26332657
}
2658+
2659+
/** See IFocusableNode.getFocusableElement. */
2660+
getFocusableElement(): HTMLElement | SVGElement {
2661+
return this.svgGroup_;
2662+
}
2663+
2664+
/** See IFocusableNode.getFocusableTree. */
2665+
getFocusableTree(): IFocusableTree {
2666+
return this;
2667+
}
2668+
2669+
/** See IFocusableNode.onNodeFocus. */
2670+
onNodeFocus(): void {}
2671+
2672+
/** See IFocusableNode.onNodeBlur. */
2673+
onNodeBlur(): void {}
2674+
2675+
/** See IFocusableTree.getRootFocusableNode. */
2676+
getRootFocusableNode(): IFocusableNode {
2677+
return this;
2678+
}
2679+
2680+
/** See IFocusableTree.getRestoredFocusableNode. */
2681+
getRestoredFocusableNode(
2682+
previousNode: IFocusableNode | null,
2683+
): IFocusableNode | null {
2684+
if (!previousNode) {
2685+
return this.getTopBlocks(true)[0] ?? null;
2686+
} else return null;
2687+
}
2688+
2689+
/** See IFocusableTree.getNestedTrees. */
2690+
getNestedTrees(): Array<IFocusableTree> {
2691+
return [];
2692+
}
2693+
2694+
/** See IFocusableTree.lookUpFocusableNode. */
2695+
lookUpFocusableNode(id: string): IFocusableNode | null {
2696+
return this.getBlockById(id) as IFocusableNode;
2697+
}
2698+
2699+
/** See IFocusableTree.onTreeFocus. */
2700+
onTreeFocus(
2701+
_node: IFocusableNode,
2702+
_previousTree: IFocusableTree | null,
2703+
): void {}
2704+
2705+
/** See IFocusableTree.onTreeBlur. */
2706+
onTreeBlur(_nextTree: IFocusableTree | null): void {}
26342707
}
26352708

26362709
/**

0 commit comments

Comments
 (0)