From 62907dc56f154f1513e956a49432ac806034d309 Mon Sep 17 00:00:00 2001 From: Ablay Keldibek Date: Mon, 16 Mar 2015 15:10:23 +0600 Subject: [PATCH 1/4] Slider: Setting values or range slider to max, doesn't lock slider When both values are set to the maximum change them in descending order Fixes #9046 --- tests/visual/slider/range_slider.html | 47 +++++++++++++++++++++++++++ ui/slider.js | 23 ++++++++++--- 2 files changed, 65 insertions(+), 5 deletions(-) create mode 100644 tests/visual/slider/range_slider.html diff --git a/tests/visual/slider/range_slider.html b/tests/visual/slider/range_slider.html new file mode 100644 index 00000000000..2b041f37740 --- /dev/null +++ b/tests/visual/slider/range_slider.html @@ -0,0 +1,47 @@ + + + + + jQuery UI Slider - Range slider + + + + + + + + + +

Range Slider

When set both values of range slider to the maximum, slider should not lock

+
+ + +

+ + + + diff --git a/ui/slider.js b/ui/slider.js index 094f0135d47..e973a9e86f0 100644 --- a/ui/slider.js +++ b/ui/slider.js @@ -410,9 +410,24 @@ return $.widget( "ui.slider", $.ui.mouse, { } }, - _setOption: function( key, value ) { + _changeAllValues: function( valuesLength ) { var i, - valsLength = 0; + rangeValues = [ this.values( 0 ), this.values( 1 ), this._valueMax() ]; + + if ( this.options.range === true && $.unique( rangeValues ).length === 1 ) { + for ( i = valuesLength - 1; i >= 0; i -= 1 ) { + this._change( null, i ); + } + return; + } + + for ( i = 0; i < valuesLength; i += 1 ) { + this._change( null, i ); + } + }, + + _setOption: function( key, value ) { + var valsLength = 0; if ( key === "range" && this.options.range === true ) { if ( value === "min" ) { @@ -453,9 +468,7 @@ return $.widget( "ui.slider", $.ui.mouse, { case "values": this._animateOff = true; this._refreshValue(); - for ( i = 0; i < valsLength; i += 1 ) { - this._change( null, i ); - } + this._changeAllValues( valsLength ); this._animateOff = false; break; case "step": From 5dac65fe149b3ec66b667ac9f177ef15fb0a433c Mon Sep 17 00:00:00 2001 From: Ablay Keldibek Date: Mon, 16 Mar 2015 17:59:08 +0600 Subject: [PATCH 2/4] Slider: Unit test for setting both values of range slider to the max Refs #9046 --- tests/unit/slider/slider_events.js | 14 +++++++++++++- ui/slider.js | 12 ++---------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/tests/unit/slider/slider_events.js b/tests/unit/slider/slider_events.js index d020d8ac616..e6cb77bfbc5 100644 --- a/tests/unit/slider/slider_events.js +++ b/tests/unit/slider/slider_events.js @@ -99,7 +99,7 @@ test( "programmatic event triggers", function() { }); test( "mouse based interaction part two: when handles overlap", function() { - expect( 4 ); + expect( 5 ); var element = $( "#slider1" ) .slider({ @@ -147,6 +147,18 @@ test( "mouse based interaction part two: when handles overlap", function() { }); handles.eq( 0 ).simulate( "drag", { dx: 10 } ); + element = $( "#slider1" ) + .slider({ + range: true, + min: 0, + max: 100, + values: [ 0, 50 ] + }), + handles = element.find( ".ui-slider-handle" ); + + element.slider( "option", { values: [ 100, 100 ] } ); + element.find( ".ui-slider-handle" ).simulate( "drag", { dx: -10 } ); + equal( element.slider( "values" )[ 0 ], 99, "setting values of range slider doesn't lock slider" ); }); test( "event data", function() { diff --git a/ui/slider.js b/ui/slider.js index e973a9e86f0..65b65864d70 100644 --- a/ui/slider.js +++ b/ui/slider.js @@ -411,17 +411,9 @@ return $.widget( "ui.slider", $.ui.mouse, { }, _changeAllValues: function( valuesLength ) { - var i, - rangeValues = [ this.values( 0 ), this.values( 1 ), this._valueMax() ]; + var i; - if ( this.options.range === true && $.unique( rangeValues ).length === 1 ) { - for ( i = valuesLength - 1; i >= 0; i -= 1 ) { - this._change( null, i ); - } - return; - } - - for ( i = 0; i < valuesLength; i += 1 ) { + for ( i = valuesLength - 1; i >= 0; i -= 1 ) { this._change( null, i ); } }, From 00ce97fe5165a48fe36beb1c8e828edd23b06e68 Mon Sep 17 00:00:00 2001 From: Ablay Keldibek Date: Tue, 17 Mar 2015 12:22:05 +0600 Subject: [PATCH 3/4] Slider: Unit test for setting both values of range slider to the min Refs #9046 --- tests/unit/slider/slider_events.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/unit/slider/slider_events.js b/tests/unit/slider/slider_events.js index e6cb77bfbc5..cc5546a9f6f 100644 --- a/tests/unit/slider/slider_events.js +++ b/tests/unit/slider/slider_events.js @@ -99,7 +99,7 @@ test( "programmatic event triggers", function() { }); test( "mouse based interaction part two: when handles overlap", function() { - expect( 5 ); + expect( 6 ); var element = $( "#slider1" ) .slider({ @@ -157,8 +157,12 @@ test( "mouse based interaction part two: when handles overlap", function() { handles = element.find( ".ui-slider-handle" ); element.slider( "option", { values: [ 100, 100 ] } ); - element.find( ".ui-slider-handle" ).simulate( "drag", { dx: -10 } ); - equal( element.slider( "values" )[ 0 ], 99, "setting values of range slider doesn't lock slider" ); + handles.eq( 0 ).simulate( "drag", { dx: -10 } ); + equal( element.slider( "values" )[ 0 ], 99, "setting both values of range slider to the maximum doesn't lock slider" ); + + element.slider( "option", { values: [ 0, 0 ] } ); + handles.eq( 1 ).simulate( "drag", { dx: 10 } ); + equal( element.slider( "values" )[ 1 ], 1, "setting both values of range slider to the minimum doesn't lock slider" ); }); test( "event data", function() { From 1f0353ae72f705908ad84c4d55d17545fbfa2d64 Mon Sep 17 00:00:00 2001 From: Ablay Keldibek Date: Tue, 17 Mar 2015 22:23:59 +0600 Subject: [PATCH 4/4] Slider: Separate method for change all values removed Refs #9046 --- ui/slider.js | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/ui/slider.js b/ui/slider.js index 65b65864d70..6c72055ea39 100644 --- a/ui/slider.js +++ b/ui/slider.js @@ -410,16 +410,9 @@ return $.widget( "ui.slider", $.ui.mouse, { } }, - _changeAllValues: function( valuesLength ) { - var i; - - for ( i = valuesLength - 1; i >= 0; i -= 1 ) { - this._change( null, i ); - } - }, - _setOption: function( key, value ) { - var valsLength = 0; + var i, + valsLength = 0; if ( key === "range" && this.options.range === true ) { if ( value === "min" ) { @@ -460,7 +453,12 @@ return $.widget( "ui.slider", $.ui.mouse, { case "values": this._animateOff = true; this._refreshValue(); - this._changeAllValues( valsLength ); + + // use reverse loop to change values, then last changed handle will be the first handle + // it need when user set both values to max ( with normal loop slider locks ) + for ( i = valsLength - 1; i >= 0; i -= 1 ) { + this._change( null, i ); + } this._animateOff = false; break; case "step":