feat: allow overriding comment icons (#7937)

* feat: add comment icon interface

* feat: have blocks construct comment icons from registry

* chore: add tests for setCommentText

* fix: typeguard

core/block.ts

@@ -34,8 +34,8 @@ import {Input} from './inputs/input.js';
 import {Align} from './inputs/align.js';
 import type {IASTNodeLocation} from './interfaces/i_ast_node_location.js';
 import type {IDeletable} from './interfaces/i_deletable.js';
-import type {IIcon} from './interfaces/i_icon.js';
-import {CommentIcon} from './icons/comment_icon.js';
+import {type IIcon} from './interfaces/i_icon.js';
+import {isCommentIcon} from './interfaces/i_comment_icon.js';
 import type {MutatorIcon} from './icons/mutator_icon.js';
 import * as Tooltip from './tooltip.js';
 import * as arrayUtils from './utils/array.js';
@@ -2212,7 +2212,7 @@ export class Block implements IASTNodeLocation, IDeletable {
    * @returns Block's comment.
    */
   getCommentText(): string | null {
-    const comment = this.getIcon(CommentIcon.TYPE) as CommentIcon | null;
+    const comment = this.getIcon(IconType.COMMENT);
     return comment?.getText() ?? null;
   }
 
@@ -2222,19 +2222,36 @@ export class Block implements IASTNodeLocation, IDeletable {
    * @param text The text, or null to delete.
    */
   setCommentText(text: string | null) {
-    const comment = this.getIcon(CommentIcon.TYPE) as CommentIcon | null;
+    const comment = this.getIcon(IconType.COMMENT);
     const oldText = comment?.getText() ?? null;
     if (oldText === text) return;
     if (text !== null) {
-      let comment = this.getIcon(CommentIcon.TYPE) as CommentIcon | undefined;
+      let comment = this.getIcon(IconType.COMMENT);
       if (!comment) {
-        comment = this.addIcon(new CommentIcon(this));
+        const commentConstructor = registry.getClass(
+          registry.Type.ICON,
+          IconType.COMMENT.toString(),
+          false,
+        );
+        if (!commentConstructor) {
+          throw new Error(
+            'No comment icon class is registered, so a comment cannot be set',
+          );
+        }
+        const icon = new commentConstructor(this);
+        if (!isCommentIcon(icon)) {
+          throw new Error(
+            'The class registered as a comment icon does not conform to the ' +
+              'ICommentIcon interface',
+          );
+        }
+        comment = this.addIcon(icon);
       }
       eventUtils.disable();
       comment.setText(text);
       eventUtils.enable();
     } else {
-      this.removeIcon(CommentIcon.TYPE);
+      this.removeIcon(IconType.COMMENT);
     }
 
     eventUtils.fire(

core/icons/icon_types.ts

@@ -4,8 +4,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import {ICommentIcon} from '../interfaces/i_comment_icon.js';
 import {IIcon} from '../interfaces/i_icon.js';
-import {CommentIcon} from './comment_icon.js';
 import {MutatorIcon} from './mutator_icon.js';
 import {WarningIcon} from './warning_icon.js';
 
@@ -28,5 +28,5 @@ export class IconType<_T extends IIcon> {
 
   static MUTATOR = new IconType<MutatorIcon>('mutator');
   static WARNING = new IconType<WarningIcon>('warning');
-  static COMMENT = new IconType<CommentIcon>('comment');
+  static COMMENT = new IconType<ICommentIcon>('comment');
 }

core/interfaces/i_comment_icon.ts

@@ -0,0 +1,33 @@
+/**
+ * @license
+ * Copyright 2024 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import {IconType} from '../icons.js';
+import {IIcon, isIcon} from './i_icon.js';
+import {Size} from '../utils/size.js';
+import {IHasBubble, hasBubble} from './i_has_bubble.js';
+
+export interface ICommentIcon extends IIcon, IHasBubble {
+  setText(text: string): void;
+
+  getText(): string;
+
+  setBubbleSize(size: Size): void;
+
+  getBubbleSize(): Size;
+}
+
+/** Checks whether the given object is an ICommentIcon. */
+export function isCommentIcon(obj: Object): obj is ICommentIcon {
+  return (
+    isIcon(obj) &&
+    hasBubble(obj) &&
+    (obj as any)['setText'] !== undefined &&
+    (obj as any)['getText'] !== undefined &&
+    (obj as any)['setBubbleSize'] !== undefined &&
+    (obj as any)['getBubbleSize'] !== undefined &&
+    obj.getType() === IconType.COMMENT
+  );
+}

tests/mocha/block_test.js

@@ -19,6 +19,7 @@ import {
   createMockEvent,
 } from './test_helpers/events.js';
 import {MockIcon, MockBubbleIcon} from './test_helpers/icon_mocks.js';
+import {IconType} from '../../build/src/core/icons/icon_types.js';
 
 suite('Blocks', function () {
   setup(function () {
@@ -1367,6 +1368,93 @@ suite('Blocks', function () {
         });
       });
     });
+
+    suite('Constructing registered comment classes', function () {
+      class MockComment extends MockIcon {
+        getType() {
+          return Blockly.icons.IconType.COMMENT;
+        }
+
+        setText() {}
+
+        getText() {
+          return '';
+        }
+
+        setBubbleSize() {}
+
+        getBubbleSize() {
+          return Blockly.utils.Size(0, 0);
+        }
+
+        bubbleIsVisible() {
+          return true;
+        }
+
+        setBubbleVisible() {}
+      }
+
+      setup(function () {
+        this.workspace = Blockly.inject('blocklyDiv', {});
+
+        this.block = this.workspace.newBlock('stack_block');
+        this.block.initSvg();
+        this.block.render();
+      });
+
+      teardown(function () {
+        workspaceTeardown.call(this, this.workspace);
+
+        Blockly.icons.registry.unregister(
+          Blockly.icons.IconType.COMMENT.toString(),
+        );
+        Blockly.icons.registry.register(
+          Blockly.icons.IconType.COMMENT,
+          Blockly.icons.CommentIcon,
+        );
+      });
+
+      test('setCommentText constructs the registered comment icon', function () {
+        Blockly.icons.registry.unregister(
+          Blockly.icons.IconType.COMMENT.toString(),
+        );
+        Blockly.icons.registry.register(
+          Blockly.icons.IconType.COMMENT,
+          MockComment,
+        );
+
+        this.block.setCommentText('test text');
+
+        chai.assert.instanceOf(
+          this.block.getIcon(Blockly.icons.IconType.COMMENT),
+          MockComment,
+        );
+      });
+
+      test('setCommentText throws if no icon is registered', function () {
+        Blockly.icons.registry.unregister(
+          Blockly.icons.IconType.COMMENT.toString(),
+        );
+
+        chai.assert.throws(() => {
+          this.block.setCommentText('test text');
+        }, 'No comment icon class is registered, so a comment cannot be set');
+      });
+
+      test('setCommentText throws if the icon is not an ICommentIcon', function () {
+        Blockly.icons.registry.unregister(
+          Blockly.icons.IconType.COMMENT.toString(),
+        );
+        Blockly.icons.registry.register(
+          Blockly.icons.IconType.COMMENT,
+          MockIcon,
+        );
+
+        chai.assert.throws(() => {
+          this.block.setCommentText('test text');
+        }, 'The class registered as a comment icon does not conform to the ICommentIcon interface');
+      });
+    });
   });
 
   suite('Getting/Setting Field (Values)', function () {