feat: improve design and add styling

Author: maribethb

core/comments/comment_editor.ts

@@ -9,29 +9,34 @@ 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';
 
+/** The part of a comment that can be typed into. */
 export class CommentEditor implements IFocusableNode {
   id?: string;
   /** The foreignObject containing the HTML text area. */
-  foreignObject: SVGForeignObjectElement;
+  private foreignObject: SVGForeignObjectElement;
 
   /** The text area where the user can type. */
-  textArea: HTMLTextAreaElement;
+  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,
-    commentSvgRoot: SVGGElement,
     commentId?: string,
   ) {
-    this.foreignObject = dom.createSvgElement(
-      Svg.FOREIGNOBJECT,
-      {
-        'class': 'blocklyCommentForeignObject',
-      },
-      commentSvgRoot,
-    );
+    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';
@@ -46,10 +51,19 @@ export class CommentEditor implements IFocusableNode {
     this.foreignObject.appendChild(body);
 
     if (commentId) {
-      this.id = commentId + '_textarea_';
+      this.id = commentId + '_comment_textarea_';
       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',
@@ -63,6 +77,74 @@ export class CommentEditor implements IFocusableNode {
     );
   }
 
+  /** 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);
+    }
+  }
+
+  /** 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;
   }
@@ -72,6 +154,7 @@ export class CommentEditor implements IFocusableNode {
   onNodeFocus(): void {}
   onNodeBlur(): void {}
   canBeFocused(): boolean {
-    return true;
+    if (this.id) return true;
+    return false;
   }
 }

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';
@@ -47,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;
@@ -65,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> =
     [];
@@ -107,8 +97,6 @@ export class CommentView implements IRenderedElement {
   /** The default size of newly created comments. */
   static defaultCommentSize = new Size(120, 100);
 
-  public commentEditor: CommentEditor | undefined;
-
   constructor(
     readonly workspace: WorkspaceSvg,
     private commentId?: string,
@@ -128,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.svgRoot);
 
     this.resizeHandle = this.createResizeHandle(this.svgRoot, workspace);
 
@@ -242,19 +229,26 @@ export class CommentView implements IRenderedElement {
   /**
    * Creates the text area where users can type. Registers event listeners.
    */
-  private createTextArea(svgRoot: SVGGElement): {
-    foreignObject: SVGForeignObjectElement;
-    textArea: HTMLTextAreaElement;
-  } {
-    this.commentEditor = new CommentEditor(
-      this.workspace,
-      svgRoot,
-      this.commentId,
-    );
-    const {foreignObject, textArea} = this.commentEditor;
-    browserEvents.conditionalBind(textArea, 'change', this, this.onTextChange);
+  private createTextArea(svgRoot: SVGGElement) {
+    const commentEditor = new CommentEditor(this.workspace, this.commentId);
+
+    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. */
@@ -316,7 +310,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(
@@ -352,7 +346,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);
@@ -400,19 +394,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.
    */
@@ -644,12 +625,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. */
@@ -670,49 +650,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. */
@@ -876,6 +836,11 @@ css.register(`
   fill: none;
 }
 
+.blocklyCommentText.blocklyActiveFocus {
+  border-color: #fc3;
+  border-width: 2px;
+}
+
 .blocklySelected .blocklyCommentHighlight {
   stroke: #fc3;
   stroke-width: 3px;

core/comments/rendered_workspace_comment.ts

@@ -333,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();