Slider: Fix to restrict handle values when set programmatically.

Fixes #3762 - Slider: handles not restricted properly when set programmatically

Author: dekajp

tests/unit/slider/slider_methods.js

@@ -100,8 +100,58 @@ test( "value", function() {
 	equal( element.slider( "value" ), 2.4, "value is set to max with 0.01 step" );
 });
 
-//test( "values", function() {
-//	ok(false, "missing test - untested code is broken code." );
-//});
+test( "values", function() {
+	expect( 12 );
+	
+	var element = $( "<div></div>" ).slider({
+		range: true,
+		min: 10,
+		max: 100,
+		step: 1
+	});
+
+	element.slider( "option", "values", [ 20, 90 ] );
+	deepEqual( element.slider( "values" ), [ 20, 90 ], "option values - get value for all handles" );
+
+	element.slider( "option", "values", [ 0, 200 ] );
+	deepEqual( element.slider( "values" ), [ 10, 100 ], "option values - restricted min and max" );
+
+	element.slider( "values", 0, 50 );
+	equal( element.slider( "values", 0 ), 50, "values - set first handle" );
+
+	element.slider( "values", 1, 75 );
+	equal( element.slider( "values", 1 ), 75, "values - set second handle" );
+
+	element.slider( "option", "values", [ 50, 75 ] );
+	element.slider( "values", 0, 5 );
+	equal( element.slider( "values", 0 ), 10, "1st handle restricted properly, against min" );
+
+	element.slider( "values", 1, 5 );
+	equal( element.slider( "values", 0 ), 10, "2nd handle restricted properly, against min" );
+
+	element.slider( "option", "values", [ 50, 75 ] );
+	element.slider( "values", 0, 110 );
+	equal( element.slider( "values", 0 ), 75, "1st handle restricted properly, against max" );
+
+	element.slider( "values", 1, 110 );
+	equal( element.slider( "values", 0 ), 75, "2nd handle restricted properly, against max" );
+
+	element.slider( "option", "values", [ 50, 75 ] );
+	element.slider( "values", 0, 90 );
+	equal( element.slider( "values", 0 ), 75, "1st handle restricted properly, against 2nd handle" );
+
+	element.slider( "option", "values", [ 50, 75 ] );
+	element.slider( "values", 1, 45 );
+	equal( element.slider( "values", 0 ), 50, "2nd handle restricted properly, against 1st handle" );
+	
+	element.slider( "option", "values", [ 50, 75 ] );
+	element.slider( "values", [ 50, 10 ] );
+	deepEqual( element.slider( "values" ), [ 50, 50 ], "handles restricted properly when set programmatically - 2nd Handle resets to 1st" );
+
+	element.slider( "option", "values", [ 50, 75 ] );
+	element.slider( "values", [ 75, 50 ] );
+	deepEqual( element.slider( "values" ), [ 75, 75 ], "handles restricted properly when set programmatically - 2nd Handle resets to 1st" );
+
+});
 
 })( jQuery );

tests/unit/slider/slider_options.js

@@ -198,18 +198,19 @@ test( "step", function() {
 //});
 
 test( "values", function() {
-	expect( 2 );
+	expect( 3 );
 
 	// testing multiple ranges on the same page, the object reference to the values
 	// property is preserved via multiple range elements, so updating options.values
 	// of 1 slider updates options.values of all the others
-	var ranges = $([
-		document.createElement( "div" ),
-		document.createElement( "div" )
-	]).slider({
-		range: true,
-		values: [ 25, 75 ]
-	});
+	var element,
+		ranges = $([
+			document.createElement( "div" ),
+			document.createElement( "div" )
+		]).slider({
+			range: true,
+			values: [ 25, 75 ]
+		});
 
 	notStrictEqual(
 		ranges.eq( 0 ).slider( "instance" ).options.values,
@@ -224,6 +225,15 @@ test( "values", function() {
 		ranges.eq( 1 ).slider( "values", 0 ),
 		"the values for multiple sliders should be different"
 	);
+
+	element = $( "<div></div>" ).slider({
+		range: true,
+		min: 10,
+		max: 100,
+		step: 1,
+		values: [ 15, 1 ]
+	});
+	deepEqual( element.slider( "values" ), [ 15, 15 ], "handles restricted properly when set programmatically - 2nd handle reset" );
 });
 
 test( "range", function() {

ui/slider.js

@@ -157,6 +157,48 @@ return $.widget( "ui.slider", $.ui.mouse, {
 		}
 	},
 
+	_cleanValues: function( index, newValue ) {
+		var values,
+			i = 0 ;
+
+		if ( $.isArray( arguments[ 0 ] ) ) {
+			values = arguments[ 0 ] || this.options.values;
+			if ( values ) {
+				for ( ; i < values.length; i++ ){
+					if ( values[ i ] < this._valueMin() ) {
+						values[ i ] = this._valueMin();
+					}
+
+					if ( values[ i ]  > this._valueMax() ) {
+						values[ i ] = this._valueMax();
+					}
+				}
+				if ( values[ 0 ] > values[ 1 ] ) {
+					values[ 1 ] = values[ 0 ];
+				}
+				return values;
+			}
+		}
+
+		if ( arguments.length > 1 ) {
+			values = this.options.values;
+			if ( index === 0 && newValue < this._valueMin() ){
+				newValue = this._valueMin();
+			}else if ( index === 1 && newValue > this._valueMax() ){
+				newValue = this._valueMax();
+			}
+
+			if ( this.options.range === true && values.length === 2 ) {
+				if ( index === 0 && newValue > values[ index + 1 ] ) {
+					newValue = values[ index + 1 ];
+				}else if ( index === 1 && newValue < values[ index - 1 ] ) {
+					newValue = values[ index - 1 ];
+				}
+			}
+			return newValue;
+		}
+	},
+
 	_setupEvents: function() {
 		this._off( this.handles );
 		this._on( this.handles, this._handleEvents );
@@ -406,6 +448,7 @@ return $.widget( "ui.slider", $.ui.mouse, {
 
 		if ( arguments.length > 1 ) {
 			this.options.values[ index ] = this._trimAlignValue( newValue );
+			this.options.values[ index ] = this._cleanValues( index, newValue );
 			this._refreshValue();
 			this._change( null, index );
 			return;
@@ -414,14 +457,15 @@ return $.widget( "ui.slider", $.ui.mouse, {
 		if ( arguments.length ) {
 			if ( $.isArray( arguments[ 0 ] ) ) {
 				vals = this.options.values;
-				newValues = arguments[ 0 ];
+				newValues = this._cleanValues( arguments[ 0 ] );
 				for ( i = 0; i < vals.length; i += 1 ) {
 					vals[ i ] = this._trimAlignValue( newValues[ i ] );
 					this._change( null, i );
 				}
 				this._refreshValue();
 			} else {
 				if ( this.options.values && this.options.values.length ) {
+					this.options.values = this._cleanValues( this.options.values );
 					return this._values( index );
 				} else {
 					return this.value();