feat!: Add support for preserving block comment locations. (#8231)

* feat: Add support for preserving block comment locations.

* chore: format the tests.

core/bubbles/textinput_bubble.ts

@@ -47,6 +47,9 @@ export class TextInputBubble extends Bubble {
   /** Functions listening for changes to the size of this bubble. */
   private sizeChangeListeners: (() => void)[] = [];
 
+  /** Functions listening for changes to the location of this bubble. */
+  private locationChangeListeners: (() => void)[] = [];
+
   /** The text of this bubble. */
   private text = '';
 
@@ -105,6 +108,11 @@ export class TextInputBubble extends Bubble {
     this.sizeChangeListeners.push(listener);
   }
 
+  /** Adds a change listener to be notified when this bubble's location changes. */
+  addLocationChangeListener(listener: () => void) {
+    this.locationChangeListeners.push(listener);
+  }
+
   /** Creates the editor UI for this bubble. */
   private createEditor(container: SVGGElement): {
     inputRoot: SVGForeignObjectElement;
@@ -212,10 +220,25 @@ export class TextInputBubble extends Bubble {
 
   /** @returns the size of this bubble. */
   getSize(): Size {
-    // Overriden to be public.
+    // Overridden to be public.
     return super.getSize();
   }
 
+  override moveDuringDrag(newLoc: Coordinate) {
+    super.moveDuringDrag(newLoc);
+    this.onLocationChange();
+  }
+
+  override setPositionRelativeToAnchor(left: number, top: number) {
+    super.setPositionRelativeToAnchor(left, top);
+    this.onLocationChange();
+  }
+
+  protected override positionByRect(rect = new Rect(0, 0, 0, 0)) {
+    super.positionByRect(rect);
+    this.onLocationChange();
+  }
+
   /** Handles mouse down events on the resize target. */
   private onResizePointerDown(e: PointerEvent) {
     this.bringToFront();
@@ -297,6 +320,13 @@ export class TextInputBubble extends Bubble {
       listener();
     }
   }
+
+  /** Handles a location change event for the text area. Calls event listeners. */
+  private onLocationChange() {
+    for (const listener of this.locationChangeListeners) {
+      listener();
+    }
+  }
 }
 
 Css.register(`

core/icons/comment_icon.ts

@@ -58,6 +58,9 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable {
   /** The size of this comment (which is applied to the editable bubble). */
   private bubbleSize = new Size(DEFAULT_BUBBLE_WIDTH, DEFAULT_BUBBLE_HEIGHT);
 
+  /** The location of the comment bubble in workspace coordinates. */
+  private bubbleLocation?: Coordinate;
+
   /**
    * The visibility of the bubble for this comment.
    *
@@ -149,7 +152,13 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable {
   }
 
   override onLocationChange(blockOrigin: Coordinate): void {
+    const oldLocation = this.workspaceLocation;
     super.onLocationChange(blockOrigin);
+    if (this.bubbleLocation) {
+      const newLocation = this.workspaceLocation;
+      const delta = Coordinate.difference(newLocation, oldLocation);
+      this.bubbleLocation = Coordinate.sum(this.bubbleLocation, delta);
+    }
     const anchorLocation = this.getAnchorLocation();
     this.textInputBubble?.setAnchorLocation(anchorLocation);
     this.textBubble?.setAnchorLocation(anchorLocation);
@@ -191,18 +200,43 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable {
     return this.bubbleSize;
   }
 
+  /**
+   * Sets the location of the comment bubble in the workspace.
+   */
+  setBubbleLocation(location: Coordinate) {
+    this.bubbleLocation = location;
+    this.textInputBubble?.moveDuringDrag(location);
+    this.textBubble?.moveDuringDrag(location);
+  }
+
+  /**
+   * @returns the location of the comment bubble in the workspace.
+   */
+  getBubbleLocation(): Coordinate | undefined {
+    return this.bubbleLocation;
+  }
+
   /**
    * @returns the state of the comment as a JSON serializable value if the
    * comment has text. Otherwise returns null.
    */
   saveState(): CommentState | null {
     if (this.text) {
-      return {
+      const state: CommentState = {
         'text': this.text,
         'pinned': this.bubbleIsVisible(),
         'height': this.bubbleSize.height,
         'width': this.bubbleSize.width,
       };
+      const location = this.getBubbleLocation();
+      if (location) {
+        state['x'] = this.sourceBlock.workspace.RTL
+          ? this.sourceBlock.workspace.getWidth() -
+            (location.x + this.bubbleSize.width)
+          : location.x;
+        state['y'] = location.y;
+      }
+      return state;
     }
     return null;
   }
@@ -216,6 +250,16 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable {
     );
     this.bubbleVisiblity = state['pinned'] ?? false;
     this.setBubbleVisible(this.bubbleVisiblity);
+    let x = state['x'];
+    const y = state['y'];
+    renderManagement.finishQueuedRenders().then(() => {
+      if (x && y) {
+        x = this.sourceBlock.workspace.RTL
+          ? this.sourceBlock.workspace.getWidth() - (x + this.bubbleSize.width)
+          : x;
+        this.setBubbleLocation(new Coordinate(x, y));
+      }
+    });
   }
 
   override onClick(): void {
@@ -259,6 +303,12 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable {
     }
   }
 
+  onBubbleLocationChange(): void {
+    if (this.textInputBubble) {
+      this.bubbleLocation = this.textInputBubble.getRelativeToSurfaceXY();
+    }
+  }
+
   bubbleIsVisible(): boolean {
     return this.bubbleVisiblity;
   }
@@ -308,8 +358,14 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable {
     );
     this.textInputBubble.setText(this.getText());
     this.textInputBubble.setSize(this.bubbleSize, true);
+    if (this.bubbleLocation) {
+      this.textInputBubble.moveDuringDrag(this.bubbleLocation);
+    }
     this.textInputBubble.addTextChangeListener(() => this.onTextChange());
     this.textInputBubble.addSizeChangeListener(() => this.onSizeChange());
+    this.textInputBubble.addLocationChangeListener(() =>
+      this.onBubbleLocationChange(),
+    );
   }
 
   /** Shows the non editable text bubble for this comment. */
@@ -320,6 +376,9 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable {
       this.getAnchorLocation(),
       this.getBubbleOwnerRect(),
     );
+    if (this.bubbleLocation) {
+      this.textBubble.moveDuringDrag(this.bubbleLocation);
+    }
   }
 
   /** Hides any open bubbles owned by this comment. */
@@ -365,6 +424,12 @@ export interface CommentState {
 
   /** The width of the comment bubble. */
   width?: number;
+
+  /** The X coordinate of the comment bubble. */
+  x?: number;
+
+  /** The Y coordinate of the comment bubble. */
+  y?: number;
 }
 
 registry.register(CommentIcon.TYPE, CommentIcon);

core/interfaces/i_comment_icon.ts

@@ -8,6 +8,7 @@ import {IconType} from '../icons/icon_types.js';
 import {CommentState} from '../icons/comment_icon.js';
 import {IIcon, isIcon} from './i_icon.js';
 import {Size} from '../utils/size.js';
+import {Coordinate} from '../utils/coordinate.js';
 import {IHasBubble, hasBubble} from './i_has_bubble.js';
 import {ISerializable, isSerializable} from './i_serializable.js';
 
@@ -20,6 +21,10 @@ export interface ICommentIcon extends IIcon, IHasBubble, ISerializable {
 
   getBubbleSize(): Size;
 
+  setBubbleLocation(location: Coordinate): void;
+
+  getBubbleLocation(): Coordinate | undefined;
+
   saveState(): CommentState;
 
   loadState(state: CommentState): void;
@@ -35,6 +40,8 @@ export function isCommentIcon(obj: Object): obj is ICommentIcon {
     (obj as any)['getText'] !== undefined &&
     (obj as any)['setBubbleSize'] !== undefined &&
     (obj as any)['getBubbleSize'] !== undefined &&
+    (obj as any)['setBubbleLocation'] !== undefined &&
+    (obj as any)['getBubbleLocation'] !== undefined &&
     obj.getType() === IconType.COMMENT
   );
 }

core/xml.ts

@@ -217,12 +217,24 @@ export function blockToDom(
     const comment = block.getIcon(IconType.COMMENT)!;
     const size = comment.getBubbleSize();
     const pinned = comment.bubbleIsVisible();
+    const location = comment.getBubbleLocation();
 
     const commentElement = utilsXml.createElement('comment');
     commentElement.appendChild(utilsXml.createTextNode(commentText));
     commentElement.setAttribute('pinned', `${pinned}`);
-    commentElement.setAttribute('h', String(size.height));
-    commentElement.setAttribute('w', String(size.width));
+    commentElement.setAttribute('h', `${size.height}`);
+    commentElement.setAttribute('w', `${size.width}`);
+    if (location) {
+      commentElement.setAttribute(
+        'x',
+        `${
+          block.workspace.RTL
+            ? block.workspace.getWidth() - (location.x + size.width)
+            : location.x
+        }`,
+      );
+      commentElement.setAttribute('y', `${location.y}`);
+    }
 
     element.appendChild(commentElement);
   }
@@ -795,6 +807,8 @@ function applyCommentTagNodes(xmlChildren: Element[], block: Block) {
     const pinned = xmlChild.getAttribute('pinned') === 'true';
     const width = parseInt(xmlChild.getAttribute('w') ?? '50', 10);
     const height = parseInt(xmlChild.getAttribute('h') ?? '50', 10);
+    let x = parseInt(xmlChild.getAttribute('x') ?? '', 10);
+    const y = parseInt(xmlChild.getAttribute('y') ?? '', 10);
 
     block.setCommentText(text);
     const comment = block.getIcon(IconType.COMMENT)!;
@@ -803,8 +817,15 @@ function applyCommentTagNodes(xmlChildren: Element[], block: Block) {
     }
     // Set the pinned state of the bubble.
     comment.setBubbleVisible(pinned);
+
     // Actually show the bubble after the block has been rendered.
-    setTimeout(() => comment.setBubbleVisible(pinned), 1);
+    setTimeout(() => {
+      if (!isNaN(x) && !isNaN(y)) {
+        x = block.workspace.RTL ? block.workspace.getWidth() - (x + width) : x;
+        comment.setBubbleLocation(new Coordinate(x, y));
+      }
+      comment.setBubbleVisible(pinned);
+    }, 1);
   }
 }
 

tests/mocha/block_test.js

@@ -1388,6 +1388,10 @@ suite('Blocks', function () {
           return Blockly.utils.Size(0, 0);
         }
 
+        setBubbleLocation() {}
+
+        getBubbleLocation() {}
+
         bubbleIsVisible() {
           return true;
         }

tests/mocha/comment_test.js

@@ -141,4 +141,30 @@ suite('Comments', function () {
       assertBubbleSize(this.comment, 100, 100);
     });
   });
+  suite('Set/Get Bubble Location', function () {
+    teardown(function () {
+      sinon.restore();
+    });
+    function assertBubbleLocation(comment, x, y) {
+      const location = comment.getBubbleLocation();
+      assert.equal(location.x, x);
+      assert.equal(location.y, y);
+    }
+    test('Set Location While Visible', function () {
+      this.comment.setBubbleVisible(true);
+
+      this.comment.setBubbleLocation(new Blockly.utils.Coordinate(100, 100));
+      assertBubbleLocation(this.comment, 100, 100);
+
+      this.comment.setBubbleVisible(false);
+      assertBubbleLocation(this.comment, 100, 100);
+    });
+    test('Set Location While Invisible', function () {
+      this.comment.setBubbleLocation(new Blockly.utils.Coordinate(100, 100));
+      assertBubbleLocation(this.comment, 100, 100);
+
+      this.comment.setBubbleVisible(true);
+      assertBubbleLocation(this.comment, 100, 100);
+    });
+  });
 });