diff --git a/blocks/procedures.ts b/blocks/procedures.ts index 7284973d9..534bfba6e 100644 --- a/blocks/procedures.ts +++ b/blocks/procedures.ts @@ -9,7 +9,6 @@ import type {Block} from '../core/block.js'; import type {BlockSvg} from '../core/block_svg.js'; import type {BlockDefinition} from '../core/blocks.js'; -import * as common from '../core/common.js'; import {defineBlocks} from '../core/common.js'; import {config} from '../core/config.js'; import type {Connection} from '../core/connection.js'; @@ -27,6 +26,7 @@ import {FieldCheckbox} from '../core/field_checkbox.js'; import {FieldLabel} from '../core/field_label.js'; import * as fieldRegistry from '../core/field_registry.js'; import {FieldTextInput} from '../core/field_textinput.js'; +import {getFocusManager} from '../core/focus_manager.js'; import '../core/icons/comment_icon.js'; import {MutatorIcon as Mutator} from '../core/icons/mutator_icon.js'; import '../core/icons/warning_icon.js'; @@ -1178,7 +1178,7 @@ const PROCEDURE_CALL_COMMON = { const def = Procedures.getDefinition(name, workspace); if (def) { (workspace as WorkspaceSvg).centerOnBlock(def.id); - common.setSelected(def as BlockSvg); + getFocusManager().focusNode(def as BlockSvg); } }, }); diff --git a/core/block_svg.ts b/core/block_svg.ts index c2a26e7f8..e4cc27c06 100644 --- a/core/block_svg.ts +++ b/core/block_svg.ts @@ -264,20 +264,14 @@ export class BlockSvg /** Selects this block. Highlights the block visually. */ select() { - if (this.isShadow()) { - this.getParent()?.select(); - return; - } this.addSelect(); + common.fireSelectedEvent(this); } /** Unselects this block. Unhighlights the block visually. */ unselect() { - if (this.isShadow()) { - this.getParent()?.unselect(); - return; - } this.removeSelect(); + common.fireSelectedEvent(null); } /** @@ -860,25 +854,6 @@ export class BlockSvg blockAnimations.disposeUiEffect(this); } - // Selecting a shadow block highlights an ancestor block, but that highlight - // should be removed if the shadow block will be deleted. So, before - // deleting blocks and severing the connections between them, check whether - // doing so would delete a selected block and make sure that any associated - // parent is updated. - const selection = common.getSelected(); - if (selection instanceof Block) { - let selectionAncestor: Block | null = selection; - while (selectionAncestor !== null) { - if (selectionAncestor === this) { - // The block to be deleted contains the selected block, so remove any - // selection highlight associated with the selected block before - // deleting them. - selection.unselect(); - } - selectionAncestor = selectionAncestor.getParent(); - } - } - super.dispose(!!healStack); dom.removeNode(this.svgGroup); } @@ -891,8 +866,7 @@ export class BlockSvg this.disposing = true; super.disposeInternal(); - if (common.getSelected() === this) { - this.unselect(); + if (getFocusManager().getFocusedNode() === this) { this.workspace.cancelCurrentGesture(); } @@ -1837,14 +1811,17 @@ export class BlockSvg /** See IFocusableNode.onNodeFocus. */ onNodeFocus(): void { - common.setSelected(this); + this.select(); } /** See IFocusableNode.onNodeBlur. */ onNodeBlur(): void { - if (common.getSelected() === this) { - common.setSelected(null); - } + this.unselect(); + } + + /** See IFocusableNode.canBeFocused. */ + canBeFocused(): boolean { + return true; } /** diff --git a/core/bubbles/bubble.ts b/core/bubbles/bubble.ts index 645e74a60..64060fe78 100644 --- a/core/bubbles/bubble.ts +++ b/core/bubbles/bubble.ts @@ -4,11 +4,13 @@ * SPDX-License-Identifier: Apache-2.0 */ -import {ISelectable} from '../blockly.js'; import * as browserEvents from '../browser_events.js'; import * as common from '../common.js'; import {BubbleDragStrategy} from '../dragging/bubble_drag_strategy.js'; +import {getFocusManager} from '../focus_manager.js'; import {IBubble} from '../interfaces/i_bubble.js'; +import type {IFocusableTree} from '../interfaces/i_focusable_tree.js'; +import {ISelectable} from '../interfaces/i_selectable.js'; import {ContainerRegion} from '../metrics_manager.js'; import {Scrollbar} from '../scrollbar.js'; import {Coordinate} from '../utils/coordinate.js'; @@ -86,17 +88,24 @@ export abstract class Bubble implements IBubble, ISelectable { private dragStrategy = new BubbleDragStrategy(this, this.workspace); + private focusableElement: SVGElement | HTMLElement; + /** * @param workspace The workspace this bubble belongs to. * @param anchor The anchor location of the thing this bubble is attached to. * The tail of the bubble will point to this location. * @param ownerRect An optional rect we don't want the bubble to overlap with * when automatically positioning. + * @param overriddenFocusableElement An optional replacement to the focusable + * element that's represented by this bubble (as a focusable node). This + * element will have its ID and tabindex overwritten. If not provided, the + * focusable element of this node will default to the bubble's SVG root. */ constructor( public readonly workspace: WorkspaceSvg, protected anchor: Coordinate, protected ownerRect?: Rect, + overriddenFocusableElement?: SVGElement | HTMLElement, ) { this.id = idGenerator.getNextUniqueId(); this.svgRoot = dom.createSvgElement( @@ -127,6 +136,10 @@ export abstract class Bubble implements IBubble, ISelectable { ); this.contentContainer = dom.createSvgElement(Svg.G, {}, this.svgRoot); + this.focusableElement = overriddenFocusableElement ?? this.svgRoot; + this.focusableElement.setAttribute('id', this.id); + this.focusableElement.setAttribute('tabindex', '-1'); + browserEvents.conditionalBind( this.background, 'pointerdown', @@ -208,11 +221,13 @@ export abstract class Bubble implements IBubble, ISelectable { this.background.setAttribute('fill', colour); } - /** Brings the bubble to the front and passes the pointer event off to the gesture system. */ + /** + * Passes the pointer event off to the gesture system and ensures the bubble + * is focused. + */ private onMouseDown(e: PointerEvent) { this.workspace.getGesture(e)?.handleBubbleStart(e, this); - this.bringToFront(); - common.setSelected(this); + getFocusManager().focusNode(this); } /** Positions the bubble relative to its anchor. Does not render its tail. */ @@ -647,9 +662,37 @@ export abstract class Bubble implements IBubble, ISelectable { select(): void { // Bubbles don't have any visual for being selected. + common.fireSelectedEvent(this); } unselect(): void { // Bubbles don't have any visual for being selected. + common.fireSelectedEvent(null); + } + + /** See IFocusableNode.getFocusableElement. */ + getFocusableElement(): HTMLElement | SVGElement { + return this.focusableElement; + } + + /** See IFocusableNode.getFocusableTree. */ + getFocusableTree(): IFocusableTree { + return this.workspace; + } + + /** See IFocusableNode.onNodeFocus. */ + onNodeFocus(): void { + this.select(); + this.bringToFront(); + } + + /** See IFocusableNode.onNodeBlur. */ + onNodeBlur(): void { + this.unselect(); + } + + /** See IFocusableNode.canBeFocused. */ + canBeFocused(): boolean { + return true; } } diff --git a/core/bubbles/textinput_bubble.ts b/core/bubbles/textinput_bubble.ts index cb13d5cae..6281ad758 100644 --- a/core/bubbles/textinput_bubble.ts +++ b/core/bubbles/textinput_bubble.ts @@ -80,11 +80,10 @@ export class TextInputBubble extends Bubble { protected anchor: Coordinate, protected ownerRect?: Rect, ) { - super(workspace, anchor, ownerRect); + super(workspace, anchor, ownerRect, TextInputBubble.createTextArea()); dom.addClass(this.svgRoot, 'blocklyTextInputBubble'); - ({inputRoot: this.inputRoot, textArea: this.textArea} = this.createEditor( - this.contentContainer, - )); + this.textArea = this.getFocusableElement() as HTMLTextAreaElement; + this.inputRoot = this.createEditor(this.contentContainer, this.textArea); this.resizeGroup = this.createResizeHandle(this.svgRoot, workspace); this.setSize(this.DEFAULT_SIZE, true); } @@ -131,11 +130,21 @@ export class TextInputBubble extends Bubble { this.locationChangeListeners.push(listener); } - /** Creates the editor UI for this bubble. */ - private createEditor(container: SVGGElement): { - inputRoot: SVGForeignObjectElement; - textArea: HTMLTextAreaElement; - } { + /** Creates and returns the editable text area for this bubble's editor. */ + private static createTextArea(): HTMLTextAreaElement { + const textArea = document.createElementNS( + dom.HTML_NS, + 'textarea', + ) as HTMLTextAreaElement; + textArea.className = 'blocklyTextarea blocklyText'; + return textArea; + } + + /** Creates and returns the UI container element for this bubble's editor. */ + private createEditor( + container: SVGGElement, + textArea: HTMLTextAreaElement, + ): SVGForeignObjectElement { const inputRoot = dom.createSvgElement( Svg.FOREIGNOBJECT, { @@ -149,22 +158,13 @@ export class TextInputBubble extends Bubble { body.setAttribute('xmlns', dom.HTML_NS); body.className = 'blocklyMinimalBody'; - const textArea = document.createElementNS( - dom.HTML_NS, - 'textarea', - ) as HTMLTextAreaElement; - textArea.className = 'blocklyTextarea blocklyText'; textArea.setAttribute('dir', this.workspace.RTL ? 'RTL' : 'LTR'); - body.appendChild(textArea); inputRoot.appendChild(body); this.bindTextAreaEvents(textArea); - setTimeout(() => { - textArea.focus(); - }, 0); - return {inputRoot, textArea}; + return inputRoot; } /** Binds events to the text area element. */ @@ -174,13 +174,6 @@ export class TextInputBubble extends Bubble { e.stopPropagation(); }); - browserEvents.conditionalBind( - textArea, - 'focus', - this, - this.onStartEdit, - true, - ); browserEvents.conditionalBind(textArea, 'change', this, this.onTextChange); } @@ -314,17 +307,6 @@ export class TextInputBubble extends Bubble { this.onSizeChange(); } - /** - * Handles starting an edit of the text area. Brings the bubble to the front. - */ - private onStartEdit() { - if (this.bringToFront()) { - // Since the act of moving this node within the DOM causes a loss of - // focus, we need to reapply the focus. - this.textArea.focus(); - } - } - /** Handles a text change event for the text area. Calls event listeners. */ private onTextChange() { this.text = this.textArea.value; diff --git a/core/clipboard/workspace_comment_paster.ts b/core/clipboard/workspace_comment_paster.ts index fdfbf0a84..00c56681d 100644 --- a/core/clipboard/workspace_comment_paster.ts +++ b/core/clipboard/workspace_comment_paster.ts @@ -5,9 +5,9 @@ */ import {RenderedWorkspaceComment} from '../comments/rendered_workspace_comment.js'; -import * as common from '../common.js'; import {EventType} from '../events/type.js'; import * as eventUtils from '../events/utils.js'; +import {getFocusManager} from '../focus_manager.js'; import {ICopyData} from '../interfaces/i_copyable.js'; import {IPaster} from '../interfaces/i_paster.js'; import * as commentSerialiation from '../serialization/workspace_comments.js'; @@ -49,7 +49,7 @@ export class WorkspaceCommentPaster if (eventUtils.isEnabled()) { eventUtils.fire(new (eventUtils.get(EventType.COMMENT_CREATE))(comment)); } - common.setSelected(comment); + getFocusManager().focusNode(comment); return comment; } } diff --git a/core/comments/rendered_workspace_comment.ts b/core/comments/rendered_workspace_comment.ts index bcb650b26..76eeb64f1 100644 --- a/core/comments/rendered_workspace_comment.ts +++ b/core/comments/rendered_workspace_comment.ts @@ -13,11 +13,13 @@ import * as common from '../common.js'; import * as contextMenu from '../contextmenu.js'; import {ContextMenuRegistry} from '../contextmenu_registry.js'; import {CommentDragStrategy} from '../dragging/comment_drag_strategy.js'; +import {getFocusManager} from '../focus_manager.js'; import {IBoundedElement} from '../interfaces/i_bounded_element.js'; import {IContextMenu} from '../interfaces/i_contextmenu.js'; import {ICopyable} from '../interfaces/i_copyable.js'; import {IDeletable} from '../interfaces/i_deletable.js'; import {IDraggable} from '../interfaces/i_draggable.js'; +import type {IFocusableTree} from '../interfaces/i_focusable_tree.js'; import {IRenderedElement} from '../interfaces/i_rendered_element.js'; import {ISelectable} from '../interfaces/i_selectable.js'; import * as layers from '../layers.js'; @@ -60,6 +62,8 @@ export class RenderedWorkspaceComment this.view.setSize(this.getSize()); this.view.setEditable(this.isEditable()); this.view.getSvgRoot().setAttribute('data-id', this.id); + this.view.getSvgRoot().setAttribute('id', this.id); + this.view.getSvgRoot().setAttribute('tabindex', '-1'); this.addModelUpdateBindings(); @@ -220,9 +224,8 @@ export class RenderedWorkspaceComment e.stopPropagation(); } else { gesture.handleCommentStart(e, this); - this.workspace.getLayerManager()?.append(this, layers.BLOCK); } - common.setSelected(this); + getFocusManager().focusNode(this); } } @@ -263,11 +266,13 @@ export class RenderedWorkspaceComment /** Visually highlights the comment. */ select(): void { dom.addClass(this.getSvgRoot(), 'blocklySelected'); + common.fireSelectedEvent(this); } /** Visually unhighlights the comment. */ unselect(): void { dom.removeClass(this.getSvgRoot(), 'blocklySelected'); + common.fireSelectedEvent(null); } /** @@ -322,4 +327,31 @@ export class RenderedWorkspaceComment this.moveTo(alignedXY, ['snap']); } } + + /** See IFocusableNode.getFocusableElement. */ + getFocusableElement(): HTMLElement | SVGElement { + return this.getSvgRoot(); + } + + /** See IFocusableNode.getFocusableTree. */ + getFocusableTree(): IFocusableTree { + return this.workspace; + } + + /** See IFocusableNode.onNodeFocus. */ + onNodeFocus(): void { + this.select(); + // Ensure that the comment is always at the top when focused. + this.workspace.getLayerManager()?.append(this, layers.BLOCK); + } + + /** See IFocusableNode.onNodeBlur. */ + onNodeBlur(): void { + this.unselect(); + } + + /** See IFocusableNode.canBeFocused. */ + canBeFocused(): boolean { + return true; + } } diff --git a/core/common.ts b/core/common.ts index bc31bf17e..1f7ba7e88 100644 --- a/core/common.ts +++ b/core/common.ts @@ -7,11 +7,12 @@ // Former goog.module ID: Blockly.common import type {Block} from './block.js'; -import {ISelectable} from './blockly.js'; import {BlockDefinition, Blocks} from './blocks.js'; import type {Connection} from './connection.js'; import {EventType} from './events/type.js'; import * as eventUtils from './events/utils.js'; +import {getFocusManager} from './focus_manager.js'; +import {ISelectable, isSelectable} from './interfaces/i_selectable.js'; import type {Workspace} from './workspace.js'; import type {WorkspaceSvg} from './workspace_svg.js'; @@ -86,38 +87,45 @@ export function setMainWorkspace(workspace: Workspace) { } /** - * Currently selected copyable object. - */ -let selected: ISelectable | null = null; - -/** - * Returns the currently selected copyable object. + * Returns the current selection. */ export function getSelected(): ISelectable | null { - return selected; + const focused = getFocusManager().getFocusedNode(); + if (focused && isSelectable(focused)) return focused; + return null; } /** - * Sets the currently selected block. This function does not visually mark the - * block as selected or fire the required events. If you wish to - * programmatically select a block, use `BlockSvg#select`. + * Sets the current selection. * - * @param newSelection The newly selected block. + * To clear the current selection, select another ISelectable or focus a + * non-selectable (like the workspace root node). + * + * @param newSelection The new selection to make. * @internal */ -export function setSelected(newSelection: ISelectable | null) { - if (selected === newSelection) return; +export function setSelected(newSelection: ISelectable) { + getFocusManager().focusNode(newSelection); +} +/** + * Fires a selection change event based on the new selection. + * + * This is only expected to be called by ISelectable implementations and should + * always be called before updating the current selection state. It does not + * change focus or selection state. + * + * @param newSelection The new selection. + * @internal + */ +export function fireSelectedEvent(newSelection: ISelectable | null) { + const selected = getSelected(); const event = new (eventUtils.get(EventType.SELECTED))( selected?.id ?? null, newSelection?.id ?? null, newSelection?.workspace.id ?? selected?.workspace.id ?? '', ); eventUtils.fire(event); - - selected?.unselect(); - selected = newSelection; - selected?.select(); } /** diff --git a/core/contextmenu.ts b/core/contextmenu.ts index 4ba09de82..f3ebbd1c6 100644 --- a/core/contextmenu.ts +++ b/core/contextmenu.ts @@ -9,7 +9,6 @@ import type {Block} from './block.js'; import type {BlockSvg} from './block_svg.js'; import * as browserEvents from './browser_events.js'; -import * as common from './common.js'; import {config} from './config.js'; import type { ContextMenuOption, @@ -17,6 +16,7 @@ import type { } from './contextmenu_registry.js'; import {EventType} from './events/type.js'; import * as eventUtils from './events/utils.js'; +import {getFocusManager} from './focus_manager.js'; import {Menu} from './menu.js'; import {MenuSeparator} from './menu_separator.js'; import {MenuItem} from './menuitem.js'; @@ -289,7 +289,7 @@ export function callbackFactory( if (eventUtils.isEnabled() && !newBlock.isShadow()) { eventUtils.fire(new (eventUtils.get(EventType.BLOCK_CREATE))(newBlock)); } - common.setSelected(newBlock); + getFocusManager().focusNode(newBlock); return newBlock; }; } diff --git a/core/contextmenu_items.ts b/core/contextmenu_items.ts index 267305e21..774bfdde2 100644 --- a/core/contextmenu_items.ts +++ b/core/contextmenu_items.ts @@ -9,7 +9,6 @@ import type {BlockSvg} from './block_svg.js'; import * as clipboard from './clipboard.js'; import {RenderedWorkspaceComment} from './comments/rendered_workspace_comment.js'; -import * as common from './common.js'; import {MANUALLY_DISABLED} from './constants.js'; import { ContextMenuRegistry, @@ -19,6 +18,7 @@ import { import * as dialog from './dialog.js'; import * as Events from './events/events.js'; import * as eventUtils from './events/utils.js'; +import {getFocusManager} from './focus_manager.js'; import {CommentIcon} from './icons/comment_icon.js'; import {Msg} from './msg.js'; import {StatementInput} from './renderers/zelos/zelos.js'; @@ -631,7 +631,7 @@ export function registerCommentCreate() { workspace, ), ); - common.setSelected(comment); + getFocusManager().focusNode(comment); eventUtils.setGroup(false); }, scopeType: ContextMenuRegistry.ScopeType.WORKSPACE, diff --git a/core/css.ts b/core/css.ts index e1a2a10d3..6b5e19a58 100644 --- a/core/css.ts +++ b/core/css.ts @@ -499,7 +499,11 @@ input[type=number] { .blocklyWorkspace, .blocklyField, .blocklyPath, - .blocklyHighlightedConnectionPath + .blocklyHighlightedConnectionPath, + .blocklyComment, + .blocklyBubble, + .blocklyIconGroup, + .blocklyTextarea ) { outline-width: 0px; } diff --git a/core/field.ts b/core/field.ts index b231b3365..589817b19 100644 --- a/core/field.ts +++ b/core/field.ts @@ -1364,6 +1364,11 @@ export abstract class Field /** See IFocusableNode.onNodeBlur. */ onNodeBlur(): void {} + /** See IFocusableNode.canBeFocused. */ + canBeFocused(): boolean { + return true; + } + /** * Subclasses should reimplement this method to construct their Field * subclass from a JSON arg object. diff --git a/core/flyout_base.ts b/core/flyout_base.ts index 82ae0c23d..2af280820 100644 --- a/core/flyout_base.ts +++ b/core/flyout_base.ts @@ -26,7 +26,7 @@ import {IAutoHideable} from './interfaces/i_autohideable.js'; import type {IFlyout} from './interfaces/i_flyout.js'; import type {IFlyoutInflater} from './interfaces/i_flyout_inflater.js'; import {IFocusableNode} from './interfaces/i_focusable_node.js'; -import {IFocusableTree} from './interfaces/i_focusable_tree.js'; +import type {IFocusableTree} from './interfaces/i_focusable_tree.js'; import type {Options} from './options.js'; import * as registry from './registry.js'; import * as renderManagement from './render_management.js'; @@ -986,6 +986,11 @@ export abstract class Flyout /** See IFocusableNode.onNodeBlur. */ onNodeBlur(): void {} + /** See IFocusableNode.canBeFocused. */ + canBeFocused(): boolean { + return true; + } + /** See IFocusableTree.getRootFocusableNode. */ getRootFocusableNode(): IFocusableNode { return this; diff --git a/core/flyout_button.ts b/core/flyout_button.ts index bb104be67..6790145a7 100644 --- a/core/flyout_button.ts +++ b/core/flyout_button.ts @@ -408,6 +408,11 @@ export class FlyoutButton /** See IFocusableNode.onNodeBlur. */ onNodeBlur(): void {} + /** See IFocusableNode.canBeFocused. */ + canBeFocused(): boolean { + return true; + } + /** * Returns whether or not this button is accessible through keyboard * navigation. diff --git a/core/focus_manager.ts b/core/focus_manager.ts index 4c5fab160..c0139aec0 100644 --- a/core/focus_manager.ts +++ b/core/focus_manager.ts @@ -81,7 +81,7 @@ export class FocusManager { } } - if (newNode) { + if (newNode && newNode.canBeFocused()) { const newTree = newNode.getFocusableTree(); const oldTree = this.focusedNode?.getFocusableTree(); if (newNode === newTree.getRootFocusableNode() && newTree !== oldTree) { @@ -232,11 +232,20 @@ export class FocusManager { * Any previously focused node will be updated to be passively highlighted (if * it's in a different focusable tree) or blurred (if it's in the same one). * + * **Important**: If the provided node is not able to be focused (e.g. its + * canBeFocused() method returns false), it will be ignored and any existing + * focus state will remain unchanged. + * * @param focusableNode The node that should receive active focus. */ focusNode(focusableNode: IFocusableNode): void { this.ensureManagerIsUnlocked(); if (this.focusedNode === focusableNode) return; // State is unchanged. + if (!focusableNode.canBeFocused()) { + // This node can't be focused. + console.warn("Trying to focus a node that can't be focused."); + return; + } const nextTree = focusableNode.getFocusableTree(); if (!this.isRegistered(nextTree)) { @@ -395,9 +404,9 @@ export class FocusManager { } /** - * Marks the specified node as actively focused, also calling related lifecycle - * callback methods for both the node and its parent tree. This ensures that - * the node is properly styled to indicate its active focus. + * Marks the specified node as actively focused, also calling related + * lifecycle callback methods for both the node and its parent tree. This + * ensures that the node is properly styled to indicate its active focus. * * This does not change the manager's currently tracked node, nor does it * change any other nodes. @@ -494,8 +503,8 @@ export class FocusManager { /** * Returns the page-global FocusManager. * - * The returned instance is guaranteed to not change across function calls, but - * may change across page loads. + * The returned instance is guaranteed to not change across function calls, + * but may change across page loads. */ static getFocusManager(): FocusManager { if (!FocusManager.focusManager) { diff --git a/core/icons/comment_icon.ts b/core/icons/comment_icon.ts index b54676964..959eb2500 100644 --- a/core/icons/comment_icon.ts +++ b/core/icons/comment_icon.ts @@ -11,6 +11,7 @@ import type {BlockSvg} from '../block_svg.js'; import {TextInputBubble} from '../bubbles/textinput_bubble.js'; import {EventType} from '../events/type.js'; import * as eventUtils from '../events/utils.js'; +import type {IBubble} from '../interfaces/i_bubble.js'; import type {IHasBubble} from '../interfaces/i_has_bubble.js'; import type {ISerializable} from '../interfaces/i_serializable.js'; import * as renderManagement from '../render_management.js'; @@ -338,6 +339,11 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable { ); } + /** See IHasBubble.getBubble. */ + getBubble(): IBubble | null { + return this.textInputBubble; + } + /** * Shows the editable text bubble for this comment, and adds change listeners * to update the state of this icon in response to changes in the bubble. diff --git a/core/icons/icon.ts b/core/icons/icon.ts index 30a6b538f..5bf61d49c 100644 --- a/core/icons/icon.ts +++ b/core/icons/icon.ts @@ -7,13 +7,16 @@ import type {Block} from '../block.js'; import type {BlockSvg} from '../block_svg.js'; import * as browserEvents from '../browser_events.js'; +import type {IFocusableTree} from '../interfaces/i_focusable_tree.js'; import {hasBubble} from '../interfaces/i_has_bubble.js'; import type {IIcon} from '../interfaces/i_icon.js'; import * as tooltip from '../tooltip.js'; import {Coordinate} from '../utils/coordinate.js'; import * as dom from '../utils/dom.js'; +import * as idGenerator from '../utils/idgenerator.js'; import {Size} from '../utils/size.js'; import {Svg} from '../utils/svg.js'; +import type {WorkspaceSvg} from '../workspace_svg.js'; import type {IconType} from './icon_types.js'; /** @@ -38,8 +41,12 @@ export abstract class Icon implements IIcon { /** The tooltip for this icon. */ protected tooltip: tooltip.TipInfo; + /** The unique ID of this icon. */ + private id: string; + constructor(protected sourceBlock: Block) { this.tooltip = sourceBlock; + this.id = idGenerator.getNextUniqueId(); } getType(): IconType { @@ -50,7 +57,11 @@ export abstract class Icon implements IIcon { if (this.svgRoot) return; // The icon has already been initialized. const svgBlock = this.sourceBlock as BlockSvg; - this.svgRoot = dom.createSvgElement(Svg.G, {'class': 'blocklyIconGroup'}); + this.svgRoot = dom.createSvgElement(Svg.G, { + 'class': 'blocklyIconGroup', + 'tabindex': '-1', + 'id': this.id, + }); svgBlock.getSvgRoot().appendChild(this.svgRoot); this.updateSvgRootOffset(); browserEvents.conditionalBind( @@ -144,4 +155,27 @@ export abstract class Icon implements IIcon { isClickableInFlyout(autoClosingFlyout: boolean): boolean { return true; } + + /** See IFocusableNode.getFocusableElement. */ + getFocusableElement(): HTMLElement | SVGElement { + const svgRoot = this.svgRoot; + if (!svgRoot) throw new Error('Attempting to focus uninitialized icon.'); + return svgRoot; + } + + /** See IFocusableNode.getFocusableTree. */ + getFocusableTree(): IFocusableTree { + return this.sourceBlock.workspace as WorkspaceSvg; + } + + /** See IFocusableNode.onNodeFocus. */ + onNodeFocus(): void {} + + /** See IFocusableNode.onNodeBlur. */ + onNodeBlur(): void {} + + /** See IFocusableNode.canBeFocused. */ + canBeFocused(): boolean { + return true; + } } diff --git a/core/icons/mutator_icon.ts b/core/icons/mutator_icon.ts index d8d91bea9..1842855fa 100644 --- a/core/icons/mutator_icon.ts +++ b/core/icons/mutator_icon.ts @@ -14,6 +14,7 @@ import {BlockChange} from '../events/events_block_change.js'; import {isBlockChange, isBlockCreate} from '../events/predicates.js'; import {EventType} from '../events/type.js'; import * as eventUtils from '../events/utils.js'; +import type {IBubble} from '../interfaces/i_bubble.js'; import type {IHasBubble} from '../interfaces/i_has_bubble.js'; import * as renderManagement from '../render_management.js'; import {Coordinate} from '../utils/coordinate.js'; @@ -203,6 +204,11 @@ export class MutatorIcon extends Icon implements IHasBubble { ); } + /** See IHasBubble.getBubble. */ + getBubble(): IBubble | null { + return this.miniWorkspaceBubble; + } + /** @returns the configuration the mini workspace should have. */ private getMiniWorkspaceConfig() { const options: BlocklyOptions = { diff --git a/core/icons/warning_icon.ts b/core/icons/warning_icon.ts index 87d932bb5..f24a6a561 100644 --- a/core/icons/warning_icon.ts +++ b/core/icons/warning_icon.ts @@ -10,6 +10,7 @@ import type {BlockSvg} from '../block_svg.js'; import {TextBubble} from '../bubbles/text_bubble.js'; import {EventType} from '../events/type.js'; import * as eventUtils from '../events/utils.js'; +import type {IBubble} from '../interfaces/i_bubble.js'; import type {IHasBubble} from '../interfaces/i_has_bubble.js'; import * as renderManagement from '../render_management.js'; import {Size} from '../utils.js'; @@ -197,6 +198,11 @@ export class WarningIcon extends Icon implements IHasBubble { ); } + /** See IHasBubble.getBubble. */ + getBubble(): IBubble | null { + return this.textBubble; + } + /** * @returns the location the bubble should be anchored to. * I.E. the middle of this icon. diff --git a/core/interfaces/i_bubble.ts b/core/interfaces/i_bubble.ts index d31ce9c9d..553f86e9e 100644 --- a/core/interfaces/i_bubble.ts +++ b/core/interfaces/i_bubble.ts @@ -9,11 +9,12 @@ import type {Coordinate} from '../utils/coordinate.js'; import type {IContextMenu} from './i_contextmenu.js'; import type {IDraggable} from './i_draggable.js'; +import {IFocusableNode} from './i_focusable_node.js'; /** * A bubble interface. */ -export interface IBubble extends IDraggable, IContextMenu { +export interface IBubble extends IDraggable, IContextMenu, IFocusableNode { /** * Return the coordinates of the top-left corner of this bubble's body * relative to the drawing surface's origin (0,0), in workspace units. diff --git a/core/interfaces/i_focusable_node.ts b/core/interfaces/i_focusable_node.ts index 53a432d30..6844e0809 100644 --- a/core/interfaces/i_focusable_node.ts +++ b/core/interfaces/i_focusable_node.ts @@ -33,6 +33,9 @@ export interface IFocusableNode { * It's expected the actual returned element will not change for the lifetime * of the node (that is, its properties can change but a new element should * never be returned). + * + * @returns The HTMLElement or SVGElement which can both receive focus and be + * visually represented as actively or passively focused for this node. */ getFocusableElement(): HTMLElement | SVGElement; @@ -40,6 +43,8 @@ export interface IFocusableNode { * Returns the closest parent tree of this node (in cases where a tree has * distinct trees underneath it), which represents the tree to which this node * belongs. + * + * @returns The node's IFocusableTree. */ getFocusableTree(): IFocusableTree; @@ -59,6 +64,34 @@ export interface IFocusableNode { * This has the same implementation restrictions as onNodeFocus(). */ onNodeBlur(): void; + + /** + * Indicates whether this node allows focus. If this returns false then none + * of the other IFocusableNode methods will be called. + * + * Note that special care must be taken if implementations of this function + * dynamically change their return value value over the lifetime of the node + * as certain environment conditions could affect the focusability of this + * node's DOM element (such as whether the element has a positive or zero + * tabindex). Also, changing from a true to a false value while the node holds + * focus will not immediately change the current focus of the node nor + * FocusManager's internal state, and thus may result in some of the node's + * functions being called later on when defocused (since it was previously + * considered focusable at the time of being focused). + * + * Implementations should generally always return true here unless there are + * circumstances under which this node should be skipped for focus + * considerations. Examples may include being disabled, read-only, a purely + * visual decoration, or a node with no visual representation that must + * implement this interface (e.g. due to a parent interface extending it). + * Keep in mind accessibility best practices when determining whether a node + * should be focusable since even disabled and read-only elements are still + * often relevant to providing organizational context to users (particularly + * when using a screen reader). + * + * @returns Whether this node can be focused by FocusManager. + */ + canBeFocused(): boolean; } /** @@ -74,6 +107,7 @@ export function isFocusableNode(object: any | null): object is IFocusableNode { 'getFocusableElement' in object && 'getFocusableTree' in object && 'onNodeFocus' in object && - 'onNodeBlur' in object + 'onNodeBlur' in object && + 'canBeFocused' in object ); } diff --git a/core/interfaces/i_has_bubble.ts b/core/interfaces/i_has_bubble.ts index 276feff21..85c6f0990 100644 --- a/core/interfaces/i_has_bubble.ts +++ b/core/interfaces/i_has_bubble.ts @@ -4,12 +4,27 @@ * SPDX-License-Identifier: Apache-2.0 */ +import type {IBubble} from './i_bubble'; + export interface IHasBubble { /** @returns True if the bubble is currently open, false otherwise. */ bubbleIsVisible(): boolean; /** Sets whether the bubble is open or not. */ setBubbleVisible(visible: boolean): Promise; + + /** + * Returns the current IBubble that implementations are managing, or null if + * there isn't one. + * + * Note that this cannot be expected to return null if bubbleIsVisible() + * returns false, i.e., the nullability of the returned bubble does not + * necessarily imply visibility. + * + * @returns The current IBubble maintained by implementations, or null if + * there is not one. + */ + getBubble(): IBubble | null; } /** Type guard that checks whether the given object is a IHasBubble. */ diff --git a/core/interfaces/i_icon.ts b/core/interfaces/i_icon.ts index a6159985f..74489dc5e 100644 --- a/core/interfaces/i_icon.ts +++ b/core/interfaces/i_icon.ts @@ -7,8 +7,9 @@ import type {IconType} from '../icons/icon_types.js'; import type {Coordinate} from '../utils/coordinate.js'; import type {Size} from '../utils/size.js'; +import {IFocusableNode, isFocusableNode} from './i_focusable_node.js'; -export interface IIcon { +export interface IIcon extends IFocusableNode { /** * @returns the IconType representing the type of the icon. This value should * also be used to register the icon via `Blockly.icons.registry.register`. @@ -109,6 +110,7 @@ export function isIcon(obj: any): obj is IIcon { obj.isShownWhenCollapsed !== undefined && obj.setOffsetInBlock !== undefined && obj.onLocationChange !== undefined && - obj.onClick !== undefined + obj.onClick !== undefined && + isFocusableNode(obj) ); } diff --git a/core/interfaces/i_selectable.ts b/core/interfaces/i_selectable.ts index 972b0adb1..639972e45 100644 --- a/core/interfaces/i_selectable.ts +++ b/core/interfaces/i_selectable.ts @@ -7,11 +7,17 @@ // Former goog.module ID: Blockly.ISelectable import type {Workspace} from '../workspace.js'; +import {IFocusableNode, isFocusableNode} from './i_focusable_node.js'; /** * The interface for an object that is selectable. + * + * Implementations are generally expected to use their implementations of + * onNodeFocus() and onNodeBlur() to call setSelected() with themselves and + * null, respectively, in order to ensure that selections are correctly updated + * and the selection change event is fired. */ -export interface ISelectable { +export interface ISelectable extends IFocusableNode { id: string; workspace: Workspace; @@ -29,6 +35,7 @@ export function isSelectable(obj: object): obj is ISelectable { typeof (obj as any).id === 'string' && (obj as any).workspace !== undefined && (obj as any).select !== undefined && - (obj as any).unselect !== undefined + (obj as any).unselect !== undefined && + isFocusableNode(obj) ); } diff --git a/core/keyboard_nav/line_cursor.ts b/core/keyboard_nav/line_cursor.ts index 97b5ae70f..1dcfdd8a2 100644 --- a/core/keyboard_nav/line_cursor.ts +++ b/core/keyboard_nav/line_cursor.ts @@ -14,7 +14,6 @@ */ import {BlockSvg} from '../block_svg.js'; -import * as common from '../common.js'; import {ConnectionType} from '../connection_type.js'; import {Field} from '../field.js'; import {FieldCheckbox} from '../field_checkbox.js'; @@ -561,12 +560,7 @@ export class LineCursor extends Marker { * @returns The current field, connection, or block the cursor is on. */ override getCurNode(): INavigable | null { - if (!this.updateCurNodeFromFocus()) { - // Fall back to selection if focus fails to sync. This can happen for - // non-focusable nodes or for cases when focus may not properly propagate - // (such as for mouse clicks). - this.updateCurNodeFromSelection(); - } + this.updateCurNodeFromFocus(); return super.getCurNode(); } @@ -593,60 +587,18 @@ export class LineCursor extends Marker { } } - /** - * 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 instanceof BlockSvg) { - this.setCurNode(null); - 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 === block || curNode === block.getParent())) { - return; - } - block = block.getParent(); - } - if (block) { - this.setCurNode(block); - } - } - } - /** * Updates the current node to match what's currently focused. - * - * @returns Whether the current node has been set successfully from the - * current focused node. */ - private updateCurNodeFromFocus(): boolean { + private updateCurNodeFromFocus() { const focused = getFocusManager().getFocusedNode(); if (focused instanceof BlockSvg) { const block: BlockSvg | null = focused; if (block && block.workspace === this.workspace) { this.setCurNode(block); - return true; } } - - return false; } /** diff --git a/core/layer_manager.ts b/core/layer_manager.ts index e7663b1b7..a7cb57934 100644 --- a/core/layer_manager.ts +++ b/core/layer_manager.ts @@ -122,7 +122,12 @@ export class LayerManager { if (!this.layers.has(layerNum)) { this.createLayer(layerNum); } - this.layers.get(layerNum)?.appendChild(elem.getSvgRoot()); + const childElem = elem.getSvgRoot(); + if (this.layers.get(layerNum)?.lastChild !== childElem) { + // Only append the child if it isn't already last (to avoid re-firing + // events like focused). + this.layers.get(layerNum)?.appendChild(childElem); + } } /** diff --git a/core/rendered_connection.ts b/core/rendered_connection.ts index 858bc3d1d..ec1b4ed0a 100644 --- a/core/rendered_connection.ts +++ b/core/rendered_connection.ts @@ -13,7 +13,6 @@ import type {Block} from './block.js'; import type {BlockSvg} from './block_svg.js'; -import * as common from './common.js'; import {config} from './config.js'; import {Connection} from './connection.js'; import type {ConnectionDB} from './connection_db.js'; @@ -198,15 +197,12 @@ export class RenderedConnection ? inferiorRootBlock : superiorRootBlock; // Raise it to the top for extra visibility. - const selected = common.getSelected() === dynamicRootBlock; - if (!selected) dynamicRootBlock.addSelect(); if (dynamicRootBlock.RTL) { offsetX = -offsetX; } const dx = staticConnection.x + offsetX - dynamicConnection.x; const dy = staticConnection.y + offsetY - dynamicConnection.y; dynamicRootBlock.moveBy(dx, dy, ['bump']); - if (!selected) dynamicRootBlock.removeSelect(); } /** @@ -559,21 +555,6 @@ export class RenderedConnection childBlock.updateDisabled(); childBlock.queueRender(); - // If either block being connected was selected, visually un- and reselect - // it. This has the effect of moving the selection path to the end of the - // list of child nodes in the DOM. Since SVG z-order is determined by node - // order in the DOM, this works around an issue where the selection outline - // path could be partially obscured by a new block inserted after it in the - // DOM. - const selection = common.getSelected(); - const selectedBlock = - (selection === parentBlock && parentBlock) || - (selection === childBlock && childBlock); - if (selectedBlock) { - selectedBlock.removeSelect(); - selectedBlock.addSelect(); - } - // The input the child block is connected to (if any). const parentInput = parentBlock.getInputWithBlock(childBlock); if (parentInput) { @@ -671,6 +652,11 @@ export class RenderedConnection this.unhighlight(); } + /** See IFocusableNode.canBeFocused. */ + canBeFocused(): boolean { + return true; + } + private findHighlightSvg(): SVGElement | null { // This cast is valid as TypeScript's definition is wrong. See: // https://github.com/microsoft/TypeScript/issues/60996. diff --git a/core/toolbox/toolbox.ts b/core/toolbox/toolbox.ts index 650630e6a..0fbb231dc 100644 --- a/core/toolbox/toolbox.ts +++ b/core/toolbox/toolbox.ts @@ -1094,6 +1094,11 @@ export class Toolbox /** See IFocusableNode.onNodeBlur. */ onNodeBlur(): void {} + /** See IFocusableNode.canBeFocused. */ + canBeFocused(): boolean { + return true; + } + /** See IFocusableTree.getRootFocusableNode. */ getRootFocusableNode(): IFocusableNode { return this; diff --git a/core/toolbox/toolbox_item.ts b/core/toolbox/toolbox_item.ts index 0d46a5ead..9fc5c160d 100644 --- a/core/toolbox/toolbox_item.ts +++ b/core/toolbox/toolbox_item.ts @@ -172,5 +172,10 @@ export class ToolboxItem implements IToolboxItem { /** See IFocusableNode.onNodeBlur. */ onNodeBlur(): void {} + + /** See IFocusableNode.canBeFocused. */ + canBeFocused(): boolean { + return true; + } } // nop by default diff --git a/core/workspace_dragger.ts b/core/workspace_dragger.ts index 7ad5651f7..312015e85 100644 --- a/core/workspace_dragger.ts +++ b/core/workspace_dragger.ts @@ -11,7 +11,6 @@ */ // Former goog.module ID: Blockly.WorkspaceDragger -import * as common from './common.js'; import {Coordinate} from './utils/coordinate.js'; import type {WorkspaceSvg} from './workspace_svg.js'; @@ -56,11 +55,7 @@ export class WorkspaceDragger { * * @internal */ - startDrag() { - if (common.getSelected()) { - common.getSelected()!.unselect(); - } - } + startDrag() {} /** * Finish dragging the workspace and put everything back where it belongs. diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index 543b099ca..2dae154e0 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -51,6 +51,7 @@ 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 type {IMetricsManager} from './interfaces/i_metrics_manager.js'; import type {INavigable} from './interfaces/i_navigable.js'; import type {IToolbox} from './interfaces/i_toolbox.js'; @@ -2694,6 +2695,11 @@ export class WorkspaceSvg /** See IFocusableNode.onNodeBlur. */ onNodeBlur(): void {} + /** See IFocusableNode.canBeFocused. */ + canBeFocused(): boolean { + return true; + } + /** See IFocusableTree.getRootFocusableNode. */ getRootFocusableNode(): IFocusableNode { return this; @@ -2728,6 +2734,7 @@ export class WorkspaceSvg } } + // Search for fields and connections (based on ID indicators). const fieldIndicatorIndex = id.indexOf('_field_'); const connectionIndicatorIndex = id.indexOf('_connection_'); if (fieldIndicatorIndex !== -1) { @@ -2750,7 +2757,33 @@ export class WorkspaceSvg return null; } - return this.getBlockById(id) as IFocusableNode; + // Search for a specific block. + const block = this.getBlockById(id); + if (block) return block; + + // Search for a workspace comment (semi-expensive). + for (const comment of this.getTopComments()) { + if ( + comment instanceof RenderedWorkspaceComment && + comment.getFocusableElement().id === id + ) { + return comment; + } + } + + // Search for icons and bubbles (which requires an expensive getAllBlocks). + const icons = this.getAllBlocks() + .map((block) => block.getIcons()) + .flat(); + for (const icon of icons) { + if (icon.getFocusableElement().id === id) return icon; + if (hasBubble(icon)) { + const bubble = icon.getBubble(); + if (bubble && bubble.getFocusableElement().id === id) return bubble; + } + } + + return null; } /** See IFocusableTree.onTreeFocus. */ diff --git a/tests/mocha/block_test.js b/tests/mocha/block_test.js index 874e5161c..eda2d82a5 100644 --- a/tests/mocha/block_test.js +++ b/tests/mocha/block_test.js @@ -4,7 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -import * as common from '../../build/src/core/common.js'; import {ConnectionType} from '../../build/src/core/connection_type.js'; import {EventType} from '../../build/src/core/events/type.js'; import * as eventUtils from '../../build/src/core/events/utils.js'; @@ -463,20 +462,6 @@ suite('Blocks', function () { teardown(function () { workspaceTeardown.call(this, this.workspace); }); - - test('Disposing selected shadow unhighlights parent', function () { - const parentBlock = this.parentBlock; - common.setSelected(this.shadowChild); - assert.isTrue( - parentBlock.pathObject.svgRoot.classList.contains('blocklySelected'), - 'Expected parent to be highlighted after selecting shadow child', - ); - this.shadowChild.dispose(); - assert.isFalse( - parentBlock.pathObject.svgRoot.classList.contains('blocklySelected'), - 'Expected parent to be unhighlighted after deleting shadow child', - ); - }); }); }); diff --git a/tests/mocha/focus_manager_test.js b/tests/mocha/focus_manager_test.js index 264d91734..b1cfb029a 100644 --- a/tests/mocha/focus_manager_test.js +++ b/tests/mocha/focus_manager_test.js @@ -31,6 +31,10 @@ class FocusableNodeImpl { onNodeFocus() {} onNodeBlur() {} + + canBeFocused() { + return true; + } } class FocusableTreeImpl { diff --git a/tests/mocha/focusable_tree_traverser_test.js b/tests/mocha/focusable_tree_traverser_test.js index d2467b6e9..66cc598cc 100644 --- a/tests/mocha/focusable_tree_traverser_test.js +++ b/tests/mocha/focusable_tree_traverser_test.js @@ -29,6 +29,10 @@ class FocusableNodeImpl { onNodeFocus() {} onNodeBlur() {} + + canBeFocused() { + return true; + } } class FocusableTreeImpl { diff --git a/tests/mocha/test_helpers/icon_mocks.js b/tests/mocha/test_helpers/icon_mocks.js index 039c4082f..5d117c712 100644 --- a/tests/mocha/test_helpers/icon_mocks.js +++ b/tests/mocha/test_helpers/icon_mocks.js @@ -34,6 +34,22 @@ export class MockIcon { onLocationChange() {} onClick() {} + + getFocusableElement() { + throw new Error('Unsupported operation in mock.'); + } + + getFocusableTree() { + throw new Error('Unsupported operation in mock.'); + } + + onNodeFocus() {} + + onNodeBlur() {} + + canBeFocused() { + return false; + } } export class MockSerializableIcon extends MockIcon {