jonthornton / Datepair.js

A javascript plugin for intelligently selecting date and time ranges, inspired by Google Calendar.
https://www.jonthornton.com/Datepair.js
358 stars 87 forks source link

Change event not firing on the time fields #44

Closed marcinjackowiak closed 9 years ago

marcinjackowiak commented 9 years ago

Whenever the time field is populated automatically no "change" event is fired. This can be seen on the jQuery Example by pasting the following code:

$('#jqueryExample .time').on('change', function(e) { 
    console.log($(e.currentTarget).attr('class')+' CHANGED'); 
});

I traced this to jquery-timepicker.js and the setTime method. You cannot pass a "source" parameter to setTime, which is then used by the _setTimeValue method. The "source" is "undefined" when that method is called and as a result it doesn't trigger the change event:

jquery-timepicker.js @ line: 778

        if (source == 'select') {                                
            self.trigger('selectTime').trigger('changeTime').trigger('change', 'timepicker');
        } else if (source != 'error') {
            self.trigger('changeTime');
        }
        ...

Added self.trigger('change'); to fix the problem:

        if (source == 'select') {                                
            self.trigger('selectTime').trigger('changeTime').trigger('change', 'timepicker');
        } else if (source != 'error') {
            self.trigger('change'); // Trigger change regardless of source
            self.trigger('changeTime');
        }
        ...
jonthornton commented 9 years ago

This is deliberate. Timepicker's setTime method does not fire a change event in order to disambiguate between user-initiated changes and code-initiated changes. The assumption is that if setTime is being called, your app already has some way to know a change event has happened. I realize this is not a perfect assumption, but it's the most flexible compromise I could make.

Your best bet is to override the setTime option when instantiating Datepair:

var d = new Datepair(d, {
    'updateTime': function(input, dateObj){
        jq(input).timepicker('setTime', dateObj).trigger('change');
    }
});
marcinjackowiak commented 9 years ago

Ok, I understand why it doesn't really make sense to make changes to the Timepicker library.

My problem was occurring when the end time was auto-populated by the plugin due to a user change made to the start time (for example when you push the start time beyond the end time). You could argue that it is a user-initiated change on the end time field, though indirectly.

Anyway, your solution will work. Edit: I modified setTime to updateTime used in Datepair:

           updateTime: function(input, dateObj) {
                    jq(input).timepicker('setTime', dateObj).trigger('change');
            }

Thanks for the feedback and the library of course, it works great :)

jonthornton commented 9 years ago

Whoops! I should read my own documentation. Glad it worked out.