feat: add registering and serializing icons (#7063)

* feat: add registry for icons

* feat: add serialization of custom icons

* feat: add deserialization of custom icons

* chore: fixup deserialization

* chore: export icons registry

* chore: add tests for serialization and deserialization

* chore: move mocks and helpers to the top level

* chore: fix doc error

* chore: remove accidental only

core/blockly.ts

@@ -122,7 +122,7 @@ import {VerticalFlyout} from './flyout_vertical.js';
 import {CodeGenerator} from './generator.js';
 import {Gesture} from './gesture.js';
 import {Grid} from './grid.js';
-import {Icon} from './icon_old.js';
+import {Icon} from './icons/icon.js';
 import * as icons from './icons.js';
 import {inject} from './inject.js';
 import {Align, Input} from './inputs/input.js';

core/icons.ts

@@ -5,5 +5,6 @@
  */
 
 import * as exceptions from './icons/exceptions.js';
+import * as registry from './icons/registry.js';
 
-export {exceptions};
+export {exceptions, registry};

core/icons/registry.ts

@@ -0,0 +1,32 @@
+/**
+ * @license
+ * Copyright 2023 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import type {Block} from '../block.js';
+import type {IIcon} from '../interfaces/i_icon.js';
+import * as registry from '../registry.js';
+
+/**
+ * Registers the given icon so that it can be deserialized.
+ *
+ * @param type The type of the icon to register. This should be the same string
+ *     that is returned from its `getType` method.
+ * @param iconConstructor The icon class/constructor to register.
+ */
+export function register(
+  type: string,
+  iconConstructor: new (block: Block) => IIcon
+) {
+  registry.register(registry.Type.ICON, type, iconConstructor);
+}
+
+/**
+ * Unregisters the icon associated with the given type.
+ *
+ * @param type The type of the icon to unregister.
+ */
+export function unregister(type: string) {
+  registry.unregister(registry.Type.ICON, type);
+}

core/registry.ts

@@ -13,6 +13,7 @@ import type {IBlockDragger} from './interfaces/i_block_dragger.js';
 import type {IConnectionChecker} from './interfaces/i_connection_checker.js';
 import type {IFlyout} from './interfaces/i_flyout.js';
 import type {IMetricsManager} from './interfaces/i_metrics_manager.js';
+import type {IIcon} from './interfaces/i_icon.js';
 import type {Input} from './inputs/input.js';
 import type {ISerializer} from './interfaces/i_serializer.js';
 import type {IToolbox} from './interfaces/i_toolbox.js';
@@ -92,6 +93,9 @@ export class Type<_T> {
 
   /** @internal */
   static SERIALIZER = new Type<ISerializer>('serializer');
+
+  /** @internal */
+  static ICON = new Type<IIcon>('icon');
 }
 
 /**

core/serialization/blocks.ts

@@ -12,7 +12,9 @@ import type {BlockSvg} from '../block_svg.js';
 import type {Connection} from '../connection.js';
 import * as eventUtils from '../events/utils.js';
 import {inputTypes} from '../inputs/input_types.js';
+import {isSerializable} from '../interfaces/i_serializable.js';
 import type {ISerializer} from '../interfaces/i_serializer.js';
+import * as registry from '../registry.js';
 import {Size} from '../utils/size.js';
 import * as utilsXml from '../utils/xml.js';
 import type {Workspace} from '../workspace.js';
@@ -23,6 +25,7 @@ import {
   MissingBlockType,
   MissingConnection,
   RealChildOfShadow,
+  UnregisteredIcon,
 } from './exceptions.js';
 import * as priorities from './priorities.js';
 import * as serializationRegistry from './registry.js';
@@ -113,7 +116,7 @@ export function save(
   saveExtraState(block, state as AnyDuringMigration);
   // AnyDuringMigration because:  Argument of type '{ type: string; id: string;
   // }' is not assignable to parameter of type 'State'.
-  saveIcons(block, state as AnyDuringMigration);
+  saveIcons(block, state as AnyDuringMigration, doFullSerialization);
   // AnyDuringMigration because:  Argument of type '{ type: string; id: string;
   // }' is not assignable to parameter of type 'State'.
   saveFields(block, state as AnyDuringMigration, doFullSerialization);
@@ -208,19 +211,30 @@ function saveExtraState(block: Block, state: State) {
  *
  * @param block The block to serialize the icon state of.
  * @param state The state object to append to.
+ * @param doFullSerialization Whether or not to serialize the full state of the
+ *     icon (rather than possibly saving a reference to some state).
  */
-function saveIcons(block: Block, state: State) {
-  // TODO(#2105): Remove this logic and put it in the icon.
+function saveIcons(block: Block, state: State, doFullSerialization: boolean) {
+  const icons = Object.create(null);
+  for (const icon of block.getIcons()) {
+    if (isSerializable(icon)) {
+      icons[icon.getType()] = icon.saveState(doFullSerialization);
+    }
+  }
+
+  // TODO(#7038): Remove this logic and put it in the comment icon.
   if (block.getCommentText()) {
-    state['icons'] = {
-      'comment': {
-        'text': block.getCommentText(),
-        'pinned': block.commentModel.pinned,
-        'height': Math.round(block.commentModel.size.height),
-        'width': Math.round(block.commentModel.size.width),
-      },
+    icons['comment'] = {
+      'text': block.getCommentText(),
+      'pinned': block.commentModel.pinned,
+      'height': Math.round(block.commentModel.size.height),
+      'width': Math.round(block.commentModel.size.width),
     };
   }
+
+  if (Object.keys(icons).length) {
+    state['icons'] = icons;
+  }
 }
 
 /**
@@ -575,10 +589,22 @@ function tryToConnectParent(
  * @param state The state object to reference.
  */
 function loadIcons(block: Block, state: State) {
-  if (!state['icons']) {
-    return;
+  if (!state['icons']) return;
+
+  const iconTypes = Object.keys(state['icons']);
+  for (const iconType of iconTypes) {
+    // TODO(#7038): Remove this special casing of comment..
+    if (iconType === 'comment') continue;
+
+    const iconState = state['icons'][iconType];
+    const constructor = registry.getClass(registry.Type.ICON, iconType, false);
+    if (!constructor) throw new UnregisteredIcon(iconType, block, state);
+    const icon = new constructor();
+    block.addIcon(icon);
+    if (isSerializable(icon)) icon.loadState(iconState);
   }
-  // TODO(#2105): Remove this logic and put it in the icon.
+
+  // TODO(#7038): Remove this logic and put it in the icon.
   const comment = state['icons']['comment'];
   if (comment) {
     block.setCommentText(comment['text']);

core/serialization/exceptions.ts

@@ -86,3 +86,20 @@ block. It is an invariant of Blockly that shadow blocks only have shadow
 children`);
   }
 }
+
+export class UnregisteredIcon extends DeserializationError {
+  /**
+   * @param iconType The type of the unregistered icon we are attempting to
+   *     deserialize.
+   * @param block The block we are attempting to add the unregistered icon to.
+   * @param state The state object representing the block.
+   */
+  constructor(iconType: string, public block: Block, public state: State) {
+    super(
+      `Cannot add an icon of type '${iconType}' to the block ` +
+        `${block.toDevString()}, because there is no icon registered with ` +
+        `type '${iconType}'. Make sure that all of your icons have been ` +
+        `registered.`
+    );
+  }
+}

core/serialization/registry.ts

@@ -7,7 +7,6 @@
 import * as goog from '../../closure/goog/goog.js';
 goog.declareModuleId('Blockly.serialization.registry');
 
-// eslint-disable-next-line no-unused-vars
 import type {ISerializer} from '../interfaces/i_serializer.js';
 import * as registry from '../registry.js';
 

tests/mocha/icon_test.js

@@ -12,7 +12,7 @@ import {
 } from './test_helpers/setup_teardown.js';
 import {defineEmptyBlock} from './test_helpers/block_definitions.js';
 
-suite.skip('Icon', function () {
+suite('Icon', function () {
   setup(function () {
     this.clock = sharedTestSetup.call(this, {fireEventsNow: false}).clock;
     defineEmptyBlock();
@@ -22,38 +22,75 @@ suite.skip('Icon', function () {
     sharedTestTeardown.call(this);
   });
 
-  suite('Hooks getting properly triggered by the block', function () {
-    class MockIcon {
-      initView() {}
-
-      applyColour() {}
-
-      updateEditable() {}
-
-      updateCollapsed() {}
+  class MockIcon {
+    getType() {
+      return 'mock icon';
     }
 
-    function createHeadlessWorkspace() {
-      return new Blockly.Workspace();
+    initView() {}
+
+    applyColour() {}
+
+    updateEditable() {}
+
+    updateCollapsed() {}
+  }
+
+  class MockSerializableIcon extends MockIcon {
+    constructor() {
+      super();
+      this.state = '';
     }
 
-    function createWorkspaceSvg() {
-      const workspace = new Blockly.WorkspaceSvg(new Blockly.Options({}));
-      workspace.createDom();
-      return workspace;
+    getType() {
+      return 'serializable icon';
     }
 
-    function createUninitializedBlock(workspace) {
-      return workspace.newBlock('empty_block');
+    getWeight() {
+      return 1;
     }
 
-    function createInitializedBlock(workspace) {
-      const block = workspace.newBlock('empty_block');
-      block.initSvg();
-      block.render();
-      return block;
+    saveState() {
+      return 'some state';
     }
 
+    loadState(state) {
+      this.state = state;
+    }
+  }
+
+  class MockNonSerializableIcon extends MockIcon {
+    getType() {
+      return 'non-serializable icon';
+    }
+  }
+
+  function createHeadlessWorkspace() {
+    return new Blockly.Workspace();
+  }
+
+  function createWorkspaceSvg() {
+    const workspace = new Blockly.WorkspaceSvg(new Blockly.Options({}));
+    workspace.createDom();
+    return workspace;
+  }
+
+  function createUninitializedBlock(workspace) {
+    return workspace.newBlock('empty_block');
+  }
+
+  function createInitializedBlock(workspace) {
+    const block = workspace.newBlock('empty_block');
+    block.initSvg();
+    block.render();
+    return block;
+  }
+
+  function createHeadlessBlock(workspace) {
+    return createUninitializedBlock(workspace);
+  }
+
+  suite.skip('Hooks getting properly triggered by the block', function () {
     suite('Triggering view initialization', function () {
       test('initView is not called by headless blocks', function () {
         const workspace = createHeadlessWorkspace();
@@ -348,4 +385,73 @@ suite.skip('Icon', function () {
       });
     });
   });
+
+  suite('Serialization', function () {
+    test('serializable icons are saved', function () {
+      const block = createHeadlessBlock(createHeadlessWorkspace());
+      block.addIcon(new MockSerializableIcon());
+      const json = Blockly.serialization.blocks.save(block);
+      chai.assert.deepNestedInclude(
+        json,
+        {'icons': {'serializable icon': 'some state'}},
+        'Expected the JSON to include the saved state of the ' +
+          'serializable icon.'
+      );
+    });
+
+    test('non-serializable icons are not saved', function () {
+      const block = createHeadlessBlock(createHeadlessWorkspace());
+      block.addIcon(new MockNonSerializableIcon());
+      const json = Blockly.serialization.blocks.save(block);
+      chai.assert.notProperty(
+        json,
+        'icons',
+        'Expected the JSON to not include any saved state for icons'
+      );
+    });
+  });
+
+  suite('Deserialization', function () {
+    test('registered icons are instantiated and added to the block', function () {
+      Blockly.icons.registry.register(
+        'serializable icon',
+        MockSerializableIcon
+      );
+
+      const workspace = createHeadlessWorkspace();
+      const json = {
+        'type': 'empty_block',
+        'icons': {
+          'serializable icon': 'some state',
+        },
+      };
+      const block = Blockly.serialization.blocks.append(json, workspace);
+      chai.assert.equal(
+        block.getIcon('serializable icon').state,
+        'some state',
+        'Expected the icon to have been properly instantiated and ' +
+          'deserialized'
+      );
+
+      Blockly.icons.registry.unregister('serializable icon');
+    });
+
+    test('trying to deserialize an unregistered icon throws an error', function () {
+      const workspace = createHeadlessWorkspace();
+      const json = {
+        'type': 'empty_block',
+        'icons': {
+          'serializable icon': 'some state',
+        },
+      };
+      chai.assert.throws(
+        () => {
+          Blockly.serialization.blocks.append(json, workspace);
+        },
+        Blockly.serialization.exceptions.UnregisteredIcon,
+        '',
+        'Expected deserializing an unregistered icon to throw'
+      );
+    });
+  });
 });