DOMUI: Implement the new Fonts and Encoding page.

chrome/browser/resources/options/pref_ui.js

-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 cr.define('options', function() {
 
   var Preferences = options.Preferences;
   /////////////////////////////////////////////////////////////////////////////
   // PrefCheckbox class:
   // TODO(jhawkins): Refactor all this copy-pasted code!
 
@@ skipping 226 matching lines @@
           });
     }
   };
 
   /////////////////////////////////////////////////////////////////////////////
   // PrefRange class:
 
   // Define a constructor that uses an input element as its underlying element.
   var PrefRange = cr.ui.define('input');
 
   PrefRange.prototype = {

[csilv, 2011/01/11 21:48:43]

Just curious... I believe PrefRange is used in CrOS for touchpad sensitivity...
any impact on that code?

[James Hawkins, 2011/01/11 23:45:54]

The only thing I removed from this class was updateIndicator(), which wasn't
being used anyway.  The remaining behavior is the same, and any new features are
opt-in.

     // Set up the prototype chain
-    __proto__: PrefNumeric.prototype,
+    __proto__: HTMLInputElement.prototype,
+
+    /**
+     * The map from input range value to the corresponding preference value.
+     * @private
+     */
+    valueMap_: undefined,
+
+    /**
+     * If true, the associated pref will be modified on each onchange event;
+     * otherwise, the pref will only be modified on the onmouseup event after

[arv (Not doing code reviews), 2011/01/11 21:50:35]

Using mouseup seems wrong since it will not work with the keyboard

[James Hawkins, 2011/01/11 23:45:54]

Not wrong, just not completely right ;-)  Added keyboard handling.

+     * the drag.
+     * @private
+     */
+    continuous_: true,
 
     /**
      * Initialization function for the cr.ui framework.
      */
     decorate: function() {
       this.type = 'range';
-      PrefNumeric.prototype.decorate.call(this);
-      var self = this;
 
-      // Additionally change the indicator as well.
-      Preferences.getInstance().addEventListener(this.pref,
-          function(event) {
-            self.updateIndicator();
-          });
+      // Update the UI when the pref changes.
+      Preferences.getInstance().addEventListener(
+          this.pref, this.onPrefChange_.bind(this));
 
       // Listen to user events.
-      this.addEventListener('input',
-          function(e) {
-            this.updateIndicator();
-          });
+      this.onchange = this.onChange_.bind(this);
+      this.onmouseup = this.onMouseUp_.bind(this);
     },
 
-    updateIndicator: function() {
-      if ($(this.id + '-value')) {
-        $(this.id + '-value').textContent = this.value;
-      }
-    }
+    /**
+     * Event listener that updates the UI when the underlying pref changes.
+     * @param {Event} event The event that details the pref change.
+     * @private
+     */
+    onPrefChange_: function(event) {
+      var value = event.value && event.value['value'] != undefined ?

[arv (Not doing code reviews), 2011/01/11 21:50:35]

I see these lines all over. Maybe we could provide another getter on the event
that does this in a single place?

[James Hawkins, 2011/01/11 23:45:54]

I have a plan to refactor this, though it's large enough to be left out of this
CL.

+          event.value['value'] : event.value;
+      if (value != undefined)
+        this.value = this.valueMap ? this.valueMap.indexOf(value) : value;
+    },
+
+    /**
+     * onchange handler that sets the pref when the user changes the value of
+     * the input element.
+     * @private
+     */
+    onChange_: function(event) {
+      if (this.continuous)
+        this.setRangePref_();
+
+      this.notifyChange(this, this.mapValueToRange_(this.value));
+    },
+
+    /**
+     * Sets the integer value of |pref| to the value of this element.
+     * @private
+     */
+    setRangePref_: function() {
+      Preferences.setIntegerPref(
+          this.pref, this.mapValueToRange_(this.value), this.metric);
+    },
+
+    /**
+     * onmouseup handler that modifies the pref if |continuous_| is false.
+     * @private
+     */
+    onMouseUp_: function() {
+      if (!this.continuous)

[arv (Not doing code reviews), 2011/01/11 21:50:35]

You also need to handle keyboard (and mousewheel?)

[James Hawkins, 2011/01/11 23:45:54]

Done.

+        this.setRangePref_();
+    },
+
+    /**
+     * Maps the value of this element into the range provided by the client,
+     * represented by |valueMap|.
+     * @param {Integer} value The value to map.

[arv (Not doing code reviews), 2011/01/11 21:50:35]

{number}

JS has no int type

[James Hawkins, 2011/01/11 23:45:54]

Done.

+     * @private
+     */
+    mapValueToRange_: function(value) {
+      return this.valueMap ? this.valueMap[value] : value;
+    },
+
+    /**
+     * Get and set the value map for the range element.
+     * @type {Array}
+     */
+    get valueMap() {

[arv (Not doing code reviews), 2011/01/11 21:50:35]

Just use a plain old property if there are no side effects.

[James Hawkins, 2011/01/11 23:45:54]

Done.

+      return this.valueMap_;
+    },
+    set valueMap(valueMap) {
+      this.valueMap_ = valueMap;
+    },
+
+    /**
+     * Get and set the continuous_ flag.
+     * @type {Boolean}
+     */
+    get continuous() {
+      return this.continuous_;

[arv (Not doing code reviews), 2011/01/11 21:50:35]

same here

[James Hawkins, 2011/01/11 23:45:54]

Done.

+    },
+    set continuous(continuous) {
+      this.continuous_ = continuous;
+    },
+
+    /**
+     * Called when the client has specified non-continuous mode and the value of
+     * the range control changes.
+     * @param {Element} el This element.
+     * @param {Integer} value The value of this element.
+     */
+    notifyChange: function(el, value) {
+    },
   };
 
+  /**
+   * The preference name.
+   * @type {string}
+   */
+  cr.defineProperty(PrefRange, 'pref', cr.PropertyKind.ATTR);
+
+  /**
+   * The user metric string.
+   * @type {string}
+   */
+  cr.defineProperty(PrefRange, 'metric', cr.PropertyKind.ATTR);
+
   /////////////////////////////////////////////////////////////////////////////
   // PrefSelect class:
 
   // Define a constructor that uses an select element as its underlying element.
   var PrefSelect = cr.ui.define('select');
 
   PrefSelect.prototype = {
     // Set up the prototype chain
     __proto__: HTMLSelectElement.prototype,
 
@@ skipping 134 matching lines @@
     PrefCheckbox: PrefCheckbox,
     PrefNumber: PrefNumber,
     PrefNumeric: PrefNumeric,
     PrefRadio: PrefRadio,
     PrefRange: PrefRange,
     PrefSelect: PrefSelect,
     PrefTextField: PrefTextField
   };
 
 });