From 26e025da85c5ad59d5c7554102ddba83f9116b87 Mon Sep 17 00:00:00 2001 From: Ablay Keldibek Date: Fri, 20 Mar 2015 12:06:39 +0600 Subject: [PATCH 1/4] Slider: Drag selected handle instead of closest Drag last changed handle if lastChangedValue is set and target is not a handle Fixes #9553 --- tests/unit/slider/slider_events.js | 51 +++++++++++++++++--------- ui/slider.js | 58 +++++++++++++++++++----------- 2 files changed, 71 insertions(+), 38 deletions(-) diff --git a/tests/unit/slider/slider_events.js b/tests/unit/slider/slider_events.js index cc5546a9f6f..0e598eb3746 100644 --- a/tests/unit/slider/slider_events.js +++ b/tests/unit/slider/slider_events.js @@ -7,28 +7,45 @@ module( "slider: events" ); // value (even if same as previous value), via mouse(mouseup) or keyboard(keyup) // or value method/option" test( "mouse based interaction", function() { - expect( 4 ); + expect( 5 ); - var element = $( "#slider1" ) + var handlePosition, + element = $( "#slider1" ) + .slider({ + start: function( event ) { + equal( event.originalEvent.type, "mousedown", "start triggered by mousedown" ); + }, + slide: function( event) { + equal( event.originalEvent.type, "mousemove", "slider triggered by mousemove" ); + }, + stop: function( event ) { + equal( event.originalEvent.type, "mouseup", "stop triggered by mouseup" ); + }, + change: function( event ) { + equal( event.originalEvent.type, "mouseup", "change triggered by mouseup" ); + } + }), + handles = element.find( ".ui-slider-handle" ); + + handles.eq( 0 ).simulate( "drag", { dx: 10, dy: 10 } ); + element.slider( "destroy" ); + + element = $( "#slider1" ) .slider({ - start: function( event ) { - equal( event.originalEvent.type, "mousedown", "start triggered by mousedown" ); - }, - slide: function( event) { - equal( event.originalEvent.type, "mousemove", "slider triggered by mousemove" ); - }, - stop: function( event ) { - equal( event.originalEvent.type, "mouseup", "stop triggered by mouseup" ); - }, - change: function( event ) { - equal( event.originalEvent.type, "mouseup", "change triggered by mouseup" ); - } + range: true, + min: 0, + max: 100, + values: [ 20, 80 ] }); + handles = element.find( ".ui-slider-handle" ); + handles.eq( 1 ).simulate( "drag", { dx: 10 } ); + handlePosition = handles.eq( 0 ).offset(); - element.find( ".ui-slider-handle" ).eq( 0 ) - .simulate( "drag", { dx: 10, dy: 10 } ); - + element.simulate( "mousedown", { clientX: handlePosition.left + 110, clientY: handlePosition.top } ); + element.simulate( "mouseup" ); + equal( element.slider( "values" )[ 1 ], 30, "should drag selected handle" ); }); + test( "keyboard based interaction", function() { expect( 3 ); diff --git a/ui/slider.js b/ui/slider.js index 4ca9c60022b..58e6d8c493b 100644 --- a/ui/slider.js +++ b/ui/slider.js @@ -72,6 +72,7 @@ return $.widget( "ui.slider", $.ui.mouse, { this._mouseSliding = false; this._animateOff = true; this._handleIndex = null; + this._lastChangedValue = null; this._detectOrientation(); this._mouseInit(); this._calculateNewMax(); @@ -176,11 +177,30 @@ return $.widget( "ui.slider", $.ui.mouse, { this._mouseDestroy(); }, - _mouseCapture: function( event ) { - var position, normValue, distance, closestHandle, index, allowed, offset, mouseOverHandle, + _closestHandleIndex: function( normValue ) { + var distance, index, that = this, o = this.options; + distance = this._valueMax() - this._valueMin() + 1; + this.handles.each(function( i ) { + var thisDistance = Math.abs( normValue - that.values(i) ); + if ( ( distance > thisDistance ) || + ( distance === thisDistance && + ( i === that._lastChangedValue || that.values(i) === o.min )) ) { + distance = thisDistance; + index = i; + } + }); + + return index; + }, + + _mouseCapture: function( event ) { + var position, normValue, handle, index, allowed, offset, mouseOverHandle, + capturedElement = $( event.target ), + o = this.options; + if ( o.disabled ) { return false; } @@ -193,17 +213,13 @@ return $.widget( "ui.slider", $.ui.mouse, { position = { x: event.pageX, y: event.pageY }; normValue = this._normValueFromMouse( position ); - distance = this._valueMax() - this._valueMin() + 1; - this.handles.each(function( i ) { - var thisDistance = Math.abs( normValue - that.values(i) ); - if (( distance > thisDistance ) || - ( distance === thisDistance && - (i === that._lastChangedValue || that.values(i) === o.min ))) { - distance = thisDistance; - closestHandle = $( this ); - index = i; - } - }); + if ( this._lastChangedValue === null || capturedElement.is( ".ui-slider-handle" ) ) { + index = this._closestHandleIndex( normValue ); + } else { + index = this._lastChangedValue; + } + + handle = this.handles.eq( index ); allowed = this._start( event, index ); if ( allowed === false ) { @@ -213,18 +229,18 @@ return $.widget( "ui.slider", $.ui.mouse, { this._handleIndex = index; - this._addClass( closestHandle, null, "ui-state-active" ); - closestHandle.focus(); + this._addClass( handle, null, "ui-state-active" ); + handle.focus(); - offset = closestHandle.offset(); + offset = handle.offset(); mouseOverHandle = !$( event.target ).parents().addBack().is( ".ui-slider-handle" ); this._clickOffset = mouseOverHandle ? { left: 0, top: 0 } : { - left: event.pageX - offset.left - ( closestHandle.width() / 2 ), + left: event.pageX - offset.left - ( handle.width() / 2 ), top: event.pageY - offset.top - - ( closestHandle.height() / 2 ) - - ( parseInt( closestHandle.css("borderTopWidth"), 10 ) || 0 ) - - ( parseInt( closestHandle.css("borderBottomWidth"), 10 ) || 0) + - ( parseInt( closestHandle.css("marginTop"), 10 ) || 0) + ( handle.height() / 2 ) - + ( parseInt( handle.css("borderTopWidth"), 10 ) || 0 ) - + ( parseInt( handle.css("borderBottomWidth"), 10 ) || 0) + + ( parseInt( handle.css("marginTop"), 10 ) || 0) }; if ( !this.handles.hasClass( "ui-state-hover" ) ) { From 6bb719927cc7d27a04c0c5e045949253b9c8d873 Mon Sep 17 00:00:00 2001 From: Ablay Keldibek Date: Sun, 29 Mar 2015 15:54:19 +0600 Subject: [PATCH 2/4] Slider: Drag selected handle instead of closest Correct test and implementation of `_closestHandleIndex` method Fixes #9553 --- tests/unit/slider/slider_events.js | 12 ++++--- ui/slider.js | 56 ++++++++++++++++++------------ 2 files changed, 41 insertions(+), 27 deletions(-) diff --git a/tests/unit/slider/slider_events.js b/tests/unit/slider/slider_events.js index 0e598eb3746..340c2501866 100644 --- a/tests/unit/slider/slider_events.js +++ b/tests/unit/slider/slider_events.js @@ -38,12 +38,14 @@ test( "mouse based interaction", function() { values: [ 20, 80 ] }); handles = element.find( ".ui-slider-handle" ); - handles.eq( 1 ).simulate( "drag", { dx: 10 } ); - handlePosition = handles.eq( 0 ).offset(); + element.slider( "values", 0, 79 ); + handlePosition = handles.eq( 1 ).offset(); + element.on( "slidestart", function( event, ui ) { + equal( ui.handleIndex, 1, "drag selected instead of closest handle" ); + }); - element.simulate( "mousedown", { clientX: handlePosition.left + 110, clientY: handlePosition.top } ); - element.simulate( "mouseup" ); - equal( element.slider( "values" )[ 1 ], 30, "should drag selected handle" ); + handles.eq( 1 ).simulate( "mousedown", { clientX: handlePosition.left } ); + handles.eq( 1 ).simulate( "mouseup" ); }); test( "keyboard based interaction", function() { diff --git a/ui/slider.js b/ui/slider.js index 58e6d8c493b..cf49a4458cb 100644 --- a/ui/slider.js +++ b/ui/slider.js @@ -72,7 +72,7 @@ return $.widget( "ui.slider", $.ui.mouse, { this._mouseSliding = false; this._animateOff = true; this._handleIndex = null; - this._lastChangedValue = null; + this._lastChangedIndex = null; this._detectOrientation(); this._mouseInit(); this._calculateNewMax(); @@ -177,28 +177,45 @@ return $.widget( "ui.slider", $.ui.mouse, { this._mouseDestroy(); }, - _closestHandleIndex: function( normValue ) { - var distance, index, - that = this, - o = this.options; + _closestHandleIndex: function( value, activeHandleIndex ) { + var that = this, + index = activeHandleIndex, + breakLoop = false, + activeSet = activeHandleIndex !== -1, + max = this._valueMax(), min = this._valueMin(), + distance = activeSet ? value - this.values( activeHandleIndex ) : max - min + 1; - distance = this._valueMax() - this._valueMin() + 1; - this.handles.each(function( i ) { - var thisDistance = Math.abs( normValue - that.values(i) ); - if ( ( distance > thisDistance ) || - ( distance === thisDistance && - ( i === that._lastChangedValue || that.values(i) === o.min )) ) { - distance = thisDistance; - index = i; + this.handles.each( function( i ) { + var handleDistance = Math.abs( value - that.values( i ) ); + if ( breakLoop || handleDistance > distance || !that._canBeDrag( i ) ) { + return; } + index = i; + distance = handleDistance; + breakLoop = i === that._lastChangedIndex ; }); return index; }, + _canBeDrag: function( index ) { + var values, + val = this.values( index ), + prev = this.values( index - 1 ), + next = this.values( index + 1 ); + + prev = isNaN( prev ) ? this._valueMin() : prev; + next = isNaN( next ) ? this._valueMax() : next; + values = [ prev, val, next ]; + + return $.grep( values, function( v, k ) { + return $.inArray( v, values ) === k; + }).length > 1; + }, + _mouseCapture: function( event ) { var position, normValue, handle, index, allowed, offset, mouseOverHandle, - capturedElement = $( event.target ), + capturedElementIndex = this.handles.index( $( event.target ) ), o = this.options; if ( o.disabled ) { @@ -210,15 +227,10 @@ return $.widget( "ui.slider", $.ui.mouse, { height: this.element.outerHeight() }; this.elementOffset = this.element.offset(); - position = { x: event.pageX, y: event.pageY }; normValue = this._normValueFromMouse( position ); - if ( this._lastChangedValue === null || capturedElement.is( ".ui-slider-handle" ) ) { - index = this._closestHandleIndex( normValue ); - } else { - index = this._lastChangedValue; - } + index = this._closestHandleIndex( normValue, capturedElementIndex ); handle = this.handles.eq( index ); allowed = this._start( event, index ); @@ -377,7 +389,7 @@ return $.widget( "ui.slider", $.ui.mouse, { _change: function( event, index ) { if ( !this._keySliding && !this._mouseSliding ) { //store the last changed value index for reference when handles overlap - this._lastChangedValue = index; + this._lastChangedIndex = index; this._trigger( "change", event, this._uiHash( index ) ); } }, @@ -471,7 +483,7 @@ return $.widget( "ui.slider", $.ui.mouse, { this._refreshValue(); // Start from the last handle to prevent unreachable handles (#9046) - for ( i = valsLength - 1; i >= 0; i-- ) { + for ( i = 0; i < valsLength; i++ ) { this._change( null, i ); } this._animateOff = false; From 9ed3a1f613eec992205cac5902622b1b301337a4 Mon Sep 17 00:00:00 2001 From: Ablay Keldibek Date: Sun, 29 Mar 2015 17:28:37 +0600 Subject: [PATCH 3/4] Slider: Drag selected handle instead of closest when user clicks on track closest handle changes Refs #9553 --- ui/slider.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/slider.js b/ui/slider.js index cf49a4458cb..fea7e2d7f00 100644 --- a/ui/slider.js +++ b/ui/slider.js @@ -187,12 +187,12 @@ return $.widget( "ui.slider", $.ui.mouse, { this.handles.each( function( i ) { var handleDistance = Math.abs( value - that.values( i ) ); - if ( breakLoop || handleDistance > distance || !that._canBeDrag( i ) ) { + if ( ( breakLoop && handleDistance >= distance ) || handleDistance > distance || !that._canBeDrag( i ) ) { return; } index = i; distance = handleDistance; - breakLoop = i === that._lastChangedIndex ; + breakLoop = breakLoop ? breakLoop : i === that._lastChangedIndex; }); return index; From 006e9a6916946677704618e375188fc34f9d1dec Mon Sep 17 00:00:00 2001 From: Ablay Keldibek Date: Tue, 7 Apr 2015 21:15:35 +0600 Subject: [PATCH 4/4] Slider: Drag selected handle instead of closest change implementation of `_canBeDrag` method Fixes #9553 --- ui/slider.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/ui/slider.js b/ui/slider.js index fea7e2d7f00..8bf315084e2 100644 --- a/ui/slider.js +++ b/ui/slider.js @@ -199,18 +199,14 @@ return $.widget( "ui.slider", $.ui.mouse, { }, _canBeDrag: function( index ) { - var values, - val = this.values( index ), + var val = this.values( index ), prev = this.values( index - 1 ), next = this.values( index + 1 ); prev = isNaN( prev ) ? this._valueMin() : prev; next = isNaN( next ) ? this._valueMax() : next; - values = [ prev, val, next ]; - return $.grep( values, function( v, k ) { - return $.inArray( v, values ) === k; - }).length > 1; + return !( prev === val && val === next ); }, _mouseCapture: function( event ) {