Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Slider: when handles overlap, clicking and dragging will now pick the last... #820

Closed
wants to merge 1 commit into from

3 participants

Juan Pablo Kaniefsky Mike Sherov Scott González
Juan Pablo Kaniefsky

...one that was moved. Fixed #3467 - Sliders Handles can overlap, only small sliver of slider is selectable

Turns out the z-index didn't have anything to do with how handles are picked upon mouse click, so I just stored the last one moved, and on _mouseCapture, if they overlap, the last one moved wins. It's a minimal change and it works. I also removed the #3736 fix because it becomes irrelevant with this change.

Mike Sherov
Collaborator

@jpka, Thanks again for following up here! Can you please add unit tests for this showing it fixes 3467, and that it doesn't regress for 3736. Also, if you haven't already, please sign the CLA so we can accept your patch! http://jquery.github.com/cla.html

Juan Pablo Kaniefsky

@mikesherov: Where I should include those tests? slider_core.js ? the other test files seem to be pretty specific in their scope.

Mike Sherov
Collaborator

I would imagine it goes in slider_events.js, as its related to behavior changes on a specific event, but core would be acceptable if you feel otherwise :)

Scott González

It could go in either file. The events file is about the events being fired by the slider and the core file is about general functionality. Since we're testing which handle is being activated, it could really go in either place, since the activation results in an event and the event will tell us which handle was activated.

Juan Pablo Kaniefsky jpka Slider: when handles overlap, clicking and dragging will now pick the…
… last one that was moved. Fixed #3467 - Sliders Handles can overlap, only small sliver of slider is selectable
eed8062
Juan Pablo Kaniefsky

Alright, I signed the CLA and amended the commit with tests. I hope you'll find them to your liking :).

Mike Sherov
Collaborator

Thanks! Landed in a188632

Mike Sherov mikesherov closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 15, 2012
  1. Juan Pablo Kaniefsky

    Slider: when handles overlap, clicking and dragging will now pick the…

    jpka authored
    … last one that was moved. Fixed #3467 - Sliders Handles can overlap, only small sliver of slider is selectable
This page is out of date. Refresh to see the latest.
Showing with 57 additions and 9 deletions.
  1. +51 −0 tests/unit/slider/slider_events.js
  2. +6 −9 ui/jquery.ui.slider.js
51 tests/unit/slider/slider_events.js
View
@@ -104,4 +104,55 @@ test( "programmatic event triggers", function() {
});
+test( "mouse based interaction part two: when handles overlap", function() {
+ expect(4);
+
+ var el = $( "#slider1" )
+ .slider({
+ values: [ 0, 0, 0 ],
+ start: function( event, ui ) {
+ equal(handles.index(ui.handle), 2, "rightmost handle activated when overlapping at minimum (#3736)");
+ }
+ }),
+ handles = el.find( ".ui-slider-handle" );
+ handles.eq(0).simulate( "drag", { dx: 10 } );
+
+ QUnit.reset();
+ el = $( "#slider1" )
+ .slider({
+ values: [ 10, 10, 10 ],
+ max: 10,
+ start: function( event, ui ) {
+ equal(handles.index(ui.handle), 0, "leftmost handle activated when overlapping at maximum");
+ }
+ }),
+ handles = el.find( ".ui-slider-handle" );
+ handles.eq(0).simulate( "drag", { dx: -10 } );
+
+ QUnit.reset();
+ el = $( "#slider1" )
+ .slider({
+ values: [ 19, 20 ]
+ }),
+ handles = el.find( ".ui-slider-handle" );
+ handles.eq(0).simulate( "drag", { dx: 10 } );
+ el.on("slidestart", function(event, ui) {
+ equal(handles.index(ui.handle), 0, "left handle activated if left was moved last");
+ });
+ handles.eq(0).simulate( "drag", { dx: 10 } );
+
+ QUnit.reset();
+ el = $( "#slider1" )
+ .slider({
+ values: [ 19, 20 ]
+ }),
+ handles = el.find( ".ui-slider-handle" );
+ handles.eq(1).simulate( "drag", { dx: -10 } );
+ el.on("slidestart", function(event, ui) {
+ equal(handles.index(ui.handle), 1, "right handle activated if right was moved last (#3467)");
+ });
+ handles.eq(0).simulate( "drag", { dx: 10 } );
+
+});
+
}( jQuery ) );
15 ui/jquery.ui.slider.js
View
@@ -233,21 +233,15 @@ $.widget( "ui.slider", $.ui.mouse, {
distance = this._valueMax() - this._valueMin() + 1;
this.handles.each(function( i ) {
var thisDistance = Math.abs( normValue - that.values(i) );
- if ( distance > thisDistance ) {
+ if (( distance > thisDistance ) ||
+ ( distance === thisDistance &&
+ (i === that._lastChangedValue || that.values(i) === o.min ))) {
distance = thisDistance;
closestHandle = $( this );
index = i;
}
});
- // workaround for bug #3736 (if both handles of a range are at 0,
- // the first is always used as the one with least distance,
- // and moving it is obviously prevented by preventing negative ranges)
- if( o.range === true && this.values(1) === o.min ) {
- index += 1;
- closestHandle = $( this.handles[index] );
- }
-
allowed = this._start( event, index );
if ( allowed === false ) {
return false;
@@ -419,6 +413,9 @@ $.widget( "ui.slider", $.ui.mouse, {
uiHash.values = this.values();
}
+ //store the last changed value index for reference when handles overlap
+ this._lastChangedValue = index;
+
this._trigger( "change", event, uiHash );
}
},
Something went wrong with that request. Please try again.