Code cleanup: widget creation (#1332)

core/contextmenu.js

@@ -88,7 +88,6 @@ Blockly.ContextMenu.populate_ = function(options, rtl) {
      callback: Blockly.MakeItSo}
   */
   var menu = new goog.ui.Menu();
-  menu.setAllowAutoFocus(true);
   menu.setRightToLeft(rtl);
   for (var i = 0, option; option = options[i]; i++) {
     var menuItem = new goog.ui.MenuItem(option.text);
@@ -119,13 +118,9 @@ Blockly.ContextMenu.position_ = function(menu, e, rtl) {
     // Record windowSize and scrollOffset before adding menu.
   var windowSize = goog.dom.getViewportSize();
   var scrollOffset = goog.style.getViewportPageOffset(document);
-  var div = Blockly.WidgetDiv.DIV;
-  menu.render(div);
+
+  Blockly.ContextMenu.createWidget_(menu);
   var menuDom = menu.getElement();
-  Blockly.utils.addClass(menuDom, 'blocklyContextMenu');
-  // Prevent system context menu when right-clicking a Blockly context menu.
-  Blockly.bindEventWithChecks_(menuDom, 'contextmenu', null,
-                               Blockly.utils.noEvent);
   // Record menuSize after adding menu.
   var menuSize = goog.style.getSize(menuDom);
 
@@ -147,6 +142,27 @@ Blockly.ContextMenu.position_ = function(menu, e, rtl) {
     }
   }
   Blockly.WidgetDiv.position(x, y, windowSize, scrollOffset, rtl);
+  // Calling menuDom.focus() has to wait until after the menu has been placed
+  // correctly.  Otherwise it will cause a page scroll to get the misplaced menu
+  // in view.  See issue #1329.
+  menuDom.focus();
+};
+
+/**
+ * Create and render the menu widget inside Blockly's widget div.
+ * @param {!goog.ui.Menu} menu The menu to add to the widget div.
+ * @private
+ */
+Blockly.ContextMenu.createWidget_ = function(menu) {
+  var div = Blockly.WidgetDiv.DIV;
+  menu.render(div);
+  var menuDom = menu.getElement();
+  Blockly.utils.addClass(menuDom, 'blocklyContextMenu');
+  // Prevent system context menu when right-clicking a Blockly context menu.
+  Blockly.bindEventWithChecks_(menuDom, 'contextmenu', null,
+                               Blockly.utils.noEvent);
+  // Enable autofocus after the initial render to avoid issue #1329.
+  menu.setAllowAutoFocus(true);
 };
 
 /**

core/field_colour.js

@@ -166,10 +166,6 @@ Blockly.FieldColour.prototype.setColumns = function(columns) {
 Blockly.FieldColour.prototype.showEditor_ = function() {
   Blockly.WidgetDiv.show(this, this.sourceBlock_.RTL,
       Blockly.FieldColour.widgetDispose_);
-  // Create the palette using Closure.
-  var picker = new goog.ui.ColorPicker();
-  picker.setSize(this.columns_ || Blockly.FieldColour.COLUMNS);
-  picker.setColors(this.colours_ || Blockly.FieldColour.COLOURS);
 
   // Position the palette to line up with the field.
   // Record windowSize and scrollOffset before adding the palette.
@@ -177,10 +173,8 @@ Blockly.FieldColour.prototype.showEditor_ = function() {
   var scrollOffset = goog.style.getViewportPageOffset(document);
   var xy = this.getAbsoluteXY_();
   var borderBBox = this.getScaledBBox_();
-  var div = Blockly.WidgetDiv.DIV;
-  picker.render(div);
-  picker.setSelectedColor(this.getValue());
   // Record paletteSize after adding the palette.
+  var picker = this.createWidget_();
   var paletteSize = goog.style.getSize(picker.getElement());
 
   // Flip the palette vertically if off the bottom.
@@ -223,6 +217,22 @@ Blockly.FieldColour.prototype.showEditor_ = function() {
       });
 };
 
+/**
+ * Create a color picker widget and render it inside the widget div.
+ * @return {!goog.ui.ColorPicker} The newly created color picker.
+ * @private
+ */
+Blockly.FieldColour.prototype.createWidget_ = function() {
+  // Create the palette using Closure.
+  var picker = new goog.ui.ColorPicker();
+  picker.setSize(this.columns_ || Blockly.FieldColour.COLUMNS);
+  picker.setColors(this.colours_ || Blockly.FieldColour.COLOURS);
+  var div = Blockly.WidgetDiv.DIV;
+  picker.render(div);
+  picker.setSelectedColor(this.getValue());
+  return picker;
+};
+
 /**
  * Hide the colour palette.
  * @private

core/field_date.js

@@ -101,11 +101,6 @@ Blockly.FieldDate.prototype.setValue = function(date) {
 Blockly.FieldDate.prototype.showEditor_ = function() {
   Blockly.WidgetDiv.show(this, this.sourceBlock_.RTL,
       Blockly.FieldDate.widgetDispose_);
-  // Create the date picker using Closure.
-  Blockly.FieldDate.loadLanguage_();
-  var picker = new goog.ui.DatePicker();
-  picker.setAllowNone(false);
-  picker.setShowWeekNum(false);
 
   // Position the picker to line up with the field.
   // Record windowSize and scrollOffset before adding the picker.
@@ -113,9 +108,7 @@ Blockly.FieldDate.prototype.showEditor_ = function() {
   var scrollOffset = goog.style.getViewportPageOffset(document);
   var xy = this.getAbsoluteXY_();
   var borderBBox = this.getScaledBBox_();
-  var div = Blockly.WidgetDiv.DIV;
-  picker.render(div);
-  picker.setDate(goog.date.fromIsoString(this.getValue()));
+  var picker = this.createWidget_();
   // Record pickerSize after adding the date picker.
   var pickerSize = goog.style.getSize(picker.getElement());
 
@@ -157,6 +150,23 @@ Blockly.FieldDate.prototype.showEditor_ = function() {
       });
 };
 
+/**
+ * Create a date picker widget and render it inside the widget div.
+ * @return {!goog.ui.DatePicker} The newly created date picker.
+ * @private
+ */
+Blockly.FieldDate.prototype.createWidget_ = function() {
+  // Create the date picker using Closure.
+  Blockly.FieldDate.loadLanguage_();
+  var picker = new goog.ui.DatePicker();
+  picker.setAllowNone(false);
+  picker.setShowWeekNum(false);
+  var div = Blockly.WidgetDiv.DIV;
+  picker.render(div);
+  picker.setDate(goog.date.fromIsoString(this.getValue()));
+  return picker;
+};
+
 /**
  * Hide the date picker.
  * @private

core/field_dropdown.js

@@ -236,10 +236,9 @@ Blockly.FieldDropdown.prototype.positionMenu_ = function(menu) {
   var scrollOffset = goog.style.getViewportPageOffset(document);
   var xy = this.getAbsoluteXY_();
   var borderBBox = this.getScaledBBox_();
-  var div = Blockly.WidgetDiv.DIV;
-  menu.render(div);
+
+  this.createWidget_(menu);
   var menuDom = menu.getElement();
-  Blockly.utils.addClass(menuDom, 'blocklyDropdownMenu');
   // Record menuSize after adding menu.
   var menuSize = goog.style.getSize(menuDom);
   // Recalculate height for the total content, not only box height.
@@ -269,10 +268,22 @@ Blockly.FieldDropdown.prototype.positionMenu_ = function(menu) {
   }
   Blockly.WidgetDiv.position(xy.x, xy.y, windowSize, scrollOffset,
                              this.sourceBlock_.RTL);
-  menu.setAllowAutoFocus(true);
   menuDom.focus();
 };
 
+/**
+ * Create and render the menu widget inside Blockly's widget div.
+ * @param {!goog.ui.Menu} menu The menu to add to the widget div.
+ * @private
+ */
+Blockly.FieldDropdown.prototype.createWidget_ = function(menu) {
+  var div = Blockly.WidgetDiv.DIV;
+  menu.render(div);
+  Blockly.utils.addClass(menu.getElement(), 'blocklyDropdownMenu');
+  // Enable autofocus after the initial render to avoid issue #1329.
+  menu.setAllowAutoFocus(true);
+};
+
 /**
  * Handle the selection of an item in the dropdown menu.
  * @param {!goog.ui.Menu} menu The Menu component clicked.