feat: add implementations for adding, removing, and getting icons (#7059)

* feat: add implementations for adding, removing, and getting icons

* chore: fix tests

* chore: switch order of adding icons

* chore: create duplicate icon error

* chore: un-only test

core/block.ts

@@ -25,6 +25,7 @@ import * as common from './common.js';
 import {Connection} from './connection.js';
 import {ConnectionType} from './connection_type.js';
 import * as constants from './constants.js';
+import {DuplicateIconType} from './icons/exceptions.js';
 import type {Abstract} from './events/events_abstract.js';
 import type {BlockMove} from './events/events_block_move.js';
 import * as eventUtils from './events/utils.js';
@@ -34,6 +35,7 @@ import * as fieldRegistry from './field_registry.js';
 import {Align, Input} from './inputs/input.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 type {Mutator} from './mutator.js';
 import * as Tooltip from './tooltip.js';
 import * as arrayUtils from './utils/array.js';
@@ -156,6 +158,7 @@ export class Block implements IASTNodeLocation, IDeletable {
   previousConnection: Connection | null = null;
   inputList: Input[] = [];
   inputsInline?: boolean;
+  icons: IIcon[] = [];
   private disabled = false;
   tooltip: Tooltip.TipInfo = '';
   contextMenu = true;
@@ -2217,16 +2220,60 @@ export class Block implements IASTNodeLocation, IDeletable {
    * @param _opt_id An optional ID for the warning text to be able to maintain
    *     multiple warnings.
    */
-  setWarningText(_text: string | null, _opt_id?: string) {}
-  // NOP.
+  setWarningText(_text: string | null, _opt_id?: string) {
+    // NOOP.
+  }
 
   /**
    * Give this block a mutator dialog.
    *
    * @param _mutator A mutator dialog instance or null to remove.
    */
-  setMutator(_mutator: Mutator) {}
-  // NOP.
+  setMutator(_mutator: Mutator) {
+    // NOOP.
+  }
+
+  /** Adds the given icon to the block. */
+  addIcon<T extends IIcon>(icon: T): T {
+    if (this.hasIcon(icon.getType())) throw new DuplicateIconType(icon);
+    this.icons.push(icon);
+    this.icons.sort((a, b) => a.getWeight() - b.getWeight());
+    return icon;
+  }
+
+  /**
+   * Removes the icon whose getType matches the given type iconType from the
+   * block.
+   *
+   * @param type The type of the icon to remove from the block.
+   * @return True if an icon with the given type was found, false otherwise.
+   */
+  removeIcon(type: string): boolean {
+    if (!this.hasIcon(type)) return false;
+    this.icons = this.icons.filter((icon) => icon.getType() !== type);
+    return true;
+  }
+
+  /**
+   * @return True if an icon with the given type exists on the block,
+   *     false otherwise.
+   */
+  hasIcon(type: string): boolean {
+    return this.icons.some((icon) => icon.getType() === type);
+  }
+
+  /**
+   * @return The icon with the given type if it exists on the block, undefined
+   *     otherwise.
+   */
+  getIcon(type: string): IIcon | undefined {
+    return this.icons.find((icon) => icon.getType() === type);
+  }
+
+  /** @return An array of the icons attached to this block. */
+  getIcons(): IIcon[] {
+    return [...this.icons];
+  }
 
   /**
    * Return the coordinates of the top-left corner of this block relative to the

core/block_svg.ts

@@ -34,12 +34,12 @@ import type {BlockMove} from './events/events_block_move.js';
 import * as eventUtils from './events/utils.js';
 import type {Field} from './field.js';
 import {FieldLabel} from './field_label.js';
-import type {Icon} from './icon_old.js';
 import type {Input} from './inputs/input.js';
 import type {IASTNodeLocationSvg} from './interfaces/i_ast_node_location_svg.js';
 import type {IBoundedElement} from './interfaces/i_bounded_element.js';
 import type {CopyData, ICopyable} from './interfaces/i_copyable.js';
 import type {IDraggable} from './interfaces/i_draggable.js';
+import type {IIcon} from './interfaces/i_icon.js';
 import * as internalConstants from './internal_constants.js';
 import {ASTNode} from './keyboard_nav/ast_node.js';
 import {TabNavigateCursor} from './keyboard_nav/tab_navigate_cursor.js';
@@ -305,25 +305,6 @@ export class BlockSvg
     this.removeSelect();
   }
 
-  /**
-   * Returns a list of mutator, comment, and warning icons.
-   *
-   * @returns List of icons.
-   */
-  getIcons(): Icon[] {
-    const icons = [];
-    if (this.mutator) {
-      icons.push(this.mutator);
-    }
-    if (this.commentIcon_) {
-      icons.push(this.commentIcon_);
-    }
-    if (this.warning) {
-      icons.push(this.warning);
-    }
-    return icons;
-  }
-
   /**
    * Sets the parent of this block to be a new block or null.
    *
@@ -1145,6 +1126,36 @@ export class BlockSvg
     }
   }
 
+  override addIcon<T extends IIcon>(icon: T): T {
+    super.addIcon(icon);
+    if (this.rendered) {
+      // TODO: Change this based on #7024.
+      this.render();
+      this.bumpNeighbours();
+    }
+    return icon;
+  }
+
+  override removeIcon(type: string): boolean {
+    const removed = super.removeIcon(type);
+    if (this.rendered) {
+      // TODO: Change this based on #7024.
+      this.render();
+      this.bumpNeighbours();
+    }
+    return removed;
+  }
+
+  // TODO: remove this implementation after #7038, #7039, and #7040 are
+  //   resolved.
+  override getIcons(): AnyDuringMigration[] {
+    const icons = [];
+    if (this.commentIcon_) icons.push(this.commentIcon_);
+    if (this.warning) icons.push(this.warning);
+    if (this.mutator) icons.push(this.mutator);
+    return icons;
+  }
+
   /**
    * Set whether the block is enabled or not.
    *

core/blockly.ts

@@ -124,6 +124,7 @@ import {CodeGenerator} from './generator.js';
 import {Gesture} from './gesture.js';
 import {Grid} from './grid.js';
 import {Icon} from './icon_old.js';
+import * as icons from './icons.js';
 import {inject} from './inject.js';
 import {Align, Input} from './inputs/input.js';
 import {inputTypes} from './inputs/input_types.js';
@@ -586,6 +587,7 @@ export {IComponent};
 export {IConnectionChecker};
 export {IContextMenu};
 export {Icon};
+export {icons};
 export {ICopyable};
 export {IDeletable};
 export {IDeleteArea};

core/icons.ts

@@ -0,0 +1,9 @@
+/**
+ * @license
+ * Copyright 2023 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import * as exceptions from './icons/exceptions.js';
+
+export {exceptions};

core/icons/exceptions.ts

@@ -0,0 +1,20 @@
+/**
+ * @license
+ * Copyright 2023 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import type {IIcon} from '../interfaces/i_icon.js';
+
+export class DuplicateIconType extends Error {
+  /**
+   * @internal
+   */
+  constructor(public icon: IIcon) {
+    super(
+      `Tried to append an icon of type ${icon.getType()} when an icon of ` +
+        `that type already exists on the block. ` +
+        `Use getIcon to access the existing icon.`
+    );
+  }
+}

tests/mocha/block_test.js

@@ -1422,22 +1422,30 @@ suite('Blocks', function () {
       getType() {
         return 'A';
       }
+
+      getWeight() {
+        return 1;
+      }
     }
 
     class MockIconB {
       getType() {
         return 'B';
       }
+
+      getWeight() {
+        return 2;
+      }
     }
 
-    suite.skip('Adding icons', function () {
+    suite('Adding icons', function () {
       setup(function () {
-        // Tear down the old headless workspace and create a new rendered one.
-        workspaceTeardown.call(this, this.workspace);
-        this.workspace = Blockly.inject('blocklyDiv');
+        this.workspace = Blockly.inject('blocklyDiv', {});
 
         this.block = this.workspace.newBlock('stack_block');
-        this.renderSpy = sinon.spy(this.block, 'queueRender');
+        this.block.initSvg();
+        this.block.render();
+        this.renderSpy = sinon.spy(this.block, 'render');
       });
 
       teardown(function () {
@@ -1455,9 +1463,14 @@ suite('Blocks', function () {
 
       test('adding two icons of the same type throws', function () {
         this.block.addIcon(new MockIconA());
-        chai.assert.throws(() => {
-          this.block.addIcon(new MockIconA());
-        }, 'Expected adding an icon of the same type to throw');
+        chai.assert.throws(
+          () => {
+            this.block.addIcon(new MockIconA());
+          },
+          Blockly.icons.DuplicateIconType,
+          '',
+          'Expected adding an icon of the same type to throw'
+        );
       });
 
       test('adding an icon triggers a render', function () {
@@ -1470,14 +1483,14 @@ suite('Blocks', function () {
       });
     });
 
-    suite.skip('Removing icons', function () {
+    suite('Removing icons', function () {
       setup(function () {
-        // Tear down the old headless workspace and create a new rendered one.
-        workspaceTeardown.call(this, this.workspace);
         this.workspace = Blockly.inject('blocklyDiv');
 
         this.block = this.workspace.newBlock('stack_block');
-        this.renderSpy = sinon.spy(this.block, 'queueRender');
+        this.block.initSvg();
+        this.block.render();
+        this.renderSpy = sinon.spy(this.block, 'render');
       });
 
       teardown(function () {
@@ -1505,8 +1518,8 @@ suite('Blocks', function () {
       });
 
       test('removing an icon triggers a render', function () {
-        this.renderSpy.resetHistory();
         this.block.addIcon(new MockIconA());
+        this.renderSpy.resetHistory();
         this.block.removeIcon('A');
         chai.assert.isTrue(
           this.renderSpy.calledOnce,
@@ -1515,20 +1528,28 @@ suite('Blocks', function () {
       });
     });
 
-    suite.skip('Getting icons', function () {
+    suite('Getting icons', function () {
       setup(function () {
         this.block = this.workspace.newBlock('stack_block');
       });
 
-      test('all icons are returned from getIcons', function () {
+      test('all icons are returned from getIcons, in order of weight', function () {
         const iconA = new MockIconA();
         const iconB = new MockIconB();
-        this.block.addIcon(iconA);
         this.block.addIcon(iconB);
-        chai.assert.sameMembers(
+        this.block.addIcon(iconA);
+        chai.assert.sameOrderedMembers(
           this.block.getIcons(),
           [iconA, iconB],
-          'Expected getIcon to return both icons'
+          'Expected getIcon to return both icons in order of weight'
+        );
+      });
+
+      test('if there are no icons, getIcons returns an empty array', function () {
+        chai.assert.isEmpty(
+          this.block.getIcons(),
+          'Expected getIcons to return an empty array ' +
+            'for a block with no icons'
         );
       });
 
@@ -1552,9 +1573,9 @@ suite('Blocks', function () {
         );
       });
 
-      test('if there is no matching icon, getIcon returns null', function () {
+      test('if there is no matching icon, getIcon returns undefined', function () {
         this.block.addIcon(new MockIconA());
-        chai.assert.isNull(
+        chai.assert.isUndefined(
           this.block.getIcon('B'),
           'Expected getIcon to return null if there is no ' +
             'icon with a matching type'
@@ -1564,8 +1585,6 @@ suite('Blocks', function () {
 
     suite('Bubbles and collapsing', function () {
       setup(function () {
-        // Tear down the old headless workspace and create a new rendered one.
-        workspaceTeardown.call(this, this.workspace);
         this.workspace = Blockly.inject('blocklyDiv');
       });