fix: misc adjustments from PR feedback

google · Oct 21, 2022 · a495529 · a495529
1 parent 5ba9648
commit a495529
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 26 deletions.
diff --git a/core/blockly.ts b/core/blockly.ts
@@ -57,7 +57,7 @@ import {Field} from './field.js';
 import {FieldAngle} from './field_angle.js';
 import {FieldCheckbox} from './field_checkbox.js';
 import {FieldColour} from './field_colour.js';
-import {FieldDropdown} from './field_dropdown.js';
+import {FieldDropdown, MenuGenerator, MenuGeneratorFunction, MenuOption} from './field_dropdown.js';
 import {FieldImage} from './field_image.js';
 import {FieldLabel} from './field_label.js';
 import {FieldLabelSerializable} from './field_label_serializable.js';
@@ -651,7 +651,7 @@ export {Field};
 export {FieldAngle};
 export {FieldCheckbox};
 export {FieldColour};
-export {FieldDropdown};
+export {FieldDropdown, MenuGenerator, MenuGeneratorFunction, MenuOption};
 export {FieldImage};
 export {FieldLabel};
 export {FieldLabelSerializable};

diff --git a/core/field_dropdown.ts b/core/field_dropdown.ts
@@ -44,7 +44,9 @@ export class FieldDropdown extends Field {
    * height.
    */
   static MAX_MENU_HEIGHT_VH = 0.45;
-  static ARROW_CHAR: '▼'|'▾';
+
+  /** Android can't (in 2014) display "▾", so use "▼" instead. */
+  static ARROW_CHAR: string = userAgent.ANDROID ? '▼' : '▾';
 
   /** A reference to the currently selected menu item. */
   private selectedMenuItem_: MenuItem|null = null;
@@ -71,7 +73,7 @@ export class FieldDropdown extends Field {
 
   /** Mouse cursor style when over the hotspot that initiates the editor. */
   override CURSOR = 'default';
-  // TODO(b/109816955): remove '!', see go/strict-prop-init-fix.
+
   protected menuGenerator_?: MenuGenerator;
 
   /** A cache of the most recently generated options. */
@@ -119,19 +121,17 @@ export class FieldDropdown extends Field {
       opt_config?: FieldConfig) {
     super(Field.SKIP_SETUP);
 
-    if (isMenuGenerator(menuGenerator)) {
-      if (Array.isArray(menuGenerator)) {
-        validateOptions(menuGenerator);
-        const trimmed = trimOptions(menuGenerator);
-        this.menuGenerator_ = trimmed.options;
-        this.prefixField = trimmed.prefix || null;
-        this.suffixField = trimmed.suffix || null;
-      } else {
-        this.menuGenerator_ = menuGenerator;
-      }
+    // If we pass SKIP_SETUP, don't do *anything* with the menu generator.
+    if (!isMenuGenerator(menuGenerator)) return;
+
+    if (Array.isArray(menuGenerator)) {
+      validateOptions(menuGenerator);
+      const trimmed = trimOptions(menuGenerator);
+      this.menuGenerator_ = trimmed.options;
+      this.prefixField = trimmed.prefix || null;
+      this.suffixField = trimmed.suffix || null;
     } else {
-      // If we pass SKIP_SETUP, don't do *anything* with the menu generator.
-      return;
+      this.menuGenerator_ = menuGenerator;
     }
 
     /**
@@ -369,13 +369,20 @@ export class FieldDropdown extends Field {
    * @throws {TypeError} If generated options are incorrectly structured.
    */
   getOptions(opt_useCache?: boolean): MenuOption[] {
-    if (!this.menuGenerator_) return [];
-    if (Array.isArray(this.menuGenerator_)) return this.menuGenerator_;
-    if (opt_useCache && this.generatedOptions_) return this.generatedOptions_;
-
-    this.generatedOptions_ = this.menuGenerator_();
-    validateOptions(this.generatedOptions_);
-    return this.generatedOptions_;
+    const options = (() => {
+      if (!this.menuGenerator_) return [];
+      if (Array.isArray(this.menuGenerator_)) return this.menuGenerator_;
+      if (opt_useCache && this.generatedOptions_) return this.generatedOptions_;
+
+      this.generatedOptions_ = this.menuGenerator_();
+      validateOptions(this.generatedOptions_);
+      return this.generatedOptions_;
+    })();
+
+    if (options.length === 0) {
+      throw new Error('A non-empty array of options should be provided.');
+    }
+    return options;
   }
 
   /**
@@ -620,8 +627,16 @@ export interface ImageProperties {
  */
 export type MenuOption = [string | ImageProperties, string];
 
+/**
+ * A function that generates an array of menu options for FieldDropdown
+ * or its descendants.
+ */
 export type MenuGeneratorFunction = (this: FieldDropdown) => MenuOption[];
 
+/**
+ * Either an array of menu options or a function that generates an array of
+ * menu options for FieldDropdown or its descendants.
+ */
 export type MenuGenerator = MenuOption[]|MenuGeneratorFunction;
 
 /**
@@ -640,9 +655,6 @@ const IMAGE_Y_OFFSET = 5;
 /** The total vertical padding above and below an image. */
 const IMAGE_Y_PADDING: number = IMAGE_Y_OFFSET * 2;
 
-/** Android can't (in 2014) display "▾", so use "▼" instead. */
-FieldDropdown.ARROW_CHAR = userAgent.ANDROID ? '▼' : '▾';
-
 /**
  * NOTE: Because Sentinel is an empty class, proving a value is Sentinel does
  * not resolve in TS that it isn't a MenuGenerator.