Search and Top Navigation
#3762 open bug ()
Opened January 05, 2009 01:09AM UTC
Last modified February 23, 2015 10:38PM UTC
Slider: handles not restricted properly when set programmatically
| Reported by: | scottjehl | Owned by: | |
|---|---|---|---|
| Priority: | major | Milestone: | none |
| Component: | ui.slider | Version: | 1.6rc4 |
| Keywords: | Cc: | ||
| Blocked by: | Blocking: |
Description
When setting handle values via the 'values' option or method, the handles are not restricted from overlapping and passing one another.
For example:
$(...).slider({values: [5,0], range: true});
This will result in a range that's not actually valid in using the slider, since the range will be negative. The second handle left of the first. The handle placement and getter value of the slider should be validated to [5,5] in this case. The option of course can be left as is.
The same goes for the values method. Both need to be restricted by the handle values.
Attachments (1)
Change History (31)
Changed March 21, 2009 06:00PM UTC by comment:1
| milestone: | → 1.8 |
|---|
Changed April 17, 2009 01:59PM UTC by comment:2
Changed April 17, 2009 05:33PM UTC by comment:3
| owner: | → andrew_ |
|---|---|
| status: | new → assigned |
Changed April 17, 2009 07:52PM UTC by comment:4
Proposed fix would be to enforce that values following one another in linear order are greater to or equal to a predecessor. ie. index:0 >= index:1 >= index:n.
Changed April 17, 2009 07:57PM UTC by comment:5
Replying to [comment:4 andrew_]:
Proposed fix would be to enforce that values following one another in linear order are greater to or equal to a predecessor. ie. index:0 >= index:1 >= index:n.
To specify, values would either;
A. be adjusted accordingly
- Invalid values would be replaced by the closest preceeding valid value
B. the plugin would not generate a ui, awaiting valid values.
Changed April 17, 2009 10:05PM UTC by comment:6
andrew_, go ahead with the proposed solution.
Each individual value should be adjusted according to the following rules:
- if less than min, set to min
- if less than previous handle value, set to previous handle value
- if greater than max, set to max
- if greater than next handle value, set to next handle value
Changed April 19, 2009 11:44PM UTC by comment:7
Add patch. Updated the tests here; http://jquery-ui.googlecode.com/svn/trunk/tests/visual/slider/slider_ticket_3762.html
Changed February 24, 2010 05:25PM UTC by comment:8
| owner: | andrew_ → rdworth |
|---|
Changed October 19, 2010 03:47PM UTC by comment:9
| priority: | critical → major |
|---|
Changed October 11, 2012 02:53PM UTC by comment:10
| milestone: | 1.9.0 → 1.11.0 |
|---|
Changed October 19, 2012 05:52PM UTC by comment:11
| keywords: | → haspatch |
|---|---|
| owner: | rdworth |
| status: | assigned → open |
Changed January 04, 2013 09:27AM UTC by comment:12
I can confirm this is still an issue.
See this example: http://jsfiddle.net/SvwTc/2/
The slider starts out with invalid values and strange things happen when you try to slide the slider.
Then you can use the button to see the values are able to be set back to their invalid state.
Changed April 08, 2013 08:26PM UTC by comment:13
| _comment0: | How can I assign myself as the owner of this ticket? I just fixed this bug. \ \ By now, if range is true, only two handles are supported. So, here goes the solution: \ \ \ {{{ \ _createRange: function() { \ var options = this.options, \ classes = ""; \ \ if ( options.range ) { \ if ( options.range === true ) { \ if ( !options.values ) { \ options.values = [ this._valueMin(), this._valueMin() ]; \ } else if ( options.values.length && options.values.length !== 2 ) { \ options.values = [ options.values[0], options.values[0] ]; \ } else if ( $.isArray( options.values ) ) { \ options.values = options.values.slice(0); \ } \ } \ \ if(options.values[0] < this._valueMin()) { \ options.values[0] = this._valueMin(); \ } \ if(options.values[1] > this._valueMax()) { \ options.values[1] = this._valueMax(); \ } \ if(options.values[0] > options.values[1]) { \ options.values[1] = options.values[0] \ } \ \ ... ... ... \ }}} → 1365458772895644 |
|---|
How can I assign myself as the owner of this ticket? I just fixed this bug.
By now, if range option is set to true, only two handles are supported. So, I created a function _verifyValues() and invoked it inside _createRange() and values( index, newValue ) whenever necessary.
Changed April 08, 2013 08:33PM UTC by comment:14
@lucas.ams Please do not paste blocks of code into tickets. If you'd like to submit a fix, please sign our CLA and submit a pull request. Make sure to include a unit test showing that the patch actually does fix the problem. Thanks.
Changed April 08, 2013 08:39PM UTC by comment:15
Sorry @scott.gonzalez, I'm new here! But how can I assign myself as the owner of this ticket? Can it be only after submit a pull request?
Changed April 08, 2013 08:40PM UTC by comment:16
Tickets don't get assigned to members of the community. Just send the pull request.
Changed April 13, 2013 11:34PM UTC by comment:17
Changed October 01, 2013 12:54AM UTC by comment:18
Replying to [comment:15 lucas.ams]:
Sorry @scott.gonzalez, I'm new here! But how can I assign myself as the owner of this ticket? Can it be only after submit a pull request?
is this still open ?
Changed October 01, 2013 12:55AM UTC by comment:19
Replying to [comment:16 scott.gonzalez]:
Tickets don't get assigned to members of the community. Just send the pull request.
Is it still open ?
Changed October 01, 2013 06:11AM UTC by comment:20
Replying to [comment:19 dekajp]:
Replying to [comment:16 scott.gonzalez]: > Tickets don't get assigned to members of the community. Just send the pull request. Is it still open ?
Changed October 02, 2013 01:30AM UTC by comment:21
Replying to [comment:20 tj.vantoll]:
Replying to [comment:19 dekajp]: > Replying to [comment:16 scott.gonzalez]: > > Tickets don't get assigned to members of the community. Just send the pull request. > > Is it still open ? Yes - https://github.com/jquery/jquery-ui/pull/1016
@TJ pull 1016 is for ticket #9376 , this request is for Range validation . i think it's different.
Changed October 02, 2013 07:34AM UTC by comment:22
| keywords: | haspatch |
|---|---|
| summary: | slider handles not restricted properly when set programmatically → Slider: handles not restricted properly when set programmatically |
@dekajp Ah you're right, thanks. But to answer your original question http://jsfiddle.net/SvwTc/2/ shows that this is still a problem and this ticket is open.
Changed October 03, 2013 03:32AM UTC by comment:23
Replying to [comment:6 scott.gonzalez]:
andrew_, go ahead with the proposed solution. Each individual value should be adjusted according to the following rules: - if less than min, set to min - if less than previous handle value, set to previous handle value - if greater than max, set to max - if greater than next handle value, set to next handle value
@scott @tj
Will this approach work ? ( if array size is more than 2 - slice it first 2 element) Sort the Array . compare the zero index with Min value if not equal to min value , replace it by min value . compare 1st index with Max value if not equal to max value ,replace it with max value.
Changed October 03, 2013 05:26AM UTC by comment:24
| _comment0: | Replying to [comment:23 dekajp]: \ > Replying to [comment:6 scott.gonzalez]: \ > > andrew_, go ahead with the proposed solution. \ > > \ > > Each individual value should be adjusted according to the following rules:[[BR]] \ > > - if less than min, set to min[[BR]] \ > > - if less than previous handle value, set to previous handle value[[BR]] \ > > - if greater than max, set to max[[BR]] \ > > - if greater than next handle value, set to next handle value[[BR]] \ > \ > @scott @tj \ > \ > Will this approach work ? ( if array size is more than 2 - slice it first 2 element) Sort the Array . compare the zero index with Min value if not equal to min value , replace it by min value . compare 1st index with Max value if not equal to max value ,replace it with max value. \ > \ \ @scott \ I think i see your point - you do not want "auto correct" - so that developer can see the problem and fix it , also the slider should behave normally. \ [min=1,max=100][5,2] = [2,2] \ [min=10,max=100][1,15] = [10,15] \ [min=10,max=100][15,1] = [1,10] expected [10,10] [ Fails at no.4 - if greater than next handle value, set to next handle value ] \ [min=1,max=20][5,25] = [5,20] \ [min=1,max=20][25,5] = [5,5] \ [min=10,max=20][25,5] =[5,10] expected [10,10] [ Fails at No. 2 - if less than previous handle value, set to previous handle value ] \ \ i think we have to add this clause - when setting to next or previous handle value if newvalue is less than min set min or if more than max set max. \ \ → 1380778159122480 |
|---|
Replying to [comment:23 dekajp]:
Replying to [comment:6 scott.gonzalez]: > andrew_, go ahead with the proposed solution. > > Each individual value should be adjusted according to the following rules: > - if less than min, set to min > - if less than previous handle value, set to previous handle value > - if greater than max, set to max > - if greater than next handle value, set to next handle value @scott @tj Will this approach work ? ( if array size is more than 2 - slice it first 2 element) Sort the Array . compare the zero index with Min value if not equal to min value , replace it by min value . compare 1st index with Max value if not equal to max value ,replace it with max value.
@scott
I think i see your point - you do not want "auto correct" - so that developer can see the problem and fix it , also the slider should behave normally.
- [min=1,max=100][5,2] = [2,2]
- [min=10,max=100][1,15] = [10,15]
- [min=10,max=100][15,1] = [1,10] expected [10,10] [ Fails at no.4 - if greater than next handle value, set to next handle value ]
- [min=1,max=20][5,25] = [5,20]
- [min=1,max=20][25,5] = [5,5]
- [min=10,max=20][25,5] =[5,10] expected [10,10] [ Fails at No. 2 - if less than previous handle value, set to previous handle value ]
i think we have to add this clause - when setting to next or previous handle value if newvalue is less than min set min or if more than max set max.
Changed October 03, 2013 06:06AM UTC by comment:25
I feel we will be better with sort and then clean the values in one pass. we can avoid all these nested if conditions.
Changed October 03, 2013 11:42AM UTC by comment:26
You can't avoid the logic by sorting, because you can set the value of an individual handle at any time. Surely you can agree that setting a single handle shouldn't modify other handles because of a sort.
Changed October 07, 2013 10:28PM UTC by comment:27
Replying to [comment:26 scott.gonzalez]:
You can't avoid the logic by sorting, because you can set the value of an individual handle at any time. Surely you can agree that setting a single handle shouldn't modify other handles because of a sort.
How about this ? in first pass let's treat individual values any handle value if is out of boundary [min-max] let's figure out the closest boundary value [min or max] and set it .
in second pass then run your suggested 4 checks.
Changed October 08, 2013 01:05PM UTC by comment:28
How about you just implement what I said? Why would we want two loops when one will do?
Changed October 09, 2013 03:14PM UTC by comment:29
Changed June 24, 2014 11:55PM UTC by comment:30
| milestone: | 1.11.0 → none |
|---|
Changed February 23, 2015 10:38PM UTC by comment:31
I think the instructions above are confusing and potentially incorrect. So here's a more detailed set of rules:
Restricting the value of a handle based on other handles only applies with range: true and therefore also only applies when exactly two handles exist. When setting both values at the same time, either via .values( array ), .option( "values", array ), or initialization, the first handle can be any value between min and max and the second handle must be between the first handle and max. When setting a single handle, it is restricted by the min, max, and existing value of the other handle.
All other forms of sliders only restrict handle values based on min and max. This includes standard single handle sliders, min range, max range, and non-range multi-handle sliders.