release: Merge branch 'develop' into rc/v11.2.0

.github/workflows/appengine_deploy.yml

@@ -42,7 +42,7 @@ jobs:
           path: _deploy/
 
       - name: Deploy to App Engine
-        uses: google-github-actions/deploy-appengine@v2.1.3
+        uses: google-github-actions/deploy-appengine@v2.1.4
         # For parameters see:
         # https://github.com/google-github-actions/deploy-appengine#inputs
         with:

blocks/lists.ts

@@ -412,6 +412,24 @@ const LISTS_GETINDEX = {
     this.appendDummyInput()
       .appendField(modeMenu, 'MODE')
       .appendField('', 'SPACE');
+    const menu = fieldRegistry.fromJson({
+      type: 'field_dropdown',
+      options: this.WHERE_OPTIONS,
+    }) as FieldDropdown;
+    menu.setValidator(
+      /** @param value The input value. */
+      function (this: FieldDropdown, value: string) {
+        const oldValue: string | null = this.getValue();
+        const oldAt = oldValue === 'FROM_START' || oldValue === 'FROM_END';
+        const newAt = value === 'FROM_START' || value === 'FROM_END';
+        if (newAt !== oldAt) {
+          const block = this.getSourceBlock() as GetIndexBlock;
+          block.updateAt_(newAt);
+        }
+        return undefined;
+      },
+    );
+    this.appendDummyInput().appendField(menu, 'WHERE');
     this.appendDummyInput('AT');
     if (Msg['LISTS_GET_INDEX_TAIL']) {
       this.appendDummyInput('TAIL').appendField(Msg['LISTS_GET_INDEX_TAIL']);
@@ -577,31 +595,6 @@ const LISTS_GETINDEX = {
     } else {
       this.appendDummyInput('AT');
     }
-    const menu = fieldRegistry.fromJson({
-      type: 'field_dropdown',
-      options: this.WHERE_OPTIONS,
-    }) as FieldDropdown;
-    menu.setValidator(
-      /**
-       * @param value The input value.
-       * @returns Null if the field has been replaced; otherwise undefined.
-       */
-      function (this: FieldDropdown, value: string) {
-        const newAt = value === 'FROM_START' || value === 'FROM_END';
-        // The 'isAt' variable is available due to this function being a
-        // closure.
-        if (newAt !== isAt) {
-          const block = this.getSourceBlock() as GetIndexBlock;
-          block.updateAt_(newAt);
-          // This menu has been destroyed and replaced.  Update the
-          // replacement.
-          block.setFieldValue(value, 'WHERE');
-          return null;
-        }
-        return undefined;
-      },
-    );
-    this.getInput('AT')!.appendField(menu, 'WHERE');
     if (Msg['LISTS_GET_INDEX_TAIL']) {
       this.moveInputBefore('TAIL', null);
     }
@@ -644,6 +637,24 @@ const LISTS_SETINDEX = {
     this.appendDummyInput()
       .appendField(operationDropdown, 'MODE')
       .appendField('', 'SPACE');
+    const menu = fieldRegistry.fromJson({
+      type: 'field_dropdown',
+      options: this.WHERE_OPTIONS,
+    }) as FieldDropdown;
+    menu.setValidator(
+      /** @param value The input value. */
+      function (this: FieldDropdown, value: string) {
+        const oldValue: string | null = this.getValue();
+        const oldAt = oldValue === 'FROM_START' || oldValue === 'FROM_END';
+        const newAt = value === 'FROM_START' || value === 'FROM_END';
+        if (newAt !== oldAt) {
+          const block = this.getSourceBlock() as SetIndexBlock;
+          block.updateAt_(newAt);
+        }
+        return undefined;
+      },
+    );
+    this.appendDummyInput().appendField(menu, 'WHERE');
     this.appendDummyInput('AT');
     this.appendValueInput('TO').appendField(Msg['LISTS_SET_INDEX_INPUT_TO']);
     this.setInputsInline(true);
@@ -756,36 +767,10 @@ const LISTS_SETINDEX = {
     } else {
       this.appendDummyInput('AT');
     }
-    const menu = fieldRegistry.fromJson({
-      type: 'field_dropdown',
-      options: this.WHERE_OPTIONS,
-    }) as FieldDropdown;
-    menu.setValidator(
-      /**
-       * @param value The input value.
-       * @returns Null if the field has been replaced; otherwise undefined.
-       */
-      function (this: FieldDropdown, value: string) {
-        const newAt = value === 'FROM_START' || value === 'FROM_END';
-        // The 'isAt' variable is available due to this function being a
-        // closure.
-        if (newAt !== isAt) {
-          const block = this.getSourceBlock() as SetIndexBlock;
-          block.updateAt_(newAt);
-          // This menu has been destroyed and replaced.  Update the
-          // replacement.
-          block.setFieldValue(value, 'WHERE');
-          return null;
-        }
-        return undefined;
-      },
-    );
     this.moveInputBefore('AT', 'TO');
     if (this.getInput('ORDINAL')) {
       this.moveInputBefore('ORDINAL', 'TO');
     }
-
-    this.getInput('AT')!.appendField(menu, 'WHERE');
   },
 };
 blocks['lists_setIndex'] = LISTS_SETINDEX;
@@ -818,7 +803,30 @@ const LISTS_GETSUBLIST = {
     this.appendValueInput('LIST')
       .setCheck('Array')
       .appendField(Msg['LISTS_GET_SUBLIST_INPUT_IN_LIST']);
+    const createMenu = (n: 1 | 2): FieldDropdown => {
+      const menu = fieldRegistry.fromJson({
+        type: 'field_dropdown',
+        options:
+          this[('WHERE_OPTIONS_' + n) as 'WHERE_OPTIONS_1' | 'WHERE_OPTIONS_2'],
+      }) as FieldDropdown;
+      menu.setValidator(
+        /** @param value The input value. */
+        function (this: FieldDropdown, value: string) {
+          const oldValue: string | null = this.getValue();
+          const oldAt = oldValue === 'FROM_START' || oldValue === 'FROM_END';
+          const newAt = value === 'FROM_START' || value === 'FROM_END';
+          if (newAt !== oldAt) {
+            const block = this.getSourceBlock() as GetSublistBlock;
+            block.updateAt_(n, newAt);
+          }
+          return undefined;
+        },
+      );
+      return menu;
+    };
+    this.appendDummyInput('WHERE1_INPUT').appendField(createMenu(1), 'WHERE1');
     this.appendDummyInput('AT1');
+    this.appendDummyInput('WHERE2_INPUT').appendField(createMenu(2), 'WHERE2');
     this.appendDummyInput('AT2');
     if (Msg['LISTS_GET_SUBLIST_TAIL']) {
       this.appendDummyInput('TAIL').appendField(Msg['LISTS_GET_SUBLIST_TAIL']);
@@ -896,35 +904,10 @@ const LISTS_GETSUBLIST = {
     } else {
       this.appendDummyInput('AT' + n);
     }
-    const menu = fieldRegistry.fromJson({
-      type: 'field_dropdown',
-      options:
-        this[('WHERE_OPTIONS_' + n) as 'WHERE_OPTIONS_1' | 'WHERE_OPTIONS_2'],
-    }) as FieldDropdown;
-    menu.setValidator(
-      /**
-       * @param value The input value.
-       * @returns Null if the field has been replaced; otherwise undefined.
-       */
-      function (this: FieldDropdown, value: string) {
-        const newAt = value === 'FROM_START' || value === 'FROM_END';
-        // The 'isAt' variable is available due to this function being a
-        // closure.
-        if (newAt !== isAt) {
-          const block = this.getSourceBlock() as GetSublistBlock;
-          block.updateAt_(n, newAt);
-          // This menu has been destroyed and replaced.
-          // Update the replacement.
-          block.setFieldValue(value, 'WHERE' + n);
-          return null;
-        }
-      },
-    );
-    this.getInput('AT' + n)!.appendField(menu, 'WHERE' + n);
     if (n === 1) {
-      this.moveInputBefore('AT1', 'AT2');
+      this.moveInputBefore('AT1', 'WHERE2_INPUT');
       if (this.getInput('ORDINAL1')) {
-        this.moveInputBefore('ORDINAL1', 'AT2');
+        this.moveInputBefore('ORDINAL1', 'WHERE2_INPUT');
       }
     }
     if (Msg['LISTS_GET_SUBLIST_TAIL']) {

blocks/text.ts

@@ -216,7 +216,30 @@ const GET_SUBSTRING_BLOCK = {
     this.appendValueInput('STRING')
       .setCheck('String')
       .appendField(Msg['TEXT_GET_SUBSTRING_INPUT_IN_TEXT']);
+    const createMenu = (n: 1 | 2): FieldDropdown => {
+      const menu = fieldRegistry.fromJson({
+        type: 'field_dropdown',
+        options:
+          this[('WHERE_OPTIONS_' + n) as 'WHERE_OPTIONS_1' | 'WHERE_OPTIONS_2'],
+      }) as FieldDropdown;
+      menu.setValidator(
+        /** @param value The input value. */
+        function (this: FieldDropdown, value: any): null | undefined {
+          const oldValue: string | null = this.getValue();
+          const oldAt = oldValue === 'FROM_START' || oldValue === 'FROM_END';
+          const newAt = value === 'FROM_START' || value === 'FROM_END';
+          if (newAt !== oldAt) {
+            const block = this.getSourceBlock() as GetSubstringBlock;
+            block.updateAt_(n, newAt);
+          }
+          return undefined;
+        },
+      );
+      return menu;
+    };
+    this.appendDummyInput('WHERE1_INPUT').appendField(createMenu(1), 'WHERE1');
     this.appendDummyInput('AT1');
+    this.appendDummyInput('WHERE2_INPUT').appendField(createMenu(2), 'WHERE2');
     this.appendDummyInput('AT2');
     if (Msg['TEXT_GET_SUBSTRING_TAIL']) {
       this.appendDummyInput('TAIL').appendField(Msg['TEXT_GET_SUBSTRING_TAIL']);
@@ -288,37 +311,10 @@ const GET_SUBSTRING_BLOCK = {
       this.removeInput('TAIL', true);
       this.appendDummyInput('TAIL').appendField(Msg['TEXT_GET_SUBSTRING_TAIL']);
     }
-    const menu = fieldRegistry.fromJson({
-      type: 'field_dropdown',
-      options:
-        this[('WHERE_OPTIONS_' + n) as 'WHERE_OPTIONS_1' | 'WHERE_OPTIONS_2'],
-    }) as FieldDropdown;
-    menu.setValidator(
-      /**
-       * @param value The input value.
-       * @returns Null if the field has been replaced; otherwise undefined.
-       */
-      function (this: FieldDropdown, value: any): null | undefined {
-        const newAt = value === 'FROM_START' || value === 'FROM_END';
-        // The 'isAt' variable is available due to this function being a
-        // closure.
-        if (newAt !== isAt) {
-          const block = this.getSourceBlock() as GetSubstringBlock;
-          block.updateAt_(n, newAt);
-          // This menu has been destroyed and replaced.
-          // Update the replacement.
-          block.setFieldValue(value, 'WHERE' + n);
-          return null;
-        }
-        return undefined;
-      },
-    );
-
-    this.getInput('AT' + n)!.appendField(menu, 'WHERE' + n);
     if (n === 1) {
-      this.moveInputBefore('AT1', 'AT2');
+      this.moveInputBefore('AT1', 'WHERE2_INPUT');
       if (this.getInput('ORDINAL1')) {
-        this.moveInputBefore('ORDINAL1', 'AT2');
+        this.moveInputBefore('ORDINAL1', 'WHERE2_INPUT');
       }
     }
   },

core/block_svg.ts

@@ -1472,9 +1472,9 @@ export class BlockSvg
         if (conn.isConnected() && neighbour.isConnected()) continue;
 
         if (conn.isSuperior()) {
-          neighbour.bumpAwayFrom(conn);
+          neighbour.bumpAwayFrom(conn, /* initiatedByThis = */ false);
         } else {
-          conn.bumpAwayFrom(neighbour);
+          conn.bumpAwayFrom(neighbour, /* initiatedByThis = */ true);
         }
       }
     }

core/bubbles/textinput_bubble.ts

@@ -9,6 +9,7 @@ import * as touch from '../touch.js';
 import {browserEvents} from '../utils.js';
 import {Coordinate} from '../utils/coordinate.js';
 import * as dom from '../utils/dom.js';
+import * as drag from '../utils/drag.js';
 import {Rect} from '../utils/rect.js';
 import {Size} from '../utils/size.js';
 import {Svg} from '../utils/svg.js';
@@ -62,6 +63,8 @@ export class TextInputBubble extends Bubble {
     20 + Bubble.DOUBLE_BORDER,
   );
 
+  private editable = true;
+
   /**
    * @param workspace The workspace this bubble belongs to.
    * @param anchor The anchor location of the thing this bubble is attached to.
@@ -95,6 +98,21 @@ export class TextInputBubble extends Bubble {
     this.onTextChange();
   }
 
+  /** Sets whether or not the text in the bubble is editable. */
+  setEditable(editable: boolean) {
+    this.editable = editable;
+    if (this.editable) {
+      this.textArea.removeAttribute('readonly');
+    } else {
+      this.textArea.setAttribute('readonly', '');
+    }
+  }
+
+  /** Returns whether or not the text in the bubble is editable. */
+  isEditable(): boolean {
+    return this.editable;
+  }
+
   /** Adds a change listener to be notified when this bubble's text changes. */
   addTextChangeListener(listener: () => void) {
     this.textChangeListeners.push(listener);
@@ -224,7 +242,8 @@ export class TextInputBubble extends Bubble {
       return;
     }
 
-    this.workspace.startDrag(
+    drag.start(
+      this.workspace,
       e,
       new Coordinate(
         this.workspace.RTL ? -this.getSize().width : this.getSize().width,
@@ -264,7 +283,7 @@ export class TextInputBubble extends Bubble {
 
   /** Handles pointer move events on the resize target. */
   private onResizePointerMove(e: PointerEvent) {
-    const delta = this.workspace.moveDrag(e);
+    const delta = drag.move(this.workspace, e);
     this.setSize(
       new Size(this.workspace.RTL ? -delta.x : delta.x, delta.y),
       false,

core/comments/comment_view.ts

@@ -11,6 +11,7 @@ import * as layers from '../layers.js';
 import * as touch from '../touch.js';
 import {Coordinate} from '../utils/coordinate.js';
 import * as dom from '../utils/dom.js';
+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';
@@ -524,8 +525,8 @@ export class CommentView implements IRenderedElement {
 
     this.preResizeSize = this.getSize();
 
-    // TODO(#7926): Move this into a utils file.
+    drag.start(
-    this.workspace.startDrag(
+      this.workspace,
       e,
       new Coordinate(
         this.workspace.RTL ? -this.getSize().width : this.getSize().width,
@@ -569,8 +570,7 @@ export class CommentView implements IRenderedElement {
 
   /** Resizes the comment in response to a drag on the resize handle. */
   private onResizePointerMove(e: PointerEvent) {
-    // TODO(#7926): Move this into a utils file.
+    const size = drag.move(this.workspace, e);
-    const size = this.workspace.moveDrag(e);
     this.setSizeWithoutFiringEvents(
       new Size(this.workspace.RTL ? -size.x : size.x, size.y),
     );
@@ -623,6 +623,7 @@ export class CommentView implements IRenderedElement {
    * event on the foldout icon.
    */
   private onFoldoutDown(e: PointerEvent) {
+    touch.clearTouchIdentifier();
     this.bringToFront();
     if (browserEvents.isRightButton(e)) {
       e.stopPropagation();
@@ -738,6 +739,7 @@ export class CommentView implements IRenderedElement {
    * delete icon.
    */
   private onDeleteDown(e: PointerEvent) {
+    touch.clearTouchIdentifier();
     if (browserEvents.isRightButton(e)) {
       e.stopPropagation();
       return;

core/connection.ts

@@ -214,11 +214,11 @@ export class Connection implements IASTNodeLocationWithBlock {
    * Called when an attempted connection fails. NOP by default (i.e. for
    * headless workspaces).
    *
-   * @param _otherConnection Connection that this connection failed to connect
+   * @param _superiorConnection Connection that this connection failed to connect
-   *     to.
+   *     to. The provided connection should be the superior connection.
    * @internal
    */
-  onFailedConnect(_otherConnection: Connection) {}
+  onFailedConnect(_superiorConnection: Connection) {}
   // NOP
 
   /**

core/field.ts

@@ -1086,57 +1086,68 @@ export abstract class Field<T = any>
       return;
     }
 
-    const classValidation = this.doClassValidation_(newValue);
+    // Field validators are allowed to make changes to the workspace, which
-    const classValue = this.processValidation_(
+    // should get grouped with the field value change event.
-      newValue,
+    const existingGroup = eventUtils.getGroup();
-      classValidation,
+    if (!existingGroup) {
-      fireChangeEvent,
+      eventUtils.setGroup(true);
-    );
-    if (classValue instanceof Error) {
-      if (doLogging) console.log('invalid class validation, return');
-      return;
     }
 
-    const localValidation = this.getValidator()?.call(this, classValue);
+    try {
-    const localValue = this.processValidation_(
+      const classValidation = this.doClassValidation_(newValue);
-      classValue,
+      const classValue = this.processValidation_(
-      localValidation,
+        newValue,
-      fireChangeEvent,
+        classValidation,
-    );
+        fireChangeEvent,
-    if (localValue instanceof Error) {
-      if (doLogging) console.log('invalid local validation, return');
-      return;
-    }
-
-    const source = this.sourceBlock_;
-    if (source && source.disposed) {
-      if (doLogging) console.log('source disposed, return');
-      return;
-    }
-
-    const oldValue = this.getValue();
-    if (oldValue === localValue) {
-      if (doLogging) console.log('same, doValueUpdate_, return');
-      this.doValueUpdate_(localValue);
-      return;
-    }
-
-    this.doValueUpdate_(localValue);
-    if (fireChangeEvent && source && eventUtils.isEnabled()) {
-      eventUtils.fire(
-        new (eventUtils.get(EventType.BLOCK_CHANGE))(
-          source,
-          'field',
-          this.name || null,
-          oldValue,
-          localValue,
-        ),
       );
+      if (classValue instanceof Error) {
+        if (doLogging) console.log('invalid class validation, return');
+        return;
+      }
+
+      const localValidation = this.getValidator()?.call(this, classValue);
+      const localValue = this.processValidation_(
+        classValue,
+        localValidation,
+        fireChangeEvent,
+      );
+      if (localValue instanceof Error) {
+        if (doLogging) console.log('invalid local validation, return');
+        return;
+      }
+
+      const source = this.sourceBlock_;
+      if (source && source.disposed) {
+        if (doLogging) console.log('source disposed, return');
+        return;
+      }
+
+      const oldValue = this.getValue();
+      if (oldValue === localValue) {
+        if (doLogging) console.log('same, doValueUpdate_, return');
+        this.doValueUpdate_(localValue);
+        return;
+      }
+
+      this.doValueUpdate_(localValue);
+      if (fireChangeEvent && source && eventUtils.isEnabled()) {
+        eventUtils.fire(
+          new (eventUtils.get(EventType.BLOCK_CHANGE))(
+            source,
+            'field',
+            this.name || null,
+            oldValue,
+            localValue,
+          ),
+        );
+      }
+      if (this.isDirty_) {
+        this.forceRerender();
+      }
+      if (doLogging) console.log(this.value_);
+    } finally {
+      eventUtils.setGroup(existingGroup);
     }
-    if (this.isDirty_) {
-      this.forceRerender();
-    }
-    if (doLogging) console.log(this.value_);
   }
 
   /**

core/field_dropdown.ts

@@ -95,6 +95,15 @@ export class FieldDropdown extends Field<string> {
   private selectedOption!: MenuOption;
   override clickTarget_: SVGElement | null = null;
 
+  /**
+   * The y offset from the top of the field to the top of the image, if an image
+   * is selected.
+   */
+  protected static IMAGE_Y_OFFSET = 5;
+
+  /** The total vertical padding above and below an image. */
+  protected static IMAGE_Y_PADDING = FieldDropdown.IMAGE_Y_OFFSET * 2;
+
   /**
    * @param menuGenerator A non-empty array of options for a dropdown list, or a
    *     function which generates these options. Also accepts Field.SKIP_SETUP
@@ -128,8 +137,8 @@ export class FieldDropdown extends Field<string> {
     if (menuGenerator === Field.SKIP_SETUP) return;
 
     if (Array.isArray(menuGenerator)) {
-      validateOptions(menuGenerator);
+      this.validateOptions(menuGenerator);
-      const trimmed = trimOptions(menuGenerator);
+      const trimmed = this.trimOptions(menuGenerator);
       this.menuGenerator_ = trimmed.options;
       this.prefixField = trimmed.prefix || null;
       this.suffixField = trimmed.suffix || null;
@@ -401,7 +410,7 @@ export class FieldDropdown extends Field<string> {
     if (useCache && this.generatedOptions) return this.generatedOptions;
 
     this.generatedOptions = this.menuGenerator_();
-    validateOptions(this.generatedOptions);
+    this.validateOptions(this.generatedOptions);
     return this.generatedOptions;
   }
 
@@ -520,7 +529,7 @@ export class FieldDropdown extends Field<string> {
     const hasBorder = !!this.borderRect_;
     const height = Math.max(
       hasBorder ? this.getConstants()!.FIELD_DROPDOWN_BORDER_RECT_HEIGHT : 0,
-      imageHeight + IMAGE_Y_PADDING,
+      imageHeight + FieldDropdown.IMAGE_Y_PADDING,
     );
     const xPadding = hasBorder
       ? this.getConstants()!.FIELD_BORDER_RECT_X_PADDING
@@ -661,6 +670,127 @@ export class FieldDropdown extends Field<string> {
     // override the static fromJson method.
     return new this(options.options, undefined, options);
   }
+
+  /**
+   * Factor out common words in statically defined options.
+   * Create prefix and/or suffix labels.
+   */
+  protected trimOptions(options: MenuOption[]): {
+    options: MenuOption[];
+    prefix?: string;
+    suffix?: string;
+  } {
+    let hasImages = false;
+    const trimmedOptions = options.map(([label, value]): MenuOption => {
+      if (typeof label === 'string') {
+        return [parsing.replaceMessageReferences(label), value];
+      }
+
+      hasImages = true;
+      // Copy the image properties so they're not influenced by the original.
+      // NOTE: No need to deep copy since image properties are only 1 level deep.
+      const imageLabel =
+        label.alt !== null
+          ? {...label, alt: parsing.replaceMessageReferences(label.alt)}
+          : {...label};
+      return [imageLabel, value];
+    });
+
+    if (hasImages || options.length < 2) return {options: trimmedOptions};
+
+    const stringOptions = trimmedOptions as [string, string][];
+    const stringLabels = stringOptions.map(([label]) => label);
+
+    const shortest = utilsString.shortestStringLength(stringLabels);
+    const prefixLength = utilsString.commonWordPrefix(stringLabels, shortest);
+    const suffixLength = utilsString.commonWordSuffix(stringLabels, shortest);
+
+    if (
+      (!prefixLength && !suffixLength) ||
+      shortest <= prefixLength + suffixLength
+    ) {
+      // One or more strings will entirely vanish if we proceed.  Abort.
+      return {options: stringOptions};
+    }
+
+    const prefix = prefixLength
+      ? stringLabels[0].substring(0, prefixLength - 1)
+      : undefined;
+    const suffix = suffixLength
+      ? stringLabels[0].substr(1 - suffixLength)
+      : undefined;
+    return {
+      options: this.applyTrim(stringOptions, prefixLength, suffixLength),
+      prefix,
+      suffix,
+    };
+  }
+
+  /**
+   * Use the calculated prefix and suffix lengths to trim all of the options in
+   * the given array.
+   *
+   * @param options Array of option tuples:
+   *     (human-readable text or image, language-neutral name).
+   * @param prefixLength The length of the common prefix.
+   * @param suffixLength The length of the common suffix
+   * @returns A new array with all of the option text trimmed.
+   */
+  private applyTrim(
+    options: [string, string][],
+    prefixLength: number,
+    suffixLength: number,
+  ): MenuOption[] {
+    return options.map(([text, value]) => [
+      text.substring(prefixLength, text.length - suffixLength),
+      value,
+    ]);
+  }
+
+  /**
+   * Validates the data structure to be processed as an options list.
+   *
+   * @param options The proposed dropdown options.
+   * @throws {TypeError} If proposed options are incorrectly structured.
+   */
+  protected validateOptions(options: MenuOption[]) {
+    if (!Array.isArray(options)) {
+      throw TypeError('FieldDropdown options must be an array.');
+    }
+    if (!options.length) {
+      throw TypeError('FieldDropdown options must not be an empty array.');
+    }
+    let foundError = false;
+    for (let i = 0; i < options.length; i++) {
+      const tuple = options[i];
+      if (!Array.isArray(tuple)) {
+        foundError = true;
+        console.error(
+          `Invalid option[${i}]: Each FieldDropdown option must be an array.
+          Found: ${tuple}`,
+        );
+      } else if (typeof tuple[1] !== 'string') {
+        foundError = true;
+        console.error(
+          `Invalid option[${i}]: Each FieldDropdown option id must be a string. 
+          Found ${tuple[1]} in: ${tuple}`,
+        );
+      } else if (
+        tuple[0] &&
+        typeof tuple[0] !== 'string' &&
+        typeof tuple[0].src !== 'string'
+      ) {
+        foundError = true;
+        console.error(
+          `Invalid option[${i}]: Each FieldDropdown option must have a string 
+          label or image description. Found ${tuple[0]} in: ${tuple}`,
+        );
+      }
+    }
+    if (foundError) {
+      throw TypeError('Found invalid FieldDropdown options.');
+    }
+  }
 }
 
 /**
@@ -721,147 +851,4 @@ export interface FieldDropdownFromJsonConfig extends FieldDropdownConfig {
  */
 export type FieldDropdownValidator = FieldValidator<string>;
 
-/**
- * The y offset from the top of the field to the top of the image, if an image
- * is selected.
- */
-const IMAGE_Y_OFFSET = 5;
-
-/** The total vertical padding above and below an image. */
-const IMAGE_Y_PADDING: number = IMAGE_Y_OFFSET * 2;
-
-/**
- * Factor out common words in statically defined options.
- * Create prefix and/or suffix labels.
- */
-function trimOptions(options: MenuOption[]): {
-  options: MenuOption[];
-  prefix?: string;
-  suffix?: string;
-} {
-  let hasImages = false;
-  const trimmedOptions = options.map(([label, value]): MenuOption => {
-    if (typeof label === 'string') {
-      return [parsing.replaceMessageReferences(label), value];
-    }
-
-    hasImages = true;
-    // Copy the image properties so they're not influenced by the original.
-    // NOTE: No need to deep copy since image properties are only 1 level deep.
-    const imageLabel =
-      label.alt !== null
-        ? {...label, alt: parsing.replaceMessageReferences(label.alt)}
-        : {...label};
-    return [imageLabel, value];
-  });
-
-  if (hasImages || options.length < 2) return {options: trimmedOptions};
-
-  const stringOptions = trimmedOptions as [string, string][];
-  const stringLabels = stringOptions.map(([label]) => label);
-
-  const shortest = utilsString.shortestStringLength(stringLabels);
-  const prefixLength = utilsString.commonWordPrefix(stringLabels, shortest);
-  const suffixLength = utilsString.commonWordSuffix(stringLabels, shortest);
-
-  if (
-    (!prefixLength && !suffixLength) ||
-    shortest <= prefixLength + suffixLength
-  ) {
-    // One or more strings will entirely vanish if we proceed.  Abort.
-    return {options: stringOptions};
-  }
-
-  const prefix = prefixLength
-    ? stringLabels[0].substring(0, prefixLength - 1)
-    : undefined;
-  const suffix = suffixLength
-    ? stringLabels[0].substr(1 - suffixLength)
-    : undefined;
-  return {
-    options: applyTrim(stringOptions, prefixLength, suffixLength),
-    prefix,
-    suffix,
-  };
-}
-
-/**
- * Use the calculated prefix and suffix lengths to trim all of the options in
- * the given array.
- *
- * @param options Array of option tuples:
- *     (human-readable text or image, language-neutral name).
- * @param prefixLength The length of the common prefix.
- * @param suffixLength The length of the common suffix
- * @returns A new array with all of the option text trimmed.
- */
-function applyTrim(
-  options: [string, string][],
-  prefixLength: number,
-  suffixLength: number,
-): MenuOption[] {
-  return options.map(([text, value]) => [
-    text.substring(prefixLength, text.length - suffixLength),
-    value,
-  ]);
-}
-
-/**
- * Validates the data structure to be processed as an options list.
- *
- * @param options The proposed dropdown options.
- * @throws {TypeError} If proposed options are incorrectly structured.
- */
-function validateOptions(options: MenuOption[]) {
-  if (!Array.isArray(options)) {
-    throw TypeError('FieldDropdown options must be an array.');
-  }
-  if (!options.length) {
-    throw TypeError('FieldDropdown options must not be an empty array.');
-  }
-  let foundError = false;
-  for (let i = 0; i < options.length; i++) {
-    const tuple = options[i];
-    if (!Array.isArray(tuple)) {
-      foundError = true;
-      console.error(
-        'Invalid option[' +
-          i +
-          ']: Each FieldDropdown option must be an ' +
-          'array. Found: ',
-        tuple,
-      );
-    } else if (typeof tuple[1] !== 'string') {
-      foundError = true;
-      console.error(
-        'Invalid option[' +
-          i +
-          ']: Each FieldDropdown option id must be ' +
-          'a string. Found ' +
-          tuple[1] +
-          ' in: ',
-        tuple,
-      );
-    } else if (
-      tuple[0] &&
-      typeof tuple[0] !== 'string' &&
-      typeof tuple[0].src !== 'string'
-    ) {
-      foundError = true;
-      console.error(
-        'Invalid option[' +
-          i +
-          ']: Each FieldDropdown option must have a ' +
-          'string label or image description. Found' +
-          tuple[0] +
-          ' in: ',
-        tuple,
-      );
-    }
-  }
-  if (foundError) {
-    throw TypeError('Found invalid FieldDropdown options.');
-  }
-}
-
 fieldRegistry.register('field_dropdown', FieldDropdown);

core/field_input.ts

@@ -28,8 +28,8 @@ import {
   UnattachedFieldError,
 } from './field.js';
 import {Msg} from './msg.js';
+import * as renderManagement from './render_management.js';
 import * as aria from './utils/aria.js';
-import {Coordinate} from './utils/coordinate.js';
 import * as dom from './utils/dom.js';
 import {Size} from './utils/size.js';
 import * as userAgent from './utils/useragent.js';
@@ -630,22 +630,22 @@ export abstract class FieldInput<T extends InputTypes> extends Field<
 
   /** Resize the editor to fit the text. */
   protected resizeEditor_() {
-    const block = this.getSourceBlock();
+    renderManagement.finishQueuedRenders().then(() => {
-    if (!block) {
+      const block = this.getSourceBlock();
-      throw new UnattachedFieldError();
+      if (!block) throw new UnattachedFieldError();
-    }
+      const div = WidgetDiv.getDiv();
-    const div = WidgetDiv.getDiv();
+      const bBox = this.getScaledBBox();
-    const bBox = this.getScaledBBox();
+      div!.style.width = bBox.right - bBox.left + 'px';
-    div!.style.width = bBox.right - bBox.left + 'px';
+      div!.style.height = bBox.bottom - bBox.top + 'px';
-    div!.style.height = bBox.bottom - bBox.top + 'px';
 
-    // In RTL mode block fields and LTR input fields the left edge moves,
+      // In RTL mode block fields and LTR input fields the left edge moves,
-    // whereas the right edge is fixed.  Reposition the editor.
+      // whereas the right edge is fixed.  Reposition the editor.
-    const x = block.RTL ? bBox.right - div!.offsetWidth : bBox.left;
+      const x = block.RTL ? bBox.right - div!.offsetWidth : bBox.left;
-    const xy = new Coordinate(x, bBox.top);
+      const y = bBox.top;
 
-    div!.style.left = xy.x + 'px';
+      div!.style.left = `${x}px`;
-    div!.style.top = xy.y + 'px';
+      div!.style.top = `${y}px`;
+    });
   }
 
   /**
@@ -657,7 +657,7 @@ export abstract class FieldInput<T extends InputTypes> extends Field<
    *     div.
    */
   override repositionForWindowResize(): boolean {
-    const block = this.getSourceBlock();
+    const block = this.getSourceBlock()?.getRootBlock();
     // This shouldn't be possible. We should never have a WidgetDiv if not using
     // rendered blocks.
     if (!(block instanceof BlockSvg)) return false;

core/icons/comment_icon.ts

@@ -8,7 +8,6 @@
 
 import type {Block} from '../block.js';
 import type {BlockSvg} from '../block_svg.js';
-import {TextBubble} from '../bubbles/text_bubble.js';
 import {TextInputBubble} from '../bubbles/textinput_bubble.js';
 import {EventType} from '../events/type.js';
 import * as eventUtils from '../events/utils.js';
@@ -47,12 +46,9 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable {
    */
   static readonly WEIGHT = 3;
 
-  /** The bubble used to show editable text to the user. */
+  /** The bubble used to show comment text to the user. */
   private textInputBubble: TextInputBubble | null = null;
 
-  /** The bubble used to show non-editable text to the user. */
-  private textBubble: TextBubble | null = null;
-
   /** The text of this comment. */
   private text = '';
 
@@ -118,7 +114,6 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable {
   override dispose() {
     super.dispose();
     this.textInputBubble?.dispose();
-    this.textBubble?.dispose();
   }
 
   override getWeight(): number {
@@ -133,7 +128,6 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable {
     super.applyColour();
     const colour = (this.sourceBlock as BlockSvg).style.colourPrimary;
     this.textInputBubble?.setColour(colour);
-    this.textBubble?.setColour(colour);
   }
 
   /**
@@ -153,7 +147,6 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable {
     super.onLocationChange(blockOrigin);
     const anchorLocation = this.getAnchorLocation();
     this.textInputBubble?.setAnchorLocation(anchorLocation);
-    this.textBubble?.setAnchorLocation(anchorLocation);
   }
 
   /** Sets the text of this comment. Updates any bubbles if they are visible. */
@@ -170,7 +163,6 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable {
     );
     this.text = text;
     this.textInputBubble?.setText(this.text);
-    this.textBubble?.setText(this.text);
   }
 
   /** Returns the text of this comment. */
@@ -302,6 +294,18 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable {
    * to update the state of this icon in response to changes in the bubble.
    */
   private showEditableBubble() {
+    this.createBubble();
+    this.textInputBubble?.addTextChangeListener(() => this.onTextChange());
+    this.textInputBubble?.addSizeChangeListener(() => this.onSizeChange());
+  }
+
+  /** Shows the non editable text bubble for this comment. */
+  private showNonEditableBubble() {
+    this.createBubble();
+    this.textInputBubble?.setEditable(false);
+  }
+
+  protected createBubble() {
     this.textInputBubble = new TextInputBubble(
       this.sourceBlock.workspace as WorkspaceSvg,
       this.getAnchorLocation(),
@@ -309,26 +313,12 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable {
     );
     this.textInputBubble.setText(this.getText());
     this.textInputBubble.setSize(this.bubbleSize, true);
-    this.textInputBubble.addTextChangeListener(() => this.onTextChange());
-    this.textInputBubble.addSizeChangeListener(() => this.onSizeChange());
-  }
-
-  /** Shows the non editable text bubble for this comment. */
-  private showNonEditableBubble() {
-    this.textBubble = new TextBubble(
-      this.getText(),
-      this.sourceBlock.workspace as WorkspaceSvg,
-      this.getAnchorLocation(),
-      this.getBubbleOwnerRect(),
-    );
   }
 
   /** Hides any open bubbles owned by this comment. */
   private hideBubble() {
     this.textInputBubble?.dispose();
     this.textInputBubble = null;
-    this.textBubble?.dispose();
-    this.textBubble = null;
   }
 
   /**

core/rendered_connection.ts

@@ -117,59 +117,85 @@ export class RenderedConnection extends Connection {
    * Move the block(s) belonging to the connection to a point where they don't
    * visually interfere with the specified connection.
    *
-   * @param staticConnection The connection to move away from.
+   * @param superiorConnection The connection to move away from. The provided
+   *     connection should be the superior connection and should not be
+   *     connected to this connection.
+   * @param initiatedByThis Whether or not the block group that was manipulated
+   *     recently causing bump checks is associated with the inferior
+   *     connection. Defaults to false.
    * @internal
    */
-  bumpAwayFrom(staticConnection: RenderedConnection) {
+  bumpAwayFrom(
+    superiorConnection: RenderedConnection,
+    initiatedByThis = false,
+  ) {
     if (this.sourceBlock_.workspace.isDragging()) {
       // Don't move blocks around while the user is doing the same.
       return;
     }
-    // Move the root block.
+    let offsetX =
-    let rootBlock = this.sourceBlock_.getRootBlock();
+      config.snapRadius + Math.floor(Math.random() * BUMP_RANDOMNESS);
-    if (rootBlock.isInFlyout) {
+    let offsetY =
+      config.snapRadius + Math.floor(Math.random() * BUMP_RANDOMNESS);
+    /* eslint-disable-next-line @typescript-eslint/no-this-alias */
+    const inferiorConnection = this;
+    const superiorRootBlock = superiorConnection.sourceBlock_.getRootBlock();
+    const inferiorRootBlock = inferiorConnection.sourceBlock_.getRootBlock();
+
+    if (superiorRootBlock.isInFlyout || inferiorRootBlock.isInFlyout) {
       // Don't move blocks around in a flyout.
       return;
     }
-    let reverse = false;
+    let moveInferior = true;
-    if (!rootBlock.isMovable()) {
+    if (!inferiorRootBlock.isMovable()) {
-      // Can't bump an uneditable block away.
+      // Can't bump an immovable block away.
       // Check to see if the other block is movable.
-      rootBlock = staticConnection.getSourceBlock().getRootBlock();
+      if (!superiorRootBlock.isMovable()) {
-      if (!rootBlock.isMovable()) {
+        // Neither block is movable, abort operation.
         return;
+      } else {
+        // Only the superior block group is movable.
+        moveInferior = false;
+        // The superior block group moves in the opposite direction.
+        offsetX = -offsetX;
+        offsetY = -offsetY;
+      }
+    } else if (superiorRootBlock.isMovable()) {
+      // Both block groups are movable. The one on the inferior side will be
+      // moved to make space for the superior one. However, it's possible that
+      // both groups of blocks have an inferior connection that bumps into a
+      // superior connection on the other group, which could result in both
+      // groups moving in the same direction and eventually bumping each other
+      // again. It would be better if one group of blocks could consistently
+      // move in an orthogonal direction from the other, so that they become
+      // separated in the end. We can designate one group the "initiator" if
+      // it's the one that was most recently manipulated, causing inputs to be
+      // checked for bumpable neighbors. As a useful heuristic, in the case
+      // where the inferior connection belongs to the initiator group, moving it
+      // in the orthogonal direction will separate the blocks better.
+      if (initiatedByThis) {
+        offsetY = -offsetY;
       }
-      // Swap the connections and move the 'static' connection instead.
-      /* eslint-disable-next-line @typescript-eslint/no-this-alias */
-      staticConnection = this;
-      reverse = true;
     }
+    const staticConnection = moveInferior
+      ? superiorConnection
+      : inferiorConnection;
+    const dynamicConnection = moveInferior
+      ? inferiorConnection
+      : superiorConnection;
+    const dynamicRootBlock = moveInferior
+      ? inferiorRootBlock
+      : superiorRootBlock;
     // Raise it to the top for extra visibility.
-    const selected = common.getSelected() == rootBlock;
+    const selected = common.getSelected() === dynamicRootBlock;
-    if (!selected) rootBlock.addSelect();
+    if (!selected) dynamicRootBlock.addSelect();
-    let dx =
+    if (dynamicRootBlock.RTL) {
-      staticConnection.x +
+      offsetX = -offsetX;
-      config.snapRadius +
-      Math.floor(Math.random() * BUMP_RANDOMNESS) -
-      this.x;
-    let dy =
-      staticConnection.y +
-      config.snapRadius +
-      Math.floor(Math.random() * BUMP_RANDOMNESS) -
-      this.y;
-    if (reverse) {
-      // When reversing a bump due to an uneditable block, bump up.
-      dy = -dy;
     }
-    if (rootBlock.RTL) {
+    const dx = staticConnection.x + offsetX - dynamicConnection.x;
-      dx =
+    const dy = staticConnection.y + offsetY - dynamicConnection.y;
-        staticConnection.x -
+    dynamicRootBlock.moveBy(dx, dy, ['bump']);
-        config.snapRadius -
+    if (!selected) dynamicRootBlock.removeSelect();
-        Math.floor(Math.random() * BUMP_RANDOMNESS) -
-        this.x;
-    }
-    rootBlock.moveBy(dx, dy, ['bump']);
-    if (!selected) rootBlock.removeSelect();
   }
 
   /**
@@ -413,11 +439,11 @@ export class RenderedConnection extends Connection {
    * Bumps this connection away from the other connection. Called when an
    * attempted connection fails.
    *
-   * @param otherConnection Connection that this connection failed to connect
+   * @param superiorConnection Connection that this connection failed to connect
-   *     to.
+   *     to. The provided connection should be the superior connection.
    * @internal
    */
-  override onFailedConnect(otherConnection: Connection) {
+  override onFailedConnect(superiorConnection: Connection) {
     const block = this.getSourceBlock();
     if (eventUtils.getRecordUndo()) {
       const group = eventUtils.getGroup();
@@ -425,7 +451,7 @@ export class RenderedConnection extends Connection {
         function (this: RenderedConnection) {
           if (!block.isDisposed() && !block.getParent()) {
             eventUtils.setGroup(group);
-            this.bumpAwayFrom(otherConnection as RenderedConnection);
+            this.bumpAwayFrom(superiorConnection as RenderedConnection);
             eventUtils.setGroup(false);
           }
         }.bind(this),

core/shortcut_registry.ts

@@ -45,31 +45,27 @@ export class ShortcutRegistry {
    * Registers a keyboard shortcut.
    *
    * @param shortcut The shortcut for this key code.
-   * @param opt_allowOverrides True to prevent a warning when overriding an
+   * @param allowOverrides True to prevent a warning when overriding an
    *     already registered item.
    * @throws {Error} if a shortcut with the same name already exists.
    */
-  register(shortcut: KeyboardShortcut, opt_allowOverrides?: boolean) {
+  register(shortcut: KeyboardShortcut, allowOverrides?: boolean) {
     const registeredShortcut = this.shortcuts.get(shortcut.name);
-    if (registeredShortcut && !opt_allowOverrides) {
+    if (registeredShortcut && !allowOverrides) {
       throw new Error(`Shortcut named "${shortcut.name}" already exists.`);
     }
     this.shortcuts.set(shortcut.name, shortcut);
 
     const keyCodes = shortcut.keyCodes;
-    if (keyCodes && keyCodes.length > 0) {
+    if (keyCodes?.length) {
-      for (let i = 0; i < keyCodes.length; i++) {
+      for (const keyCode of keyCodes) {
-        this.addKeyMapping(
+        this.addKeyMapping(keyCode, shortcut.name, !!shortcut.allowCollision);
-          keyCodes[i],
-          shortcut.name,
-          !!shortcut.allowCollision,
-        );
       }
     }
   }
 
   /**
-   * Unregisters a keyboard shortcut registered with the given key code. This
+   * Unregisters a keyboard shortcut registered with the given name. This
    * will also remove any key mappings that reference this shortcut.
    *
    * @param shortcutName The name of the shortcut to unregister.
@@ -92,27 +88,34 @@ export class ShortcutRegistry {
   /**
    * Adds a mapping between a keycode and a keyboard shortcut.
    *
+   * Normally only one shortcut can be mapped to any given keycode,
+   * but setting allowCollisions to true allows a keyboard to be
+   * mapped to multiple shortcuts.  In that case, when onKeyDown is
+   * called with the given keystroke, it will process the mapped
+   * shortcuts in reverse order, from the most- to least-recently
+   * mapped).
+   *
    * @param keyCode The key code for the keyboard shortcut. If registering a key
    *     code with a modifier (ex: ctrl+c) use
    *     ShortcutRegistry.registry.createSerializedKey;
    * @param shortcutName The name of the shortcut to execute when the given
    *     keycode is pressed.
-   * @param opt_allowCollision True to prevent an error when adding a shortcut
+   * @param allowCollision True to prevent an error when adding a shortcut
    *     to a key that is already mapped to a shortcut.
    * @throws {Error} if the given key code is already mapped to a shortcut.
    */
   addKeyMapping(
     keyCode: string | number | KeyCodes,
     shortcutName: string,
-    opt_allowCollision?: boolean,
+    allowCollision?: boolean,
   ) {
     keyCode = `${keyCode}`;
     const shortcutNames = this.keyMap.get(keyCode);
-    if (shortcutNames && !opt_allowCollision) {
+    if (shortcutNames && !allowCollision) {
       throw new Error(
         `Shortcut named "${shortcutName}" collides with shortcuts "${shortcutNames}"`,
       );
-    } else if (shortcutNames && opt_allowCollision) {
+    } else if (shortcutNames && allowCollision) {
       shortcutNames.unshift(shortcutName);
     } else {
       this.keyMap.set(keyCode, [shortcutName]);
@@ -127,19 +130,19 @@ export class ShortcutRegistry {
    *     ShortcutRegistry.registry.createSerializedKey;
    * @param shortcutName The name of the shortcut to execute when the given
    *     keycode is pressed.
-   * @param opt_quiet True to not console warn when there is no shortcut to
+   * @param quiet True to not console warn when there is no shortcut to
    *     remove.
    * @returns True if a key mapping was removed, false otherwise.
    */
   removeKeyMapping(
     keyCode: string,
     shortcutName: string,
-    opt_quiet?: boolean,
+    quiet?: boolean,
   ): boolean {
     const shortcutNames = this.keyMap.get(keyCode);
 
     if (!shortcutNames) {
-      if (!opt_quiet) {
+      if (!quiet) {
         console.warn(
           `No keyboard shortcut named "${shortcutName}" registered with key code "${keyCode}"`,
         );
@@ -155,7 +158,7 @@ export class ShortcutRegistry {
       }
       return true;
     }
-    if (!opt_quiet) {
+    if (!quiet) {
       console.warn(
         `No keyboard shortcut named "${shortcutName}" registered with key code "${keyCode}"`,
       );
@@ -172,7 +175,7 @@ export class ShortcutRegistry {
    */
   removeAllKeyMappings(shortcutName: string) {
     for (const keyCode of this.keyMap.keys()) {
-      this.removeKeyMapping(keyCode, shortcutName, true);
+      this.removeKeyMapping(keyCode, shortcutName, /* quiet= */ true);
     }
   }
 
@@ -219,6 +222,21 @@ export class ShortcutRegistry {
   /**
    * Handles key down events.
    *
+   * - Any `KeyboardShortcut`(s) mapped to the keycodes that cause
+   *   event `e` to be fired will be processed, in order from least-
+   *   to most-recently registered.
+   * - If the shortcut's `preconditionFn` exists it will be called.
+   *   If `preconditionFn` returns false the shortcut's `callback`
+   *   function will be skipped.  Processing will continue with the
+   *   next shortcut, if any.
+   * - The shortcut's `callback` function will then be called.  If it
+   *   returns true, processing will terminate and `onKeyDown` will
+   *   return true.  If it returns false, processing will continue
+   *   with with the next shortcut, if any.
+   * - If all registered shortcuts for the given keycode have been
+   *   processed without any having returned true, `onKeyDown` will
+   *   return false.
+   *
    * @param workspace The main workspace where the event was captured.
    * @param e The key down event.
    * @returns True if the event was handled, false otherwise.
@@ -226,17 +244,17 @@ export class ShortcutRegistry {
   onKeyDown(workspace: WorkspaceSvg, e: KeyboardEvent): boolean {
     const key = this.serializeKeyEvent_(e);
     const shortcutNames = this.getShortcutNamesByKeyCode(key);
-    if (!shortcutNames) {
+    if (!shortcutNames) return false;
-      return false;
+    for (const shortcutName of shortcutNames) {
-    }
-    for (let i = 0, shortcutName; (shortcutName = shortcutNames[i]); i++) {
       const shortcut = this.shortcuts.get(shortcutName);
-      if (!shortcut?.preconditionFn || shortcut?.preconditionFn(workspace)) {
+      if (
-        // If the key has been handled, stop processing shortcuts.
+        !shortcut ||
-        if (shortcut?.callback && shortcut?.callback(workspace, e, shortcut)) {
+        (shortcut.preconditionFn && !shortcut.preconditionFn(workspace))
-          return true;
+      ) {
-        }
+        continue;
       }
+      // If the key has been handled, stop processing shortcuts.
+      if (shortcut.callback?.(workspace, e, shortcut)) return true;
     }
     return false;
   }
@@ -301,7 +319,7 @@ export class ShortcutRegistry {
    * @throws {Error} if the modifier is not in the valid modifiers list.
    */
   private checkModifiers_(modifiers: KeyCodes[]) {
-    for (let i = 0, modifier; (modifier = modifiers[i]); i++) {
+    for (const modifier of modifiers) {
       if (!(modifier in ShortcutRegistry.modifierKeys)) {
         throw new Error(modifier + ' is not a valid modifier key.');
       }
@@ -313,7 +331,7 @@ export class ShortcutRegistry {
    *
    * @param keyCode Number code representing the key.
    * @param modifiers List of modifier key codes to be used with the key. All
-   *     valid modifiers can be found in the ShortcutRegistry.modifierKeys.
+   *     valid modifiers can be found in the `ShortcutRegistry.modifierKeys`.
    * @returns The serialized key code for the given modifiers and key.
    */
   createSerializedKey(keyCode: number, modifiers: KeyCodes[] | null): string {
@@ -344,12 +362,59 @@ export class ShortcutRegistry {
 }
 
 export namespace ShortcutRegistry {
+  /** Interface defining a keyboard shortcut. */
   export interface KeyboardShortcut {
-    callback?: (p1: WorkspaceSvg, p2: Event, p3: KeyboardShortcut) => boolean;
+    /**
+     * The function to be called when the shorctut is invoked.
+     *
+     * @param workspace The `WorkspaceSvg` when the shortcut was
+     *     invoked.
+     * @param e The event that caused the shortcut to be activated.
+     * @param shortcut The `KeyboardShortcut` that was activated
+     *     (i.e., the one this callback is attached to).
+     * @returns Returning true ends processing of the invoked keycode.
+     *     Returning false causes processing to continue with the
+     *     next-most-recently registered shortcut for the invoked
+     *     keycode.
+     */
+    callback?: (
+      workspace: WorkspaceSvg,
+      e: Event,
+      shortcut: KeyboardShortcut,
+    ) => boolean;
+
+    /** The name of the shortcut.  Should be unique. */
     name: string;
-    preconditionFn?: (p1: WorkspaceSvg) => boolean;
+
+    /**
+     * A function to be called when the shortcut is invoked, before
+     * calling `callback`, to decide if this shortcut is applicable in
+     * the current situation.
+     *
+     * @param workspace The `WorkspaceSvg` where the shortcut was
+     *     invoked.
+     * @returns True iff `callback` function should be called.
+     */
+    preconditionFn?: (workspace: WorkspaceSvg) => boolean;
+
+    /** Optional arbitray extra data attached to the shortcut. */
     metadata?: object;
+
+    /**
+     * Optional list of key codes to be bound (via
+     * ShortcutRegistry.prototype.addKeyMapping) to this shortcut.
+     */
     keyCodes?: (number | string)[];
+
+    /**
+     * Value of `allowCollision` to pass to `addKeyMapping` when
+     * binding this shortcut's `.keyCodes` (if any).
+     *
+     * N.B.: this is only used for binding keycodes at the time this
+     * shortcut is initially registered, not for any subsequent
+     * `addKeyMapping` calls that happen to reference this shortcut's
+     * name.
+     */
     allowCollision?: boolean;
   }
 

core/utils/drag.ts

@@ -0,0 +1,74 @@
+/**
+ * @license
+ * Copyright 2024 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import * as browserEvents from '../browser_events.js';
+import type {WorkspaceSvg} from '../workspace_svg.js';
+import {Coordinate} from './coordinate.js';
+
+const workspaceToDragDelta: WeakMap<WorkspaceSvg, Coordinate> = new WeakMap();
+
+/**
+ * Convert from mouse coordinates to workspace coordinates.
+ *
+ * @param workspace The workspace where the pointer event is occurring.
+ * @param e The pointer event with the source coordinates.
+ */
+function mouseToWorkspacePoint(
+  workspace: WorkspaceSvg,
+  e: PointerEvent,
+): SVGPoint {
+  const point = browserEvents.mouseToSvg(
+    e,
+    workspace.getParentSvg(),
+    workspace.getInverseScreenCTM(),
+  );
+  // Fix scale of mouse event.
+  point.x /= workspace.scale;
+  point.y /= workspace.scale;
+  return point;
+}
+
+/**
+ * Start tracking a drag of an object on this workspace by recording the offset
+ * between the pointer's current location and the object's starting location.
+ *
+ * Used for resizing block comments and workspace comments.
+ *
+ * @param workspace The workspace where the drag is occurring.
+ * @param e Pointer down event.
+ * @param xy Starting location of object.
+ */
+export function start(
+  workspace: WorkspaceSvg,
+  e: PointerEvent,
+  xy: Coordinate,
+) {
+  const point = mouseToWorkspacePoint(workspace, e);
+  // Record the starting offset between the bubble's location and the mouse.
+  workspaceToDragDelta.set(workspace, Coordinate.difference(xy, point));
+}
+
+/**
+ * Compute the new position of a dragged object in this workspace based on the
+ * current pointer position and the offset between the pointer's starting
+ * location and the object's starting location.
+ *
+ * The start function should have be called previously, when the drag started.
+ *
+ * Used for resizing block comments and workspace comments.
+ *
+ * @param workspace The workspace where the drag is occurring.
+ * @param e Pointer move event.
+ * @returns New location of object.
+ */
+export function move(workspace: WorkspaceSvg, e: PointerEvent): Coordinate {
+  const point = mouseToWorkspacePoint(workspace, e);
+  const dragDelta = workspaceToDragDelta.get(workspace);
+  if (!dragDelta) {
+    throw new Error('Drag not initialized');
+  }
+  return Coordinate.sum(dragDelta, point);
+}

core/utils/rect.ts

@@ -13,6 +13,8 @@
  */
 // Former goog.module ID: Blockly.utils.Rect
 
+import {Coordinate} from './coordinate.js';
+
 /**
  * Class for representing rectangular regions.
  */
@@ -30,10 +32,21 @@ export class Rect {
     public right: number,
   ) {}
 
+  /**
+   * Creates a new copy of this rectangle.
+   *
+   * @returns A copy of this Rect.
+   */
+  clone(): Rect {
+    return new Rect(this.top, this.bottom, this.left, this.right);
+  }
+
+  /** Returns the height of this rectangle. */
   getHeight(): number {
     return this.bottom - this.top;
   }
 
+  /** Returns the width of this rectangle. */
   getWidth(): number {
     return this.right - this.left;
   }
@@ -59,11 +72,56 @@ export class Rect {
    * @returns Whether this rectangle intersects the provided rectangle.
    */
   intersects(other: Rect): boolean {
-    return !(
+    // The following logic can be derived and then simplified from a longer form symmetrical check
-      this.left > other.right ||
+    // of verifying each rectangle's borders with the other rectangle by checking if either end of
-      this.right < other.left ||
+    // the border's line segment is contained within the other rectangle. The simplified version
-      this.top > other.bottom ||
+    // used here can be logically interpreted as ensuring that each line segment of 'this' rectangle
-      this.bottom < other.top
+    // is not outside the bounds of the 'other' rectangle (proving there's an intersection).
+    return (
+      this.left <= other.right &&
+      this.right >= other.left &&
+      this.bottom >= other.top &&
+      this.top <= other.bottom
     );
   }
+
+  /**
+   * Compares bounding rectangles for equality.
+   *
+   * @param a A Rect.
+   * @param b A Rect.
+   * @returns True iff the bounding rectangles are equal, or if both are null.
+   */
+  static equals(a?: Rect | null, b?: Rect | null): boolean {
+    if (a === b) {
+      return true;
+    }
+    if (!a || !b) {
+      return false;
+    }
+    return (
+      a.top === b.top &&
+      a.bottom === b.bottom &&
+      a.left === b.left &&
+      a.right === b.right
+    );
+  }
+
+  /**
+   * Creates a new Rect using a position and supplied dimensions.
+   *
+   * @param position The upper left coordinate of the new rectangle.
+   * @param width The width of the rectangle, in pixels.
+   * @param height The height of the rectangle, in pixels.
+   * @returns A newly created Rect using the provided Coordinate and dimensions.
+   */
+  static createFromPoint(
+    position: Coordinate,
+    width: number,
+    height: number,
+  ): Rect {
+    const left = position.x;
+    const top = position.y;
+    return new Rect(top, top + height, left, left + width);
+  }
 }

core/workspace_svg.ts

@@ -63,6 +63,7 @@ import type {Trashcan} from './trashcan.js';
 import * as arrayUtils from './utils/array.js';
 import {Coordinate} from './utils/coordinate.js';
 import * as dom from './utils/dom.js';
+import * as drag from './utils/drag.js';
 import type {Metrics} from './utils/metrics.js';
 import {Rect} from './utils/rect.js';
 import {Size} from './utils/size.js';
@@ -181,9 +182,6 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg {
   /** Vertical scroll value when scrolling started in pixel units. */
   startScrollY = 0;
 
-  /** Distance from mouse to object being dragged. */
-  private dragDeltaXY: Coordinate | null = null;
-
   /** Current scale. */
   scale = 1;
 
@@ -1447,16 +1445,7 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg {
    * @param xy Starting location of object.
    */
   startDrag(e: PointerEvent, xy: Coordinate) {
-    // Record the starting offset between the bubble's location and the mouse.
+    drag.start(this, e, xy);
-    const point = browserEvents.mouseToSvg(
-      e,
-      this.getParentSvg(),
-      this.getInverseScreenCTM(),
-    );
-    // Fix scale of mouse event.
-    point.x /= this.scale;
-    point.y /= this.scale;
-    this.dragDeltaXY = Coordinate.difference(xy, point);
   }
 
   /**
@@ -1466,15 +1455,7 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg {
    * @returns New location of object.
    */
   moveDrag(e: PointerEvent): Coordinate {
-    const point = browserEvents.mouseToSvg(
+    return drag.move(this, e);
-      e,
-      this.getParentSvg(),
-      this.getInverseScreenCTM(),
-    );
-    // Fix scale of mouse event.
-    point.x /= this.scale;
-    point.y /= this.scale;
-    return Coordinate.sum(this.dragDeltaXY!, point);
   }
 
   /**
@@ -1645,23 +1626,56 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg {
     return boundary;
   }
 
-  /** Clean up the workspace by ordering all the blocks in a column. */
+  /** Clean up the workspace by ordering all the blocks in a column such that none overlap. */
   cleanUp() {
     this.setResizesEnabled(false);
     eventUtils.setGroup(true);
+
     const topBlocks = this.getTopBlocks(true);
-    let cursorY = 0;
+    const movableBlocks = topBlocks.filter((block) => block.isMovable());
-    for (let i = 0, block; (block = topBlocks[i]); i++) {
+    const immovableBlocks = topBlocks.filter((block) => !block.isMovable());
-      if (!block.isMovable()) {
+
-        continue;
+    const immovableBlockBounds = immovableBlocks.map((block) =>
+      block.getBoundingRectangle(),
+    );
+
+    const getNextIntersectingImmovableBlock = function (
+      rect: Rect,
+    ): Rect | null {
+      for (const immovableRect of immovableBlockBounds) {
+        if (rect.intersects(immovableRect)) {
+          return immovableRect;
+        }
       }
-      const xy = block.getRelativeToSurfaceXY();
+      return null;
-      block.moveBy(-xy.x, cursorY - xy.y, ['cleanup']);
+    };
+
+    let cursorY = 0;
+    const minBlockHeight = this.renderer.getConstants().MIN_BLOCK_HEIGHT;
+    for (const block of movableBlocks) {
+      // Make the initial movement of shifting the block to its best possible position.
+      let boundingRect = block.getBoundingRectangle();
+      block.moveBy(-boundingRect.left, cursorY - boundingRect.top, ['cleanup']);
       block.snapToGrid();
+
+      boundingRect = block.getBoundingRectangle();
+      let conflictingRect = getNextIntersectingImmovableBlock(boundingRect);
+      while (conflictingRect != null) {
+        // If the block intersects with an immovable block, move it down past that immovable block.
+        cursorY =
+          conflictingRect.top + conflictingRect.getHeight() + minBlockHeight;
+        block.moveBy(0, cursorY - boundingRect.top, ['cleanup']);
+        block.snapToGrid();
+        boundingRect = block.getBoundingRectangle();
+        conflictingRect = getNextIntersectingImmovableBlock(boundingRect);
+      }
+
+      // Ensure all next blocks start past the most recent (which will also put them past all
+      // previously intersecting immovable blocks).
       cursorY =
         block.getRelativeToSurfaceXY().y +
         block.getHeightWidth().height +
-        this.renderer.getConstants().MIN_BLOCK_HEIGHT;
+        minBlockHeight;
     }
     eventUtils.setGroup(false);
     this.setResizesEnabled(true);

demos/blockfactory/block_library_controller.js

@@ -109,9 +109,9 @@ BlockLibraryController.prototype.clearBlockLibrary = function() {
 BlockLibraryController.prototype.saveToBlockLibrary = function() {
   var blockType = this.getCurrentBlockType();
   // If user has not changed the name of the starter block.
-  if (blockType === 'block_type') {
+  if (reservedBlockFactoryBlocks.has(blockType) || blockType === 'block_type') {
     // Do not save block if it has the default type, 'block_type'.
-    var msg = 'You cannot save a block under the name "block_type". Try ' +
+    var msg = `You cannot save a block under the name "${blockType}". Try ` +
         'changing the name before saving. Then, click on the "Block Library"' +
         ' button to view your saved blocks.';
     alert(msg);

demos/blockfactory/block_library_view.js

@@ -104,36 +104,36 @@ BlockLibraryView.prototype.updateButtons =
     // User is editing a block.
 
     if (!isInLibrary) {
-      // Block type has not been saved to library yet. Disable the delete button
+      // Block type has not been saved to the library yet.
-      // and allow user to save.
+      // Disable the delete button.
       this.saveButton.textContent = 'Save "' + blockType + '"';
-      this.saveButton.disabled = false;
       this.deleteButton.disabled = true;
     } else {
-      // Block type has already been saved. Disable the save button unless the
+      // A version of the block type has already been saved.
-      // there are unsaved changes (checked below).
+      // Enable the delete button.
       this.saveButton.textContent = 'Update "' + blockType + '"';
-      this.saveButton.disabled = true;
       this.deleteButton.disabled = false;
     }
     this.deleteButton.textContent = 'Delete "' + blockType + '"';
 
-    // If changes to block have been made and are not saved, make button
+    this.saveButton.classList.remove('button_alert', 'button_warn');
-    // green to encourage user to save the block.
     if (!savedChanges) {
-      var buttonFormatClass = 'button_warn';
+      var buttonFormatClass;
 
-      // If block type is the default, 'block_type', make button red to alert
+      var isReserved = reservedBlockFactoryBlocks.has(blockType);
-      // user.
+      if (isReserved || blockType === 'block_type') {
-      if (blockType === 'block_type') {
+        // Make button red to alert user that the block type can't be saved.
         buttonFormatClass = 'button_alert';
+      } else {
+        // Block type has not been saved to library yet or has unsaved changes.
+        // Make the button green to encourage the user to save the block.
+        buttonFormatClass = 'button_warn';
       }
       this.saveButton.classList.add(buttonFormatClass);
       this.saveButton.disabled = false;
 
     } else {
       // No changes to save.
-      this.saveButton.classList.remove('button_alert', 'button_warn');
       this.saveButton.disabled = true;
     }
 

demos/blockfactory/blocks.js

@@ -914,3 +914,7 @@ function inputNameCheck(referenceBlock) {
       'There are ' + count + ' input blocks\n with this name.' : null;
   referenceBlock.setWarningText(msg);
 }
+
+// Make a set of all of block types that are required for the block factory.
+var reservedBlockFactoryBlocks =
+    new Set(Object.getOwnPropertyNames(Blockly.Blocks));

demos/blockfactory/factory.js

@@ -187,8 +187,9 @@ BlockFactory.updatePreview = function() {
   // Don't let the user create a block type that already exists,
   // because it doesn't work.
   var warnExistingBlock = function(blockType) {
-    if (blockType in Blockly.Blocks) {
+    if (reservedBlockFactoryBlocks.has(blockType)) {
-      var text = `You can't make a block called ${blockType} in this tool because that name already exists.`;
+      var text = `You can't make a block called ${blockType} in this tool ` +
+          `because that name is reserved.`;
       FactoryUtils.getRootBlock(BlockFactory.mainWorkspace).setWarningText(text);
       console.error(text);
       return true;

package.json

@@ -111,11 +111,11 @@
     "@typescript-eslint/parser": "^8.1.0",
     "async-done": "^2.0.0",
     "chai": "^5.1.1",
-    "concurrently": "^8.0.1",
+    "concurrently": "^9.0.1",
     "eslint": "^8.4.1",
     "eslint-config-google": "^0.14.0",
     "eslint-config-prettier": "^9.0.0",
-    "eslint-plugin-jsdoc": "^48.0.2",
+    "eslint-plugin-jsdoc": "^50.4.3",
     "glob": "^10.3.4",
     "google-closure-compiler": "^20240317.0.0",
     "gulp": "^5.0.0",
@@ -143,7 +143,7 @@
     "yargs": "^17.2.1"
   },
   "dependencies": {
-    "jsdom": "25.0.0"
+    "jsdom": "25.0.1"
   },
   "engines": {
     "node": ">=18"

tests/mocha/comment_test.js

@@ -41,12 +41,12 @@ suite('Comments', function () {
     });
 
     function assertEditable(comment) {
-      assert.isNotOk(comment.textBubble);
       assert.isOk(comment.textInputBubble);
+      assert.isTrue(comment.textInputBubble.isEditable());
     }
     function assertNotEditable(comment) {
-      assert.isNotOk(comment.textInputBubble);
+      assert.isOk(comment.textInputBubble);
-      assert.isOk(comment.textBubble);
+      assert.isFalse(comment.textInputBubble.isEditable());
     }
     test('Editable', async function () {
       await this.comment.setBubbleVisible(true);

tests/mocha/contextmenu_items_test.js

@@ -123,7 +123,7 @@ suite('Context Menu Items', function () {
     suite('Cleanup', function () {
       setup(function () {
         this.cleanupOption = this.registry.getItem('cleanWorkspace');
-        this.cleanupStub = sinon.stub(this.workspace, 'cleanUp');
+        this.cleanUpStub = sinon.stub(this.workspace, 'cleanUp');
       });
 
       test('Enabled when multiple blocks', function () {
@@ -153,9 +153,9 @@ suite('Context Menu Items', function () {
         );
       });
 
-      test('Calls workspace cleanup', function () {
+      test('Calls workspace cleanUp', function () {
         this.cleanupOption.callback(this.scope);
-        sinon.assert.calledOnce(this.cleanupStub);
+        sinon.assert.calledOnce(this.cleanUpStub);
       });
 
       test('Has correct label', function () {

tests/mocha/index.html

@@ -110,6 +110,7 @@
       import './old_workspace_comment_test.js';
       import './procedure_map_test.js';
       import './blocks/procedures_test.js';
+      import './rect_test.js';
       import './registry_test.js';
       import './render_management_test.js';
       import './serializer_test.js';

tests/mocha/jso_serialization_test.js

@@ -533,7 +533,7 @@ suite('JSO Serialization', function () {
             },
             'block': {
               'type': 'text',
-              'id': 'id3',
+              'id': 'id4',
               'fields': {
                 'TEXT': '',
               },

tests/mocha/workspace_svg_test.js

@@ -406,6 +406,560 @@ suite('WorkspaceSvg', function () {
       });
     });
   });
+
+  suite('cleanUp', function () {
+    test('empty workspace does not change', function () {
+      this.workspace.cleanUp();
+
+      const blocks = this.workspace.getTopBlocks(true);
+      assert.equal(blocks.length, 0, 'workspace is empty');
+    });
+
+    test('single block at (0, 0) does not change', function () {
+      const blockJson = {
+        'type': 'math_number',
+        'x': 0,
+        'y': 0,
+        'fields': {
+          'NUM': 123,
+        },
+      };
+      Blockly.serialization.blocks.append(blockJson, this.workspace);
+
+      this.workspace.cleanUp();
+
+      const blocks = this.workspace.getTopBlocks(true);
+      const origin = new Blockly.utils.Coordinate(0, 0);
+      assert.equal(blocks.length, 1, 'workspace has one top-level block');
+      assert.deepEqual(
+        blocks[0].getRelativeToSurfaceXY(),
+        origin,
+        'block is at origin',
+      );
+    });
+
+    test('single block at (10, 15) is moved to (0, 0)', function () {
+      const blockJson = {
+        'type': 'math_number',
+        'x': 10,
+        'y': 15,
+        'fields': {
+          'NUM': 123,
+        },
+      };
+      Blockly.serialization.blocks.append(blockJson, this.workspace);
+
+      this.workspace.cleanUp();
+
+      const topBlocks = this.workspace.getTopBlocks(true);
+      const allBlocks = this.workspace.getAllBlocks(false);
+      const origin = new Blockly.utils.Coordinate(0, 0);
+      assert.equal(topBlocks.length, 1, 'workspace has one top-level block');
+      assert.equal(allBlocks.length, 1, 'workspace has one block overall');
+      assert.deepEqual(
+        topBlocks[0].getRelativeToSurfaceXY(),
+        origin,
+        'block is at origin',
+      );
+    });
+
+    test('single block at (10, 15) with child is moved as unit to (0, 0)', function () {
+      const blockJson = {
+        'type': 'logic_negate',
+        'id': 'parent',
+        'x': 10,
+        'y': 15,
+        'inputs': {
+          'BOOL': {
+            'block': {
+              'type': 'logic_boolean',
+              'id': 'child',
+              'fields': {
+                'BOOL': 'TRUE',
+              },
+            },
+          },
+        },
+      };
+      Blockly.serialization.blocks.append(blockJson, this.workspace);
+
+      this.workspace.cleanUp();
+
+      const topBlocks = this.workspace.getTopBlocks(true);
+      const allBlocks = this.workspace.getAllBlocks(false);
+      const origin = new Blockly.utils.Coordinate(0, 0);
+      assert.equal(topBlocks.length, 1, 'workspace has one top-level block');
+      assert.equal(allBlocks.length, 2, 'workspace has two blocks overall');
+      assert.deepEqual(
+        topBlocks[0].getRelativeToSurfaceXY(),
+        origin,
+        'block is at origin',
+      );
+      assert.notDeepEqual(
+        allBlocks[1].getRelativeToSurfaceXY(),
+        origin,
+        'child is not at origin',
+      );
+    });
+
+    test('two blocks first at (10, 15) second at (0, 0) do not switch places', function () {
+      const blockJson1 = {
+        'type': 'math_number',
+        'id': 'block1',
+        'x': 10,
+        'y': 15,
+        'fields': {
+          'NUM': 123,
+        },
+      };
+      const blockJson2 = {...blockJson1, 'id': 'block2', 'x': 0, 'y': 0};
+      Blockly.serialization.blocks.append(blockJson1, this.workspace);
+      Blockly.serialization.blocks.append(blockJson2, this.workspace);
+
+      this.workspace.cleanUp();
+
+      // block1 and block2 do not switch places since blocks are pre-sorted by their position before
+      // being tidied up, so the order they were added to the workspace doesn't matter.
+      const topBlocks = this.workspace.getTopBlocks(true);
+      const block1 = this.workspace.getBlockById('block1');
+      const block2 = this.workspace.getBlockById('block2');
+      const origin = new Blockly.utils.Coordinate(0, 0);
+      const belowBlock2 = new Blockly.utils.Coordinate(0, 50);
+      assert.equal(topBlocks.length, 2, 'workspace has two top-level blocks');
+      assert.deepEqual(
+        block2.getRelativeToSurfaceXY(),
+        origin,
+        'block2 is at origin',
+      );
+      assert.deepEqual(
+        block1.getRelativeToSurfaceXY(),
+        belowBlock2,
+        'block1 is below block2',
+      );
+    });
+
+    test('two overlapping blocks are moved to origin and below', function () {
+      const blockJson1 = {
+        'type': 'math_number',
+        'id': 'block1',
+        'x': 25,
+        'y': 15,
+        'fields': {
+          'NUM': 123,
+        },
+      };
+      const blockJson2 = {
+        ...blockJson1,
+        'id': 'block2',
+        'x': 15.25,
+        'y': 20.25,
+      };
+      Blockly.serialization.blocks.append(blockJson1, this.workspace);
+      Blockly.serialization.blocks.append(blockJson2, this.workspace);
+
+      this.workspace.cleanUp();
+
+      const topBlocks = this.workspace.getTopBlocks(true);
+      const block1 = this.workspace.getBlockById('block1');
+      const block2 = this.workspace.getBlockById('block2');
+      const origin = new Blockly.utils.Coordinate(0, 0);
+      const belowBlock1 = new Blockly.utils.Coordinate(0, 50);
+      assert.equal(topBlocks.length, 2, 'workspace has two top-level blocks');
+      assert.deepEqual(
+        block1.getRelativeToSurfaceXY(),
+        origin,
+        'block1 is at origin',
+      );
+      assert.deepEqual(
+        block2.getRelativeToSurfaceXY(),
+        belowBlock1,
+        'block2 is below block1',
+      );
+    });
+
+    test('two overlapping blocks with snapping are moved to grid-aligned positions', function () {
+      const blockJson1 = {
+        'type': 'math_number',
+        'id': 'block1',
+        'x': 25,
+        'y': 15,
+        'fields': {
+          'NUM': 123,
+        },
+      };
+      const blockJson2 = {
+        ...blockJson1,
+        'id': 'block2',
+        'x': 15.25,
+        'y': 20.25,
+      };
+      Blockly.serialization.blocks.append(blockJson1, this.workspace);
+      Blockly.serialization.blocks.append(blockJson2, this.workspace);
+      this.workspace.getGrid().setSpacing(20);
+      this.workspace.getGrid().setSnapToGrid(true);
+
+      this.workspace.cleanUp();
+
+      const topBlocks = this.workspace.getTopBlocks(true);
+      const block1 = this.workspace.getBlockById('block1');
+      const block2 = this.workspace.getBlockById('block2');
+      const snappedOffOrigin = new Blockly.utils.Coordinate(10, 10);
+      const belowBlock1 = new Blockly.utils.Coordinate(10, 70);
+      assert.equal(topBlocks.length, 2, 'workspace has two top-level blocks');
+      assert.deepEqual(
+        block1.getRelativeToSurfaceXY(),
+        snappedOffOrigin,
+        'block1 is near origin',
+      );
+      assert.deepEqual(
+        block2.getRelativeToSurfaceXY(),
+        belowBlock1,
+        'block2 is below block1',
+      );
+    });
+
+    test('two overlapping blocks are moved to origin and below including children', function () {
+      const blockJson1 = {
+        'type': 'logic_negate',
+        'id': 'block1',
+        'x': 10,
+        'y': 15,
+        'inputs': {
+          'BOOL': {
+            'block': {
+              'type': 'logic_boolean',
+              'fields': {
+                'BOOL': 'TRUE',
+              },
+            },
+          },
+        },
+      };
+      const blockJson2 = {
+        ...blockJson1,
+        'id': 'block2',
+        'x': 15.25,
+        'y': 20.25,
+      };
+      Blockly.serialization.blocks.append(blockJson1, this.workspace);
+      Blockly.serialization.blocks.append(blockJson2, this.workspace);
+
+      this.workspace.cleanUp();
+
+      const topBlocks = this.workspace.getTopBlocks(true);
+      const allBlocks = this.workspace.getAllBlocks(false);
+      const block1 = this.workspace.getBlockById('block1');
+      const block2 = this.workspace.getBlockById('block2');
+      const origin = new Blockly.utils.Coordinate(0, 0);
+      const belowBlock1 = new Blockly.utils.Coordinate(0, 50);
+      const block1Pos = block1.getRelativeToSurfaceXY();
+      const block2Pos = block2.getRelativeToSurfaceXY();
+      const block1ChildPos = block1.getChildren()[0].getRelativeToSurfaceXY();
+      const block2ChildPos = block2.getChildren()[0].getRelativeToSurfaceXY();
+      assert.equal(topBlocks.length, 2, 'workspace has two top-level block2');
+      assert.equal(allBlocks.length, 4, 'workspace has four blocks overall');
+      assert.deepEqual(block1Pos, origin, 'block1 is at origin');
+      assert.deepEqual(block2Pos, belowBlock1, 'block2 is below block1');
+      assert.isAbove(
+        block1ChildPos.x,
+        block1Pos.x,
+        "block1's child is right of it",
+      );
+      assert.isBelow(
+        block1ChildPos.y,
+        block2Pos.y,
+        "block1's child is above block 2",
+      );
+      assert.isAbove(
+        block2ChildPos.x,
+        block2Pos.x,
+        "block2's child is right of it",
+      );
+      assert.isAbove(
+        block2ChildPos.y,
+        block1Pos.y,
+        "block2's child is below block 1",
+      );
+    });
+
+    test('two large overlapping blocks are moved to origin and below', function () {
+      const blockJson1 = {
+        'type': 'controls_repeat_ext',
+        'id': 'block1',
+        'x': 10,
+        'y': 20,
+        'inputs': {
+          'TIMES': {
+            'shadow': {
+              'type': 'math_number',
+              'fields': {
+                'NUM': 10,
+              },
+            },
+          },
+          'DO': {
+            'block': {
+              'type': 'controls_if',
+              'inputs': {
+                'IF0': {
+                  'block': {
+                    'type': 'logic_boolean',
+                    'fields': {
+                      'BOOL': 'TRUE',
+                    },
+                  },
+                },
+                'DO0': {
+                  'block': {
+                    'type': 'text_print',
+                    'inputs': {
+                      'TEXT': {
+                        'shadow': {
+                          'type': 'text',
+                          'fields': {
+                            'TEXT': 'abc',
+                          },
+                        },
+                      },
+                    },
+                  },
+                },
+              },
+            },
+          },
+        },
+      };
+      const blockJson2 = {...blockJson1, 'id': 'block2', 'x': 20, 'y': 30};
+      Blockly.serialization.blocks.append(blockJson1, this.workspace);
+      Blockly.serialization.blocks.append(blockJson2, this.workspace);
+
+      this.workspace.cleanUp();
+
+      const topBlocks = this.workspace.getTopBlocks(true);
+      const block1 = this.workspace.getBlockById('block1');
+      const block2 = this.workspace.getBlockById('block2');
+      const origin = new Blockly.utils.Coordinate(0, 0);
+      const belowBlock1 = new Blockly.utils.Coordinate(0, 144);
+      assert.equal(topBlocks.length, 2, 'workspace has two top-level blocks');
+      assert.deepEqual(
+        block1.getRelativeToSurfaceXY(),
+        origin,
+        'block1 is at origin',
+      );
+      assert.deepEqual(
+        block2.getRelativeToSurfaceXY(),
+        belowBlock1,
+        'block2 is below block1',
+      );
+    });
+
+    test('five overlapping blocks are moved in-order as one column', function () {
+      const blockJson1 = {
+        'type': 'math_number',
+        'id': 'block1',
+        'x': 1,
+        'y': 2,
+        'fields': {
+          'NUM': 123,
+        },
+      };
+      const blockJson2 = {...blockJson1, 'id': 'block2', 'x': 3, 'y': 4};
+      const blockJson3 = {...blockJson1, 'id': 'block3', 'x': 5, 'y': 6};
+      const blockJson4 = {...blockJson1, 'id': 'block4', 'x': 7, 'y': 8};
+      const blockJson5 = {...blockJson1, 'id': 'block5', 'x': 9, 'y': 10};
+      Blockly.serialization.blocks.append(blockJson1, this.workspace);
+      Blockly.serialization.blocks.append(blockJson2, this.workspace);
+      Blockly.serialization.blocks.append(blockJson3, this.workspace);
+      Blockly.serialization.blocks.append(blockJson4, this.workspace);
+      Blockly.serialization.blocks.append(blockJson5, this.workspace);
+
+      this.workspace.cleanUp();
+
+      const topBlocks = this.workspace.getTopBlocks(true);
+      const block1Pos = this.workspace
+        .getBlockById('block1')
+        .getRelativeToSurfaceXY();
+      const block2Pos = this.workspace
+        .getBlockById('block2')
+        .getRelativeToSurfaceXY();
+      const block3Pos = this.workspace
+        .getBlockById('block3')
+        .getRelativeToSurfaceXY();
+      const block4Pos = this.workspace
+        .getBlockById('block4')
+        .getRelativeToSurfaceXY();
+      const block5Pos = this.workspace
+        .getBlockById('block5')
+        .getRelativeToSurfaceXY();
+      const origin = new Blockly.utils.Coordinate(0, 0);
+      assert.equal(topBlocks.length, 5, 'workspace has five top-level blocks');
+      assert.deepEqual(block1Pos, origin, 'block1 is at origin');
+      assert.equal(block2Pos.x, 0, 'block2.x is at 0');
+      assert.equal(block3Pos.x, 0, 'block3.x is at 0');
+      assert.equal(block4Pos.x, 0, 'block4.x is at 0');
+      assert.equal(block5Pos.x, 0, 'block5.x is at 0');
+      assert.isAbove(block2Pos.y, block1Pos.y, 'block2 is below block1');
+      assert.isAbove(block3Pos.y, block2Pos.y, 'block3 is below block2');
+      assert.isAbove(block4Pos.y, block3Pos.y, 'block4 is below block3');
+      assert.isAbove(block5Pos.y, block4Pos.y, 'block5 is below block4');
+    });
+
+    test('single immovable block at (10, 15) is not moved', function () {
+      const blockJson = {
+        'type': 'math_number',
+        'x': 10,
+        'y': 15,
+        'movable': false,
+        'fields': {
+          'NUM': 123,
+        },
+      };
+      Blockly.serialization.blocks.append(blockJson, this.workspace);
+
+      this.workspace.cleanUp();
+
+      const topBlocks = this.workspace.getTopBlocks(true);
+      const allBlocks = this.workspace.getAllBlocks(false);
+      const origPos = new Blockly.utils.Coordinate(10, 15);
+      assert.equal(topBlocks.length, 1, 'workspace has one top-level block');
+      assert.equal(allBlocks.length, 1, 'workspace has one block overall');
+      assert.deepEqual(
+        topBlocks[0].getRelativeToSurfaceXY(),
+        origPos,
+        'block is at (10, 15)',
+      );
+    });
+
+    test('multiple block types immovable blocks are not moved', function () {
+      const smallBlockJson = {
+        'type': 'math_number',
+        'fields': {
+          'NUM': 123,
+        },
+      };
+      const largeBlockJson = {
+        'type': 'controls_repeat_ext',
+        'inputs': {
+          'TIMES': {
+            'shadow': {
+              'type': 'math_number',
+              'fields': {
+                'NUM': 10,
+              },
+            },
+          },
+          'DO': {
+            'block': {
+              'type': 'controls_if',
+              'inputs': {
+                'IF0': {
+                  'block': {
+                    'type': 'logic_boolean',
+                    'fields': {
+                      'BOOL': 'TRUE',
+                    },
+                  },
+                },
+                'DO0': {
+                  'block': {
+                    'type': 'text_print',
+                    'inputs': {
+                      'TEXT': {
+                        'shadow': {
+                          'type': 'text',
+                          'fields': {
+                            'TEXT': 'abc',
+                          },
+                        },
+                      },
+                    },
+                  },
+                },
+              },
+            },
+          },
+        },
+      };
+      // Block 1 overlaps block 2 (immovable) from above.
+      const blockJson1 = {...smallBlockJson, 'id': 'block1', 'x': 1, 'y': 2};
+      const blockJson2 = {
+        ...smallBlockJson,
+        'id': 'block2',
+        'x': 10,
+        'y': 20,
+        'movable': false,
+      };
+      // Block 3 overlaps block 2 (immovable) from below.
+      const blockJson3 = {...smallBlockJson, 'id': 'block3', 'x': 2, 'y': 30};
+      const blockJson4 = {...largeBlockJson, 'id': 'block4', 'x': 3, 'y': 40};
+      // Block 5 (immovable) will end up overlapping with block 4 since it's large and will be
+      // moved.
+      const blockJson5 = {
+        ...smallBlockJson,
+        'id': 'block5',
+        'x': 20,
+        'y': 200,
+        'movable': false,
+      };
+      Blockly.serialization.blocks.append(blockJson1, this.workspace);
+      Blockly.serialization.blocks.append(blockJson2, this.workspace);
+      Blockly.serialization.blocks.append(blockJson3, this.workspace);
+      Blockly.serialization.blocks.append(blockJson4, this.workspace);
+      Blockly.serialization.blocks.append(blockJson5, this.workspace);
+
+      this.workspace.cleanUp();
+
+      const topBlocks = this.workspace.getTopBlocks(true);
+      const block1Rect = this.workspace
+        .getBlockById('block1')
+        .getBoundingRectangle();
+      const block2Rect = this.workspace
+        .getBlockById('block2')
+        .getBoundingRectangle();
+      const block3Rect = this.workspace
+        .getBlockById('block3')
+        .getBoundingRectangle();
+      const block4Rect = this.workspace
+        .getBlockById('block4')
+        .getBoundingRectangle();
+      const block5Rect = this.workspace
+        .getBlockById('block5')
+        .getBoundingRectangle();
+      assert.equal(topBlocks.length, 5, 'workspace has five top-level blocks');
+      // Check that immovable blocks haven't moved.
+      assert.equal(block2Rect.left, 10, 'block2.x is at 10');
+      assert.equal(block2Rect.top, 20, 'block2.y is at 20');
+      assert.equal(block5Rect.left, 20, 'block5.x is at 20');
+      assert.equal(block5Rect.top, 200, 'block5.y is at 200');
+      // Check that movable positions have correctly been left-aligned.
+      assert.equal(block1Rect.left, 0, 'block1.x is at 0');
+      assert.equal(block3Rect.left, 0, 'block3.x is at 0');
+      assert.equal(block4Rect.left, 0, 'block4.x is at 0');
+      // Block order should be: 2, 1, 3, 5, 4 since 2 and 5 are immovable.
+      assert.isAbove(block1Rect.top, block2Rect.top, 'block1 is below block2');
+      assert.isAbove(block3Rect.top, block1Rect.top, 'block3 is below block1');
+      assert.isAbove(block5Rect.top, block3Rect.top, 'block5 is below block3');
+      assert.isAbove(block4Rect.top, block5Rect.top, 'block4 is below block5');
+      // Ensure no blocks intersect (can check in order due to the position verification above).
+      assert.isFalse(
+        block2Rect.intersects(block1Rect),
+        'block2/block1 do not intersect',
+      );
+      assert.isFalse(
+        block1Rect.intersects(block3Rect),
+        'block1/block3 do not intersect',
+      );
+      assert.isFalse(
+        block3Rect.intersects(block5Rect),
+        'block3/block5 do not intersect',
+      );
+      assert.isFalse(
+        block5Rect.intersects(block4Rect),
+        'block5/block4 do not intersect',
+      );
+    });
+  });
+
   suite('Workspace Base class', function () {
     testAWorkspace();
   });