From 3c404be94266e877ea5639f40d1d3c6ff47d6d91 Mon Sep 17 00:00:00 2001 From: Sam El-Husseini Date: Tue, 17 Sep 2019 14:25:43 -0700 Subject: [PATCH 1/6] Cache dynamic dropdown options and only re-generate on initialization and render. --- core/field_dropdown.js | 33 ++++++++++++++++++++++++--------- tests/blocks/test_blocks.js | 18 ++++++++++++++++++ tests/playground.html | 1 + 3 files changed, 43 insertions(+), 9 deletions(-) diff --git a/core/field_dropdown.js b/core/field_dropdown.js index 45ce9bf9131..fd5116694e6 100644 --- a/core/field_dropdown.js +++ b/core/field_dropdown.js @@ -68,15 +68,23 @@ Blockly.FieldDropdown = function(menuGenerator, opt_validator) { this.menuGenerator_ = menuGenerator; /** - * The currently selected index. A value of -1 indicates no option - * has been selected. + * A cache of the most recently generated options. + * @type {Array.>} + * @private + */ + this.generatedOptions_ = null; + + /** + * The currently selected index. The field is initialized with the + * first option selected. * @type {number} * @private */ - this.selectedIndex_ = -1; + this.selectedIndex_ = 0; this.trimOptions_(); - var firstTuple = this.getOptions()[0]; + var options = this.getOptions(true); + var firstTuple = options[0]; // Call parent's constructor. Blockly.FieldDropdown.superClass_.constructor.call(this, firstTuple[1], @@ -230,7 +238,8 @@ Blockly.FieldDropdown.prototype.widgetCreate_ = function() { menu.setRightToLeft(this.sourceBlock_.RTL); menu.setRole('listbox'); - var options = this.getOptions(); + var options = this.getOptions(true); + this.selectedMenuItem_ = null; for (var i = 0; i < options.length; i++) { var content = options[i][0]; // Human-readable text or image. var value = options[i][1]; // Language-neutral value. @@ -421,15 +430,18 @@ Blockly.FieldDropdown.prototype.isOptionListDynamic = function() { /** * Return a list of the options for this dropdown. + * @param {boolean=} opt_regenerate Whether or not to re-generate the options. * @return {!Array.} Array of option tuples: * (human-readable text or image, language-neutral name). * @throws If generated options are incorrectly structured. */ -Blockly.FieldDropdown.prototype.getOptions = function() { +Blockly.FieldDropdown.prototype.getOptions = function(opt_regenerate) { if (this.isOptionListDynamic()) { - var generatedOptions = this.menuGenerator_.call(this); - Blockly.FieldDropdown.validateOptions_(generatedOptions); - return generatedOptions; + if (!this.generatedOptions_ || opt_regenerate) { + this.generatedOptions_ = this.menuGenerator_.call(this); + Blockly.FieldDropdown.validateOptions_(this.generatedOptions_); + } + return this.generatedOptions_; } return /** @type {!Array.>} */ (this.menuGenerator_); }; @@ -617,6 +629,9 @@ Blockly.FieldDropdown.validateOptions_ = function(options) { tuple); } } + if (!options.length) { + throw TypeError('FieldDropdown options must not be an empty array.'); + } if (foundError) { throw TypeError('Found invalid FieldDropdown options.'); } diff --git a/tests/blocks/test_blocks.js b/tests/blocks/test_blocks.js index f8aa6c0502a..22a1a32c170 100644 --- a/tests/blocks/test_blocks.js +++ b/tests/blocks/test_blocks.js @@ -1210,3 +1210,21 @@ Blockly.TestBlocks.removeDynamicDropdownOption_ = function() { } }) }; + +Blockly.Blocks['test_dropdowns_dynamic_random'] = { + init: function() { + var dropdown = new Blockly.FieldDropdown(this.dynamicOptions); + this.appendDummyInput() + .appendField('dynamic random') + .appendField(dropdown, 'OPTIONS'); + }, + + dynamicOptions: function() { + var random = Math.floor(Math.random() * 10) + 1; + var options = []; + for (var i = 0; i < random; i++) { + options.push([String(i), String(i)]); + } + return options; + } +}; diff --git a/tests/playground.html b/tests/playground.html index acacd898f54..28a18c58f7c 100644 --- a/tests/playground.html +++ b/tests/playground.html @@ -1468,6 +1468,7 @@

Blockly Playground

+ From 1fa3b06bdd8957e8f7bc120be0d31501504c5025 Mon Sep 17 00:00:00 2001 From: Sam El-Husseini Date: Wed, 18 Sep 2019 11:00:31 -0700 Subject: [PATCH 2/6] PR comments --- core/field_dropdown.js | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/core/field_dropdown.js b/core/field_dropdown.js index fd5116694e6..47b8a8f195a 100644 --- a/core/field_dropdown.js +++ b/core/field_dropdown.js @@ -45,8 +45,8 @@ goog.require('Blockly.utils.userAgent'); /** * Class for an editable dropdown field. - * @param {(!Array.|!Function)} menuGenerator An array of options - * for a dropdown list, or a function which generates these options. + * @param {(!Array.|!Function)} menuGenerator A non-empty array of + * options for a dropdown list, or a function which generates these options. * @param {Function=} opt_validator A function that is called to validate * changes to the field's value. Takes in a language-neutral dropdown * option & returns a validated language-neutral dropdown option, or null to @@ -430,14 +430,15 @@ Blockly.FieldDropdown.prototype.isOptionListDynamic = function() { /** * Return a list of the options for this dropdown. - * @param {boolean=} opt_regenerate Whether or not to re-generate the options. - * @return {!Array.} Array of option tuples: + * @param {boolean=} opt_useCache For dynamic options, whether or not to use the + * cached options or to re-generate them. + * @return {!Array.} A non-empty array of option tuples: * (human-readable text or image, language-neutral name). * @throws If generated options are incorrectly structured. */ -Blockly.FieldDropdown.prototype.getOptions = function(opt_regenerate) { +Blockly.FieldDropdown.prototype.getOptions = function(opt_useCache) { if (this.isOptionListDynamic()) { - if (!this.generatedOptions_ || opt_regenerate) { + if (!opt_useCache || !this.generatedOptions_) { this.generatedOptions_ = this.menuGenerator_.call(this); Blockly.FieldDropdown.validateOptions_(this.generatedOptions_); } @@ -454,7 +455,7 @@ Blockly.FieldDropdown.prototype.getOptions = function(opt_regenerate) { */ Blockly.FieldDropdown.prototype.doClassValidation_ = function(opt_newValue) { var isValueValid = false; - var options = this.getOptions(); + var options = this.getOptions(false); for (var i = 0, option; option = options[i]; i++) { // Options are tuples of human-readable text and language-neutral values. if (option[1] == opt_newValue) { @@ -480,7 +481,7 @@ Blockly.FieldDropdown.prototype.doClassValidation_ = function(opt_newValue) { */ Blockly.FieldDropdown.prototype.doValueUpdate_ = function(newValue) { Blockly.FieldDropdown.superClass_.doValueUpdate_.call(this, newValue); - var options = this.getOptions(); + var options = this.getOptions(false); for (var i = 0, option; option = options[i]; i++) { if (option[1] == this.value_) { this.selectedIndex_ = i; @@ -513,7 +514,7 @@ Blockly.FieldDropdown.prototype.render_ = function() { this.imageElement_.style.display = 'none'; // Show correct element. - var options = this.getOptions(); + var options = this.getOptions(false); var selectedOption = this.selectedIndex_ >= 0 && options[this.selectedIndex_][0]; if (selectedOption && typeof selectedOption == 'object') { @@ -588,7 +589,7 @@ Blockly.FieldDropdown.prototype.getText_ = function() { if (this.selectedIndex_ < 0) { return null; } - var options = this.getOptions(); + var options = this.getOptions(false); var selectedOption = options[this.selectedIndex_][0]; if (typeof selectedOption == 'object') { return selectedOption['alt']; @@ -606,6 +607,9 @@ Blockly.FieldDropdown.validateOptions_ = function(options) { if (!Array.isArray(options)) { throw TypeError('FieldDropdown options must be an array.'); } + if (!options.length) { + throw TypeError('FieldDropdown options must not be an empty array.'); + } var foundError = false; for (var i = 0; i < options.length; ++i) { var tuple = options[i]; @@ -629,9 +633,6 @@ Blockly.FieldDropdown.validateOptions_ = function(options) { tuple); } } - if (!options.length) { - throw TypeError('FieldDropdown options must not be an empty array.'); - } if (foundError) { throw TypeError('Found invalid FieldDropdown options.'); } From 2f75db642be66471d13895bf325980266f0bf9a3 Mon Sep 17 00:00:00 2001 From: Sam El-Husseini Date: Wed, 18 Sep 2019 11:05:54 -0700 Subject: [PATCH 3/6] Other way around --- core/field_dropdown.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/core/field_dropdown.js b/core/field_dropdown.js index 47b8a8f195a..6fc504c8cd6 100644 --- a/core/field_dropdown.js +++ b/core/field_dropdown.js @@ -83,7 +83,7 @@ Blockly.FieldDropdown = function(menuGenerator, opt_validator) { this.selectedIndex_ = 0; this.trimOptions_(); - var options = this.getOptions(true); + var options = this.getOptions(false); var firstTuple = options[0]; // Call parent's constructor. @@ -238,7 +238,7 @@ Blockly.FieldDropdown.prototype.widgetCreate_ = function() { menu.setRightToLeft(this.sourceBlock_.RTL); menu.setRole('listbox'); - var options = this.getOptions(true); + var options = this.getOptions(false); this.selectedMenuItem_ = null; for (var i = 0; i < options.length; i++) { var content = options[i][0]; // Human-readable text or image. @@ -438,7 +438,7 @@ Blockly.FieldDropdown.prototype.isOptionListDynamic = function() { */ Blockly.FieldDropdown.prototype.getOptions = function(opt_useCache) { if (this.isOptionListDynamic()) { - if (!opt_useCache || !this.generatedOptions_) { + if (!this.generatedOptions_ || !opt_useCache) { this.generatedOptions_ = this.menuGenerator_.call(this); Blockly.FieldDropdown.validateOptions_(this.generatedOptions_); } @@ -455,7 +455,7 @@ Blockly.FieldDropdown.prototype.getOptions = function(opt_useCache) { */ Blockly.FieldDropdown.prototype.doClassValidation_ = function(opt_newValue) { var isValueValid = false; - var options = this.getOptions(false); + var options = this.getOptions(true); for (var i = 0, option; option = options[i]; i++) { // Options are tuples of human-readable text and language-neutral values. if (option[1] == opt_newValue) { @@ -481,7 +481,7 @@ Blockly.FieldDropdown.prototype.doClassValidation_ = function(opt_newValue) { */ Blockly.FieldDropdown.prototype.doValueUpdate_ = function(newValue) { Blockly.FieldDropdown.superClass_.doValueUpdate_.call(this, newValue); - var options = this.getOptions(false); + var options = this.getOptions(true); for (var i = 0, option; option = options[i]; i++) { if (option[1] == this.value_) { this.selectedIndex_ = i; @@ -514,7 +514,7 @@ Blockly.FieldDropdown.prototype.render_ = function() { this.imageElement_.style.display = 'none'; // Show correct element. - var options = this.getOptions(false); + var options = this.getOptions(true); var selectedOption = this.selectedIndex_ >= 0 && options[this.selectedIndex_][0]; if (selectedOption && typeof selectedOption == 'object') { @@ -589,7 +589,7 @@ Blockly.FieldDropdown.prototype.getText_ = function() { if (this.selectedIndex_ < 0) { return null; } - var options = this.getOptions(false); + var options = this.getOptions(true); var selectedOption = options[this.selectedIndex_][0]; if (typeof selectedOption == 'object') { return selectedOption['alt']; From 4b89799d16929fa47e95c3eec29ed600d5ff758f Mon Sep 17 00:00:00 2001 From: Sam El-Husseini Date: Wed, 18 Sep 2019 11:59:34 -0700 Subject: [PATCH 4/6] nit --- core/field_dropdown.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/field_dropdown.js b/core/field_dropdown.js index 6fc504c8cd6..78cc6bda6a9 100644 --- a/core/field_dropdown.js +++ b/core/field_dropdown.js @@ -83,8 +83,7 @@ Blockly.FieldDropdown = function(menuGenerator, opt_validator) { this.selectedIndex_ = 0; this.trimOptions_(); - var options = this.getOptions(false); - var firstTuple = options[0]; + var firstTuple = this.getOptions(false)[0]; // Call parent's constructor. Blockly.FieldDropdown.superClass_.constructor.call(this, firstTuple[1], From 6da2d4fed8a37033a247b9e650609324cf74ed1e Mon Sep 17 00:00:00 2001 From: Sam El-Husseini Date: Wed, 18 Sep 2019 12:45:05 -0700 Subject: [PATCH 5/6] PR comments. --- core/field_dropdown.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/field_dropdown.js b/core/field_dropdown.js index 78cc6bda6a9..e3f7497bbf9 100644 --- a/core/field_dropdown.js +++ b/core/field_dropdown.js @@ -433,7 +433,6 @@ Blockly.FieldDropdown.prototype.isOptionListDynamic = function() { * cached options or to re-generate them. * @return {!Array.} A non-empty array of option tuples: * (human-readable text or image, language-neutral name). - * @throws If generated options are incorrectly structured. */ Blockly.FieldDropdown.prototype.getOptions = function(opt_useCache) { if (this.isOptionListDynamic()) { @@ -599,7 +598,7 @@ Blockly.FieldDropdown.prototype.getText_ = function() { /** * Validates the data structure to be processed as an options list. * @param {?} options The proposed dropdown options. - * @throws If proposed options are incorrectly structured. + * @throws {TypeError} If proposed options are incorrectly structured. * @private */ Blockly.FieldDropdown.validateOptions_ = function(options) { From d749a603189da3090ee834e86f46865f3cea632e Mon Sep 17 00:00:00 2001 From: Sam El-Husseini Date: Wed, 18 Sep 2019 12:46:49 -0700 Subject: [PATCH 6/6] Throws in constructor --- core/field_dropdown.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/field_dropdown.js b/core/field_dropdown.js index e3f7497bbf9..5e5f48e10fc 100644 --- a/core/field_dropdown.js +++ b/core/field_dropdown.js @@ -53,6 +53,7 @@ goog.require('Blockly.utils.userAgent'); * abort the change. * @extends {Blockly.Field} * @constructor + * @throws {TypeError} If `menuGenerator` options are incorrectly structured. */ Blockly.FieldDropdown = function(menuGenerator, opt_validator) { if (typeof menuGenerator != 'function') { @@ -433,6 +434,7 @@ Blockly.FieldDropdown.prototype.isOptionListDynamic = function() { * cached options or to re-generate them. * @return {!Array.} A non-empty array of option tuples: * (human-readable text or image, language-neutral name). + * @throws {TypeError} If generated options are incorrectly structured. */ Blockly.FieldDropdown.prototype.getOptions = function(opt_useCache) { if (this.isOptionListDynamic()) {