jQRangeSlider icon indicating copy to clipboard operation
jQRangeSlider copied to clipboard

Implement limits

Open nicofrand opened this issue 11 years ago • 14 comments

See https://github.com/ghusse/jQRangeSlider/issues/85

nicofrand avatar Apr 25 '13 13:04 nicofrand

I created a branch named "limits" in order to propose one modification in your code:

https://github.com/ghusse/jQRangeSlider/tree/limits

Take a look at it, and if it seems good to you, merge it in your branch.

I removed the option limits in the bar, removed the code in _mouseDrag, and let handles constrain values. The bar just tries to keep the same distance between both handles.

ghusse avatar Apr 26 '13 12:04 ghusse

Thanks. I merged it with my branch and also updated the other things you reported. The handles are still bouncing in Chrome but besides that, it works juste fine to me.

nicofrand avatar Apr 26 '13 13:04 nicofrand

We need to investigate on this bouncing issue.

We also need to make the build to pass, and to add unit tests to verify that:

  • Option is correctly set when passed in constructor (with undefined, false, define values for each property)
  • Option is correctly modified after construction (with same values)
  • When activated, user cannot slide the bar outside the limits
  • When activated, user cannot slide handles outside the limits

ghusse avatar Apr 26 '13 18:04 ghusse

I still need to investigate further but, for the bouncing issue, this seems the root of the issue is that sometimes the offset values in cache are rounded integers instead of floats. For example, I expect "308.5263565" and the value in cache is "308".

nicofrand avatar May 02 '13 09:05 nicofrand

jQuery offset methods always returns integers (in Chrome at least) whereas we sometimes set the position from a position value based on event.pageX, which is a float.

Rounding all the position fixes the issue for me. In jQrangeSliderDraggable, _applyPosition :

_applyPosition: function(position){
            position = position >> 0;
            var offset = {
                top: this.cache.offset.top,
                left: position
            };

            this.element.offset({left:position});

            this.cache.offset = offset;
        }

Would that be a problem @ghusse ? If this is not, the positions should always be rounded of course (to obtain correct values at least).

nicofrand avatar May 02 '13 09:05 nicofrand

I'm ok with that. Just a question, do you know how it react when we call offset with a float value ? How does it round the value ? Is it a mathematical rounding, a truncation ?

ghusse avatar May 02 '13 09:05 ghusse

I did it to follow your code (in jQDateRangeSlider#_setOption and #values). I will update the code and commit it today.

nicofrand avatar May 13 '13 08:05 nicofrand

OK. So, this might be not optimal but I found a solution. The issue came some, I think so at least, a matter of js handling the numbers : there were some really really slight difference between two positions (whereas it should not) like between "196" and "195.999999999956". I modified jQRangeSliderHandle#position to ignore those minor differences (0.005 error margin), that should not cause any other issues…

nicofrand avatar May 14 '13 15:05 nicofrand

Ok, great. It's not JS specific, it's a known limitation of float numbers binary encoding.

ghusse avatar May 14 '13 15:05 ghusse

Argh, once again, I did not test enough. This breaks the bar dragging (partially). I guess I did not put it in the good place.

PS: indeed, this is not JS specific.

nicofrand avatar May 14 '13 15:05 nicofrand

There are some things I do not understand.

For example, I logged the positions I received in several methods. This is what happens when the bar is not expected to move (and the values to change) :

Draggable drag: (old) 195.9953930144298 (new)  197 jQRangeSliderDraggable.js:77
Constraint position (before):  197 jQRangeSliderBar.js:337
Draggable constraint position:  197 jQRangeSliderDraggable.js:105
Pos before constraint (handle):  197 jQRangeSliderHandle.js:151
Pos after constraint (handle):  197 jQRangeSliderHandle.js:158
set values 1360261205072 jQRangeSliderHandle.js:188
Pos before constraint (handle):  1593 jQRangeSliderHandle.js:151
Pos after constraint (handle):  1592.0046069855703 jQRangeSliderHandle.js:158
Pos before constraint (handle):  196.00460698557026 jQRangeSliderHandle.js:151
Pos after constraint (handle):  196.00460698557026 jQRangeSliderHandle.js:158
set values 1360255373976 jQRangeSliderHandle.js:188
Constraint position (after):  196.00460698557026 jQRangeSliderBar.js:351

In jQRangeSliderDraggable.js:77 the old position from the first line is this.cache.offset.left. The position in the last line (the one returned) is correct since it is the same (almost, due to float precision and calculating "errors"). But during the process, the values were set two times.

nicofrand avatar May 16 '13 13:05 nicofrand

What kind of interaction are you doing in this case ?

ghusse avatar May 16 '13 13:05 ghusse

I am dragging the bar. Each handle of the bar is stuck to the limits, so the bar is not supposed to move (which is almost the case since) and, the values are not supposed to change (since the bar did not move).

nicofrand avatar May 16 '13 13:05 nicofrand

It seems that constraintValues is not returning exactly the same value + I cannot see a test in the code verifying that before and after values are different before setting new positions.

Where position is constrained: https://github.com/ghusse/jQRangeSlider/blob/master/jQRangeSliderDraggable.js#L73

Then applied: https://github.com/ghusse/jQRangeSlider/blob/master/jQRangeSliderDraggable.js#L100

ghusse avatar May 16 '13 13:05 ghusse