Removing stopPropagation

[andreeib]

Hello, I'm using the slider inside a sweetalert2 modal and have a problem with users trying to slide to the min or max value and accidentally closing the modal.

Clicking and sliding to min/max value with the mouse going outside the modal then releasing the click will close the modal because of click event on background overlay.

Removing stopPropagation fixes the issue because sweetalert2/boostrap knows if the background overlay click was intended or not based on mousedown/mouseup.

Made some tests with stopPropagation removed and seems to work correctly. Will a pull request with this change be merged?

https://css-tricks.com/dangers-stopping-event-propagation/

Expected behaviour

The mousedown events should propagate upwards.

Actual behaviour

The mousedown events don't propagate because stopPropagation is called.

[ValentinH]

Hi, thanks for raising this issue :)

To be honest, I don't remember why this is here... It might be to prevent selecting the text around the slider while draging. Did you test that?

[andreeib]

I ran the tests locally and they passed by removing the stopPropagation call check. Played with the demos and the text is not selected while dragging.

Made some demos:

• With stopPropagation: http://jsfiddle.net/se14jem6/1/
• Without: http://jsfiddle.net/rzq5Lbsy/2/

It's reproducible with the bootstrap modal demo as well: http://angular-slider.github.io/angularjs-slider/

The stopPropagation call is there from the initial commit in 2013.

[ValentinH]

You convinced me, thanks for the investigation :)

Thus, a PR is more than welcome! 👍

[ValentinH]

Released with 6.4.4: https://github.com/angular-slider/angularjs-slider/releases/tag/6.4.4

[andreeib]

Awesome, thank you for the quick response.

[stripathix]

In ionic, there is an event attached to swipe left/right to show/hide side menu. Removing event.stopPropagation() on start causes opening of side menu when the slider is dragged to right. There can be many different cases or framework which has an event attached to swipe especially for mobile application, I am not sure removing event.stopPropagation() from the library was a good idea because now every developer who uses this library for mobile apps will have to write onStart option to ensure event.stopPropagation(); to prevent side menu from opening.

To solve this I have added an option for slider configuration.

options: { onStart: function () { event.stopPropagation(); }, }

[andreeib]

Hey @stripathix you could solve this with the following options:

onStart: function () {
    $ionicSideMenuDelegate.canDragContent(false);
},
onEnd: function () {
    $ionicSideMenuDelegate.canDragContent(true);
}

Take a look at this example: http://jsfiddle.net/313p8bag/15/

[ValentinH]

You can also set the options provided by @andreeib globally by using the Slider config function.

[MaximCrabbe]

Since 6.4.4 the slider behavior went bad for my application: instead of only dragging the slider it also started dragging the whole block wich was draggable with angular ui sortable, however for now I just set bower to keep 6.4.3 so I am able to keep using this... Imo This (and 6.4.4) with removing the stoppropagation, is a breaking change

[stripathix]

@ValentinH @andreeib I do use that configuration to stop Ionic canDrag when slider drag starts, but I would say that performance is not that smooth as it used to be. Many a time I can see that due to slider drag menu does open. This happens when quicky strat drag/stop drag then starts drag of the slider.

The performance with stopPropogation was really smooth.

[ValentinH]

OK I will rollback this then.

[MaximCrabbe]

Best solution is to make this an optional option paramter I think. Something like DisableStopPropogation, then it should be default like it was before 6.4.3 so people can override it, if they have other needs. (Just a suggestion)

[ValentinH]

Yes definitely agreed. To be quick, I just reverted the commit that had removed the stopPropagation.

@andreeib , if you still need this, could you resubmit your PR and add the option mentionned by @MaximCrabbe ?

[ValentinH]

6.5.1 fixes your issues @MaximCrabbe @stripathix

[stripathix]

@ValentinH Thanks :-)