seiyria / bootstrap-slider

A slider control for Bootstrap 3 & 4.
http://seiyria.github.io/bootstrap-slider/
Other
3k stars 1.14k forks source link

mouseup does not trigger change event #896

Closed viceice closed 5 years ago

viceice commented 5 years ago

The mouseup event should trigger change event.

_mousedown uses setValue

https://github.com/seiyria/bootstrap-slider/blob/d41fa60264f5177a9c4e646b8a8a641b44e1cda2/src/js/bootstrap-slider.js#L1571-L1575

_mousemove too https://github.com/seiyria/bootstrap-slider/blob/d41fa60264f5177a9c4e646b8a8a641b44e1cda2/src/js/bootstrap-slider.js#L1678-L1679

But not _mouseup. I think _mouseup should call setValue(val, false,true) too.

https://github.com/seiyria/bootstrap-slider/blob/d41fa60264f5177a9c4e646b8a8a641b44e1cda2/src/js/bootstrap-slider.js#L1750-L1754

I can send a PR if required.

jespirit commented 5 years ago

https://github.com/seiyria/bootstrap-slider/blob/d41fa60264f5177a9c4e646b8a8a641b44e1cda2/src/js/bootstrap-slider.js#L1750-L1754

You're right, there should be a call to setValue() in _mouseup(). I ran into a bug when fixing the lock_to_ticks feature in https://github.com/seiyria/bootstrap-slider/pull/744 where the _mouseup() would recalculate the percentages but not recalculate the value which causes problems when "snapping" to ticks.

You're welcome to send a PR.

I also noticed on line 1752 there's a call to this._layout(), this should also be removed because _layout() is also called in setValue().

viceice commented 5 years ago

I'll send it tomorrow morning. 😁