MD Settings: cr-slider: Make display consistent and clean up.

ui/webui/resources/cr_elements/cr_slider/cr_slider.js

Index: ui/webui/resources/cr_elements/cr_slider/cr_slider.js
diff --git a/ui/webui/resources/cr_elements/cr_slider/cr_slider.js b/ui/webui/resources/cr_elements/cr_slider/cr_slider.js
index 32e5c5ff57b9eccd8104e470fce160c44986adb1..6f07059afe46aa056ca8f2d676736b658bcd2676 100644
--- a/ui/webui/resources/cr_elements/cr_slider/cr_slider.js
+++ b/ui/webui/resources/cr_elements/cr_slider/cr_slider.js
@@ -30,12 +30,9 @@ Polymer({
       reflectToAttribute: true,
     },
 
-    snaps: {
-      type: Boolean,
-      value: false,
-    },
+    labelMin: String,
 
-    maxMarkers: Number,
+    labelMax: String,
   },
 
   observers: [
@@ -47,8 +44,9 @@ Polymer({
    * after a user action.
    * @private
    */
-  onSliderChange_: function() {
-    this.value = this.tickValues[this.$.slider.immediateValue];
+  onSliderChanged_: function() {
+    if (this.tickValues && this.tickValues.length > 0)
+      this.value = this.tickValues[this.$.slider.immediateValue];
   },
 
   /**
@@ -58,9 +56,12 @@ Polymer({
    */
   valueChanged_: function() {
     // First update the slider settings if |tickValues| was set.
-    this.$.slider.max = this.tickValues.length - 1;
+    let numTicks = Math.max(1, this.tickValues.length);
+    this.$.slider.max = numTicks - 1;
+    this.$.slider.snaps = numTicks < 10;
+    this.$.slider.maxMarkers = numTicks < 10 ? numTicks : 0;
 
-    if (this.$.slider.dragging &&
+    if (this.$.slider.dragging && this.tickValues.length > 0 &&

[dschuyler, 2016/12/22 02:36:35]

I'm not seeing why the this.tickValues.length > 0 are being added here and
elsewhere. Would having a length <= 0 just be a developer error, or could this
happen normally at run-time?

[stevenjb, 2016/12/22 02:55:25]

It can happen briefly in display.js:157 when the list of modes has not
initialized. It seemed better to handle it here then requiring some slight of
hand in any client that has a similar problem (even though that is likely).

         this.value != this.tickValues[this.$.slider.immediateValue]) {
       // The value changed outside cr-slider but we're still holding the knob,
       // so set the value back to where the knob was.
@@ -71,9 +72,15 @@ Polymer({
       return;
     }
 
+    // If |tickValues| is set explicitly before |value| is set, this might
+    // get called with an undefined value.

[dschuyler, 2016/12/22 02:36:35]

Can the observer on line 39 be altered to account for this (so that this check
becomes unnecessary)?

[stevenjb, 2016/12/22 02:55:25]

I'm not entirely sure why this gets called with value == undefined to be honest.
I didn't think that was possible with the way the observer is defined, but it
happened which is why I added this. Sometimes Polymer baffles me.

[michaelpg, 2016/12/22 07:31:32]

i thought multi-property observers are called when a dependent parameter changes
from not-undefined to undefined. Just not when one property is undefined, and
another property changes.

Line 49 could be setting this.value to |undefined| if immediateValue can be
invalid (maybe paper-sider sets it to -1 in some cases?)

If that's what's happening then the comment is wrong. If you logged/stepped thru
and found that this really is getting called before |value| is *ever* set, then
damn.

[stevenjb, 2016/12/22 20:42:18]

Yeah, you're right, this really shouldn't be happening, and it doesn't appear to
be happening any more. Maybe I was confused when it did, or seemed to. Removed.

+    if (this.value === undefined)
+      return;
+
     // Convert from the public |value| to the slider index (where the knob
     // should be positioned on the slider).
-    var sliderIndex = this.tickValues.indexOf(this.value);
+    var sliderIndex =
+        this.tickValues.length > 0 ? this.tickValues.indexOf(this.value) : 0;
     if (sliderIndex == -1) {
       // No exact match.
       sliderIndex = this.findNearestIndex_(this.tickValues, this.value);