search

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

Bug with setValue: hours is 1hr less than selected #462

Closed Hirurg103 closed 3 years ago

Hirurg103 commented 5 years ago

I use setValue to set date values in the app form. But the issue is that the displayed value is 1hr less than selected by the user.

datepicker-error-example.html

This bug is reproducible when I select Greenwich Mean Time time zone on my computer screen shot 2018-10-28 at 12 44 24 pm

I am adding a source code how to reproduce this bug:

<!doctype html>
<html>
  <head>
    <script src="https://rawgit.com/jquery/jquery/1.12.4/dist/jquery.min.js"></script>
    <script src="https://rawgit.com/moment/moment/develop/min/moment-with-locales.min.js"></script>
    <script src="https://rawgit.com/longbill/jquery-date-range-picker/master/dist/jquery.daterangepicker.min.js"></script>
    <link href="https://rawgit.com/longbill/jquery-date-range-picker/master/dist/daterangepicker.min.css" rel="stylesheet"/>
  </head>

  <body>
    <input type="text" id="from"/>
    <input type="text" id="to"/>
    <a id="datepicker">
      select dates
    </a>
    <script>
      $('#datepicker').dateRangePicker({
        separator: ' to ',
        time: { enabled: true },
        format: 'DD-MM-YYYY HH:mm',
        showShortcuts: true,
        setValue: function(s, s1, s2) {
          console.log(s)
          console.log(s1)
          console.log(s2)
          $(this).val(s)
          $("#from").val(s1)
          $("#to").val(s2)
        },
        shortcuts: {
          'prev-days': null,
          'next-days': null,
          prev: null,
          next: null
        },
        startOfWeek: 'monday'
      })
    </script>
  </body>
</html>

There is a gist with this source code

monovertex commented 5 years ago

Please see this JSFiddle that contains the gist you posted and let me know if you can reproduce the bug there. On my Windows machine it seems to be setting the values correctly.

image

image

If the JSFiddle is OK for you, then the issue is caused by some other code in your app. Are you setting the timezone for the moment object, by any chance?

Hirurg103 commented 5 years ago

Hi @monovertex, yes I am unable to reproduce this bug today. I tried to set my computer's time to yesterdays date (28 Oct 2018) and it is reproducible! See screenshot

screen shot 2018-10-28 at 9 15 33 am - highlighted

Hirurg103 commented 5 years ago

Hi @monovertex , what do you think about it?

monovertex commented 5 years ago

@Hirurg103 Sorry, I've been caught up with work and real life. I'm currently on a Linux machine, which gives me grief with date changes, I'll check it out on 28 October as soon as I get home on my Windows machine and come back with my feedback.

Hirurg103 commented 5 years ago

@monovertex no problem at all. Thank you for the response!

monovertex commented 5 years ago

@Hirurg103, some first thoughts:

I'll have to take a look through the code and see where is the hour difference coming from, but this is definitely a confirmed bug now. PRs are welcome 😁.

brainumbc commented 5 years ago

This is probably related to an issue I had. When function x() fires (the one that fires when you click a date), the math to determine date range is based on milliseconds. On November 4th this year the clocks rolled back 1 hour so technically November 4th is 25 hours long in countries that use daylight savings. So if you do an alert to spit out the times "a.start" and "a.end" you'll see a UTC offset such as 28 Oct 2018 00:00:00T-04:00 and 28 Nov 2018 00:00:00T-05:00

My requirement was to make the maxdays=31 but because this range above is 31 days and 1 hour, daterangepicker rounds up to 32. I imagine you'll have a similar issue when daylight savings starts and you'll be able to select 1 day more than your date range, or the range will report as 1 day less. Your problem is likely similar to mine. Here's what I added at the very top of function x() to fix this issue. You might want to use this as starting point. This assumes that all dates are midnight, though:

` var mystart = new Date(a.start); var myend = new Date(a.end);

        if (mystart.getTimezoneOffset() < myend.getTimezoneOffset()) {
            mystart = mystart.setMinutes(myend.getTimezoneOffset() - mystart.getTimezoneOffset()); //if end date is after daylight savings ends (end date is 1 hour later, add 1 hour to start date)
        } else if (myend.getTimezoneOffset() < mystart.getTimezoneOffset()) {
            myend = myend.setMinutes(mystart.getTimezoneOffset() - myend.getTimezoneOffset()); // opposite if end date is 1 hour earlier offset than start date
        } else {
            mystart = mystart.setMinutes(0); 
            myend = myend.setMinutes(0);
        }

        a.start = new Date(mystart).getTime();
        a.end = new Date(myend).getTime();`

Essentially what you get is when datlight savings ends, the start date (before it ended) will have the same UTC offset as it did before, so the start date time thrown in the calculation will be 60 minutes shorter so the calculation will be exactly 31 days... or you could just subtract 60 minutes from the last day.. either way.

brainumbc commented 5 years ago

Actually strike that second condition where "myend" is set. This takes a range that would be 30 days and 23 hours and adds 60 minutes to make the range an even 31, which is unnecessary because date range calcs in JS are going to round up to 31 anyways. However, if you're looking for exact time between ranges and not just days, I'd keep it in.

Hirurg103 commented 3 years ago

Hello @monovertex @longbill . Could you please take a look at my PR that fixes this bug when you have time. Thank you!