From 8050ce929c63f8ce093f3f4f34f6c5fa6dd30a76 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Wed, 20 Sep 2017 16:38:13 -0700 Subject: [PATCH] Code cleanup: widget creation (#1332) --- core/contextmenu.js | 30 +++++++++++++++++++++++------- core/field_colour.js | 24 +++++++++++++++++------- core/field_date.js | 26 ++++++++++++++++++-------- core/field_dropdown.js | 19 +++++++++++++++---- 4 files changed, 73 insertions(+), 26 deletions(-) diff --git a/core/contextmenu.js b/core/contextmenu.js index 3f1566310..08dd5de85 100644 --- a/core/contextmenu.js +++ b/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); }; /** diff --git a/core/field_colour.js b/core/field_colour.js index 425e4a35c..1d4766cc1 100644 --- a/core/field_colour.js +++ b/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 diff --git a/core/field_date.js b/core/field_date.js index 9e19c63ac..afa5de221 100644 --- a/core/field_date.js +++ b/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 diff --git a/core/field_dropdown.js b/core/field_dropdown.js index 5ceac3f42..96dcf52cf 100644 --- a/core/field_dropdown.js +++ b/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.