trentrichardson / jQuery-Timepicker-Addon

Adds a timepicker to jQueryUI Datepicker
http://trentrichardson.com/examples/timepicker/
MIT License
2.66k stars 1.05k forks source link

onChange event fires before the time is actually changed #923

Open beleester opened 7 years ago

beleester commented 7 years ago

I was trying to make one datetimepicker update in response to another, so that they'd always show the same time. So each of them had an onChange event that checked if the other needed to update, and then updated it. I'd done this before with datetime-local inputs, but when I changed to your datetimepickers, it kept crashing due to an infinite recursion.

When I stepped through with a debugger, I found this was the sequence of events: The first picker called setDate to change the date on the second one. This fired the onChange event of the second picker. However, at the time it was called, the second picker still had its old value. So it looked at its value (old), and, compared it to the value in the first picker (new), and concluded that the other picker needed to be updated to match. That triggered another onChange() event, and so on and so forth until it died from recursions.

After digging into the code, these lines in _setDateDatepicker seem to be the culprit:

this._updateDatepicker(inst);
this._base_setDateDatepicker.apply(this, arguments);
this._setTimeDatepicker(target, tp_date, true);

It updates the datepicker. It calls the base datepicker's set method to change the date, which triggers the onChange() event. And then it calls _setTimeDatePicker to update the time. So at the time of the onChange() event, the datetimepicker still has the old time.

If you swap the last two lines, it should be fixed:

this._updateDatepicker(inst);
this._setTimeDatepicker(target, tp_date, true);
this._base_setDateDatepicker.apply(this, arguments);