From 2c2970df57446ca71a412b3600cac41edd87d47c Mon Sep 17 00:00:00 2001
From: Karishma Chadha <kchadha@scratch.mit.edu>
Date: Wed, 6 Jun 2018 18:45:32 -0400
Subject: [PATCH] Code cleanup addressing PR comments

---
 core/comment_events.js               | 17 ++++++-------
 core/contextmenu.js                  |  4 ++-
 core/events.js                       | 12 +++++++++
 core/scratch_block_comment.js        | 38 +++++++++++++++++++++++++---
 core/scratch_bubble.js               | 10 ++++++--
 core/workspace_comment_render_svg.js |  9 ++++---
 core/xml.js                          |  2 +-
 7 files changed, 73 insertions(+), 19 deletions(-)

diff --git a/core/comment_events.js b/core/comment_events.js
index 9e8f9e28..aba4bb1c 100644
--- a/core/comment_events.js
+++ b/core/comment_events.js
@@ -111,8 +111,8 @@ Blockly.Events.CommentBase.prototype.fromJson = function(json) {
 
 /**
  * Helper function for finding the comment this event pertains to.
- * @return {Blockly.WorkspaceComment | Blockly.ScratchBlockComment}
- *     The comment this event pertains to.
+ * @return {?(Blockly.WorkspaceComment | Blockly.ScratchBlockComment)}
+ *     The comment this event pertains to, or null if it no longer exists.
  * @private
  */
 Blockly.Events.CommentBase.prototype.getComment_ = function() {
@@ -202,13 +202,12 @@ Blockly.Events.CommentChange.prototype.run = function(forward) {
   }
 
   console.warn('Unrecognized comment change: ' + JSON.stringify(contents) + '; cannot undo/redo');
-  return;
 };
 
 /**
  * Class for a comment creation event.
- * @param {Blockly.WorkspaceComment} comment The created comment.
- *     Null for a blank event.
+ * @param {Blockly.WorkspaceComment | Blockly.ScratchBlockComment} comment
+ *     The created comment. Null for a blank event.
  * @param {string=} opt_blockId Optional id for the block this comment belongs
  *     to, if it is a block comment.
  * @extends {Blockly.Events.CommentBase}
@@ -222,13 +221,13 @@ Blockly.Events.CommentCreate = function(comment) {
 
   /**
    * The text content of this comment.
-   * @type{string}
+   * @type {string}
    */
   this.text = comment.getText();
 
   /**
    * The XY position of this comment on the workspace.
-   * @type{goog.math.Coordinate}
+   * @type {goog.math.Coordinate}
    */
   this.xy = comment.getXY();
 
@@ -236,13 +235,13 @@ Blockly.Events.CommentCreate = function(comment) {
 
   /**
    * The width of this comment when it is full size.
-   * @type{number}
+   * @type {number}
    */
   this.width = hw.width;
 
   /**
    * The height of this comment when it is full size.
-   * @type{number}
+   * @type {number}
    */
   this.height = hw.height;
 
diff --git a/core/contextmenu.js b/core/contextmenu.js
index 834be531..7c3276d3 100644
--- a/core/contextmenu.js
+++ b/core/contextmenu.js
@@ -499,7 +499,9 @@ Blockly.ContextMenu.workspaceCommentOption = function(ws, e) {
       comment.render(false);
       comment.select();
     }
-    if (disabled) Blockly.Events.enable();
+    if (disabled) {
+      Blockly.Events.enable();
+    }
     Blockly.WorkspaceComment.fireCreateEvent(comment);
   };
 
diff --git a/core/events.js b/core/events.js
index 0cab8d32..9598fa7c 100644
--- a/core/events.js
+++ b/core/events.js
@@ -361,6 +361,18 @@ Blockly.Events.fromJson = function(json, workspace) {
     case Blockly.Events.VAR_RENAME:
       event = new Blockly.Events.VarRename(null);
       break;
+    case Blockly.Events.COMMENT_CREATE:
+      event = new Blockly.Events.CommentCreate(null);
+      break;
+    case Blockly.Events.COMMENT_CHANGE:
+      event = new Blockly.Events.CommentChange(null);
+      break;
+    case Blockly.Events.COMMENT_MOVE:
+      event = new Blockly.Events.CommentMove(null);
+      break;
+    case Blockly.Events.COMMENT_DELETE:
+      event = new Blockly.Events.CommentDelete(null);
+      break;
     case Blockly.Events.UI:
       event = new Blockly.Events.Ui(null);
       break;
diff --git a/core/scratch_block_comment.js b/core/scratch_block_comment.js
index 8be81c9b..7b671015 100644
--- a/core/scratch_block_comment.js
+++ b/core/scratch_block_comment.js
@@ -51,16 +51,44 @@ goog.require('goog.userAgent');
  */
 Blockly.ScratchBlockComment = function(block, text, id, x, y, minimized) {
   Blockly.ScratchBlockComment.superClass_.constructor.call(this, block);
+  /**
+   * The text content of this comment.
+   * @type {string}
+   */
   this.text_ = text;
+  /**
+   * The x position of this comment in workspace coordinates.
+   * @type {number}
+   */
   this.x_ = x;
+  /**
+   * The y position of this comment in workspace coordinates.
+   * @type {number}
+   */
   this.y_ = y;
+  /**
+   * Whether this comment is minimized.
+   * @type {boolean}
+   */
   this.isMinimized_ = minimized || false;
 
+  /**
+   * The workspace this comment belongs to.
+   * @type {Blockly.Workspace}
+   */
   this.workspace = block.workspace;
+  /**
+   * The unique identifier for this comment.
+   * @type {string}
+   */
   this.id = goog.isString(id) && !this.workspace.getCommentById(id) ?
       id : Blockly.utils.genUid();
   this.workspace.addTopComment(this);
 
+  /**
+   * The id of the block this comment belongs to.
+   * @type {string}
+   */
   this.blockId = block.id;
 
   if (!block.rendered) {
@@ -303,7 +331,9 @@ Blockly.ScratchBlockComment.prototype.setVisible = function(visible) {
   // Restore the bubble stats after the visibility switch.
   this.setText(text);
   this.setBubbleSize(size.width, size.height);
-  if (visible) Blockly.ScratchBlockComment.fireCreateEvent(this);
+  if (visible) {
+    Blockly.ScratchBlockComment.fireCreateEvent(this);
+  }
 };
 
 /**
@@ -320,7 +350,9 @@ Blockly.ScratchBlockComment.prototype.toggleMinimize_ = function() {
  * @package
  */
 Blockly.ScratchBlockComment.prototype.setMinimized = function(minimize) {
-  if (this.isMinimized_ == minimize) return;
+  if (this.isMinimized_ == minimize) {
+    return;
+  }
   Blockly.Events.fire(new Blockly.Events.CommentChange(this,
       {minimized: this.isMinimized_}, {minimized: minimize}));
   this.isMinimized_ = minimize;
@@ -360,6 +392,7 @@ Blockly.ScratchBlockComment.prototype.setBubbleSize = function(width, height) {
  * bubble, also set the bubble's size.
  * @param {number} width Width of the unminimized comment.
  * @param {number} height Height of the unminimized comment.
+ * @package
  */
 Blockly.ScratchBlockComment.prototype.setSize = function(width, height) {
   var oldWidth = this.width_;
@@ -520,7 +553,6 @@ Blockly.ScratchBlockComment.prototype.toXmlWithXY = function() {
   return element;
 };
 
-
 /**
  * Fire a create event for the given workspace comment, if comments are enabled.
  * @param {!Blockly.WorkspaceComment} comment The comment that was just created.
diff --git a/core/scratch_bubble.js b/core/scratch_bubble.js
index abaa101a..fb14ec41 100644
--- a/core/scratch_bubble.js
+++ b/core/scratch_bubble.js
@@ -56,6 +56,11 @@ Blockly.ScratchBubble = function(comment, workspace, content, anchorXY,
     bubbleWidth, bubbleHeight, bubbleX, bubbleY, minimized) {
 
   // Needed for Events
+  /**
+   * The comment this bubble belongs to.
+   * @type {Blockly.ScratchBlockComment}
+   * @package
+   */
   this.comment = comment;
 
   this.workspace_ = workspace;
@@ -363,8 +368,9 @@ Blockly.ScratchBubble.prototype.resizeMouseDown_ = function(e) {
 Blockly.ScratchBubble.prototype.resizeMouseUp_ = function(_e) {
   var oldHW = this.resizeStartSize_;
   this.resizeStartSize_ = null;
-  if (this.width_ == oldHW.width &&
-      this.height_ == oldHW.height) return;
+  if (this.width_ == oldHW.width && this.height_ == oldHW.height) {
+    return;
+  }
   // Fire a change event for the new width/height after
   // resize mouse up
   Blockly.Events.fire(new Blockly.Events.CommentChange(
diff --git a/core/workspace_comment_render_svg.js b/core/workspace_comment_render_svg.js
index 975a9e1a..5c6eee30 100644
--- a/core/workspace_comment_render_svg.js
+++ b/core/workspace_comment_render_svg.js
@@ -337,8 +337,9 @@ Blockly.WorkspaceCommentSvg.prototype.resizeMouseUp_ = function(/*e*/) {
   this.unbindDragEvents_();
   var oldHW = this.resizeStartSize_;
   this.resizeStartSize_ = null;
-  if (this.width_ == oldHW.width &&
-      this.height_ == oldHW.height) return;
+  if (this.width_ == oldHW.width && this.height_ == oldHW.height) {
+    return;
+  }
   // Fire a change event for the new width/height after
   // resize mouse up
   Blockly.Events.fire(new Blockly.Events.CommentChange(
@@ -365,7 +366,9 @@ Blockly.WorkspaceCommentSvg.prototype.resizeMouseMove_ = function(e) {
     disabled = true;
   }
   this.setSize(this.RTL ? -newXY.x : newXY.x, newXY.y);
-  if (disabled) Blockly.Events.enable();
+  if (disabled) {
+    Blockly.Events.enable();
+  }
 };
 
 /**
diff --git a/core/xml.js b/core/xml.js
index 90b8560a..2b5f71f3 100644
--- a/core/xml.js
+++ b/core/xml.js
@@ -706,7 +706,7 @@ Blockly.Xml.domToBlockHeadless_ = function(xmlBlock, workspace) {
         if (!isNaN(bubbleW) && !isNaN(bubbleH) &&
             block.comment && block.comment.setVisible) {
           if (block.comment instanceof Blockly.ScratchBlockComment) {
-            block.commet.setSize(bubbleW, bubbleH);
+            block.comment.setSize(bubbleW, bubbleH);
           } else {
             block.comment.setBubbleSize(bubbleW, bubbleH);
           }