longbill / jquery-date-range-picker

A jQuery plugin that allows user to select a date range
MIT License
1.12k stars 579 forks source link

Fix days difference in checkSelectionValid() #378

Closed niutech closed 6 years ago

niutech commented 6 years ago

In function checkSelectionValid() (jquery.daterangepicker.js:1722) the days difference is counted as follows:

var days = Math.ceil((opt.end - opt.start) / 86400000) + 1;

which does not factor daylight saving time. Instead, it should be:

var days = countDays(opt.end, opt.start);

or using moment.js:

var days = moment(opt.end).diff(moment(opt.start), 'days');
holtkamp commented 6 years ago

Thanks for the report. I also noticed some "strange" behaviour around midnight.

You are referencing to this section:

https://github.com/longbill/jquery-date-range-https://github.com/longbill/jquery-date-range-picker/blob/79f6e9149b0a120b3de3bbb508dc2711a0ca33ed/src/jquery.daterangepicker.js#L1722-L1723

Would it be an idea to change the (custom) countDays() to using moment.js as well?

https://github.com/longbill/jquery-date-range-picker/blob/79f6e9149b0a120b3de3bbb508dc2711a0ca33ed/src/jquery.daterangepicker.js#L1805-L1807

niutech commented 6 years ago

Yes, I refer to this line. I think it is a good idea to use moment.js for date diffing, since it it well tested.

holtkamp commented 6 years ago

Ok, a pull-request would be very welcome

niutech commented 6 years ago

Done, but please test it.

holtkamp commented 6 years ago

https://github.com/longbill/jquery-date-range-picker/issues/310#issuecomment-350327431

holtkamp commented 6 years ago

Probably resolved with https://github.com/longbill/jquery-date-range-picker/pull/423, released in https://github.com/longbill/jquery-date-range-picker/releases/tag/0.16.2