abpetkov / powerange

iOS 7 style range slider
http://abpetkov.github.io/powerange/
423 stars 85 forks source link

avoid endless loops if size of slider or step is 0 #2

Closed orotemo closed 10 years ago

orotemo commented 10 years ago

I really loved the cleanliness of this plugin, however, running it without css crashed my browser, and when I have investigated, it boiled down to the endless loop that is happening if interval is 0.

I would like to start with an apology: couldn't build your project. not even using the recommended build flow. after a lot of playing around, got to this:

$ component install components/events

       error : no remote found for dependency "components/events".

$ component build components/events
       build : resolved in 38ms
$ grunt build
Running "componentbuild:development" (componentbuild) task
.
Fatal error: failed to lookup "powerange"'s dependency "component-events"

The point:

The proposed fix is laid out the way it is for completeness. it protects from theoretical negative and positive values to any of the variables interval and dimension, as well as zero interval, which causes infinite loops.

WDYT?

abpetkov commented 10 years ago

Even though the issue is present, the solution is not quite the best, imo.

With your solution provided, when a negative step size is set, the slider becomes unusable, since it's steps array not initialized properly. It becomes inactive, but nothing indicates it is and it looks more like broken.

Therefore, I think what a good approach to fix this issue, will be to set the step to the same number, but positive. (btw no infinite loops appear when step is 0 as you mentioned). That way the slider will work fine. Other solution, will be to set the step to the minimum closest positive number, which is in all cases will be 1, but I like the other one more.

How I've handled this - check out this commit.

Thanks for your feedback. Cheers.