angular-ui / ui-slider

jQuery UI Slider for AngularJS
https://htmlpreview.github.io/?https://github.com/angular-ui/ui-slider/master/demo/index.html
MIT License
265 stars 176 forks source link

Slide event cancellation doesn't keep previous slider values #117

Open dorellang opened 7 years ago

dorellang commented 7 years ago

I don't know if this project is still alive, but anyway according to the JQuery UI Slider docs you can cancel an slider event in the slider callback and keep the values that were just before the user started sliding. (This is the documentation http://api.jqueryui.com/slider/#event-slide)

However if you use angular-ui-slider and try to cancel the slider event in the callback in the slider options, the values will change as if the event was never cancelled. After hours of debugging I found that the culprit is this listener which updates the model value from slider:

                // Update model value from slider
                elm.bind(updateOn, function(event, ui) {
                    var valuesChanged;

                    if (ui.values) {
                        var boundedValues = ui.values.slice();

                        if (options.lowerBound && boundedValues[0] < options.lowerBound) {
                            boundedValues[0] = Math.max(boundedValues[0], options.lowerBound);
                        }
                        if (options.upperBound && boundedValues[1] > options.upperBound) {
                            boundedValues[1] = Math.min(boundedValues[1], options.upperBound);
                        }

                        if (boundedValues[0] !== ui.values[0] || boundedValues[1] !== ui.values[1]) {
                            valuesChanged = true;
                            ui.values = boundedValues;
                        }
                    } else {
                        var boundedValue = ui.value;

                        if (options.lowerBound && boundedValue < options.lowerBound) {
                            boundedValue = Math.max(boundedValue, options.lowerBound);
                        }
                        if (options.upperBound && boundedValue > options.upperBound) {
                            boundedValue = Math.min(boundedValue, options.upperBound);
                        }

                        if (boundedValue !== ui.value) {
                            valuesChanged = true;
                            ui.value = boundedValue;
                        }
                    }

                    ngModel.$setViewValue(ui.values || ui.value);
                    $(ui.handle).find('.ui-slider-tip').text(ui.value);
                    scope.$apply();

                    if (valuesChanged) {
                        setTimeout(function() {
                            elm.slider('value', ui.values || ui.value);
                        }, 0);

                        return false;
                    }
                });

which is called regardless the slide event was cancelled or not; and, in fact, it's called before the callback supplied in the options.

I think the proper fix for this is to move that listener to a callback that is always supplied in the underlying JQuery UI widget that in turn calls the actual callback specified in the angular directive and only updates the view and triggers a digest if the event was not cancelled.

What do you think?

Cheers.

dorellang commented 7 years ago

A workaround for this issue is to pass a custom event in the updateOn option and trigger it in the slider callback only when you don't cancel the event. For instance:

HTML:

<div ui-slider='{slide: onSlide, updateOn: "mySliderUpdate"}'></div>

Javascript (angular controller):

...
$scope.onSlide = function(event, ui){
  if(shouldCancelSlide(ui)){
    return false;
  }
  var slider = angular.element(event.target);
  slider.trigger('mySliderUpdate', ui);
};
...

I've tested this in the app I'm working on and it works :D

PowerKiKi commented 7 years ago

if this project is still alive

This project is kept alive only via contributions like yours. I merge PRs, but I can't do much more than that. If you think your fix should be implemented, please submit a PR, and I'll merge it.