icambron / twix.js

:hourglass::left_right_arrow: A date range plugin for moment.js
https://isaaccambron.com/twix.js/
MIT License
379 stars 54 forks source link

Possible bug with contains and allDay #75

Closed jfosnight closed 9 years ago

jfosnight commented 9 years ago

While checking to see if the range 2015-07-01 - 2015-07-31 contained certain dates, I noticed that 2015-08-01 was evaluating as true.

I was using: moment("2015-07-01").twix("2015-07-31", {allDay: true}).contains("2015-08-01");

After doing some more testing I noticed that if i add a 'Z' to all the dates, then it evaluates as false. moment("2015-07-01Z").twix("2015-07-31Z", {allDay: true}).contains("2015-08-01Z");

Now one thing of importance is that the allDay flag is set to true, when setting that to false, then it does not matter if I use 'Z' or not, they both evaluate to false. (as they should)

Perhaps I am missing something, but this doesn't produce the behavior I was expecting. It was my understanding that it should be evaluating to see if Aug 1st is contained in the date range 2015-07-01 00:00:00 to 2015-07-31 23:59:59. It shouldn't matter if I use the local timezone or UTC, as long as they are all using the same timezone.

Tested with Moment 2.10.6 & Twix 0.6.4

2015-07-31 21_08_03-developer tools - https___dev ideatek com_wallaby_v3_invoice_

icambron commented 9 years ago

Definitely a bug. Fixed, pushed, and released in 0.6.5.

In general, Twix is complicated by the fact that sometimes all-day ranges should be treated as closing over the first milli of the next day (e.g. when displaying) and sometimes not (e.g. contains). So there are some funny edges I don't always quite nail. I've made some changes internally that should help prevent this kind of issue in the future.

Thanks for the detailed bug report!

icambron commented 9 years ago

Well, apparently my memory is bad, because this wasn't a bug; this was a deliberate decision (see, e.g. #44). So now I'm less sure this was a good idea. I may pull the 0.6.5 release and leave this in the 0.7.0 release.