MD Settings: Simplify settings-dropdown-menu

chrome/browser/resources/settings/controls/settings_dropdown_menu.js

Index: chrome/browser/resources/settings/controls/settings_dropdown_menu.js
diff --git a/chrome/browser/resources/settings/controls/settings_dropdown_menu.js b/chrome/browser/resources/settings/controls/settings_dropdown_menu.js
index 28bddf25f5cd52809968372660c5846e359f8733..ecc4202778cc3b26f8e18fa05aca75ca833ceae6 100644
--- a/chrome/browser/resources/settings/controls/settings_dropdown_menu.js
+++ b/chrome/browser/resources/settings/controls/settings_dropdown_menu.js
@@ -2,6 +2,23 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+ /**
+ * This tuple is made up of a (value, name, attribute). The value and name are
+ * used by the dropdown menu. The attribute is optional 'user data' that is
+ * ignored by the dropdown menu.
+ * @typedef {{
+ *   0: (number|string),
+ *   1: string,
+ *   2: (string|undefined)
+ * }}
+ */
+var DropdownMenuOption;
+
+/**
+ * @typedef {!Array<!DropdownMenuOption>}
+ */
+var DropdownMenuOptionList;
+
 /**
  * 'settings-dropdown-menu' is a control for displaying options
  * in the settings.
@@ -18,33 +35,34 @@ Polymer({
   is: 'settings-dropdown-menu',
 
   properties: {
-    /**
-     * A text label for the drop-down menu.
-     */
-    label: {
-      type: String,
-    },
+    /** A text label for the drop-down menu. */
+    label: String,
 
     /**
      * List of options for the drop-down menu.
-     * @type {!Array<{0: (Object|number|string), 1: string,
-     *   2: (string|undefined)}>}
+     * TODO(michaelpg): use named properties instead of indices.
+     * @type {DropdownMenuOptionList}
      */
     menuOptions: {
-      notify: true,
       type: Array,
       value: function() { return []; },
     },
 
     /**
      * A single Preference object being tracked.
-     * @type {?PrefObject}
+     * @type {!chrome.settingsPrivate.PrefObject|undefined}
      */
     pref: {
       type: Object,
       notify: true,
     },
 
+    /** Whether the dropdown menu should be disabled. */
+    disabled: {
+      type: Boolean,
+      reflectToAttribute: true,
+    },
+
     /**
      * Either loading text or the label for the drop-down menu.
      * @private
@@ -54,124 +72,95 @@ Polymer({
       value: function() { return loadTimeData.getString('loading'); },
     },
 
-     /**
-     * A reverse lookup from the menu value back to the index in the
-     * menuOptions array.
-     * @private {!Object<string, string>}
-     */
-    menuMap_: {
-      type: Object,
-      value: function() { return {}; },
-    },
-
     /**
-     * The current selected item (an index number as a string).
+     * The current selected value, as a string.
      * @private
      */
-    selected_: {
-      notify: true,
-      observer: 'onSelectedChanged_',
-      type: String,
-    },
+    selected_: String,
 
     /**
-     * The current selected pref value.
+     * Whether to show the 'custom' item.
      * @private
      */
-    selectedValue_: {
-      type: String,
+    showNotFoundItem_: {
+      type: Boolean,
+      computed: 'isSelectedNotFound_(selected_)',

[stevenjb, 2015/11/17 18:19:23]

Question: Is this more efficient than just embedding
isSelectedNotFound_(selected_) in the HTML? (i.e. should we prefer one over the
other?)

[michaelpg, 2015/11/18 01:08:39]

Done: Not having data, it's simpler to embed it since we don't use this computed
property for anything else. (If anything, my guess is that embedding it might be
more efficient than what I had.)

     },
 
     /**
-     * Whether to show the 'custom' item.
+     * The value of the 'custom' item.
      * @private
      */
-    showNotFoundValue_: {
-      type: Boolean,
+    notFoundValue_: {
+      type: String,
+      value: 'SETTINGS_DROPDOWN_NOT_FOUND_ITEM',
     },
   },
 
   behaviors: [
-    I18nBehavior
+    I18nBehavior,
   ],
 
   observers: [
-    'checkSetup_(menuOptions, selectedValue_)',
-    'prefChanged_(pref.value)',
+    'checkSetup_(menuOptions)',
+    'updateSelected_(pref.value)',
   ],
 
+  ready: function() {
+    this.checkSetup_(this.menuOptions);
+  },
+
   /**
    * Check to see if we have all the pieces needed to enable the control.
-   * @param {!Array<{0: (Object|number|string), 1: string,
-   *   2: (string|undefined)}>} menuOptions
-   * @param {string} selectedValue_
+   * @param {DropdownMenuOptionList} menuOptions
    * @private
    */
-  checkSetup_: function(menuOptions, selectedValue_) {
-    if (!this.menuOptions.length) {
+  checkSetup_: function(menuOptions) {
+    if (!this.menuOptions.length)
       return;
-    }
-
-    if (!Object.keys(this.menuMap_).length) {
-      // Create a map from index value [0] back to the index i.
-      var result = {};
-      for (var i = 0; i < this.menuOptions.length; ++i)
-        result[JSON.stringify(this.menuOptions[i][0])] = i.toString();
-      this.menuMap_ = result;
-    }
 
-    // We need the menuOptions and the selectedValue_.  They may arrive
-    // at different times (each is asynchronous).
-    this.selected_ = this.getItemIndex(this.selectedValue_);
     this.menuLabel_ = this.label;
-    this.$.dropdownMenu.disabled = false;
+    this.updateSelected_();
   },
 
   /**
-   * @param {string} item A value from the menuOptions array.
-   * @return {string}
-   * @private
-   */
-  getItemIndex: function(item) {
-    var result = this.menuMap_[item];
-    if (result)
-      return result;
-    this.showNotFoundValue_ = true;
-    // The 'not found' item is added as the last of the options.
-    return (this.menuOptions.length).toString();
-  },
-
-  /**
-   * @param {string} index An index into the menuOptions array.
-   * @return {Object|number|string|undefined}
+   * Pass the selection change to the pref value.
    * @private
    */
-  getItemValue: function(index) {
-    if (this.menuOptions.length) {
-      var result = this.menuOptions[index];
-      if (result)
-        return result[0];
+  onSelect_: function() {
+    if (!this.pref || this.selected_ == undefined ||
+        this.selected_ == this.notFoundValue_) {
+      return;
     }
-    return undefined;
+    var prefValue = Settings.PrefUtil.stringToPrefValue(
+        this.selected_, this.pref);
+    if (prefValue !== undefined)
+      this.set('pref.value', prefValue);
   },
 
   /**
-   * Pass the selection change to the pref value.
+   * Updates the selected item when the pref or menuOptions change.
    * @private
    */
-  onSelectedChanged_: function() {
-    var prefValue = this.getItemValue(this.selected_);
-    if (prefValue !== undefined) {
-      this.selectedValue_ = JSON.stringify(prefValue);
-      this.set('pref.value', prefValue);
-    }
+  updateSelected_: function() {
+    if (!this.pref)
+      return;
+    var prefValue = this.pref.value;
+    var option = this.menuOptions.find(function(menuItem) {
+      return menuItem[0] == prefValue;
+    });
+    if (option == undefined)
+      this.selected_ = this.notFoundValue_;
+    else
+      this.selected_ = Settings.PrefUtil.prefToString(this.pref);
   },
 
   /**
-   * @param {number|string} value A value from the menuOptions array.
+   * @param {string} selected
+   * @return {boolean}
    * @private
    */
-  prefChanged_: function(value) {
-    this.selectedValue_ = JSON.stringify(value);
+  isSelectedNotFound_: function(selected) {
+    return this.menuOptions && selected == this.notFoundValue_;
   },
 });