From 3802a0c9604226eed595a495dccf4d7f5a0cc175 Mon Sep 17 00:00:00 2001
From: Rachel Fenichel <fenichel@google.com>
Date: Mon, 5 Aug 2019 16:01:50 -0700
Subject: [PATCH 1/4] Remove getCorrectedSize for fields

---
 core/field.js                                 | 49 ++++++++++++-------
 core/field_checkbox.js                        | 16 +-----
 core/field_colour.js                          | 21 +-------
 core/field_dropdown.js                        | 49 +++++++++----------
 core/field_image.js                           | 32 ++++--------
 core/field_label.js                           | 20 ++------
 core/field_textinput.js                       | 14 ------
 .../block_rendering_rewrite/measurables.js    |  2 +-
 8 files changed, 69 insertions(+), 134 deletions(-)

diff --git a/core/field.js b/core/field.js
index f6591c8a7..946da2d09 100644
--- a/core/field.js
+++ b/core/field.js
@@ -47,7 +47,12 @@ goog.require('goog.style');
  * @constructor
  */
 Blockly.Field = function(value, opt_validator) {
-  this.size_ = new Blockly.utils.Size(0, Blockly.BlockSvg.MIN_BLOCK_Y);
+  /**
+   * The size of the area rendered by the field.
+   * @type {Blockly.utils.Size}
+   */
+  this.size_ = new Blockly.utils.Size(0, 0);
+
   this.setValue(value);
   opt_validator && this.setValidator(opt_validator);
 };
@@ -124,6 +129,17 @@ Blockly.Field.cacheReference_ = 0;
  */
 Blockly.Field.BORDER_RECT_DEFAULT_HEIGHT = 16;
 
+/**
+ * The default height of the text element on any field.
+ * @type {number}
+ * @package
+ */
+Blockly.Field.TEXT_DEFAULT_HEIGHT = 12.5;
+
+Blockly.Field.WIDTH_PADDING = 10;
+
+Blockly.Field.HEIGHT_PADDING = 10;
+
 /**
  * Name of field.  Unique within each block.
  * Static labels are usually unnamed.
@@ -279,14 +295,16 @@ Blockly.Field.prototype.initView = function() {
  * @protected
  */
 Blockly.Field.prototype.createBorderRect_ = function() {
+  this.size_.height =
+      Math.max(this.size_.height, Blockly.Field.BORDER_RECT_DEFAULT_HEIGHT);
   this.borderRect_ = Blockly.utils.dom.createSvgElement('rect',
       {
         'rx': 4,
         'ry': 4,
-        'x': -Blockly.BlockSvg.SEP_SPACE_X / 2,
+        'x': -Blockly.Field.WIDTH_PADDING / 2,
         'y': 0,
-        'height': Blockly.Field.BORDER_RECT_DEFAULT_HEIGHT,
-        'width': this.size_.width + Blockly.BlockSvg.SEP_SPACE_X
+        'height': this.size_.height,
+        'width': this.size_.width
       }, this.fieldGroup_);
 };
 
@@ -300,7 +318,9 @@ Blockly.Field.prototype.createTextElement_ = function() {
   this.textElement_ = Blockly.utils.dom.createSvgElement('text',
       {
         'class': 'blocklyText',
-        'y': this.size_.height - 12.5
+        // This may just be trying to vertically center the text?
+        'y': Blockly.Field.TEXT_DEFAULT_HEIGHT,
+        'x': 0
       }, this.fieldGroup_);
   this.textContent_ = document.createTextNode('');
   this.textElement_.appendChild(this.textContent_);
@@ -564,12 +584,13 @@ Blockly.Field.prototype.updateWidth = function() {
  * @protected
  */
 Blockly.Field.prototype.updateSize_ = function() {
-  var width = Blockly.Field.getCachedWidth(this.textElement_);
+  var textWidth = Blockly.Field.getCachedWidth(this.textElement_);
+  var totalWidth = textWidth;
   if (this.borderRect_) {
-    this.borderRect_.setAttribute('width',
-        width + Blockly.BlockSvg.SEP_SPACE_X);
+    totalWidth += Blockly.Field.WIDTH_PADDING;
+    this.borderRect_.setAttribute('width', totalWidth);
   }
-  this.size_.width = width;
+  this.size_.width = totalWidth;
 };
 
 /**
@@ -657,16 +678,6 @@ Blockly.Field.prototype.getSize = function() {
   return this.size_;
 };
 
-/**
- * Get the size of the visible field, as used in new rendering.
- * @return {!Blockly.utils.Size} The size of the visible field.
- * @package
- */
-Blockly.Field.prototype.getCorrectedSize = function() {
-  // TODO (#2562): Remove getCorrectedSize.
-  return this.getSize();
-};
-
 /**
  * Returns the bounding box of the rendered field, accounting for workspace
  * scaling.
diff --git a/core/field_checkbox.js b/core/field_checkbox.js
index 0b1c204a7..caae6d9eb 100644
--- a/core/field_checkbox.js
+++ b/core/field_checkbox.js
@@ -70,7 +70,7 @@ Blockly.FieldCheckbox.fromJson = function(options) {
  * @type {number}
  * @const
  */
-Blockly.FieldCheckbox.WIDTH = 5;
+Blockly.FieldCheckbox.WIDTH = 15;
 
 /**
  * Character for the checkmark.
@@ -210,18 +210,4 @@ Blockly.FieldCheckbox.prototype.convertValueToBool_ = function(value) {
   }
 };
 
-/**
- * Get the size of the visible field, as used in new rendering.
- * The checkbox field fills the entire border rect, rather than just using the
- * text element.
- * @return {!Blockly.utils.Size} The size of the visible field.
- * @package
- */
-Blockly.FieldCheckbox.prototype.getCorrectedSize = function() {
-  this.getSize();
-
-  // TODO (#2562): Remove getCorrectedSize.
-  return new Blockly.utils.Size(this.size_.width + Blockly.BlockSvg.SEP_SPACE_X,
-      Blockly.Field.BORDER_RECT_DEFAULT_HEIGHT);
-};
 Blockly.Field.register('field_checkbox', Blockly.FieldCheckbox);
diff --git a/core/field_colour.js b/core/field_colour.js
index 9363fbb47..d2acbf9ca 100644
--- a/core/field_colour.js
+++ b/core/field_colour.js
@@ -78,7 +78,7 @@ Blockly.FieldColour.fromJson = function(options) {
  * @private
  * @const
  */
-Blockly.FieldColour.DEFAULT_WIDTH = 16;
+Blockly.FieldColour.DEFAULT_WIDTH = 16 + Blockly.Field.WIDTH_PADDING;
 
 /**
  * Default height of a colour field.
@@ -86,7 +86,7 @@ Blockly.FieldColour.DEFAULT_WIDTH = 16;
  * @private
  * @const
  */
-Blockly.FieldColour.DEFAULT_HEIGHT = 12;
+Blockly.FieldColour.DEFAULT_HEIGHT = Blockly.Field.BORDER_RECT_DEFAULT_HEIGHT;
 
 /**
  * Serializable fields are saved by the XML renderer, non-serializable fields
@@ -346,21 +346,4 @@ Blockly.FieldColour.prototype.dropdownDispose_ = function() {
   Blockly.unbindEvent_(this.onUpWrapper_);
 };
 
-/**
- * Get the size of the visible field, as used in new rendering.
- * The colour field fills the bounding box with colour and takes up the full
- * space of the bounding box.
- * @return {!Blockly.utils.Size} The size of the visible field.
- * @package
- */
-Blockly.FieldColour.prototype.getCorrectedSize = function() {
-  // getSize also renders and updates the size if needed.  Rather than duplicate
-  // the logic to figure out whether to rerender, just call getSize.
-  this.getSize();
-  // TODO (#2562): Remove getCorrectedSize.
-  return new Blockly.utils.Size(
-      this.size_.width + Blockly.BlockSvg.SEP_SPACE_X,
-      Blockly.Field.BORDER_RECT_DEFAULT_HEIGHT - 1);
-};
-
 Blockly.Field.register('field_colour', Blockly.FieldColour);
diff --git a/core/field_dropdown.js b/core/field_dropdown.js
index 10155267b..88a368c8f 100644
--- a/core/field_dropdown.js
+++ b/core/field_dropdown.js
@@ -105,6 +105,13 @@ Blockly.FieldDropdown.MAX_MENU_HEIGHT_VH = 0.45;
  */
 Blockly.FieldDropdown.IMAGE_Y_OFFSET = 5;
 
+/**
+ * The total vertical padding above and below an image.
+ * @type {number}
+ * @const
+ */
+Blockly.FieldDropdown.IMAGE_Y_PADDING = Blockly.FieldDropdown.IMAGE_Y_OFFSET * 2;
+
 /**
  * Android can't (in 2014) display "▾", so use "▼" instead.
  */
@@ -153,6 +160,8 @@ Blockly.FieldDropdown.prototype.initView = function() {
   } else {
     this.textElement_.appendChild(this.arrow_);
   }
+
+  this.contentDimensions_ = new Blockly.utils.Size(0, 0);
 };
 
 /**
@@ -467,9 +476,8 @@ Blockly.FieldDropdown.prototype.render_ = function() {
   } else {
     this.renderSelectedText_();
   }
-  this.borderRect_.setAttribute('height', this.size_.height - 9);
-  this.borderRect_.setAttribute('width',
-      this.size_.width + Blockly.BlockSvg.SEP_SPACE_X);
+  this.borderRect_.setAttribute('height', this.size_.height);
+  this.borderRect_.setAttribute('width', this.size_.width);
 };
 
 /**
@@ -484,18 +492,19 @@ Blockly.FieldDropdown.prototype.renderSelectedImage_ = function() {
   this.imageElement_.setAttribute('width', this.imageJson_.width);
 
   var arrowWidth = Blockly.Field.getCachedWidth(this.arrow_);
-  // TODO: Standardize sizing, need to talk to rachel and abby about rendering
-  //  redux.
-  // I think really this means plus 10?
-  this.size_.height = Number(this.imageJson_.height) + 19;
-  this.size_.width = Number(this.imageJson_.width) + arrowWidth;
+
+  // Height and width include the border rect.
+  var imageHeight = Number(this.imageJson_.height);
+  var imageWidth = Number(this.imageJson_.width);
+  this.size_.height = imageHeight + Blockly.FieldDropdown.IMAGE_Y_PADDING;
+  this.size_.width = imageWidth + arrowWidth + Blockly.Field.WIDTH_PADDING;
 
   if (this.sourceBlock_.RTL) {
     this.imageElement_.setAttribute('x', arrowWidth);
     this.textElement_.setAttribute('x', -1);
   } else {
     this.textElement_.setAttribute('text-anchor', 'end');
-    this.textElement_.setAttribute('x', this.size_.width + 1);
+    this.textElement_.setAttribute('x', imageWidth + arrowWidth + 1);
   }
 };
 
@@ -507,8 +516,10 @@ Blockly.FieldDropdown.prototype.renderSelectedText_ = function() {
   this.textContent_.nodeValue = this.getDisplayText_();
   this.textElement_.setAttribute('text-anchor', 'start');
   this.textElement_.setAttribute('x', 0);
-  this.size_.height = Blockly.BlockSvg.MIN_BLOCK_Y;
-  this.size_.width = Blockly.Field.getCachedWidth(this.textElement_);
+  // Height and width include the border rect.
+  this.size_.height = Blockly.Field.BORDER_RECT_DEFAULT_HEIGHT;
+  this.size_.width =
+      Blockly.Field.getCachedWidth(this.textElement_) + Blockly.Field.WIDTH_PADDING;
 };
 
 /**
@@ -548,20 +559,4 @@ Blockly.FieldDropdown.validateOptions_ = function(options) {
   }
 };
 
-/**
- * Get the size of the visible field, as used in new rendering.
- * @return {!Blockly.utils.Size} The size of the visible field.
- * @package
- */
-Blockly.FieldDropdown.prototype.getCorrectedSize = function() {
-  // getSize also renders and updates the size if needed.  Rather than duplicate
-  // the logic to figure out whether to rerender, just call getSize.
-  this.getSize();
-  // This extra 9 was probably to add padding between rows.
-  // It's also found in render_, renderSelectedImage_, and renderSelectedText_.
-  // TODO (#2562): Remove getCorrectedSize.
-  return new Blockly.utils.Size(this.size_.width + Blockly.BlockSvg.SEP_SPACE_X,
-      this.size_.height - 9);
-};
-
 Blockly.Field.register('field_dropdown', Blockly.FieldDropdown);
diff --git a/core/field_image.js b/core/field_image.js
index b17f0e1f7..f154e8b36 100644
--- a/core/field_image.js
+++ b/core/field_image.js
@@ -59,14 +59,15 @@ Blockly.FieldImage = function(src, width, height,
   }
 
   // Ensure height and width are numbers.  Strings are bad at math.
-  this.height_ = Number(height);
-  this.width_ = Number(width);
-  if (this.height_ <= 0 || this.width_ <= 0) {
+  var imageHeight = Number(height);
+  var imageWidth = Number(width);
+  if (imageHeight <= 0 || imageWidth <= 0) {
     throw Error('Height and width values of an image field must be greater' +
       ' than 0.');
   }
-  this.size_ = new Blockly.utils.Size(this.width_,
-      this.height_ + 2 * Blockly.BlockSvg.INLINE_PADDING_Y);
+  this.imageHeight_ = imageHeight;
+  this.size_ = new Blockly.utils.Size(imageWidth,
+      imageHeight + Blockly.FieldImage.Y_PADDING);
 
   this.flipRtl_ = opt_flipRtl;
   this.text_ = opt_alt || '';
@@ -114,6 +115,8 @@ Blockly.FieldImage.prototype.EDITABLE = false;
  */
 Blockly.FieldImage.prototype.isDirty_ = false;
 
+Blockly.FieldImage.Y_PADDING = 1;
+
 /**
  * Create the block UI for this image.
  * @package
@@ -122,8 +125,8 @@ Blockly.FieldImage.prototype.initView = function() {
   this.imageElement_ = Blockly.utils.dom.createSvgElement(
       'image',
       {
-        'height': this.height_ + 'px',
-        'width': this.width_ + 'px',
+        'height': this.imageHeight_ + 'px',
+        'width': this.size_.width + 'px',
         'alt': this.text_
       },
       this.fieldGroup_);
@@ -211,20 +214,5 @@ Blockly.FieldImage.prototype.showEditor_ = function() {
 Blockly.FieldImage.prototype.setOnClickHandler = function(func) {
   this.clickHandler_ = func;
 };
-/*
- * Get the size of the visible field, as used in new rendering.
- * @return {!Blockly.utils.Size} The size of the visible field.
- * @package
- */
-Blockly.FieldImage.prototype.getCorrectedSize = function() {
-  // getSize also renders and updates the size if needed.  Rather than duplicate
-  // the logic to figure out whether to rerender, just call getSize.
-  this.getSize();
-  // Old rendering adds an extra pixel under the image.  We include this in the
-  // height of the image in new rendering, rather than having the spacer below
-  // know that there was an image in the previous row.
-  // TODO (#2562): Remove getCorrectedSize.
-  return new Blockly.utils.Size(this.size_.width, this.height_ + 1);
-};
 
 Blockly.Field.register('field_image', Blockly.FieldImage);
diff --git a/core/field_label.js b/core/field_label.js
index 90d6d6431..abfe0b438 100644
--- a/core/field_label.js
+++ b/core/field_label.js
@@ -43,7 +43,7 @@ goog.require('Blockly.utils.Size');
  * @constructor
  */
 Blockly.FieldLabel = function(opt_value, opt_class) {
-  this.size_ = new Blockly.utils.Size(0, 17.5);
+  this.size_ = new Blockly.utils.Size(0, Blockly.Field.TEXT_DEFAULT_HEIGHT);
   this.class_ = opt_class;
   opt_value = this.doClassValidation_(opt_value);
   if (opt_value === null) {
@@ -80,7 +80,8 @@ Blockly.FieldLabel.prototype.EDITABLE = false;
  */
 Blockly.FieldLabel.prototype.initView = function() {
   this.createTextElement_();
-  this.textElement_.setAttribute('y', this.size_.height - 5);
+  // The y attribute of an svg text element is the baseline.
+  this.textElement_.setAttribute('y', this.size_.height);
   if (this.class_) {
     Blockly.utils.dom.addClass(this.textElement_, this.class_);
   }
@@ -99,19 +100,4 @@ Blockly.FieldLabel.prototype.doClassValidation_ = function(opt_newValue) {
   return String(opt_newValue);
 };
 
-/**
- * Get the size of the visible field, as used in new rendering.
- * @return {!Blockly.utils.Size} The size of the visible field.
- * @package
- */
-Blockly.FieldLabel.prototype.getCorrectedSize = function() {
-  // getSize also renders and updates the size if needed.  Rather than duplicate
-  // the logic to figure out whether to rerender, just call getSize.
-  this.getSize();
-  // This extra 5 was probably to add padding between rows.
-  // It's also found in the constructor and in initView.
-  // TODO (#2562): Remove getCorrectedSize.
-  return new Blockly.utils.Size(this.size_.width, this.size_.height - 5);
-};
-
 Blockly.Field.register('field_label', Blockly.FieldLabel);
diff --git a/core/field_textinput.js b/core/field_textinput.js
index 923807ea1..e2f7c6232 100644
--- a/core/field_textinput.js
+++ b/core/field_textinput.js
@@ -428,18 +428,4 @@ Blockly.FieldTextInput.nonnegativeIntegerValidator = function(text) {
   return n;
 };
 
-/**
- * Get the size of the visible field, as used in new rendering.
- * @return {!Blockly.utils.Size} The size of the visible field.
- * @package
- */
-Blockly.FieldTextInput.prototype.getCorrectedSize = function() {
-  // getSize also renders and updates the size if needed.  Rather than duplicate
-  // the logic to figure out whether to rerender, just call getSize.
-  this.getSize();
-  // TODO (#2562): Remove getCorrectedSize.
-  return new Blockly.utils.Size(this.size_.width + Blockly.BlockSvg.SEP_SPACE_X,
-      Blockly.Field.BORDER_RECT_DEFAULT_HEIGHT);
-};
-
 Blockly.Field.register('field_input', Blockly.FieldTextInput);
diff --git a/core/renderers/block_rendering_rewrite/measurables.js b/core/renderers/block_rendering_rewrite/measurables.js
index f2c81fefe..7d59ed047 100644
--- a/core/renderers/block_rendering_rewrite/measurables.js
+++ b/core/renderers/block_rendering_rewrite/measurables.js
@@ -217,7 +217,7 @@ Blockly.blockRendering.Field = function(field, parentInput) {
   this.isEditable = field.isCurrentlyEditable();
   this.type = 'field';
 
-  var size = this.field.getCorrectedSize();
+  var size = this.field.getSize();
   this.height = size.height;
   this.width = size.width;
   this.parentInput = parentInput;

From c6463acc70532bc5b4066e763678b70edc4bbb37 Mon Sep 17 00:00:00 2001
From: Rachel Fenichel <fenichel@google.com>
Date: Tue, 6 Aug 2019 11:03:53 -0700
Subject: [PATCH 2/4] Cleanup and comments

---
 core/field.js          | 17 +++++++++++------
 core/field_colour.js   |  2 +-
 core/field_dropdown.js | 14 ++++++++------
 core/field_image.js    |  7 +++++++
 4 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/core/field.js b/core/field.js
index 946da2d09..3e218c538 100644
--- a/core/field.js
+++ b/core/field.js
@@ -136,9 +136,12 @@ Blockly.Field.BORDER_RECT_DEFAULT_HEIGHT = 16;
  */
 Blockly.Field.TEXT_DEFAULT_HEIGHT = 12.5;
 
-Blockly.Field.WIDTH_PADDING = 10;
-
-Blockly.Field.HEIGHT_PADDING = 10;
+/**
+ * The padding added to the width by the border rect, if it exists.
+ * @type {number}
+ * @package
+ */
+Blockly.Field.X_PADDING = 10;
 
 /**
  * Name of field.  Unique within each block.
@@ -297,11 +300,13 @@ Blockly.Field.prototype.initView = function() {
 Blockly.Field.prototype.createBorderRect_ = function() {
   this.size_.height =
       Math.max(this.size_.height, Blockly.Field.BORDER_RECT_DEFAULT_HEIGHT);
+  this.size_.width =
+      Math.max(this.size_.width, Blockly.Field.X_PADDING);
   this.borderRect_ = Blockly.utils.dom.createSvgElement('rect',
       {
         'rx': 4,
         'ry': 4,
-        'x': -Blockly.Field.WIDTH_PADDING / 2,
+        'x': -Blockly.Field.X_PADDING / 2,
         'y': 0,
         'height': this.size_.height,
         'width': this.size_.width
@@ -318,7 +323,7 @@ Blockly.Field.prototype.createTextElement_ = function() {
   this.textElement_ = Blockly.utils.dom.createSvgElement('text',
       {
         'class': 'blocklyText',
-        // This may just be trying to vertically center the text?
+        // The y position is the baseline of the text.
         'y': Blockly.Field.TEXT_DEFAULT_HEIGHT,
         'x': 0
       }, this.fieldGroup_);
@@ -587,7 +592,7 @@ Blockly.Field.prototype.updateSize_ = function() {
   var textWidth = Blockly.Field.getCachedWidth(this.textElement_);
   var totalWidth = textWidth;
   if (this.borderRect_) {
-    totalWidth += Blockly.Field.WIDTH_PADDING;
+    totalWidth += Blockly.Field.X_PADDING;
     this.borderRect_.setAttribute('width', totalWidth);
   }
   this.size_.width = totalWidth;
diff --git a/core/field_colour.js b/core/field_colour.js
index d2acbf9ca..0dab6ac69 100644
--- a/core/field_colour.js
+++ b/core/field_colour.js
@@ -78,7 +78,7 @@ Blockly.FieldColour.fromJson = function(options) {
  * @private
  * @const
  */
-Blockly.FieldColour.DEFAULT_WIDTH = 16 + Blockly.Field.WIDTH_PADDING;
+Blockly.FieldColour.DEFAULT_WIDTH = 16 + Blockly.Field.X_PADDING;
 
 /**
  * Default height of a colour field.
diff --git a/core/field_dropdown.js b/core/field_dropdown.js
index 88a368c8f..6d1d5ed6f 100644
--- a/core/field_dropdown.js
+++ b/core/field_dropdown.js
@@ -99,9 +99,11 @@ Blockly.FieldDropdown.CHECKMARK_OVERHANG = 25;
 Blockly.FieldDropdown.MAX_MENU_HEIGHT_VH = 0.45;
 
 /**
- * Used to position the imageElement_ correctly.
+ * The y offset from the top of the field to the top of the image, if an image
+ * is selected.
  * @type {number}
  * @const
+ * @private
  */
 Blockly.FieldDropdown.IMAGE_Y_OFFSET = 5;
 
@@ -109,6 +111,7 @@ Blockly.FieldDropdown.IMAGE_Y_OFFSET = 5;
  * The total vertical padding above and below an image.
  * @type {number}
  * @const
+ * @private
  */
 Blockly.FieldDropdown.IMAGE_Y_PADDING = Blockly.FieldDropdown.IMAGE_Y_OFFSET * 2;
 
@@ -160,8 +163,6 @@ Blockly.FieldDropdown.prototype.initView = function() {
   } else {
     this.textElement_.appendChild(this.arrow_);
   }
-
-  this.contentDimensions_ = new Blockly.utils.Size(0, 0);
 };
 
 /**
@@ -493,11 +494,12 @@ Blockly.FieldDropdown.prototype.renderSelectedImage_ = function() {
 
   var arrowWidth = Blockly.Field.getCachedWidth(this.arrow_);
 
-  // Height and width include the border rect.
   var imageHeight = Number(this.imageJson_.height);
   var imageWidth = Number(this.imageJson_.width);
+
+  // Height and width include the border rect.
   this.size_.height = imageHeight + Blockly.FieldDropdown.IMAGE_Y_PADDING;
-  this.size_.width = imageWidth + arrowWidth + Blockly.Field.WIDTH_PADDING;
+  this.size_.width = imageWidth + arrowWidth + Blockly.Field.X_PADDING;
 
   if (this.sourceBlock_.RTL) {
     this.imageElement_.setAttribute('x', arrowWidth);
@@ -519,7 +521,7 @@ Blockly.FieldDropdown.prototype.renderSelectedText_ = function() {
   // Height and width include the border rect.
   this.size_.height = Blockly.Field.BORDER_RECT_DEFAULT_HEIGHT;
   this.size_.width =
-      Blockly.Field.getCachedWidth(this.textElement_) + Blockly.Field.WIDTH_PADDING;
+      Blockly.Field.getCachedWidth(this.textElement_) + Blockly.Field.X_PADDING;
 };
 
 /**
diff --git a/core/field_image.js b/core/field_image.js
index f154e8b36..8608a6b5b 100644
--- a/core/field_image.js
+++ b/core/field_image.js
@@ -65,6 +65,7 @@ Blockly.FieldImage = function(src, width, height,
     throw Error('Height and width values of an image field must be greater' +
       ' than 0.');
   }
+  // Store the image height, since it is different from the field height.
   this.imageHeight_ = imageHeight;
   this.size_ = new Blockly.utils.Size(imageWidth,
       imageHeight + Blockly.FieldImage.Y_PADDING);
@@ -115,6 +116,12 @@ Blockly.FieldImage.prototype.EDITABLE = false;
  */
 Blockly.FieldImage.prototype.isDirty_ = false;
 
+/**
+ * Vertical padding below the image, which is included in the reported height of
+ * the field.
+ * @type {number}
+ * @private
+ */
 Blockly.FieldImage.Y_PADDING = 1;
 
 /**

From ed245a3c9e64cd60a98e4ef523689970b01c7fde Mon Sep 17 00:00:00 2001
From: Rachel Fenichel <fenichel@google.com>
Date: Tue, 6 Aug 2019 11:44:03 -0700
Subject: [PATCH 3/4] Remove dealWithOffsetFields

---
 core/field.js                                 | 11 ++++++++--
 core/field_checkbox.js                        |  2 +-
 core/field_dropdown.js                        | 12 +++++++----
 core/field_image.js                           | 16 +++++++--------
 .../block_render_draw.js                      | 20 -------------------
 5 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/core/field.js b/core/field.js
index 3e218c538..244a3e8d7 100644
--- a/core/field.js
+++ b/core/field.js
@@ -143,6 +143,12 @@ Blockly.Field.TEXT_DEFAULT_HEIGHT = 12.5;
  */
 Blockly.Field.X_PADDING = 10;
 
+/**
+ * The default offset between the left of the text element and the left of the
+ * border rect, if the border rect exists.
+ * @type {[type]}
+ */
+Blockly.Field.DEFAULT_TEXT_OFFSET = Blockly.Field.X_PADDING / 2;
 /**
  * Name of field.  Unique within each block.
  * Static labels are usually unnamed.
@@ -306,7 +312,7 @@ Blockly.Field.prototype.createBorderRect_ = function() {
       {
         'rx': 4,
         'ry': 4,
-        'x': -Blockly.Field.X_PADDING / 2,
+        'x': 0,
         'y': 0,
         'height': this.size_.height,
         'width': this.size_.width
@@ -320,12 +326,13 @@ Blockly.Field.prototype.createBorderRect_ = function() {
  * @protected
  */
 Blockly.Field.prototype.createTextElement_ = function() {
+  var xOffset = this.borderRect_ ? Blockly.Field.DEFAULT_TEXT_OFFSET : 0;
   this.textElement_ = Blockly.utils.dom.createSvgElement('text',
       {
         'class': 'blocklyText',
         // The y position is the baseline of the text.
         'y': Blockly.Field.TEXT_DEFAULT_HEIGHT,
-        'x': 0
+        'x': xOffset
       }, this.fieldGroup_);
   this.textContent_ = document.createTextNode('');
   this.textElement_.appendChild(this.textContent_);
diff --git a/core/field_checkbox.js b/core/field_checkbox.js
index caae6d9eb..f75f0a800 100644
--- a/core/field_checkbox.js
+++ b/core/field_checkbox.js
@@ -84,7 +84,7 @@ Blockly.FieldCheckbox.CHECK_CHAR = '\u2713';
  * @type {number}
  * @const
  */
-Blockly.FieldCheckbox.CHECK_X_OFFSET = -3;
+Blockly.FieldCheckbox.CHECK_X_OFFSET = Blockly.Field.DEFAULT_TEXT_OFFSET - 3;
 
 /**
  * Used to correctly position the check mark.
diff --git a/core/field_dropdown.js b/core/field_dropdown.js
index 6d1d5ed6f..210e424ca 100644
--- a/core/field_dropdown.js
+++ b/core/field_dropdown.js
@@ -502,11 +502,15 @@ Blockly.FieldDropdown.prototype.renderSelectedImage_ = function() {
   this.size_.width = imageWidth + arrowWidth + Blockly.Field.X_PADDING;
 
   if (this.sourceBlock_.RTL) {
-    this.imageElement_.setAttribute('x', arrowWidth);
-    this.textElement_.setAttribute('x', -1);
+    var imageX = Blockly.Field.DEFAULT_TEXT_OFFSET + arrowWidth;
+    var arrowX = Blockly.Field.DEFAULT_TEXT_OFFSET - 1;
+    this.imageElement_.setAttribute('x', imageX);
+    this.textElement_.setAttribute('x', arrowX);
   } else {
+    var arrowX = imageWidth + arrowWidth + Blockly.Field.DEFAULT_TEXT_OFFSET + 1;
     this.textElement_.setAttribute('text-anchor', 'end');
-    this.textElement_.setAttribute('x', imageWidth + arrowWidth + 1);
+    this.textElement_.setAttribute('x', arrowX);
+    this.imageElement_.setAttribute('x', Blockly.Field.DEFAULT_TEXT_OFFSET);
   }
 };
 
@@ -517,7 +521,7 @@ Blockly.FieldDropdown.prototype.renderSelectedImage_ = function() {
 Blockly.FieldDropdown.prototype.renderSelectedText_ = function() {
   this.textContent_.nodeValue = this.getDisplayText_();
   this.textElement_.setAttribute('text-anchor', 'start');
-  this.textElement_.setAttribute('x', 0);
+  this.textElement_.setAttribute('x', Blockly.Field.DEFAULT_TEXT_OFFSET);
   // Height and width include the border rect.
   this.size_.height = Blockly.Field.BORDER_RECT_DEFAULT_HEIGHT;
   this.size_.width =
diff --git a/core/field_image.js b/core/field_image.js
index 8608a6b5b..b0a13ec0c 100644
--- a/core/field_image.js
+++ b/core/field_image.js
@@ -99,6 +99,14 @@ Blockly.FieldImage.fromJson = function(options) {
   return new Blockly.FieldImage(src, width, height, alt, null, flipRtl);
 };
 
+/**
+ * Vertical padding below the image, which is included in the reported height of
+ * the field.
+ * @type {number}
+ * @private
+ */
+Blockly.FieldImage.Y_PADDING = 1;
+
 /**
  * Editable fields usually show some sort of UI indicating they are
  * editable. This field should not.
@@ -116,14 +124,6 @@ Blockly.FieldImage.prototype.EDITABLE = false;
  */
 Blockly.FieldImage.prototype.isDirty_ = false;
 
-/**
- * Vertical padding below the image, which is included in the reported height of
- * the field.
- * @type {number}
- * @private
- */
-Blockly.FieldImage.Y_PADDING = 1;
-
 /**
  * Create the block UI for this image.
  * @package
diff --git a/core/renderers/block_rendering_rewrite/block_render_draw.js b/core/renderers/block_rendering_rewrite/block_render_draw.js
index c9b683632..c4f5b2ec4 100644
--- a/core/renderers/block_rendering_rewrite/block_render_draw.js
+++ b/core/renderers/block_rendering_rewrite/block_render_draw.js
@@ -300,24 +300,6 @@ Blockly.blockRendering.Drawer.prototype.drawInternals_ = function() {
   }
 };
 
-/**
- * Some fields are terrible and render offset from where they claim to be
- * rendered.  This function calculates an x offset for fields that need it.
- * No one is happy about this.
- * @param {!Blockly.Field} field The field to find an offset for.
- * @return {number} How far to offset the field in the x direction.
- * @private
- */
-Blockly.blockRendering.Drawer.prototype.dealWithOffsetFields_ = function(field) {
-  if (field instanceof Blockly.FieldDropdown ||
-      field instanceof Blockly.FieldTextInput ||
-      field instanceof Blockly.FieldColour ||
-      field instanceof Blockly.FieldCheckbox) {
-    return 5;
-  }
-  return 0;
-};
-
 /**
  * Push a field or icon's new position to its SVG root.
  * @param {!Blockly.blockRendering.Icon|!Blockly.blockRendering.Field} fieldInfo
@@ -341,8 +323,6 @@ Blockly.blockRendering.Drawer.prototype.layoutField_ = function(fieldInfo) {
     svgGroup.setAttribute('transform', 'translate(' + xPos + ',' + yPos + ')');
     fieldInfo.icon.computeIconLocation();
   } else {
-    xPos += this.dealWithOffsetFields_(fieldInfo.field);
-
     svgGroup.setAttribute('transform', 'translate(' + xPos + ',' + yPos + ')');
   }
 

From 40ba284c3f94bbc0e8fdb574d9af500c34f75fc5 Mon Sep 17 00:00:00 2001
From: Rachel Fenichel <fenichel@google.com>
Date: Tue, 6 Aug 2019 13:40:06 -0700
Subject: [PATCH 4/4] Change constant

---
 core/field_colour.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/core/field_colour.js b/core/field_colour.js
index 0dab6ac69..2b5c529d2 100644
--- a/core/field_colour.js
+++ b/core/field_colour.js
@@ -78,7 +78,7 @@ Blockly.FieldColour.fromJson = function(options) {
  * @private
  * @const
  */
-Blockly.FieldColour.DEFAULT_WIDTH = 16 + Blockly.Field.X_PADDING;
+Blockly.FieldColour.DEFAULT_WIDTH = 26;
 
 /**
  * Default height of a colour field.