From 272dd3946578c8e2fe6aa8372dd490e6b5e6a80f Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Fri, 27 Jun 2025 08:26:16 -0700 Subject: [PATCH 01/16] feat: Enhance the Rect API. --- core/utils/rect.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/core/utils/rect.ts b/core/utils/rect.ts index c7da2a6860b..5a6822633e1 100644 --- a/core/utils/rect.ts +++ b/core/utils/rect.ts @@ -32,6 +32,16 @@ export class Rect { public right: number, ) {} + /** + * Converts a DOM or SVG Rect to a Blockly Rect. + * + * @param rect The rectangle to convert. + * @returns A representation of the same rectangle as a Blockly Rect. + */ + static from(rect: DOMRect | SVGRect): Rect { + return new Rect(rect.y, rect.y + rect.height, rect.x, rect.x + rect.width); + } + /** * Creates a new copy of this rectangle. * @@ -51,6 +61,11 @@ export class Rect { return this.right - this.left; } + /** Returns the top left coordinate of this rectangle. */ + getOrigin(): Coordinate { + return new Coordinate(this.left, this.top); + } + /** * Tests whether this rectangle contains a x/y coordinate. * From 466b6c8900145af6533f781bf41818265087d089 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Fri, 27 Jun 2025 12:45:56 -0700 Subject: [PATCH 02/16] feat: Add support for sorting IBoundedElements in general. --- core/workspace.ts | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/core/workspace.ts b/core/workspace.ts index f7b866447c4..5f205193912 100644 --- a/core/workspace.ts +++ b/core/workspace.ts @@ -21,6 +21,7 @@ import * as common from './common.js'; import type {ConnectionDB} from './connection_db.js'; import type {Abstract} from './events/events_abstract.js'; import * as eventUtils from './events/utils.js'; +import type {IBoundedElement} from './interfaces/i_bounded_element.js'; import type {IConnectionChecker} from './interfaces/i_connection_checker.js'; import {IProcedureMap} from './interfaces/i_procedure_map.js'; import type {IVariableMap} from './interfaces/i_variable_map.js'; @@ -35,6 +36,7 @@ import * as arrayUtils from './utils/array.js'; import * as deprecation from './utils/deprecation.js'; import * as idGenerator from './utils/idgenerator.js'; import * as math from './utils/math.js'; +import {Rect} from './utils/rect.js'; import type * as toolbox from './utils/toolbox.js'; import {deleteVariable, getVariableUsesById} from './variables.js'; @@ -181,10 +183,31 @@ export class Workspace { a: Block | WorkspaceComment, b: Block | WorkspaceComment, ): number { + const wrap = (element: Block | WorkspaceComment) => { + return { + getBoundingRectangle: () => { + const xy = element.getRelativeToSurfaceXY(); + return new Rect(xy.y, xy.y, xy.x, xy.x); + }, + moveBy: () => {}, + }; + }; + return this.sortByOrigin(wrap(a), wrap(b)); + } + + /** + * Sorts bounded elements on the workspace by their relative position, top to + * bottom (with slight LTR or RTL bias). + * + * @param a The first element to sort. + * @param b The second elment to sort. + * @returns -1, 0 or 1 depending on the sort order. + */ + protected sortByOrigin(a: IBoundedElement, b: IBoundedElement): number { const offset = Math.sin(math.toRadians(Workspace.SCAN_ANGLE)) * (this.RTL ? -1 : 1); - const aXY = a.getRelativeToSurfaceXY(); - const bXY = b.getRelativeToSurfaceXY(); + const aXY = a.getBoundingRectangle().getOrigin(); + const bXY = b.getBoundingRectangle().getOrigin(); return aXY.y + offset * aXY.x - (bXY.y + offset * bXY.x); } From e158c8ad2f551a9f6ab2c21c91a099b987c03539 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Fri, 27 Jun 2025 13:03:09 -0700 Subject: [PATCH 03/16] fix: Improve typings of getTopElement/Comment methods. --- core/workspace_svg.ts | 39 +++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index 3033eacd74f..87c4d2c9cdb 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -2266,8 +2266,8 @@ export class WorkspaceSvg * * @param comment comment to add. */ - override addTopComment(comment: WorkspaceComment) { - this.addTopBoundedElement(comment as RenderedWorkspaceComment); + override addTopComment(comment: RenderedWorkspaceComment) { + this.addTopBoundedElement(comment); super.addTopComment(comment); } @@ -2276,11 +2276,31 @@ export class WorkspaceSvg * * @param comment comment to remove. */ - override removeTopComment(comment: WorkspaceComment) { - this.removeTopBoundedElement(comment as RenderedWorkspaceComment); + override removeTopComment(comment: RenderedWorkspaceComment) { + this.removeTopBoundedElement(comment); super.removeTopComment(comment); } + /** + * Returns a list of comments on this workspace. + * + * @param ordered If true, sorts the comments based on their position. + * @returns A list of workspace comments. + */ + override getTopComments(ordered = false): RenderedWorkspaceComment[] { + return super.getTopComments(ordered) as RenderedWorkspaceComment[]; + } + + /** + * Returns the workspace comment with the given ID, if any. + * + * @param id The ID of the comment to retrieve. + * @returns The workspace comment with the given ID, or null. + */ + override getCommentById(id: string): RenderedWorkspaceComment | null { + return super.getCommentById(id) as RenderedWorkspaceComment | null; + } + override getRootWorkspace(): WorkspaceSvg | null { return super.getRootWorkspace() as WorkspaceSvg | null; } @@ -2308,8 +2328,15 @@ export class WorkspaceSvg * * @returns The top-level bounded elements. */ - getTopBoundedElements(): IBoundedElement[] { - return new Array().concat(this.topBoundedElements); + getTopBoundedElements(ordered = false): IBoundedElement[] { + const elements = new Array().concat( + this.topBoundedElements, + ); + if (ordered) { + elements.sort(this.sortByOrigin.bind(this)); + } + + return elements; } /** From 864efbef86d29fdde38577327d3925ab915237af Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Fri, 27 Jun 2025 13:41:32 -0700 Subject: [PATCH 04/16] feat: Add classes to represent comment icons. --- core/comments.ts | 3 + core/comments/collapse_comment_icon.ts | 100 ++++++++++++++++++++++++ core/comments/comment_icon.ts | 103 +++++++++++++++++++++++++ core/comments/delete_comment_icon.ts | 99 ++++++++++++++++++++++++ 4 files changed, 305 insertions(+) create mode 100644 core/comments/collapse_comment_icon.ts create mode 100644 core/comments/comment_icon.ts create mode 100644 core/comments/delete_comment_icon.ts diff --git a/core/comments.ts b/core/comments.ts index 86e8f50b95c..0cae186e2c7 100644 --- a/core/comments.ts +++ b/core/comments.ts @@ -4,7 +4,10 @@ * SPDX-License-Identifier: Apache-2.0 */ +export {CollapseCommentIcon} from './comments/collapse_comment_icon.js'; export {CommentEditor} from './comments/comment_editor.js'; +export {CommentIcon} from './comments/comment_icon.js'; export {CommentView} from './comments/comment_view.js'; +export {DeleteCommentIcon} from './comments/delete_comment_icon.js'; export {RenderedWorkspaceComment} from './comments/rendered_workspace_comment.js'; export {WorkspaceComment} from './comments/workspace_comment.js'; diff --git a/core/comments/collapse_comment_icon.ts b/core/comments/collapse_comment_icon.ts new file mode 100644 index 00000000000..dac66657053 --- /dev/null +++ b/core/comments/collapse_comment_icon.ts @@ -0,0 +1,100 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as browserEvents from '../browser_events.js'; +import * as touch from '../touch.js'; +import * as dom from '../utils/dom.js'; +import {Svg} from '../utils/svg.js'; +import type {WorkspaceSvg} from '../workspace_svg.js'; +import {CommentIcon} from './comment_icon.js'; + +/** + * Magic string appended to the comment ID to create a unique ID for this icon. + */ +export const COMMENT_COLLAPSE_ICON_FOCUS_IDENTIFIER = '_collapse_icon'; + +/** + * Icon that toggles the collapsed state of a comment. + */ +export class CollapseCommentIcon extends CommentIcon { + /** + * Opaque ID used to unbind event handlers during disposal. + */ + private readonly bindId: browserEvents.Data; + + /** + * SVG image displayed by this icon. + */ + protected override readonly icon: SVGImageElement; + + /** + * Creates a new CollapseCommentIcon instance. + * + * @param id The ID of this icon's parent comment. + * @param workspace The workspace this icon's parent comment is displayed on. + * @param container An SVG group that this icon should be a child of. + */ + constructor( + protected readonly id: string, + protected readonly workspace: WorkspaceSvg, + protected readonly container: SVGGElement, + ) { + super(id, workspace, container); + + this.icon = dom.createSvgElement( + Svg.IMAGE, + { + 'class': 'blocklyFoldoutIcon', + 'href': `${this.workspace.options.pathToMedia}foldout-icon.svg`, + 'id': `${this.id}${COMMENT_COLLAPSE_ICON_FOCUS_IDENTIFIER}`, + }, + this.container, + ); + this.bindId = browserEvents.conditionalBind( + this.icon, + 'pointerdown', + this, + this.performAction.bind(this), + ); + } + + /** + * Disposes of this icon. + */ + dispose() { + browserEvents.unbind(this.bindId); + } + + /** + * Adjusts the positioning of this icon within its container. + */ + override reposition() { + const margin = this.getMargin(); + this.icon.setAttribute('y', `${margin}`); + this.icon.setAttribute('x', `${margin}`); + } + + /** + * Toggles the collapsed state of the parent comment. + * + * @param e The event that triggered this action. + */ + override performAction(e?: Event) { + touch.clearTouchIdentifier(); + + const comment = this.getParentComment(); + comment.view.bringToFront(); + if (e && e instanceof PointerEvent && browserEvents.isRightButton(e)) { + e.stopPropagation(); + return; + } + + comment.setCollapsed(!comment.isCollapsed()); + this.workspace.hideChaff(); + + e?.stopPropagation(); + } +} diff --git a/core/comments/comment_icon.ts b/core/comments/comment_icon.ts new file mode 100644 index 00000000000..4450b420d3d --- /dev/null +++ b/core/comments/comment_icon.ts @@ -0,0 +1,103 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import type {IFocusableNode} from '../interfaces/i_focusable_node.js'; +import {Rect} from '../utils/rect.js'; +import type {WorkspaceSvg} from '../workspace_svg.js'; +import type {RenderedWorkspaceComment} from './rendered_workspace_comment.js'; + +/** + * Actionable icon displayed on a comment's top bar. + */ +export abstract class CommentIcon implements IFocusableNode { + /** + * SVG image displayed by this icon. + */ + protected abstract readonly icon: SVGImageElement; + + /** + * Creates a new CollapseCommentIcon instance. + * + * @param id The ID of this icon's parent comment. + * @param workspace The workspace this icon's parent comment is displayed on. + * @param container An SVG group that this icon should be a child of. + */ + constructor( + protected readonly id: string, + protected readonly workspace: WorkspaceSvg, + protected readonly container: SVGGElement, + ) {} + + /** + * Returns whether or not this icon is currently visible. + */ + isVisible(): boolean { + return this.icon.checkVisibility(); + } + + /** + * Returns the parent comment of this comment icon. + */ + getParentComment(): RenderedWorkspaceComment { + const comment = this.workspace.getCommentById(this.id); + if (!comment) { + throw new Error(`Comment icon ${this.id} has no corresponding comment`); + } + + return comment; + } + + /** Adjusts the position of this icon within its parent container. */ + abstract reposition(): void; + + /** Perform the action this icon should take when it is acted on. */ + abstract performAction(e?: Event): void; + + /** + * Returns the dimensions of this icon in SVG units. + * + * @param includeMargin True to include the margin when calculating the size. + * @returns The size of this icon. + */ + getSize(includeMargin = false): Rect { + const bounds = this.icon.getBBox(); + const rect = Rect.from(bounds); + if (includeMargin) { + const margin = this.getMargin(); + rect.left -= margin; + rect.top -= margin; + rect.bottom += margin; + rect.right += margin; + } + return rect; + } + + /** Returns the margin in SVG units surrounding this icon. */ + getMargin(): number { + return (this.container.getBBox().height - this.icon.getBBox().height) / 2; + } + + /** Returns a DOM element representing this icon that can receive focus. */ + getFocusableElement() { + return this.icon; + } + + /** Returns the workspace this icon is a child of. */ + getFocusableTree() { + return this.workspace; + } + + /** Called when this icon's focusable DOM element gains focus. */ + onNodeFocus() {} + + /** Called when this icon's focusable DOM element loses focus. */ + onNodeBlur() {} + + /** Returns whether or not this icon can be focused. True if it is visible. */ + canBeFocused() { + return this.isVisible(); + } +} diff --git a/core/comments/delete_comment_icon.ts b/core/comments/delete_comment_icon.ts new file mode 100644 index 00000000000..a27b0a23d5d --- /dev/null +++ b/core/comments/delete_comment_icon.ts @@ -0,0 +1,99 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as browserEvents from '../browser_events.js'; +import * as touch from '../touch.js'; +import * as dom from '../utils/dom.js'; +import {Svg} from '../utils/svg.js'; +import type {WorkspaceSvg} from '../workspace_svg.js'; +import {CommentIcon} from './comment_icon.js'; + +/** + * Magic string appended to the comment ID to create a unique ID for this icon. + */ +export const COMMENT_DELETE_ICON_FOCUS_IDENTIFIER = '_delete_icon'; + +/** + * Icon that deletes a comment. + */ +export class DeleteCommentIcon extends CommentIcon { + /** + * Opaque ID used to unbind event handlers during disposal. + */ + private readonly bindId: browserEvents.Data; + + /** + * SVG image displayed by this icon. + */ + protected override readonly icon: SVGImageElement; + + /** + * Creates a new DeleteCommentIcon instance. + * + * @param id The ID of this icon's parent comment. + * @param workspace The workspace this icon's parent comment is displayed on. + * @param container An SVG group that this icon should be a child of. + */ + constructor( + protected readonly id: string, + protected readonly workspace: WorkspaceSvg, + protected readonly container: SVGGElement, + ) { + super(id, workspace, container); + + this.icon = dom.createSvgElement( + Svg.IMAGE, + { + 'class': 'blocklyDeleteIcon', + 'href': `${this.workspace.options.pathToMedia}delete-icon.svg`, + 'id': `${this.id}${COMMENT_DELETE_ICON_FOCUS_IDENTIFIER}`, + }, + container, + ); + this.bindId = browserEvents.conditionalBind( + this.icon, + 'pointerdown', + this, + this.performAction.bind(this), + ); + } + + /** + * Disposes of this icon. + */ + dispose() { + browserEvents.unbind(this.bindId); + } + + /** + * Adjusts the positioning of this icon within its container. + */ + override reposition() { + const margin = this.getMargin(); + const containerSize = this.container.getBBox(); + this.icon.setAttribute('y', `${margin}`); + this.icon.setAttribute( + 'x', + `${containerSize.width - this.getSize(true).getWidth()}`, + ); + } + + /** + * Deletes parent comment. + * + * @param e The event that triggered this action. + */ + override performAction(e?: Event) { + touch.clearTouchIdentifier(); + if (e && e instanceof PointerEvent && browserEvents.isRightButton(e)) { + e.stopPropagation(); + return; + } + + this.getParentComment().dispose(); + e?.stopPropagation(); + } +} From d8c82cb4346ecba93b87b7e890804f26c66f4fe9 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Fri, 27 Jun 2025 13:50:23 -0700 Subject: [PATCH 05/16] refactor: Use comment icons in comment view. --- core/comments/comment_view.ts | 171 ++++++++-------------------------- 1 file changed, 38 insertions(+), 133 deletions(-) diff --git a/core/comments/comment_view.ts b/core/comments/comment_view.ts index 1e5ad4a52c2..1106913799e 100644 --- a/core/comments/comment_view.ts +++ b/core/comments/comment_view.ts @@ -16,14 +16,17 @@ import * as drag from '../utils/drag.js'; import {Size} from '../utils/size.js'; import {Svg} from '../utils/svg.js'; import {WorkspaceSvg} from '../workspace_svg.js'; +import {CollapseCommentIcon} from './collapse_comment_icon.js'; import {CommentEditor} from './comment_editor.js'; +import type {CommentIcon} from './comment_icon.js'; +import {DeleteCommentIcon} from './delete_comment_icon.js'; export class CommentView implements IRenderedElement { /** The root group element of the comment view. */ private svgRoot: SVGGElement; /** - * The svg rect element that we use to create a hightlight around the comment. + * The SVG rect element that we use to create a highlight around the comment. */ private highlightRect: SVGRectElement; @@ -34,10 +37,10 @@ export class CommentView implements IRenderedElement { private topBarBackground: SVGRectElement; /** The delete icon that goes in the top bar. */ - private deleteIcon: SVGImageElement; + private deleteIcon: DeleteCommentIcon; /** The foldout icon that goes in the top bar. */ - private foldoutIcon: SVGImageElement; + private foldoutIcon: CollapseCommentIcon; /** The text element that goes in the top bar. */ private textPreview: SVGTextElement; @@ -99,7 +102,7 @@ export class CommentView implements IRenderedElement { constructor( readonly workspace: WorkspaceSvg, - private commentId?: string, + private commentId: string, ) { this.svgRoot = dom.createSvgElement(Svg.G, { 'class': 'blocklyComment blocklyEditable blocklyDraggable', @@ -153,8 +156,8 @@ export class CommentView implements IRenderedElement { ): { topBarGroup: SVGGElement; topBarBackground: SVGRectElement; - deleteIcon: SVGImageElement; - foldoutIcon: SVGImageElement; + deleteIcon: DeleteCommentIcon; + foldoutIcon: CollapseCommentIcon; textPreview: SVGTextElement; textPreviewNode: Text; } { @@ -172,22 +175,14 @@ export class CommentView implements IRenderedElement { }, topBarGroup, ); - // TODO: Before merging, does this mean to override an individual image, - // folks need to replace the whole media folder? - const deleteIcon = dom.createSvgElement( - Svg.IMAGE, - { - 'class': 'blocklyDeleteIcon', - 'href': `${workspace.options.pathToMedia}delete-icon.svg`, - }, + const deleteIcon = new DeleteCommentIcon( + this.commentId, + this.workspace, topBarGroup, ); - const foldoutIcon = dom.createSvgElement( - Svg.IMAGE, - { - 'class': 'blocklyFoldoutIcon', - 'href': `${workspace.options.pathToMedia}foldout-icon.svg`, - }, + const foldoutIcon = new CollapseCommentIcon( + this.commentId, + this.workspace, topBarGroup, ); const textPreview = dom.createSvgElement( @@ -200,22 +195,6 @@ export class CommentView implements IRenderedElement { const textPreviewNode = document.createTextNode(''); textPreview.appendChild(textPreviewNode); - // TODO(toychest): Triggering this on pointerdown means that we can't start - // drags on the foldout icon. We need to open up the gesture system - // to fix this. - browserEvents.conditionalBind( - foldoutIcon, - 'pointerdown', - this, - this.onFoldoutDown, - ); - browserEvents.conditionalBind( - deleteIcon, - 'pointerdown', - this, - this.onDeleteDown, - ); - return { topBarGroup, topBarBackground, @@ -300,15 +279,10 @@ export class CommentView implements IRenderedElement { */ setSizeWithoutFiringEvents(size: Size) { const topBarSize = this.topBarBackground.getBBox(); - const deleteSize = this.deleteIcon.getBBox(); - const foldoutSize = this.foldoutIcon.getBBox(); const textPreviewSize = this.textPreview.getBBox(); const resizeSize = this.resizeHandle.getBBox(); - size = Size.max( - size, - this.calcMinSize(topBarSize, foldoutSize, deleteSize), - ); + size = Size.max(size, this.calcMinSize(topBarSize)); this.size = size; this.svgRoot.setAttribute('height', `${size.height}`); @@ -317,15 +291,9 @@ export class CommentView implements IRenderedElement { this.updateHighlightRect(size); this.updateTopBarSize(size); this.commentEditor.updateSize(size, topBarSize); - this.updateDeleteIconPosition(size, topBarSize, deleteSize); - this.updateFoldoutIconPosition(topBarSize, foldoutSize); - this.updateTextPreviewSize( - size, - topBarSize, - textPreviewSize, - deleteSize, - resizeSize, - ); + this.deleteIcon.reposition(); + this.foldoutIcon.reposition(); + this.updateTextPreviewSize(size, topBarSize, textPreviewSize); this.updateResizeHandlePosition(size, resizeSize); } @@ -347,25 +315,18 @@ export class CommentView implements IRenderedElement { * * The minimum height is based on the height of the top bar. */ - private calcMinSize( - topBarSize: Size, - foldoutSize: Size, - deleteSize: Size, - ): Size { + private calcMinSize(topBarSize: Size): Size { this.updateTextPreview(this.commentEditor.getText() ?? ''); const textPreviewWidth = dom.getTextWidth(this.textPreview); - const foldoutMargin = this.calcFoldoutMargin(topBarSize, foldoutSize); - const deleteMargin = this.calcDeleteMargin(topBarSize, deleteSize); - let width = textPreviewWidth; - if (this.foldoutIcon.checkVisibility()) { - width += foldoutSize.width + foldoutMargin * 2; + if (this.foldoutIcon.isVisible()) { + width += this.foldoutIcon.getSize(true).getWidth(); } else if (textPreviewWidth) { width += 4; // Arbitrary margin before text. } - if (this.deleteIcon.checkVisibility()) { - width += deleteSize.width + deleteMargin * 2; + if (this.deleteIcon.isVisible()) { + width += this.deleteIcon.getSize(true).getWidth(); } else if (textPreviewWidth) { width += 4; // Arbitrary margin after text. } @@ -376,16 +337,6 @@ export class CommentView implements IRenderedElement { return new Size(width, height); } - /** Calculates the margin that should exist around the delete icon. */ - private calcDeleteMargin(topBarSize: Size, deleteSize: Size) { - return (topBarSize.height - deleteSize.height) / 2; - } - - /** Calculates the margin that should exist around the foldout icon. */ - private calcFoldoutMargin(topBarSize: Size, foldoutSize: Size) { - return (topBarSize.height - foldoutSize.height) / 2; - } - /** Updates the size of the highlight rect to reflect the new size. */ private updateHighlightRect(size: Size) { this.highlightRect.setAttribute('height', `${size.height}`); @@ -400,31 +351,6 @@ export class CommentView implements IRenderedElement { this.topBarBackground.setAttribute('width', `${size.width}`); } - /** - * Updates the position of the delete icon elements to reflect the new size. - */ - private updateDeleteIconPosition( - size: Size, - topBarSize: Size, - deleteSize: Size, - ) { - const deleteMargin = this.calcDeleteMargin(topBarSize, deleteSize); - this.deleteIcon.setAttribute('y', `${deleteMargin}`); - this.deleteIcon.setAttribute( - 'x', - `${size.width - deleteSize.width - deleteMargin}`, - ); - } - - /** - * Updates the position of the foldout icon elements to reflect the new size. - */ - private updateFoldoutIconPosition(topBarSize: Size, foldoutSize: Size) { - const foldoutMargin = this.calcFoldoutMargin(topBarSize, foldoutSize); - this.foldoutIcon.setAttribute('y', `${foldoutMargin}`); - this.foldoutIcon.setAttribute('x', `${foldoutMargin}`); - } - /** * Updates the size and position of the text preview elements to reflect the new size. */ @@ -432,25 +358,14 @@ export class CommentView implements IRenderedElement { size: Size, topBarSize: Size, textPreviewSize: Size, - deleteSize: Size, - foldoutSize: Size, ) { const textPreviewMargin = (topBarSize.height - textPreviewSize.height) / 2; - const deleteMargin = this.calcDeleteMargin(topBarSize, deleteSize); - const foldoutMargin = this.calcFoldoutMargin(topBarSize, foldoutSize); + const foldoutSize = this.foldoutIcon.getSize(true); + const deleteSize = this.deleteIcon.getSize(true); const textPreviewWidth = - size.width - - foldoutSize.width - - foldoutMargin * 2 - - deleteSize.width - - deleteMargin * 2; - this.textPreview.setAttribute( - 'x', - `${ - foldoutSize.width + foldoutMargin * 2 * (this.workspace.RTL ? -1 : 1) - }`, - ); + size.width - foldoutSize.getWidth() - deleteSize.getWidth(); + this.textPreview.setAttribute('x', `${foldoutSize.getWidth()}`); this.textPreview.setAttribute( 'y', `${textPreviewMargin + textPreviewSize.height / 2}`, @@ -601,25 +516,6 @@ export class CommentView implements IRenderedElement { ); } - /** - * Toggles the collapsedness of the block when we receive a pointer down - * event on the foldout icon. - */ - private onFoldoutDown(e: PointerEvent) { - touch.clearTouchIdentifier(); - this.bringToFront(); - if (browserEvents.isRightButton(e)) { - e.stopPropagation(); - return; - } - - this.setCollapsed(!this.collapsed); - - this.workspace.hideChaff(); - - e.stopPropagation(); - } - /** Returns true if the comment is currently editable. */ isEditable(): boolean { return this.editable; @@ -692,7 +588,7 @@ export class CommentView implements IRenderedElement { } /** Brings the workspace comment to the front of its layer. */ - private bringToFront() { + bringToFront() { const parent = this.svgRoot.parentNode; const childNodes = parent!.childNodes; // Avoid moving the comment if it's already at the bottom. @@ -719,6 +615,8 @@ export class CommentView implements IRenderedElement { /** Disposes of this comment view. */ dispose() { this.disposing = true; + this.foldoutIcon.dispose(); + this.deleteIcon.dispose(); dom.removeNode(this.svgRoot); // Loop through listeners backwards in case they remove themselves. for (let i = this.disposeListeners.length - 1; i >= 0; i--) { @@ -749,6 +647,13 @@ export class CommentView implements IRenderedElement { removeDisposeListener(listener: () => void) { this.disposeListeners.splice(this.disposeListeners.indexOf(listener), 1); } + + /** + * @internal + */ + getCommentIcons(): CommentIcon[] { + return [this.foldoutIcon, this.deleteIcon]; + } } css.register(` From a6c3101162a553b36294f4bcf61ca13e028220b9 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Fri, 27 Jun 2025 13:50:59 -0700 Subject: [PATCH 06/16] feat: Update navigation policies to support workspace comments. --- core/keyboard_nav/block_navigation_policy.ts | 36 +++++--- .../comment_icon_navigation_policy.ts | 86 +++++++++++++++++++ .../workspace_comment_navigation_policy.ts | 77 +++++++++++++++++ 3 files changed, 186 insertions(+), 13 deletions(-) create mode 100644 core/keyboard_nav/comment_icon_navigation_policy.ts create mode 100644 core/keyboard_nav/workspace_comment_navigation_policy.ts diff --git a/core/keyboard_nav/block_navigation_policy.ts b/core/keyboard_nav/block_navigation_policy.ts index 2637ad49df5..96e2b5807ee 100644 --- a/core/keyboard_nav/block_navigation_policy.ts +++ b/core/keyboard_nav/block_navigation_policy.ts @@ -8,7 +8,9 @@ import {BlockSvg} from '../block_svg.js'; import {ConnectionType} from '../connection_type.js'; import type {Field} from '../field.js'; import type {Icon} from '../icons/icon.js'; +import type {IBoundedElement} from '../interfaces/i_bounded_element.js'; import type {IFocusableNode} from '../interfaces/i_focusable_node.js'; +import {isFocusableNode} from '../interfaces/i_focusable_node.js'; import type {INavigationPolicy} from '../interfaces/i_navigation_policy.js'; import {RenderedConnection} from '../rendered_connection.js'; import {WorkspaceSvg} from '../workspace_svg.js'; @@ -61,7 +63,7 @@ export class BlockNavigationPolicy implements INavigationPolicy { } else if (current.getSurroundParent()) { return navigateBlock(current.getTopStackBlock(), 1); } else if (this.getParent(current) instanceof WorkspaceSvg) { - return navigateStacks(current, 1); + return navigateStacks(current, current.workspace, 1); } return null; @@ -80,7 +82,7 @@ export class BlockNavigationPolicy implements INavigationPolicy { } else if (current.outputConnection?.targetBlock()) { return navigateBlock(current, -1); } else if (this.getParent(current) instanceof WorkspaceSvg) { - return navigateStacks(current, -1); + return navigateStacks(current, current.workspace, -1); } return null; @@ -143,21 +145,29 @@ function getBlockNavigationCandidates( } /** - * Returns the next/previous stack relative to the given block's stack. + * Returns the next/previous stack relative to the given element's stack. * - * @param current The block whose stack will be navigated relative to. + * @param current The element whose stack will be navigated relative to. * @param delta The difference in index to navigate; positive values navigate * to the nth next stack, while negative values navigate to the nth previous * stack. - * @returns The first block in the stack offset by `delta` relative to the - * current block's stack, or the last block in the stack offset by `delta` - * relative to the current block's stack when navigating backwards. + * @returns The first element in the stack offset by `delta` relative to the + * current element's stack, or the last element in the stack offset by + * `delta` relative to the current element's stack when navigating backwards. */ -export function navigateStacks(current: BlockSvg, delta: number) { - const stacks = current.workspace.getTopBlocks(true); - const currentIndex = stacks.indexOf(current.getRootBlock()); +export function navigateStacks( + current: IFocusableNode, + workspace: WorkspaceSvg, + delta: number, +) { + const stacks: IFocusableNode[] = workspace + .getTopBoundedElements(true) + .filter((element: IBoundedElement) => isFocusableNode(element)); + const currentIndex = stacks.indexOf( + current instanceof BlockSvg ? current.getRootBlock() : current, + ); const targetIndex = currentIndex + delta; - let result: BlockSvg | null = null; + let result: IFocusableNode | null = null; if (targetIndex >= 0 && targetIndex < stacks.length) { result = stacks[targetIndex]; } else if (targetIndex < 0) { @@ -166,9 +176,9 @@ export function navigateStacks(current: BlockSvg, delta: number) { result = stacks[0]; } - // When navigating to a previous stack, our previous sibling is the last + // When navigating to a previous block stack, our previous sibling is the last // block in it. - if (delta < 0 && result) { + if (delta < 0 && result instanceof BlockSvg) { return result.lastConnectionInStack(false)?.getSourceBlock() ?? result; } diff --git a/core/keyboard_nav/comment_icon_navigation_policy.ts b/core/keyboard_nav/comment_icon_navigation_policy.ts new file mode 100644 index 00000000000..9ee64f35319 --- /dev/null +++ b/core/keyboard_nav/comment_icon_navigation_policy.ts @@ -0,0 +1,86 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {CommentIcon} from '../comments/comment_icon.js'; +import type {IFocusableNode} from '../interfaces/i_focusable_node.js'; +import type {INavigationPolicy} from '../interfaces/i_navigation_policy.js'; + +/** + * Set of rules controlling keyboard navigation from a CommentIcon. + */ +export class CommentIconNavigationPolicy + implements INavigationPolicy +{ + /** + * Returns the first child of the given CommentIcon. + * + * @param _current The CommentIcon to return the first child of. + * @returns Null. + */ + getFirstChild(_current: CommentIcon): IFocusableNode | null { + return null; + } + + /** + * Returns the parent of the given CommentIcon. + * + * @param current The CommentIcon to return the parent of. + * @returns The parent comment of the given CommentIcon. + */ + getParent(current: CommentIcon): IFocusableNode | null { + return current.getParentComment(); + } + + /** + * Returns the next peer node of the given CommentIcon. + * + * @param current The CommentIcon to find the following element of. + * @returns The next CommentIcon, if any. + */ + getNextSibling(current: CommentIcon): IFocusableNode | null { + const children = current.getParentComment().view.getCommentIcons(); + const currentIndex = children.indexOf(current); + if (currentIndex >= 0 && currentIndex + 1 < children.length) { + return children[currentIndex + 1]; + } + return null; + } + + /** + * Returns the previous peer node of the given CommentIcon. + * + * @param current The CommentIcon to find the preceding element of. + * @returns The CommentIcon's previous CommentIcon, if any. + */ + getPreviousSibling(current: CommentIcon): IFocusableNode | null { + const children = current.getParentComment().view.getCommentIcons(); + const currentIndex = children.indexOf(current); + if (currentIndex > 0) { + return children[currentIndex - 1]; + } + return null; + } + + /** + * Returns whether or not the given CommentIcon can be navigated to. + * + * @param current The instance to check for navigability. + * @returns True if the given CommentIcon can be focused. + */ + isNavigable(current: CommentIcon): boolean { + return current.canBeFocused(); + } + + /** + * Returns whether the given object can be navigated from by this policy. + * + * @param current The object to check if this policy applies to. + * @returns True if the object is an CommentIcon. + */ + isApplicable(current: any): current is CommentIcon { + return current instanceof CommentIcon; + } +} diff --git a/core/keyboard_nav/workspace_comment_navigation_policy.ts b/core/keyboard_nav/workspace_comment_navigation_policy.ts new file mode 100644 index 00000000000..66f314c38b9 --- /dev/null +++ b/core/keyboard_nav/workspace_comment_navigation_policy.ts @@ -0,0 +1,77 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {RenderedWorkspaceComment} from '../comments/rendered_workspace_comment.js'; +import type {IFocusableNode} from '../interfaces/i_focusable_node.js'; +import type {INavigationPolicy} from '../interfaces/i_navigation_policy.js'; +import {navigateStacks} from './block_navigation_policy.js'; + +/** + * Set of rules controlling keyboard navigation from an RenderedWorkspaceComment. + */ +export class WorkspaceCommentNavigationPolicy + implements INavigationPolicy +{ + /** + * Returns the first child of the given workspace comment. + * + * @param current The workspace comment to return the first child of. + * @returns The first child icon of the given comment. + */ + getFirstChild(current: RenderedWorkspaceComment): IFocusableNode | null { + return current.view.getCommentIcons()[0]; + } + + /** + * Returns the parent of the given workspace comment. + * + * @param current The workspace comment to return the parent of. + * @returns The parent workspace of the given comment. + */ + getParent(current: RenderedWorkspaceComment): IFocusableNode | null { + return current.workspace; + } + + /** + * Returns the next peer node of the given workspace comment. + * + * @param current The workspace comment to find the following element of. + * @returns The next workspace comment or block stack, if any. + */ + getNextSibling(current: RenderedWorkspaceComment): IFocusableNode | null { + return navigateStacks(current, current.workspace, 1); + } + + /** + * Returns the previous peer node of the given workspace comment. + * + * @param current The workspace comment to find the preceding element of. + * @returns The previous workspace comment or block stack, if any. + */ + getPreviousSibling(current: RenderedWorkspaceComment): IFocusableNode | null { + return navigateStacks(current, current.workspace, -1); + } + + /** + * Returns whether or not the given workspace comment can be navigated to. + * + * @param current The instance to check for navigability. + * @returns True if the given workspace comment can be focused. + */ + isNavigable(current: RenderedWorkspaceComment): boolean { + return current.canBeFocused(); + } + + /** + * Returns whether the given object can be navigated from by this policy. + * + * @param current The object to check if this policy applies to. + * @returns True if the object is an RenderedWorkspaceComment. + */ + isApplicable(current: any): current is RenderedWorkspaceComment { + return current instanceof RenderedWorkspaceComment; + } +} From afbc1f88cc5001a6613920d5ca026ba64ab284c2 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Fri, 27 Jun 2025 13:51:21 -0700 Subject: [PATCH 07/16] feat: Make the navigator and workspace handle workspace comments. --- core/navigator.ts | 4 ++++ core/workspace_svg.ts | 29 ++++++++++++++++++++++------- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/core/navigator.ts b/core/navigator.ts index 77bb64cd8c7..dfb2812dba0 100644 --- a/core/navigator.ts +++ b/core/navigator.ts @@ -7,9 +7,11 @@ import type {IFocusableNode} from './interfaces/i_focusable_node.js'; import type {INavigationPolicy} from './interfaces/i_navigation_policy.js'; import {BlockNavigationPolicy} from './keyboard_nav/block_navigation_policy.js'; +import {CommentIconNavigationPolicy} from './keyboard_nav/comment_icon_navigation_policy.js'; import {ConnectionNavigationPolicy} from './keyboard_nav/connection_navigation_policy.js'; import {FieldNavigationPolicy} from './keyboard_nav/field_navigation_policy.js'; import {IconNavigationPolicy} from './keyboard_nav/icon_navigation_policy.js'; +import {WorkspaceCommentNavigationPolicy} from './keyboard_nav/workspace_comment_navigation_policy.js'; import {WorkspaceNavigationPolicy} from './keyboard_nav/workspace_navigation_policy.js'; type RuleList = INavigationPolicy[]; @@ -29,6 +31,8 @@ export class Navigator { new ConnectionNavigationPolicy(), new WorkspaceNavigationPolicy(), new IconNavigationPolicy(), + new WorkspaceCommentNavigationPolicy(), + new CommentIconNavigationPolicy(), ]; /** diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index 87c4d2c9cdb..6945560bb4d 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -22,7 +22,9 @@ import type {Block} from './block.js'; import type {BlockSvg} from './block_svg.js'; import type {BlocklyOptions} from './blockly_options.js'; import * as browserEvents from './browser_events.js'; +import {COMMENT_COLLAPSE_ICON_FOCUS_IDENTIFIER} from './comments/collapse_comment_icon.js'; import {COMMENT_EDITOR_FOCUS_IDENTIFIER} from './comments/comment_editor.js'; +import {COMMENT_DELETE_ICON_FOCUS_IDENTIFIER} from './comments/delete_comment_icon.js'; import {RenderedWorkspaceComment} from './comments/rendered_workspace_comment.js'; import {WorkspaceComment} from './comments/workspace_comment.js'; import * as common from './common.js'; @@ -2821,19 +2823,32 @@ export class WorkspaceSvg return null; } - // Search for a specific workspace comment editor - // (only if id seems like it is one). - const commentEditorIndicator = id.indexOf(COMMENT_EDITOR_FOCUS_IDENTIFIER); - if (commentEditorIndicator !== -1) { - const commentId = id.substring(0, commentEditorIndicator); + // Search for a specific workspace comment or comment icon if the ID + // indicates the presence of one. + const commentIdSeparatorIndex = Math.max( + id.indexOf(COMMENT_EDITOR_FOCUS_IDENTIFIER), + id.indexOf(COMMENT_COLLAPSE_ICON_FOCUS_IDENTIFIER), + id.indexOf(COMMENT_DELETE_ICON_FOCUS_IDENTIFIER), + ); + if (commentIdSeparatorIndex !== -1) { + const commentId = id.substring(0, commentIdSeparatorIndex); const comment = this.searchForWorkspaceComment(commentId); if (comment) { - return comment.getEditorFocusableNode(); + if (id.indexOf(COMMENT_EDITOR_FOCUS_IDENTIFIER) > -1) { + return comment.getEditorFocusableNode(); + } else { + return ( + comment.view + .getCommentIcons() + .find((icon) => icon.getFocusableElement().id.includes(id)) ?? + null + ); + } } } // Search for a specific block. - // Don't use `getBlockById` because the block ID is not guaranteeed + // Don't use `getBlockById` because the block ID is not guaranteed // to be globally unique, but the ID on the focusable element is. const block = this.getAllBlocks(false).find( (block) => block.getFocusableElement().id === id, From 65fddfe92e4540eef920384513670791bb7f6bc3 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Fri, 27 Jun 2025 13:51:41 -0700 Subject: [PATCH 08/16] feat: Visit workspace comments when navigating with the up/down arrows. --- core/keyboard_nav/line_cursor.ts | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/core/keyboard_nav/line_cursor.ts b/core/keyboard_nav/line_cursor.ts index 89668dedb49..aeb80cff170 100644 --- a/core/keyboard_nav/line_cursor.ts +++ b/core/keyboard_nav/line_cursor.ts @@ -14,6 +14,7 @@ */ import {BlockSvg} from '../block_svg.js'; +import {RenderedWorkspaceComment} from '../comments/rendered_workspace_comment.js'; import {Field} from '../field.js'; import {getFocusManager} from '../focus_manager.js'; import type {IFocusableNode} from '../interfaces/i_focusable_node.js'; @@ -38,11 +39,11 @@ export class LineCursor extends Marker { } /** - * Moves the cursor to the next previous connection, next connection or block - * in the pre order traversal. Finds the next node in the pre order traversal. + * Moves the cursor to the next block or workspace comment in the pre-order + * traversal. * - * @returns The next node, or null if the current node is - * not set or there is no next value. + * @returns The next node, or null if the current node is not set or there is + * no next value. */ next(): IFocusableNode | null { const curNode = this.getCurNode(); @@ -53,8 +54,9 @@ export class LineCursor extends Marker { curNode, (candidate: IFocusableNode | null) => { return ( - candidate instanceof BlockSvg && - !candidate.outputConnection?.targetBlock() + (candidate instanceof BlockSvg && + !candidate.outputConnection?.targetBlock()) || + candidate instanceof RenderedWorkspaceComment ); }, true, @@ -87,11 +89,11 @@ export class LineCursor extends Marker { return newNode; } /** - * Moves the cursor to the previous next connection or previous connection in - * the pre order traversal. + * Moves the cursor to the previous block or workspace comment in the + * pre-order traversal. * - * @returns The previous node, or null if the current node - * is not set or there is no previous value. + * @returns The previous node, or null if the current node is not set or there + * is no previous value. */ prev(): IFocusableNode | null { const curNode = this.getCurNode(); @@ -102,8 +104,9 @@ export class LineCursor extends Marker { curNode, (candidate: IFocusableNode | null) => { return ( - candidate instanceof BlockSvg && - !candidate.outputConnection?.targetBlock() + (candidate instanceof BlockSvg && + !candidate.outputConnection?.targetBlock()) || + candidate instanceof RenderedWorkspaceComment ); }, true, From 3eb964b197e1ad977e173a2c571c129202fab6f8 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Fri, 27 Jun 2025 13:52:37 -0700 Subject: [PATCH 09/16] chore: Make the linter happy. --- core/comments/comment_view.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/core/comments/comment_view.ts b/core/comments/comment_view.ts index 1106913799e..84da5ae6a05 100644 --- a/core/comments/comment_view.ts +++ b/core/comments/comment_view.ts @@ -117,7 +117,7 @@ export class CommentView implements IRenderedElement { foldoutIcon: this.foldoutIcon, textPreview: this.textPreview, textPreviewNode: this.textPreviewNode, - } = this.createTopBar(this.svgRoot, workspace)); + } = this.createTopBar(this.svgRoot)); this.commentEditor = this.createTextArea(); @@ -150,10 +150,7 @@ export class CommentView implements IRenderedElement { * Creates the top bar and the elements visually within it. * Registers event listeners. */ - private createTopBar( - svgRoot: SVGGElement, - workspace: WorkspaceSvg, - ): { + private createTopBar(svgRoot: SVGGElement): { topBarGroup: SVGGElement; topBarBackground: SVGRectElement; deleteIcon: DeleteCommentIcon; From 84ab935e539bd93cee983e35ddfcbb0a22a9bd24 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Tue, 1 Jul 2025 10:45:38 -0700 Subject: [PATCH 10/16] chore: Rename comment icons to bar buttons. --- .../{collapse_comment_icon.ts => collapse_comment_bar_button.ts} | 0 core/comments/{comment_icon.ts => comment_bar_button.ts} | 0 .../{delete_comment_icon.ts => delete_comment_bar_button.ts} | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename core/comments/{collapse_comment_icon.ts => collapse_comment_bar_button.ts} (100%) rename core/comments/{comment_icon.ts => comment_bar_button.ts} (100%) rename core/comments/{delete_comment_icon.ts => delete_comment_bar_button.ts} (100%) diff --git a/core/comments/collapse_comment_icon.ts b/core/comments/collapse_comment_bar_button.ts similarity index 100% rename from core/comments/collapse_comment_icon.ts rename to core/comments/collapse_comment_bar_button.ts diff --git a/core/comments/comment_icon.ts b/core/comments/comment_bar_button.ts similarity index 100% rename from core/comments/comment_icon.ts rename to core/comments/comment_bar_button.ts diff --git a/core/comments/delete_comment_icon.ts b/core/comments/delete_comment_bar_button.ts similarity index 100% rename from core/comments/delete_comment_icon.ts rename to core/comments/delete_comment_bar_button.ts From 8a5a58503e9ae7a3d17308eaf42ee2227a94cb74 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Tue, 1 Jul 2025 11:16:25 -0700 Subject: [PATCH 11/16] refactor: Rename CommentIcons to CommentBarButtons. --- core/comments.ts | 6 +- core/comments/collapse_comment_bar_button.ts | 25 +++--- core/comments/comment_bar_button.ts | 42 ++++----- core/comments/comment_view.ts | 60 ++++++------- core/comments/delete_comment_bar_button.ts | 26 +++--- .../comment_bar_button_navigation_policy.ts | 86 +++++++++++++++++++ .../comment_icon_navigation_policy.ts | 86 ------------------- .../workspace_comment_navigation_policy.ts | 4 +- core/navigator.ts | 4 +- core/workspace_svg.ts | 12 +-- 10 files changed, 177 insertions(+), 174 deletions(-) create mode 100644 core/keyboard_nav/comment_bar_button_navigation_policy.ts delete mode 100644 core/keyboard_nav/comment_icon_navigation_policy.ts diff --git a/core/comments.ts b/core/comments.ts index 0cae186e2c7..179ab4a33d0 100644 --- a/core/comments.ts +++ b/core/comments.ts @@ -4,10 +4,10 @@ * SPDX-License-Identifier: Apache-2.0 */ -export {CollapseCommentIcon} from './comments/collapse_comment_icon.js'; +export {CollapseCommentBarButton} from './comments/collapse_comment_bar_button.js'; +export {CommentBarButton} from './comments/comment_bar_button.js'; export {CommentEditor} from './comments/comment_editor.js'; -export {CommentIcon} from './comments/comment_icon.js'; export {CommentView} from './comments/comment_view.js'; -export {DeleteCommentIcon} from './comments/delete_comment_icon.js'; +export {DeleteCommentBarButton} from './comments/delete_comment_bar_button.js'; export {RenderedWorkspaceComment} from './comments/rendered_workspace_comment.js'; export {WorkspaceComment} from './comments/workspace_comment.js'; diff --git a/core/comments/collapse_comment_bar_button.ts b/core/comments/collapse_comment_bar_button.ts index dac66657053..f7f44f74552 100644 --- a/core/comments/collapse_comment_bar_button.ts +++ b/core/comments/collapse_comment_bar_button.ts @@ -9,33 +9,34 @@ import * as touch from '../touch.js'; import * as dom from '../utils/dom.js'; import {Svg} from '../utils/svg.js'; import type {WorkspaceSvg} from '../workspace_svg.js'; -import {CommentIcon} from './comment_icon.js'; +import {CommentBarButton} from './comment_bar_button.js'; /** - * Magic string appended to the comment ID to create a unique ID for this icon. + * Magic string appended to the comment ID to create a unique ID for this button. */ -export const COMMENT_COLLAPSE_ICON_FOCUS_IDENTIFIER = '_collapse_icon'; +export const COMMENT_COLLAPSE_BAR_BUTTON_FOCUS_IDENTIFIER = + '_collapse_bar_button'; /** * Icon that toggles the collapsed state of a comment. */ -export class CollapseCommentIcon extends CommentIcon { +export class CollapseCommentBarButton extends CommentBarButton { /** * Opaque ID used to unbind event handlers during disposal. */ private readonly bindId: browserEvents.Data; /** - * SVG image displayed by this icon. + * SVG image displayed on this button. */ protected override readonly icon: SVGImageElement; /** - * Creates a new CollapseCommentIcon instance. + * Creates a new CollapseCommentBarButton instance. * - * @param id The ID of this icon's parent comment. - * @param workspace The workspace this icon's parent comment is displayed on. - * @param container An SVG group that this icon should be a child of. + * @param id The ID of this button's parent comment. + * @param workspace The workspace this button's parent comment is displayed on. + * @param container An SVG group that this button should be a child of. */ constructor( protected readonly id: string, @@ -49,7 +50,7 @@ export class CollapseCommentIcon extends CommentIcon { { 'class': 'blocklyFoldoutIcon', 'href': `${this.workspace.options.pathToMedia}foldout-icon.svg`, - 'id': `${this.id}${COMMENT_COLLAPSE_ICON_FOCUS_IDENTIFIER}`, + 'id': `${this.id}${COMMENT_COLLAPSE_BAR_BUTTON_FOCUS_IDENTIFIER}`, }, this.container, ); @@ -62,14 +63,14 @@ export class CollapseCommentIcon extends CommentIcon { } /** - * Disposes of this icon. + * Disposes of this button. */ dispose() { browserEvents.unbind(this.bindId); } /** - * Adjusts the positioning of this icon within its container. + * Adjusts the positioning of this button within its container. */ override reposition() { const margin = this.getMargin(); diff --git a/core/comments/comment_bar_button.ts b/core/comments/comment_bar_button.ts index 4450b420d3d..38c4ad8b089 100644 --- a/core/comments/comment_bar_button.ts +++ b/core/comments/comment_bar_button.ts @@ -10,20 +10,20 @@ import type {WorkspaceSvg} from '../workspace_svg.js'; import type {RenderedWorkspaceComment} from './rendered_workspace_comment.js'; /** - * Actionable icon displayed on a comment's top bar. + * Button displayed on a comment's top bar. */ -export abstract class CommentIcon implements IFocusableNode { +export abstract class CommentBarButton implements IFocusableNode { /** - * SVG image displayed by this icon. + * SVG image displayed on this button. */ protected abstract readonly icon: SVGImageElement; /** - * Creates a new CollapseCommentIcon instance. + * Creates a new CommentBarButton instance. * - * @param id The ID of this icon's parent comment. - * @param workspace The workspace this icon's parent comment is displayed on. - * @param container An SVG group that this icon should be a child of. + * @param id The ID of this button's parent comment. + * @param workspace The workspace this button's parent comment is on. + * @param container An SVG group that this button should be a child of. */ constructor( protected readonly id: string, @@ -32,35 +32,37 @@ export abstract class CommentIcon implements IFocusableNode { ) {} /** - * Returns whether or not this icon is currently visible. + * Returns whether or not this button is currently visible. */ isVisible(): boolean { return this.icon.checkVisibility(); } /** - * Returns the parent comment of this comment icon. + * Returns the parent comment of this comment bar button. */ getParentComment(): RenderedWorkspaceComment { const comment = this.workspace.getCommentById(this.id); if (!comment) { - throw new Error(`Comment icon ${this.id} has no corresponding comment`); + throw new Error( + `Comment bar button ${this.id} has no corresponding comment`, + ); } return comment; } - /** Adjusts the position of this icon within its parent container. */ + /** Adjusts the position of this button within its parent container. */ abstract reposition(): void; - /** Perform the action this icon should take when it is acted on. */ + /** Perform the action this button should take when it is acted on. */ abstract performAction(e?: Event): void; /** - * Returns the dimensions of this icon in SVG units. + * Returns the dimensions of this button in SVG units. * * @param includeMargin True to include the margin when calculating the size. - * @returns The size of this icon. + * @returns The size of this button. */ getSize(includeMargin = false): Rect { const bounds = this.icon.getBBox(); @@ -75,28 +77,28 @@ export abstract class CommentIcon implements IFocusableNode { return rect; } - /** Returns the margin in SVG units surrounding this icon. */ + /** Returns the margin in SVG units surrounding this button. */ getMargin(): number { return (this.container.getBBox().height - this.icon.getBBox().height) / 2; } - /** Returns a DOM element representing this icon that can receive focus. */ + /** Returns a DOM element representing this button that can receive focus. */ getFocusableElement() { return this.icon; } - /** Returns the workspace this icon is a child of. */ + /** Returns the workspace this button is a child of. */ getFocusableTree() { return this.workspace; } - /** Called when this icon's focusable DOM element gains focus. */ + /** Called when this button's focusable DOM element gains focus. */ onNodeFocus() {} - /** Called when this icon's focusable DOM element loses focus. */ + /** Called when this button's focusable DOM element loses focus. */ onNodeBlur() {} - /** Returns whether or not this icon can be focused. True if it is visible. */ + /** Returns whether this button can be focused. True if it is visible. */ canBeFocused() { return this.isVisible(); } diff --git a/core/comments/comment_view.ts b/core/comments/comment_view.ts index 84da5ae6a05..05faf9ea4cc 100644 --- a/core/comments/comment_view.ts +++ b/core/comments/comment_view.ts @@ -16,10 +16,10 @@ import * as drag from '../utils/drag.js'; import {Size} from '../utils/size.js'; import {Svg} from '../utils/svg.js'; import {WorkspaceSvg} from '../workspace_svg.js'; -import {CollapseCommentIcon} from './collapse_comment_icon.js'; +import {CollapseCommentBarButton} from './collapse_comment_bar_button.js'; +import {CommentBarButton} from './comment_bar_button.js'; import {CommentEditor} from './comment_editor.js'; -import type {CommentIcon} from './comment_icon.js'; -import {DeleteCommentIcon} from './delete_comment_icon.js'; +import {DeleteCommentBarButton} from './delete_comment_bar_button.js'; export class CommentView implements IRenderedElement { /** The root group element of the comment view. */ @@ -36,11 +36,11 @@ export class CommentView implements IRenderedElement { /** The rect background for the top bar. */ private topBarBackground: SVGRectElement; - /** The delete icon that goes in the top bar. */ - private deleteIcon: DeleteCommentIcon; + /** The delete button that goes in the top bar. */ + private deleteButton: DeleteCommentBarButton; - /** The foldout icon that goes in the top bar. */ - private foldoutIcon: CollapseCommentIcon; + /** The foldout button that goes in the top bar. */ + private foldoutButton: CollapseCommentBarButton; /** The text element that goes in the top bar. */ private textPreview: SVGTextElement; @@ -113,8 +113,8 @@ export class CommentView implements IRenderedElement { ({ topBarGroup: this.topBarGroup, topBarBackground: this.topBarBackground, - deleteIcon: this.deleteIcon, - foldoutIcon: this.foldoutIcon, + deleteButton: this.deleteButton, + foldoutButton: this.foldoutButton, textPreview: this.textPreview, textPreviewNode: this.textPreviewNode, } = this.createTopBar(this.svgRoot)); @@ -153,8 +153,8 @@ export class CommentView implements IRenderedElement { private createTopBar(svgRoot: SVGGElement): { topBarGroup: SVGGElement; topBarBackground: SVGRectElement; - deleteIcon: DeleteCommentIcon; - foldoutIcon: CollapseCommentIcon; + deleteButton: DeleteCommentBarButton; + foldoutButton: CollapseCommentBarButton; textPreview: SVGTextElement; textPreviewNode: Text; } { @@ -172,12 +172,12 @@ export class CommentView implements IRenderedElement { }, topBarGroup, ); - const deleteIcon = new DeleteCommentIcon( + const deleteButton = new DeleteCommentBarButton( this.commentId, this.workspace, topBarGroup, ); - const foldoutIcon = new CollapseCommentIcon( + const foldoutButton = new CollapseCommentBarButton( this.commentId, this.workspace, topBarGroup, @@ -195,8 +195,8 @@ export class CommentView implements IRenderedElement { return { topBarGroup, topBarBackground, - deleteIcon, - foldoutIcon, + deleteButton, + foldoutButton, textPreview, textPreviewNode, }; @@ -288,8 +288,8 @@ export class CommentView implements IRenderedElement { this.updateHighlightRect(size); this.updateTopBarSize(size); this.commentEditor.updateSize(size, topBarSize); - this.deleteIcon.reposition(); - this.foldoutIcon.reposition(); + this.deleteButton.reposition(); + this.foldoutButton.reposition(); this.updateTextPreviewSize(size, topBarSize, textPreviewSize); this.updateResizeHandlePosition(size, resizeSize); } @@ -317,13 +317,13 @@ export class CommentView implements IRenderedElement { const textPreviewWidth = dom.getTextWidth(this.textPreview); let width = textPreviewWidth; - if (this.foldoutIcon.isVisible()) { - width += this.foldoutIcon.getSize(true).getWidth(); + if (this.foldoutButton.isVisible()) { + width += this.foldoutButton.getSize(true).getWidth(); } else if (textPreviewWidth) { width += 4; // Arbitrary margin before text. } - if (this.deleteIcon.isVisible()) { - width += this.deleteIcon.getSize(true).getWidth(); + if (this.deleteButton.isVisible()) { + width += this.deleteButton.getSize(true).getWidth(); } else if (textPreviewWidth) { width += 4; // Arbitrary margin after text. } @@ -357,8 +357,8 @@ export class CommentView implements IRenderedElement { textPreviewSize: Size, ) { const textPreviewMargin = (topBarSize.height - textPreviewSize.height) / 2; - const foldoutSize = this.foldoutIcon.getSize(true); - const deleteSize = this.deleteIcon.getSize(true); + const foldoutSize = this.foldoutButton.getSize(true); + const deleteSize = this.deleteButton.getSize(true); const textPreviewWidth = size.width - foldoutSize.getWidth() - deleteSize.getWidth(); @@ -612,8 +612,8 @@ export class CommentView implements IRenderedElement { /** Disposes of this comment view. */ dispose() { this.disposing = true; - this.foldoutIcon.dispose(); - this.deleteIcon.dispose(); + this.foldoutButton.dispose(); + this.deleteButton.dispose(); dom.removeNode(this.svgRoot); // Loop through listeners backwards in case they remove themselves. for (let i = this.disposeListeners.length - 1; i >= 0; i--) { @@ -648,8 +648,8 @@ export class CommentView implements IRenderedElement { /** * @internal */ - getCommentIcons(): CommentIcon[] { - return [this.foldoutIcon, this.deleteIcon]; + getCommentBarButtons(): CommentBarButton[] { + return [this.foldoutButton, this.deleteButton]; } } @@ -675,14 +675,14 @@ css.register(` cursor: inherit; } -.blocklyDeleteIcon { +.blocklydeleteButton { width: 20px; height: 20px; display: none; cursor: pointer; } -.blocklyFoldoutIcon { +.blocklyfoldoutButton { width: 20px; height: 20px; transform-origin: 12px 12px; @@ -717,7 +717,7 @@ css.register(` display: none; } -.blocklyCollapsed.blocklyComment .blocklyFoldoutIcon { +.blocklyCollapsed.blocklyComment .blocklyfoldoutButton { transform: rotate(-90deg); } diff --git a/core/comments/delete_comment_bar_button.ts b/core/comments/delete_comment_bar_button.ts index a27b0a23d5d..5b5368f7718 100644 --- a/core/comments/delete_comment_bar_button.ts +++ b/core/comments/delete_comment_bar_button.ts @@ -9,33 +9,33 @@ import * as touch from '../touch.js'; import * as dom from '../utils/dom.js'; import {Svg} from '../utils/svg.js'; import type {WorkspaceSvg} from '../workspace_svg.js'; -import {CommentIcon} from './comment_icon.js'; +import {CommentBarButton} from './comment_bar_button.js'; /** - * Magic string appended to the comment ID to create a unique ID for this icon. + * Magic string appended to the comment ID to create a unique ID for this button. */ -export const COMMENT_DELETE_ICON_FOCUS_IDENTIFIER = '_delete_icon'; +export const COMMENT_DELETE_BAR_BUTTON_FOCUS_IDENTIFIER = '_delete_bar_button'; /** - * Icon that deletes a comment. + * Button that deletes a comment. */ -export class DeleteCommentIcon extends CommentIcon { +export class DeleteCommentBarButton extends CommentBarButton { /** * Opaque ID used to unbind event handlers during disposal. */ private readonly bindId: browserEvents.Data; /** - * SVG image displayed by this icon. + * SVG image displayed on this button. */ protected override readonly icon: SVGImageElement; /** - * Creates a new DeleteCommentIcon instance. + * Creates a new DeleteCommentBarButton instance. * - * @param id The ID of this icon's parent comment. - * @param workspace The workspace this icon's parent comment is displayed on. - * @param container An SVG group that this icon should be a child of. + * @param id The ID of this button's parent comment. + * @param workspace The workspace this button's parent comment is shown on. + * @param container An SVG group that this button should be a child of. */ constructor( protected readonly id: string, @@ -49,7 +49,7 @@ export class DeleteCommentIcon extends CommentIcon { { 'class': 'blocklyDeleteIcon', 'href': `${this.workspace.options.pathToMedia}delete-icon.svg`, - 'id': `${this.id}${COMMENT_DELETE_ICON_FOCUS_IDENTIFIER}`, + 'id': `${this.id}${COMMENT_DELETE_BAR_BUTTON_FOCUS_IDENTIFIER}`, }, container, ); @@ -62,14 +62,14 @@ export class DeleteCommentIcon extends CommentIcon { } /** - * Disposes of this icon. + * Disposes of this button. */ dispose() { browserEvents.unbind(this.bindId); } /** - * Adjusts the positioning of this icon within its container. + * Adjusts the positioning of this button within its container. */ override reposition() { const margin = this.getMargin(); diff --git a/core/keyboard_nav/comment_bar_button_navigation_policy.ts b/core/keyboard_nav/comment_bar_button_navigation_policy.ts new file mode 100644 index 00000000000..f676f465582 --- /dev/null +++ b/core/keyboard_nav/comment_bar_button_navigation_policy.ts @@ -0,0 +1,86 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {CommentBarButton} from '../comments/comment_bar_button.js'; +import type {IFocusableNode} from '../interfaces/i_focusable_node.js'; +import type {INavigationPolicy} from '../interfaces/i_navigation_policy.js'; + +/** + * Set of rules controlling keyboard navigation from a CommentBarButton. + */ +export class CommentBarButtonNavigationPolicy + implements INavigationPolicy +{ + /** + * Returns the first child of the given CommentBarButton. + * + * @param _current The CommentBarButton to return the first child of. + * @returns Null. + */ + getFirstChild(_current: CommentBarButton): IFocusableNode | null { + return null; + } + + /** + * Returns the parent of the given CommentBarButton. + * + * @param current The CommentBarButton to return the parent of. + * @returns The parent comment of the given CommentBarButton. + */ + getParent(current: CommentBarButton): IFocusableNode | null { + return current.getParentComment(); + } + + /** + * Returns the next peer node of the given CommentBarButton. + * + * @param current The CommentBarButton to find the following element of. + * @returns The next CommentBarButton, if any. + */ + getNextSibling(current: CommentBarButton): IFocusableNode | null { + const children = current.getParentComment().view.getCommentBarButtons(); + const currentIndex = children.indexOf(current); + if (currentIndex >= 0 && currentIndex + 1 < children.length) { + return children[currentIndex + 1]; + } + return null; + } + + /** + * Returns the previous peer node of the given CommentBarButton. + * + * @param current The CommentBarButton to find the preceding element of. + * @returns The CommentBarButton's previous CommentBarButton, if any. + */ + getPreviousSibling(current: CommentBarButton): IFocusableNode | null { + const children = current.getParentComment().view.getCommentBarButtons(); + const currentIndex = children.indexOf(current); + if (currentIndex > 0) { + return children[currentIndex - 1]; + } + return null; + } + + /** + * Returns whether or not the given CommentBarButton can be navigated to. + * + * @param current The instance to check for navigability. + * @returns True if the given CommentBarButton can be focused. + */ + isNavigable(current: CommentBarButton): boolean { + return current.canBeFocused(); + } + + /** + * Returns whether the given object can be navigated from by this policy. + * + * @param current The object to check if this policy applies to. + * @returns True if the object is an CommentBarButton. + */ + isApplicable(current: any): current is CommentBarButton { + return current instanceof CommentBarButton; + } +} diff --git a/core/keyboard_nav/comment_icon_navigation_policy.ts b/core/keyboard_nav/comment_icon_navigation_policy.ts deleted file mode 100644 index 9ee64f35319..00000000000 --- a/core/keyboard_nav/comment_icon_navigation_policy.ts +++ /dev/null @@ -1,86 +0,0 @@ -/** - * @license - * Copyright 2025 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -import {CommentIcon} from '../comments/comment_icon.js'; -import type {IFocusableNode} from '../interfaces/i_focusable_node.js'; -import type {INavigationPolicy} from '../interfaces/i_navigation_policy.js'; - -/** - * Set of rules controlling keyboard navigation from a CommentIcon. - */ -export class CommentIconNavigationPolicy - implements INavigationPolicy -{ - /** - * Returns the first child of the given CommentIcon. - * - * @param _current The CommentIcon to return the first child of. - * @returns Null. - */ - getFirstChild(_current: CommentIcon): IFocusableNode | null { - return null; - } - - /** - * Returns the parent of the given CommentIcon. - * - * @param current The CommentIcon to return the parent of. - * @returns The parent comment of the given CommentIcon. - */ - getParent(current: CommentIcon): IFocusableNode | null { - return current.getParentComment(); - } - - /** - * Returns the next peer node of the given CommentIcon. - * - * @param current The CommentIcon to find the following element of. - * @returns The next CommentIcon, if any. - */ - getNextSibling(current: CommentIcon): IFocusableNode | null { - const children = current.getParentComment().view.getCommentIcons(); - const currentIndex = children.indexOf(current); - if (currentIndex >= 0 && currentIndex + 1 < children.length) { - return children[currentIndex + 1]; - } - return null; - } - - /** - * Returns the previous peer node of the given CommentIcon. - * - * @param current The CommentIcon to find the preceding element of. - * @returns The CommentIcon's previous CommentIcon, if any. - */ - getPreviousSibling(current: CommentIcon): IFocusableNode | null { - const children = current.getParentComment().view.getCommentIcons(); - const currentIndex = children.indexOf(current); - if (currentIndex > 0) { - return children[currentIndex - 1]; - } - return null; - } - - /** - * Returns whether or not the given CommentIcon can be navigated to. - * - * @param current The instance to check for navigability. - * @returns True if the given CommentIcon can be focused. - */ - isNavigable(current: CommentIcon): boolean { - return current.canBeFocused(); - } - - /** - * Returns whether the given object can be navigated from by this policy. - * - * @param current The object to check if this policy applies to. - * @returns True if the object is an CommentIcon. - */ - isApplicable(current: any): current is CommentIcon { - return current instanceof CommentIcon; - } -} diff --git a/core/keyboard_nav/workspace_comment_navigation_policy.ts b/core/keyboard_nav/workspace_comment_navigation_policy.ts index 66f314c38b9..04ddbe1a6aa 100644 --- a/core/keyboard_nav/workspace_comment_navigation_policy.ts +++ b/core/keyboard_nav/workspace_comment_navigation_policy.ts @@ -19,10 +19,10 @@ export class WorkspaceCommentNavigationPolicy * Returns the first child of the given workspace comment. * * @param current The workspace comment to return the first child of. - * @returns The first child icon of the given comment. + * @returns The first child button of the given comment. */ getFirstChild(current: RenderedWorkspaceComment): IFocusableNode | null { - return current.view.getCommentIcons()[0]; + return current.view.getCommentBarButtons()[0]; } /** diff --git a/core/navigator.ts b/core/navigator.ts index dfb2812dba0..2f095f6f962 100644 --- a/core/navigator.ts +++ b/core/navigator.ts @@ -7,7 +7,7 @@ import type {IFocusableNode} from './interfaces/i_focusable_node.js'; import type {INavigationPolicy} from './interfaces/i_navigation_policy.js'; import {BlockNavigationPolicy} from './keyboard_nav/block_navigation_policy.js'; -import {CommentIconNavigationPolicy} from './keyboard_nav/comment_icon_navigation_policy.js'; +import {CommentBarButtonNavigationPolicy} from './keyboard_nav/comment_bar_button_navigation_policy.js'; import {ConnectionNavigationPolicy} from './keyboard_nav/connection_navigation_policy.js'; import {FieldNavigationPolicy} from './keyboard_nav/field_navigation_policy.js'; import {IconNavigationPolicy} from './keyboard_nav/icon_navigation_policy.js'; @@ -32,7 +32,7 @@ export class Navigator { new WorkspaceNavigationPolicy(), new IconNavigationPolicy(), new WorkspaceCommentNavigationPolicy(), - new CommentIconNavigationPolicy(), + new CommentBarButtonNavigationPolicy(), ]; /** diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index 6945560bb4d..00eef565394 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -22,9 +22,9 @@ import type {Block} from './block.js'; import type {BlockSvg} from './block_svg.js'; import type {BlocklyOptions} from './blockly_options.js'; import * as browserEvents from './browser_events.js'; -import {COMMENT_COLLAPSE_ICON_FOCUS_IDENTIFIER} from './comments/collapse_comment_icon.js'; +import {COMMENT_COLLAPSE_BAR_BUTTON_FOCUS_IDENTIFIER} from './comments/collapse_comment_bar_button.js'; import {COMMENT_EDITOR_FOCUS_IDENTIFIER} from './comments/comment_editor.js'; -import {COMMENT_DELETE_ICON_FOCUS_IDENTIFIER} from './comments/delete_comment_icon.js'; +import {COMMENT_DELETE_BAR_BUTTON_FOCUS_IDENTIFIER} from './comments/delete_comment_bar_button.js'; import {RenderedWorkspaceComment} from './comments/rendered_workspace_comment.js'; import {WorkspaceComment} from './comments/workspace_comment.js'; import * as common from './common.js'; @@ -2827,8 +2827,8 @@ export class WorkspaceSvg // indicates the presence of one. const commentIdSeparatorIndex = Math.max( id.indexOf(COMMENT_EDITOR_FOCUS_IDENTIFIER), - id.indexOf(COMMENT_COLLAPSE_ICON_FOCUS_IDENTIFIER), - id.indexOf(COMMENT_DELETE_ICON_FOCUS_IDENTIFIER), + id.indexOf(COMMENT_COLLAPSE_BAR_BUTTON_FOCUS_IDENTIFIER), + id.indexOf(COMMENT_DELETE_BAR_BUTTON_FOCUS_IDENTIFIER), ); if (commentIdSeparatorIndex !== -1) { const commentId = id.substring(0, commentIdSeparatorIndex); @@ -2839,8 +2839,8 @@ export class WorkspaceSvg } else { return ( comment.view - .getCommentIcons() - .find((icon) => icon.getFocusableElement().id.includes(id)) ?? + .getCommentBarButtons() + .find((button) => button.getFocusableElement().id.includes(id)) ?? null ); } From deef1c9018ab9d3720d43d0ff4fb3b0685616e23 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Tue, 1 Jul 2025 11:30:31 -0700 Subject: [PATCH 12/16] chore: Improve docstrings. --- core/comments/collapse_comment_bar_button.ts | 2 +- core/comments/comment_bar_button.ts | 2 +- core/keyboard_nav/block_navigation_policy.ts | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/core/comments/collapse_comment_bar_button.ts b/core/comments/collapse_comment_bar_button.ts index f7f44f74552..b0738d70705 100644 --- a/core/comments/collapse_comment_bar_button.ts +++ b/core/comments/collapse_comment_bar_button.ts @@ -18,7 +18,7 @@ export const COMMENT_COLLAPSE_BAR_BUTTON_FOCUS_IDENTIFIER = '_collapse_bar_button'; /** - * Icon that toggles the collapsed state of a comment. + * Button that toggles the collapsed state of a comment. */ export class CollapseCommentBarButton extends CommentBarButton { /** diff --git a/core/comments/comment_bar_button.ts b/core/comments/comment_bar_button.ts index 38c4ad8b089..4ba1b8d3c28 100644 --- a/core/comments/comment_bar_button.ts +++ b/core/comments/comment_bar_button.ts @@ -59,7 +59,7 @@ export abstract class CommentBarButton implements IFocusableNode { abstract performAction(e?: Event): void; /** - * Returns the dimensions of this button in SVG units. + * Returns the dimensions of this button in workspace coordinates. * * @param includeMargin True to include the margin when calculating the size. * @returns The size of this button. diff --git a/core/keyboard_nav/block_navigation_policy.ts b/core/keyboard_nav/block_navigation_policy.ts index 96e2b5807ee..499eabca704 100644 --- a/core/keyboard_nav/block_navigation_policy.ts +++ b/core/keyboard_nav/block_navigation_policy.ts @@ -148,6 +148,7 @@ function getBlockNavigationCandidates( * Returns the next/previous stack relative to the given element's stack. * * @param current The element whose stack will be navigated relative to. + * @param workspace The workspace to navigate between stacks on. * @param delta The difference in index to navigate; positive values navigate * to the nth next stack, while negative values navigate to the nth previous * stack. From 3a0fc622e1f2721248ae3588f0e0421ca5938c5e Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Tue, 1 Jul 2025 13:09:55 -0700 Subject: [PATCH 13/16] chore: Clarify unit type. --- core/comments/comment_bar_button.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/comments/comment_bar_button.ts b/core/comments/comment_bar_button.ts index 4ba1b8d3c28..d78a7fd86a1 100644 --- a/core/comments/comment_bar_button.ts +++ b/core/comments/comment_bar_button.ts @@ -77,7 +77,7 @@ export abstract class CommentBarButton implements IFocusableNode { return rect; } - /** Returns the margin in SVG units surrounding this button. */ + /** Returns the margin in workspace coordinates surrounding this button. */ getMargin(): number { return (this.container.getBBox().height - this.icon.getBBox().height) / 2; } From ec0fea0355caa67228bb0b1733bfaf0fda84e432 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Tue, 1 Jul 2025 13:12:07 -0700 Subject: [PATCH 14/16] refactor: Remove workspace argument from `navigateStacks()`. --- core/keyboard_nav/block_navigation_policy.ts | 14 +++++--------- .../workspace_comment_navigation_policy.ts | 4 ++-- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/core/keyboard_nav/block_navigation_policy.ts b/core/keyboard_nav/block_navigation_policy.ts index 499eabca704..9f56b538455 100644 --- a/core/keyboard_nav/block_navigation_policy.ts +++ b/core/keyboard_nav/block_navigation_policy.ts @@ -12,6 +12,7 @@ import type {IBoundedElement} from '../interfaces/i_bounded_element.js'; import type {IFocusableNode} from '../interfaces/i_focusable_node.js'; import {isFocusableNode} from '../interfaces/i_focusable_node.js'; import type {INavigationPolicy} from '../interfaces/i_navigation_policy.js'; +import type {ISelectable} from '../interfaces/i_selectable.js'; import {RenderedConnection} from '../rendered_connection.js'; import {WorkspaceSvg} from '../workspace_svg.js'; @@ -63,7 +64,7 @@ export class BlockNavigationPolicy implements INavigationPolicy { } else if (current.getSurroundParent()) { return navigateBlock(current.getTopStackBlock(), 1); } else if (this.getParent(current) instanceof WorkspaceSvg) { - return navigateStacks(current, current.workspace, 1); + return navigateStacks(current, 1); } return null; @@ -82,7 +83,7 @@ export class BlockNavigationPolicy implements INavigationPolicy { } else if (current.outputConnection?.targetBlock()) { return navigateBlock(current, -1); } else if (this.getParent(current) instanceof WorkspaceSvg) { - return navigateStacks(current, current.workspace, -1); + return navigateStacks(current, -1); } return null; @@ -148,7 +149,6 @@ function getBlockNavigationCandidates( * Returns the next/previous stack relative to the given element's stack. * * @param current The element whose stack will be navigated relative to. - * @param workspace The workspace to navigate between stacks on. * @param delta The difference in index to navigate; positive values navigate * to the nth next stack, while negative values navigate to the nth previous * stack. @@ -156,12 +156,8 @@ function getBlockNavigationCandidates( * current element's stack, or the last element in the stack offset by * `delta` relative to the current element's stack when navigating backwards. */ -export function navigateStacks( - current: IFocusableNode, - workspace: WorkspaceSvg, - delta: number, -) { - const stacks: IFocusableNode[] = workspace +export function navigateStacks(current: ISelectable, delta: number) { + const stacks: IFocusableNode[] = (current.workspace as WorkspaceSvg) .getTopBoundedElements(true) .filter((element: IBoundedElement) => isFocusableNode(element)); const currentIndex = stacks.indexOf( diff --git a/core/keyboard_nav/workspace_comment_navigation_policy.ts b/core/keyboard_nav/workspace_comment_navigation_policy.ts index 04ddbe1a6aa..7fe70ceadef 100644 --- a/core/keyboard_nav/workspace_comment_navigation_policy.ts +++ b/core/keyboard_nav/workspace_comment_navigation_policy.ts @@ -42,7 +42,7 @@ export class WorkspaceCommentNavigationPolicy * @returns The next workspace comment or block stack, if any. */ getNextSibling(current: RenderedWorkspaceComment): IFocusableNode | null { - return navigateStacks(current, current.workspace, 1); + return navigateStacks(current, 1); } /** @@ -52,7 +52,7 @@ export class WorkspaceCommentNavigationPolicy * @returns The previous workspace comment or block stack, if any. */ getPreviousSibling(current: RenderedWorkspaceComment): IFocusableNode | null { - return navigateStacks(current, current.workspace, -1); + return navigateStacks(current, -1); } /** From a2c248bec2afc33e59c81b067079bbb45a5e25fb Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Tue, 1 Jul 2025 15:09:35 -0700 Subject: [PATCH 15/16] fix: Fix errant find and replace in CSS. --- core/comments/comment_view.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/comments/comment_view.ts b/core/comments/comment_view.ts index 05faf9ea4cc..936d746508f 100644 --- a/core/comments/comment_view.ts +++ b/core/comments/comment_view.ts @@ -675,14 +675,14 @@ css.register(` cursor: inherit; } -.blocklydeleteButton { +.blocklyDeleteIcon { width: 20px; height: 20px; display: none; cursor: pointer; } -.blocklyfoldoutButton { +.blocklyFoldoutIcon { width: 20px; height: 20px; transform-origin: 12px 12px; @@ -717,7 +717,7 @@ css.register(` display: none; } -.blocklyCollapsed.blocklyComment .blocklyfoldoutButton { +.blocklyCollapsed.blocklyComment .blocklyFoldoutIcon { transform: rotate(-90deg); } From 2c0b0d13553a73937fa19eae3e4989d14a468379 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Tue, 1 Jul 2025 15:09:49 -0700 Subject: [PATCH 16/16] fix: Fix issue that could cause delete button to become misaligned. --- core/comments/delete_comment_bar_button.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/comments/delete_comment_bar_button.ts b/core/comments/delete_comment_bar_button.ts index 5b5368f7718..ccdd0253916 100644 --- a/core/comments/delete_comment_bar_button.ts +++ b/core/comments/delete_comment_bar_button.ts @@ -73,6 +73,9 @@ export class DeleteCommentBarButton extends CommentBarButton { */ override reposition() { const margin = this.getMargin(); + // Reset to 0 so that our position doesn't force the parent container to + // grow. + this.icon.setAttribute('x', `0`); const containerSize = this.container.getBBox(); this.icon.setAttribute('y', `${margin}`); this.icon.setAttribute(