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

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

 // Copyright 2016 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.
 
 /**
  * @fileoverview
  * cr-slider wraps a paper-slider. It maps the slider's values from a linear UI
  * range to a range of real values.  When |value| does not map exactly to a
  * tick mark, it interpolates to the nearest tick.
  *
@@ skipping 12 matching lines @@
 
     /** @type {!Array<number>} Values corresponding to each tick. */
     tickValues: Array,
 
     disabled: {
       type: Boolean,
       value: false,
       reflectToAttribute: true,
     },
 
-    snaps: {
-      type: Boolean,
-      value: false,
-    },
+    labelMin: String,
 
-    maxMarkers: Number,
+    labelMax: String,
   },
 
   observers: [
     'valueChanged_(value, tickValues.*)',
   ],
 
   /**
    * Sets the |value| property to the value corresponding to the knob position
    * 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];
   },
 
   /**
    * Updates the knob position when |value| changes. If the knob is still being
    * dragged, this instead forces |value| back to the current position.
    * @private
    */
   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.
       // Async so we don't confuse Polymer's data binding.
       this.async(function() {
         this.value = this.tickValues[this.$.slider.immediateValue];
       });
       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);
     }
     this.$.slider.value = sliderIndex;
   },
 
   /**
    * Returns the index of the item in |arr| closest to |value|.
    * @param {!Array<number>} arr
    * @param {number} value
    * @return {number}
    * @private
    */
   findNearestIndex_: function(arr, value) {
     var closestIndex;
     var minDifference = Number.MAX_VALUE;
     for (var i = 0; i < arr.length; i++) {
       var difference = Math.abs(arr[i] - value);
       if (difference < minDifference) {
         closestIndex = i;
         minDifference = difference;
       }
     }
 
     assert(typeof closestIndex != 'undefined');
     return closestIndex;
   },
 });