datetimepicker icon indicating copy to clipboard operation
datetimepicker copied to clipboard

Does not work with AM/PM since v2.5.3

Open PapillonUK opened this issue 6 years ago • 10 comments

Please see the discussion on issue: #596 Specifically this comment from @dsnopek

The root of the problem appears to be that npm install is pulling the wrong version of php-date-formatter.

Does anyone have the required knowledge to fix that in the main release build?

It appears that the bug which prevents the DateTimePicker control being used with AM/PM has already been fixed in the latest version of the php-date-formatter, but DateTimePicker is still bringing in an old, broken version.

We can fix this by inserting our own code, but it would be better if the main release was fixed.

Thanks to anyone that has the knowledge to help!

PapillonUK avatar Apr 19 '18 13:04 PapillonUK

To add some more details...

The packages.json lists these dependencies:

  "dependencies": {
    "jquery": ">= 1.7.2",
    "jquery-mousewheel": ">= 3.1.13",
    "php-date-formatter": "^1.3.4"
  },

That should mean that it'll pull in php-date-formatter version 1.3.4 or higher. This bug is fixed in 1.3.4, so, that'd be great!

However, running npm install doesn't pull in version 1.3.4 - it gets some earlier commit between 1.3.3 and 1.3.4, that doesn't include the bug fix. Confusingly, the version number in the php-date-formatter that gets downloaded is 1.3.4, but the code isn't the same as if you download the release of 1.3.4 manually.

I have no idea why npm is pulling this weird version. It could be that the upstream maintainer did something weird? I'm really not sure, this is as far as I managed to get.

dsnopek avatar Apr 19 '18 14:04 dsnopek

If you use AM/PM to format your times (formatTime: "g:i A") the specific 3 issues caused by this bug are...

  1. When the DateTimePicker opens it will open with the previous hour selected rather than the time entered in your input. E.g. if your input has the time "6:00 AM", the DateTimePicker will open with "5:00 AM" selected instead.

  2. If you have a "PM" time entered in your input, when the DateTimePicker opens it will select the equivalent "AM" time (again an hour earlier). The "PM" suffix is ignored and it looks like DateTimePicker just searches for the first hit based on the numerical hour. For example, if you have "6:00PM" in your input the DateTimePicker will open with "5:00AM" selected.

  3. Possibly the most severe bug. Every time the user clicks in and out of the input box the time is decremented by one hour on the onblur event.

From what people (with much better knowledge than me) are saying, all these bugs would be resolved if DateTimePicker linked to v1.3.4 (or later) of the php-date-formatter during the build process which is obviously the intention of the syntax in packages.json

PapillonUK avatar Apr 22 '18 14:04 PapillonUK

For the purposes of fully documenting this issue:

From what people (with much better knowledge than me) are saying, all these bugs would be resolved if DateTimePicker linked to v1.3.4 (or later) of the php-date-formatter

My fix for this problem is manually downloading php-date-formatter 1.3.4 and it fixes the bugs! See full instructions in my comment here:

https://github.com/xdan/datetimepicker/issues/596#issuecomment-359866104

dsnopek avatar Apr 23 '18 13:04 dsnopek

Thanks @dsnopek

I'm a simpleton and don't understand all that complicated npm stuff but I did the equivalent of you by manually downloading the minified v1.3.4 of php-date-formatter and pasting the DateFormatter code into v2.5.30 of the minified version of datetimepicker.

It's pretty simple to overwrite the incorrect DateFormatter code with the correct code.

The result is a minified datetimepicker at v2.5.30 which contains the code it's supposed to and works fine with AM/PM.

I still hope someone that understands npm and the broken linking fixes this on GitHub though as that would benefit everyone!

PapillonUK avatar Apr 25 '18 14:04 PapillonUK

This issue seems to still be there. I am using the full.js file in this repo that has v1.3.4 of php-date-formatter hard coded at the top and I'm still getting this error.

roneesh avatar Jun 26 '18 16:06 roneesh

Hi @roneesh - that's strange as I'm not seeing the error in my manually fixed file. I wonder if the version number you're seeing is accurately reflecting the version bought into the code.

PapillonUK avatar Jun 26 '18 19:06 PapillonUK

Hi all, any news about that issues? Seems that changing time with PM/AM caused by minification, if you're using import 'jquery-datetimepicker/build/jquery.datetimepicker.full' then everything is ok.

agriboed avatar Jul 20 '18 09:07 agriboed

I wonder if the version number you're seeing is accurately reflecting the version bought into the code.

There is an issue with the version number in the file. Like many projects, it appears they increment the version number in the files in the Git repo right after the last release. So, the code file says 1.3.4, but it's actually a revision between 1.3.3 and 1.3.4. You can verify this by manually downloading the 1.3.4 release and comparing the files - they are not the same.

I noted this version number discrepancy in my comment above https://github.com/xdan/datetimepicker/issues/657#issuecomment-382767171

dsnopek avatar Jul 20 '18 13:07 dsnopek

Here is the fix. The Class which draws is xdsoft_current.

Old Buggy Code: Line 1891

if ((options.initTime || options.defaultSelect || datetimepicker.data('changed')) && current_time.getHours() === parseInt(h, 10) && ((!isALlowTimesInit && options.step > 59) || current_time.getMinutes() === parseInt(m, 10))) {

Fixed by Adding +1 after current_time.getHours()

if ((options.initTime || options.defaultSelect || datetimepicker.data('changed')) && current_time.getHours() +1 === parseInt(h, 10) && ((!isALlowTimesInit && options.step > 59) || current_time.getMinutes() === parseInt(m, 10))) {

sprograms avatar Feb 15 '19 06:02 sprograms

Hi all, any news about that issues? Seems that changing time with PM/AM caused by minification, if you're using import 'jquery-datetimepicker/build/jquery.datetimepicker.full' then everything is ok.

What exactly is ok? I tried the same and I am still seeing: If you have a "PM" time entered in your input, when the DateTimePicker opens it will select the equivalent "AM" time. Did anyone succeeded in resolving this?

quake3 avatar Nov 21 '19 19:11 quake3