-
-
Notifications
You must be signed in to change notification settings - Fork 497
Add a rzSliderOnChange attribute. #67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ValentinH
commented
Jun 5, 2015
…der" rz-slider-on-change="onSliderChange()"></rzslider>
Similar functionality is already implemented with using |
I disagree because it fires only on moveEnd. If I want to update something everytime the value change, I couldn't. Now with this function, it is possible. Maybe we can replace my implementation by firing a slideMoved event. |
How does it differ from watching the model with scope.$watch() ? |
The result is the same but it is better to avoid $watches. This is why there is a ng-change on default inputs. My goal is to provide a ng-change like feature. |
+ 1 Better than $watch |
I think ng-change could be pretty useful. By the way, I see "This branch is up-to-date with the base branch", was ng-change added to master branch? I can't see it now, maybe I misunderstood this message. |
Nope it is not yet merged. For now, there are 2 people that want it merged (3 with me) and one person @rzajac that doesn't really want to merge it. Let's see if there are more people that request it... |
Well, it think this approach is much more viable when I have a variable list of objects which have a range. With this pull I am able to use simple markup to achieve my goals, while with the watch I would have to manage the contents of my list and add/remove watchers accordingly. Implementing this is extremely simple and much more efficient than using watchers, I don't know what the argument against it could possibly be. |
So @ValentinH , 4 to 1 :) |
OK let's merge it ;) |
Add a rzSliderOnChange attribute.