Skip to content

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

Merged
merged 1 commit into from
Aug 3, 2015
Merged

Add a rzSliderOnChange attribute. #67

merged 1 commit into from
Aug 3, 2015

Conversation

ValentinH
Copy link
Member

<rzslider rz-slider-model="priceSlider" rz-slider-on-change="onSliderChange()"></rzslider>

…der" rz-slider-on-change="onSliderChange()"></rzslider>
@rzajac
Copy link
Member

rzajac commented Jun 5, 2015

Similar functionality is already implemented with using slideEnded event: https://github.com/rzajac/angularjs-slider#slider-events

@ValentinH
Copy link
Member Author

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.

@rzajac
Copy link
Member

rzajac commented Jun 5, 2015

How does it differ from watching the model with scope.$watch() ?

@ValentinH
Copy link
Member Author

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.

@antoescu
Copy link

antoescu commented Jul 2, 2015

+ 1 Better than $watch

@radek-anuszewski
Copy link

I think ng-change could be pretty useful.
Imagine a situation:
I have 5 sliders, each value is displayed to the user, for example in blue circle. When user touch/move Slider, his circle turns into green color - to show user that he set value. Or another use case - for some of Sliders additional work has to be done, like in (real-world app use case) Glycemia, where values can be in two units: mmol/L or mg/dL, where is natural to show 2 separate Sliders for each unit - if we don't know which values user will choose we have to do recalculations between .mmol/L and mg/dL.
So maybe @ValentinH changes, if it works correctly, are worth merging?

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.

@ValentinH
Copy link
Member Author

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...

@cosme-benito
Copy link

How does it differ from watching the model with scope.$watch() ?

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.

@radek-anuszewski
Copy link

So @ValentinH , 4 to 1 :)

@ValentinH
Copy link
Member Author

OK let's merge it ;)

ValentinH added a commit that referenced this pull request Aug 3, 2015
Add a rzSliderOnChange attribute.
@ValentinH ValentinH merged commit 8a7fcbc into angular-slider:master Aug 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants