fix: comment move and change events (#7947)

* fix: comment move event

* feat: add support for a drag reason

* fix: comment change events

* chore: add tests for move and change events

core/comments/rendered_workspace_comment.ts

@@ -109,16 +109,15 @@ export class RenderedWorkspaceComment
   }
 
   /** Move the comment by the given amounts in workspace coordinates. */
-  moveBy(dx: number, dy: number, _reason?: string[] | undefined): void {
-    // TODO(#7909): Deal with reason when we add events.
+  moveBy(dx: number, dy: number, reason?: string[] | undefined): void {
     const loc = this.getRelativeToSurfaceXY();
     const newLoc = new Coordinate(loc.x + dx, loc.y + dy);
-    this.moveTo(newLoc);
+    this.moveTo(newLoc, reason);
   }
 
   /** Moves the comment to the given location in workspace coordinates. */
-  override moveTo(location: Coordinate): void {
-    super.moveTo(location);
+  override moveTo(location: Coordinate, reason?: string[] | undefined): void {
+    super.moveTo(location, reason);
     this.view.moveTo(location);
   }
 

core/comments/workspace_comment.ts

@@ -9,6 +9,7 @@ import {Size} from '../utils/size.js';
 import {Coordinate} from '../utils/coordinate.js';
 import * as idGenerator from '../utils/idgenerator.js';
 import * as eventUtils from '../events/utils.js';
+import {CommentMove} from '../events/events_comment_move.js';
 
 export class WorkspaceComment {
   /** The unique identifier for this comment. */
@@ -72,9 +73,20 @@ export class WorkspaceComment {
     }
   }
 
+  /** Fires a comment change event. */
+  private fireChangeEvent(oldText: string, newText: string) {
+    if (eventUtils.isEnabled()) {
+      eventUtils.fire(
+        new (eventUtils.get(eventUtils.COMMENT_CHANGE))(this, oldText, newText),
+      );
+    }
+  }
+
   /** Sets the text of the comment. */
   setText(text: string) {
+    const oldText = this.text;
     this.text = text;
+    this.fireChangeEvent(oldText, text);
   }
 
   /** Returns the text of the comment. */
@@ -166,8 +178,16 @@ export class WorkspaceComment {
   }
 
   /** Moves the comment to the given location in workspace coordinates. */
-  moveTo(location: Coordinate) {
+  moveTo(location: Coordinate, reason?: string[] | undefined) {
+    const event = new (eventUtils.get(eventUtils.COMMENT_MOVE))(
+      this,
+    ) as CommentMove;
+    if (reason) event.setReason(reason);
+
     this.location = location;
+
+    event.recordNew();
+    if (eventUtils.isEnabled()) eventUtils.fire(event);
   }
 
   /** Returns the position of the comment in workspace coordinates. */

core/events/events_comment_change.ts

@@ -124,13 +124,16 @@ export class CommentChange extends CommentBase {
           'the constructor, or call fromJson',
       );
     }
-    const comment = workspace.getCommentById(this.commentId);
+    // TODO: Remove the cast when we fix the type of getCommentById.
+    const comment = workspace.getCommentById(
+      this.commentId,
+    ) as unknown as WorkspaceComment;
     if (!comment) {
       console.warn("Can't change non-existent comment: " + this.commentId);
       return;
     }
     const contents = forward ? this.newContents_ : this.oldContents_;
-    if (!contents) {
+    if (contents === undefined) {
       if (forward) {
         throw new Error(
           'The new contents is undefined. Either pass a value to ' +
@@ -142,7 +145,7 @@ export class CommentChange extends CommentBase {
           'the constructor, or call fromJson',
       );
     }
-    comment.setContent(contents);
+    comment.setText(contents);
   }
 }
 

core/events/events_comment_move.ts

@@ -35,6 +35,17 @@ export class CommentMove extends CommentBase {
   /** The location of the comment after the move, in workspace coordinates. */
   newCoordinate_?: Coordinate;
 
+  /**
+   * An explanation of what this move is for.  Known values include:
+   *  'drag' -- A drag operation completed.
+   *  'snap' -- Comment got shifted to line up with the grid.
+   *  'inbounds' -- Block got pushed back into a non-scrolling workspace.
+   *  'create' -- Block created via deserialization.
+   *  'cleanup' -- Workspace aligned top-level blocks.
+   * Event merging may create multiple reasons: ['drag', 'inbounds', 'snap'].
+   */
+  reason?: string[];
+
   /**
    * @param opt_comment The comment that is being moved.  Undefined for a blank
    *     event.
@@ -70,6 +81,15 @@ export class CommentMove extends CommentBase {
     this.newCoordinate_ = this.comment_.getRelativeToSurfaceXY();
   }
 
+  /**
+   * Sets the reason for a move event.
+   *
+   * @param reason Why is this move happening?  'drag', 'bump', 'snap', ...
+   */
+  setReason(reason: string[]) {
+    this.reason = reason;
+  }
+
   /**
    * Override the location before the move.  Use this if you don't create the
    * event until the end of the move, but you know the original location.
@@ -158,7 +178,10 @@ export class CommentMove extends CommentBase {
           'the constructor, or call fromJson',
       );
     }
-    const comment = workspace.getCommentById(this.commentId);
+    // TODO: Remove cast when we update getCommentById.
+    const comment = workspace.getCommentById(
+      this.commentId,
+    ) as unknown as WorkspaceComment;
     if (!comment) {
       console.warn("Can't move non-existent comment: " + this.commentId);
       return;
@@ -172,9 +195,7 @@ export class CommentMove extends CommentBase {
           'or call fromJson',
       );
     }
-    // TODO: Check if the comment is being dragged, and give up if so.
-    const current = comment.getRelativeToSurfaceXY();
-    comment.moveBy(target.x - current.x, target.y - current.y);
+    comment.moveTo(target);
   }
 }
 

tests/mocha/workspace_comment_test.js

@@ -54,5 +54,45 @@ suite('Workspace comment', function () {
         this.workspace.id,
       );
     });
+
+    test('move events are fired when a comment is moved', function () {
+      this.renderedComment = new Blockly.comments.RenderedWorkspaceComment(
+        this.workspace,
+      );
+      const spy = createChangeListenerSpy(this.workspace);
+
+      this.renderedComment.moveTo(new Blockly.utils.Coordinate(42, 42));
+
+      assertEventFired(
+        spy,
+        Blockly.Events.CommentMove,
+        {
+          commentId: this.renderedComment.id,
+          oldCoordinate_: {x: 0, y: 0},
+          newCoordinate_: {x: 42, y: 42},
+        },
+        this.workspace.id,
+      );
+    });
+
+    test('change events are fired when a comments text is edited', function () {
+      this.renderedComment = new Blockly.comments.RenderedWorkspaceComment(
+        this.workspace,
+      );
+      const spy = createChangeListenerSpy(this.workspace);
+
+      this.renderedComment.setText('test text');
+
+      assertEventFired(
+        spy,
+        Blockly.Events.CommentChange,
+        {
+          commentId: this.renderedComment.id,
+          oldContents_: '',
+          newContents_: 'test text',
+        },
+        this.workspace.id,
+      );
+    });
   });
 });