From eaf5eea98ec7722c3bac442ef9e840e817de3aa2 Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Tue, 24 Jun 2025 12:40:23 -0700 Subject: [PATCH] feat: make comment editor separately focusable from comment itself (#9154) * feat: make comment editor separately focusable from comment itself * feat: improve design and add styling * chore: fix lint * fix: add event listeners to focus parent comment * fix: export CommentEditor * fix: export CommentEditor * fix: extract comment identifier to constant --- core/comments.ts | 1 + core/comments/comment_editor.ts | 188 ++++++++++++++++++++ core/comments/comment_view.ts | 133 +++++--------- core/comments/rendered_workspace_comment.ts | 19 +- core/workspace_svg.ts | 45 ++++- 5 files changed, 284 insertions(+), 102 deletions(-) create mode 100644 core/comments/comment_editor.ts diff --git a/core/comments.ts b/core/comments.ts index ee8591987..86e8f50b9 100644 --- a/core/comments.ts +++ b/core/comments.ts @@ -4,6 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +export {CommentEditor} from './comments/comment_editor.js'; export {CommentView} from './comments/comment_view.js'; export {RenderedWorkspaceComment} from './comments/rendered_workspace_comment.js'; export {WorkspaceComment} from './comments/workspace_comment.js'; diff --git a/core/comments/comment_editor.ts b/core/comments/comment_editor.ts new file mode 100644 index 000000000..f921168fa --- /dev/null +++ b/core/comments/comment_editor.ts @@ -0,0 +1,188 @@ +/** + * @license + * Copyright 2024 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as browserEvents from '../browser_events.js'; +import {getFocusManager} from '../focus_manager.js'; +import {IFocusableNode} from '../interfaces/i_focusable_node.js'; +import {IFocusableTree} from '../interfaces/i_focusable_tree.js'; +import * as dom from '../utils/dom.js'; +import {Size} from '../utils/size.js'; +import {Svg} from '../utils/svg.js'; +import {WorkspaceSvg} from '../workspace_svg.js'; + +/** + * String added to the ID of a workspace comment to identify + * the focusable node for the comment editor. + */ +export const COMMENT_EDITOR_FOCUS_IDENTIFIER = '_comment_textarea_'; + +/** The part of a comment that can be typed into. */ +export class CommentEditor implements IFocusableNode { + id?: string; + /** The foreignObject containing the HTML text area. */ + private foreignObject: SVGForeignObjectElement; + + /** The text area where the user can type. */ + private textArea: HTMLTextAreaElement; + + /** Listeners for changes to text. */ + private textChangeListeners: Array< + (oldText: string, newText: string) => void + > = []; + + /** The current text of the comment. Updates on text area change. */ + private text: string = ''; + + constructor( + public workspace: WorkspaceSvg, + commentId?: string, + private onFinishEditing?: () => void, + ) { + this.foreignObject = dom.createSvgElement(Svg.FOREIGNOBJECT, { + 'class': 'blocklyCommentForeignObject', + }); + const body = document.createElementNS(dom.HTML_NS, 'body'); + body.setAttribute('xmlns', dom.HTML_NS); + body.className = 'blocklyMinimalBody'; + this.textArea = document.createElementNS( + dom.HTML_NS, + 'textarea', + ) as HTMLTextAreaElement; + dom.addClass(this.textArea, 'blocklyCommentText'); + dom.addClass(this.textArea, 'blocklyTextarea'); + dom.addClass(this.textArea, 'blocklyText'); + body.appendChild(this.textArea); + this.foreignObject.appendChild(body); + + if (commentId) { + this.id = commentId + COMMENT_EDITOR_FOCUS_IDENTIFIER; + this.textArea.setAttribute('id', this.id); + } + + // Register browser event listeners for the user typing in the textarea. + browserEvents.conditionalBind( + this.textArea, + 'change', + this, + this.onTextChange, + ); + + // Register listener for pointerdown to focus the textarea. + browserEvents.conditionalBind( + this.textArea, + 'pointerdown', + this, + (e: PointerEvent) => { + // don't allow this event to bubble up + // and steal focus away from the editor/comment. + e.stopPropagation(); + getFocusManager().focusNode(this); + }, + ); + + // Register listener for keydown events that would finish editing. + browserEvents.conditionalBind( + this.textArea, + 'keydown', + this, + this.handleKeyDown, + ); + } + + /** Gets the dom structure for this comment editor. */ + getDom(): SVGForeignObjectElement { + return this.foreignObject; + } + + /** Gets the current text of the comment. */ + getText(): string { + return this.text; + } + + /** Sets the current text of the comment and fires change listeners. */ + setText(text: string) { + this.textArea.value = text; + this.onTextChange(); + } + + /** + * Triggers listeners when the text of the comment changes, either + * programmatically or manually by the user. + */ + private onTextChange() { + const oldText = this.text; + this.text = this.textArea.value; + // Loop through listeners backwards in case they remove themselves. + for (let i = this.textChangeListeners.length - 1; i >= 0; i--) { + this.textChangeListeners[i](oldText, this.text); + } + } + + /** + * Do something when the user indicates they've finished editing. + * + * @param e Keyboard event. + */ + private handleKeyDown(e: KeyboardEvent) { + if (e.key === 'Escape' || (e.key === 'Enter' && (e.ctrlKey || e.metaKey))) { + if (this.onFinishEditing) this.onFinishEditing(); + e.stopPropagation(); + } + } + + /** Registers a callback that listens for text changes. */ + addTextChangeListener(listener: (oldText: string, newText: string) => void) { + this.textChangeListeners.push(listener); + } + + /** Removes the given listener from the list of text change listeners. */ + removeTextChangeListener(listener: () => void) { + this.textChangeListeners.splice( + this.textChangeListeners.indexOf(listener), + 1, + ); + } + + /** Sets the placeholder text displayed for an empty comment. */ + setPlaceholderText(text: string) { + this.textArea.placeholder = text; + } + + /** Sets whether the textarea is editable. If not, the textarea will be readonly. */ + setEditable(isEditable: boolean) { + if (isEditable) { + this.textArea.removeAttribute('readonly'); + } else { + this.textArea.setAttribute('readonly', 'true'); + } + } + + /** Update the size of the comment editor element. */ + updateSize(size: Size, topBarSize: Size) { + this.foreignObject.setAttribute( + 'height', + `${size.height - topBarSize.height}`, + ); + this.foreignObject.setAttribute('width', `${size.width}`); + this.foreignObject.setAttribute('y', `${topBarSize.height}`); + if (this.workspace.RTL) { + this.foreignObject.setAttribute('x', `${-size.width}`); + } + } + + getFocusableElement(): HTMLElement | SVGElement { + return this.textArea; + } + getFocusableTree(): IFocusableTree { + return this.workspace; + } + onNodeFocus(): void {} + onNodeBlur(): void {} + canBeFocused(): boolean { + if (this.id) return true; + return false; + } +} diff --git a/core/comments/comment_view.ts b/core/comments/comment_view.ts index 26623d40f..1e5ad4a52 100644 --- a/core/comments/comment_view.ts +++ b/core/comments/comment_view.ts @@ -6,6 +6,7 @@ import * as browserEvents from '../browser_events.js'; import * as css from '../css.js'; +import type {IFocusableNode} from '../interfaces/i_focusable_node'; import {IRenderedElement} from '../interfaces/i_rendered_element.js'; import * as layers from '../layers.js'; import * as touch from '../touch.js'; @@ -15,6 +16,7 @@ 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 {CommentEditor} from './comment_editor.js'; export class CommentView implements IRenderedElement { /** The root group element of the comment view. */ @@ -46,11 +48,8 @@ export class CommentView implements IRenderedElement { /** The resize handle element. */ private resizeHandle: SVGImageElement; - /** The foreignObject containing the HTML text area. */ - private foreignObject: SVGForeignObjectElement; - - /** The text area where the user can type. */ - private textArea: HTMLTextAreaElement; + /** The part of the comment view that contains the textarea to edit the comment. */ + private commentEditor: CommentEditor; /** The current size of the comment in workspace units. */ private size: Size; @@ -64,14 +63,6 @@ export class CommentView implements IRenderedElement { /** The current location of the comment in workspace coordinates. */ private location: Coordinate = new Coordinate(0, 0); - /** The current text of the comment. Updates on text area change. */ - private text: string = ''; - - /** Listeners for changes to text. */ - private textChangeListeners: Array< - (oldText: string, newText: string) => void - > = []; - /** Listeners for changes to size. */ private sizeChangeListeners: Array<(oldSize: Size, newSize: Size) => void> = []; @@ -106,7 +97,10 @@ export class CommentView implements IRenderedElement { /** The default size of newly created comments. */ static defaultCommentSize = new Size(120, 100); - constructor(readonly workspace: WorkspaceSvg) { + constructor( + readonly workspace: WorkspaceSvg, + private commentId?: string, + ) { this.svgRoot = dom.createSvgElement(Svg.G, { 'class': 'blocklyComment blocklyEditable blocklyDraggable', }); @@ -122,8 +116,7 @@ export class CommentView implements IRenderedElement { textPreviewNode: this.textPreviewNode, } = this.createTopBar(this.svgRoot, workspace)); - ({foreignObject: this.foreignObject, textArea: this.textArea} = - this.createTextArea(this.svgRoot)); + this.commentEditor = this.createTextArea(); this.resizeHandle = this.createResizeHandle(this.svgRoot, workspace); @@ -236,33 +229,32 @@ export class CommentView implements IRenderedElement { /** * Creates the text area where users can type. Registers event listeners. */ - private createTextArea(svgRoot: SVGGElement): { - foreignObject: SVGForeignObjectElement; - textArea: HTMLTextAreaElement; - } { - const foreignObject = dom.createSvgElement( - Svg.FOREIGNOBJECT, - { - 'class': 'blocklyCommentForeignObject', - }, - svgRoot, + private createTextArea() { + // When the user is done editing comment, focus the entire comment. + const onFinishEditing = () => this.svgRoot.focus(); + const commentEditor = new CommentEditor( + this.workspace, + this.commentId, + onFinishEditing, ); - const body = document.createElementNS(dom.HTML_NS, 'body'); - body.setAttribute('xmlns', dom.HTML_NS); - body.className = 'blocklyMinimalBody'; - const textArea = document.createElementNS( - dom.HTML_NS, - 'textarea', - ) as HTMLTextAreaElement; - dom.addClass(textArea, 'blocklyCommentText'); - dom.addClass(textArea, 'blocklyTextarea'); - dom.addClass(textArea, 'blocklyText'); - body.appendChild(textArea); - foreignObject.appendChild(body); - browserEvents.conditionalBind(textArea, 'change', this, this.onTextChange); + this.svgRoot.appendChild(commentEditor.getDom()); - return {foreignObject, textArea}; + commentEditor.addTextChangeListener((oldText, newText) => { + this.updateTextPreview(newText); + // Update size in case our minimum size increased. + this.setSize(this.size); + }); + + return commentEditor; + } + + /** + * + * @returns The FocusableNode representing the editor portion of this comment. + */ + getEditorFocusableNode(): IFocusableNode { + return this.commentEditor; } /** Creates the DOM elements for the comment resize handle. */ @@ -324,7 +316,7 @@ export class CommentView implements IRenderedElement { this.updateHighlightRect(size); this.updateTopBarSize(size); - this.updateTextAreaSize(size, topBarSize); + this.commentEditor.updateSize(size, topBarSize); this.updateDeleteIconPosition(size, topBarSize, deleteSize); this.updateFoldoutIconPosition(topBarSize, foldoutSize); this.updateTextPreviewSize( @@ -360,7 +352,7 @@ export class CommentView implements IRenderedElement { foldoutSize: Size, deleteSize: Size, ): Size { - this.updateTextPreview(this.textArea.value ?? ''); + this.updateTextPreview(this.commentEditor.getText() ?? ''); const textPreviewWidth = dom.getTextWidth(this.textPreview); const foldoutMargin = this.calcFoldoutMargin(topBarSize, foldoutSize); @@ -408,19 +400,6 @@ export class CommentView implements IRenderedElement { this.topBarBackground.setAttribute('width', `${size.width}`); } - /** Updates the size of the text area elements to reflect the new size. */ - private updateTextAreaSize(size: Size, topBarSize: Size) { - this.foreignObject.setAttribute( - 'height', - `${size.height - topBarSize.height}`, - ); - this.foreignObject.setAttribute('width', `${size.width}`); - this.foreignObject.setAttribute('y', `${topBarSize.height}`); - if (this.workspace.RTL) { - this.foreignObject.setAttribute('x', `${-size.width}`); - } - } - /** * Updates the position of the delete icon elements to reflect the new size. */ @@ -652,12 +631,11 @@ export class CommentView implements IRenderedElement { if (this.editable) { dom.addClass(this.svgRoot, 'blocklyEditable'); dom.removeClass(this.svgRoot, 'blocklyReadonly'); - this.textArea.removeAttribute('readonly'); } else { dom.removeClass(this.svgRoot, 'blocklyEditable'); dom.addClass(this.svgRoot, 'blocklyReadonly'); - this.textArea.setAttribute('readonly', 'true'); } + this.commentEditor.setEditable(editable); } /** Returns the current location of the comment in workspace coordinates. */ @@ -678,49 +656,29 @@ export class CommentView implements IRenderedElement { ); } - /** Retursn the current text of the comment. */ + /** Returns the current text of the comment. */ getText() { - return this.text; + return this.commentEditor.getText(); } /** Sets the current text of the comment. */ setText(text: string) { - this.textArea.value = text; - this.onTextChange(); + this.commentEditor.setText(text); } /** Sets the placeholder text displayed for an empty comment. */ setPlaceholderText(text: string) { - this.textArea.placeholder = text; + this.commentEditor.setPlaceholderText(text); } - /** Registers a callback that listens for text changes. */ + /** Registers a callback that listens for text changes on the comment editor. */ addTextChangeListener(listener: (oldText: string, newText: string) => void) { - this.textChangeListeners.push(listener); + this.commentEditor.addTextChangeListener(listener); } - /** Removes the given listener from the list of text change listeners. */ + /** Removes the given listener from the comment editor. */ removeTextChangeListener(listener: () => void) { - this.textChangeListeners.splice( - this.textChangeListeners.indexOf(listener), - 1, - ); - } - - /** - * Triggers listeners when the text of the comment changes, either - * programmatically or manually by the user. - */ - private onTextChange() { - const oldText = this.text; - this.text = this.textArea.value; - this.updateTextPreview(this.text); - // Update size in case our minimum size increased. - this.setSize(this.size); - // Loop through listeners backwards in case they remove themselves. - for (let i = this.textChangeListeners.length - 1; i >= 0; i--) { - this.textChangeListeners[i](oldText, this.text); - } + this.commentEditor.removeTextChangeListener(listener); } /** Updates the preview text element to reflect the given text. */ @@ -884,6 +842,11 @@ css.register(` fill: none; } +.blocklyCommentText.blocklyActiveFocus { + border-color: #fc3; + border-width: 2px; +} + .blocklySelected .blocklyCommentHighlight { stroke: #fc3; stroke-width: 3px; diff --git a/core/comments/rendered_workspace_comment.ts b/core/comments/rendered_workspace_comment.ts index 42fb1fda4..3457e611a 100644 --- a/core/comments/rendered_workspace_comment.ts +++ b/core/comments/rendered_workspace_comment.ts @@ -47,7 +47,7 @@ export class RenderedWorkspaceComment IFocusableNode { /** The class encompassing the svg elements making up the workspace comment. */ - private view: CommentView; + view: CommentView; public readonly workspace: WorkspaceSvg; @@ -59,7 +59,7 @@ export class RenderedWorkspaceComment this.workspace = workspace; - this.view = new CommentView(workspace); + this.view = new CommentView(workspace, this.id); // Set the size to the default size as defined in the superclass. this.view.setSize(this.getSize()); this.view.setEditable(this.isEditable()); @@ -224,13 +224,7 @@ export class RenderedWorkspaceComment private startGesture(e: PointerEvent) { const gesture = this.workspace.getGesture(e); if (gesture) { - if (browserEvents.isTargetInput(e)) { - // If the text area was the focus, don't allow this event to bubble up - // and steal focus away from the editor/comment. - e.stopPropagation(); - } else { - gesture.handleCommentStart(e, this); - } + gesture.handleCommentStart(e, this); getFocusManager().focusNode(this); } } @@ -339,6 +333,13 @@ export class RenderedWorkspaceComment } } + /** + * @returns The FocusableNode representing the editor portion of this comment. + */ + getEditorFocusableNode(): IFocusableNode { + return this.view.getEditorFocusableNode(); + } + /** See IFocusableNode.getFocusableElement. */ getFocusableElement(): HTMLElement | SVGElement { return this.getSvgRoot(); diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index 552d37061..3033eacd7 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -22,6 +22,7 @@ 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_EDITOR_FOCUS_IDENTIFIER} from './comments/comment_editor.js'; import {RenderedWorkspaceComment} from './comments/rendered_workspace_comment.js'; import {WorkspaceComment} from './comments/workspace_comment.js'; import * as common from './common.js'; @@ -2729,6 +2730,26 @@ export class WorkspaceSvg return nestedWorkspaces; } + /** + * Used for searching for a specific workspace comment. + * We can't use this.getWorkspaceCommentById because the workspace + * comment ids might not be globally unique, but the id assigned to + * the focusable element for the comment should be. + */ + private searchForWorkspaceComment( + id: string, + ): RenderedWorkspaceComment | undefined { + for (const comment of this.getTopComments()) { + if ( + comment instanceof RenderedWorkspaceComment && + comment.canBeFocused() && + comment.getFocusableElement().id === id + ) { + return comment; + } + } + } + /** See IFocusableTree.lookUpFocusableNode. */ lookUpFocusableNode(id: string): IFocusableNode | null { // Check against flyout items if this workspace is part of a flyout. Note @@ -2773,21 +2794,29 @@ 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); + const comment = this.searchForWorkspaceComment(commentId); + if (comment) { + return comment.getEditorFocusableNode(); + } + } + // Search for a specific block. + // Don't use `getBlockById` because the block ID is not guaranteeed + // to be globally unique, but the ID on the focusable element is. const block = this.getAllBlocks(false).find( (block) => block.getFocusableElement().id === id, ); if (block) return block; // Search for a workspace comment (semi-expensive). - for (const comment of this.getTopComments()) { - if ( - comment instanceof RenderedWorkspaceComment && - comment.canBeFocused() && - comment.getFocusableElement().id === id - ) { - return comment; - } + const comment = this.searchForWorkspaceComment(id); + if (comment) { + return comment; } // Search for icons and bubbles (which requires an expensive getAllBlocks).