Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Date picker problem in auto format dd/mm/Y (pt_BR) #300

Open
mateusneves opened this issue Apr 22, 2015 · 33 comments
Open

Date picker problem in auto format dd/mm/Y (pt_BR) #300

mateusneves opened this issue Apr 22, 2015 · 33 comments

Comments

@mateusneves
Copy link

Hello,
I'm using WordPress in pt_BR version. With this the date picker carries been translated into my language, and enter the date in the field in the format dd / mm / Y. And when the post unless the date is not saved when for example the day is greater than 12 , because by default the CMB2 will consider the day as month , and there could not convert the date in the format dd / mm / Y for the timestamp format .

@yivi
Copy link
Contributor

yivi commented May 7, 2015

Both datepicker and timepicker are messed up in international versions.

date_format and time_format are ignored, and instead the defaults for the translation strings are used.

And later on, as you said, validation is made against American date format, no matter what. So you choose May 12th and after save you see December 5th; or if you choose any day after the 12th it just wont save. (pt_BR, or es_ES; or most other locales for that matter).

Confirming bug.

@yivi
Copy link
Contributor

yivi commented May 7, 2015

I guess #282 and this one are related in some ways.
Pickers work well on first use, but on subsequent save/display, they are broken for international users.

I choose these values:
agregar_nuevo_anuncio_ yosilose_com _wordpress

(May 15th, May 8th, 8:35 PM)
And I get these values after saving:
editar_anuncio_ yosilose_com _wordpress

Discarded the first value, saved the wrong value for the second (its 1438732800 in the DB, which is August 5th instead of May 8th), and messed up the display of the time picker (although the value saved is the right one...)

@yivi
Copy link
Contributor

yivi commented May 7, 2015

Problem lays at the sanitation method text_date_timestamp, in CMB2_Sanitize.php.

It just puts the incoming value through strtotime, which is too naive to handle international dates, and there not even an attempt to respect date_format.

I replaced that line with a call to DateTime::createFromFormat; now I'll try to see that the date picker respects date_format as well, read the contributing docs more throughly, and try to submit a PR ot fix this.

Regards

@mateusneves
Copy link
Author

Thanks, for your answer.
This bugfix is native now in the CMB2, or I need to make this change in my files?

Regards

@yivi
Copy link
Contributor

yivi commented May 11, 2015

@mateusneves, I made the fix in a pull request, but it still need to be reviewed and accepted by the project owners, and then it would need to get integrated into a proper release.

You are of course free to pull a copy from my fork at https://github.com/yivi/CMB2/tree/trunk and test the changes for yourself. I'm using them and havent found problems yet, but more people checking would always be welcome.

@jtsternberg
Copy link
Member

@mateusneves what @yivi said. If you want to test his version, that can only help the project. Thanks for the PR @yivi. I'll try to look at it as soon as I can.

@mateusneves
Copy link
Author

Thanks, @yivi and @jtsternberg
This project is the best. Congratulations for all involved.

@Lindorie
Copy link

Hello @yivi,
I have the same problem with French language and date format 'd/m/Y'.
I tried to replace the code into the text_date_timestamp function (in CMB2_Sanitize.php) with your correction but it seems not working. Have you made another code modification related to this issue I could have missed?

 array(
                'name' => __('Date de fin (si applicable)', 'carine'),
                'id' => $prefix.'date_fin',
                'type' => 'text_date',
                'date_format' => 'd/m/Y'
            ),
public function text_date_timestamp( $value ) {
        //return is_array( $value ) ? array_map( 'strtotime', $value ) : strtotime( $value );
        if ( is_array( $this->value ) ) {
            $returnee = array();
            foreach ( $this->value as $value ) {
                $date_object = DateTime::createFromFormat( $this->field->args['date_format'], $value );
                $returnee[]  = $date_object ? $date_object->setTime( 0, 0, 0 )->getTimeStamp() : '';
            }
        } else {
            $date_object = DateTime::createFromFormat( $this->field->args['date_format'], $this->value );
            $returnee    = $date_object ? $date_object->setTime( 0, 0, 0 )->getTimeStamp() : '';
        }
        return $returnee;
    }

Thanks for helping me!

@ctlcltd
Copy link

ctlcltd commented May 23, 2015

You have to update .po and .mo files to reflect _x( 'mm/dd/yy', 'Valid formatDate string for jquery-ui datepicker', 'cmb2' ); instead the previous __( 'mm/dd/yy', 'cmb2' );

@yivi
Copy link
Contributor

yivi commented Jun 10, 2015

@Lindorie, sorry for taking so long, I was in the middle of a very convoluted project and couldn't make time to come around here.

There are a few more changes. It would be easier if you just checked out my fork. It's updated to 2.08 and working (I'm using it at the moment). I'm going to take another look to see if there is anything else I'm missing out.

Hopefully at some point @jtsternberg will have time to review my PR and see if its merge-worthy... :)

Regards!

@fabianpimminger
Copy link

Thanks @yivi. Your fork really helped me to get the thing working in german. I hope this will be integrated into the main branch!

@yivi
Copy link
Contributor

yivi commented Jul 9, 2015

Glad you found it useful @fabianpimminger.

Still waiting for word from the project owners about accepting my PR. I'm willing to do more work on it if it's not up to scratch, but I guess there wasn't enough time to properly review it.

Regards!

@jtsternberg
Copy link
Member

@yivi yah, unfortunately time is not on my side. I try to test every PR thoroughly before merging, and as you can imagine, that takes some time. I'll leave some comments on your PR.

@jtsternberg jtsternberg added the bug label Jul 9, 2015
@dantewebmaster
Copy link

any fix for this issue yet?

@mateusneves
Copy link
Author

Already has provision for the solution ?
And tanks for this fantastic project.

@Lindorie
Copy link

Hi there,

I've created another fork to fix temporarily the issue with non-US dates.
I tried to use the yivi's fork but it couldn't work and I needed to quickly fix the problem.
If you are interested, see: https://github.com/Lindorie/CMB2
I've edited the readme file for the usage and configuration.

The fix is only for the text_date_timestamp field but I think you can use the concept for all the dates fields.

Please note that my solution is not the best, but it works and until a fix on the original project it's better than nothing, I guess. :)

@yivi
Copy link
Contributor

yivi commented Oct 22, 2015

Hi there @Lindorie,

Could you please elaborate on what problems you had using my fork? Did you checkout the "yivi-date-time-picker-fixes" branch, which is up-to-date one?

Thanks and regards.

@Lindorie
Copy link

@yivi,

I have tried with the fork I found in this comment. But apparently you made some changes? (The link is now broken)
I didn't notice that you had a specific branch for the date-time-picker, sorry.
I'm going to test it and I'll tell you.

@yivi
Copy link
Contributor

yivi commented Oct 23, 2015

Thanks Carine. Let me know, please. Regards! 


Sent from Mailbox

On Fri, Oct 23, 2015 at 3:06 PM, Carine Pic notifications@github.com
wrote:

@yivi,
I have tried with the fork I found in this comment. But apparently you made some changes? (The link is now broken)
I didn't notice that you had a specific branch for the date-time-picker, sorry.

I'm going to test it and I'll tell you.

Reply to this email directly or view it on GitHub:
#300 (comment)

@Lindorie
Copy link

@yivi
Your fork is working on my project ;-)

@yivi
Copy link
Contributor

yivi commented Oct 23, 2015

Glad to hear it @Lindorie .

Let me know if you find any issues.

Regards!

@jtsternberg
Copy link
Member

#446 has been merged. Please test trunk to verify these issues are good. thanks.

@jason-murray
Copy link

Just a little more input on this, we worked around this issue today by using hyphens instead of slashes as the notes on strtotime state that it disambiguates US and EU format dates by the separator used: hyphen for EU, slash for US. So as long as date_format used - as the separator it worked.

For our use case this swap was acceptable, I'm not sure if this make it easier to solve for you CMB devs.

@onnimonni
Copy link
Contributor

I had problems with this using the latest tagged version as of now: 2.1.2.

When I tried to use trunk it worked like a charm!

@jtsternberg could you release new version? I would love to use tagged version in my composer.json.

@jtsternberg
Copy link
Member

@onnimonni Yes, we'll be releasing very soon. For best results, tune in to @webdevstudios on twitter.

@JohEngstrom
Copy link

I can't get this to work in trunk either. I set date_format on my text_datetime_timestamp field to d/m/Y for UK date format but if in the picker I select a day greater than 12 it doesn't save the value and returns a blank field.

Even if the day in the picker is 12 or less it only saves the correct format every other post update which I found very odd.

The time field now works perfectly though in trunk. I could never set it to 24h format but that works now.

I really hope this can be fixed very soon. In the project I'm working on now I use date/time fields quite heavily but I'm completely stuck now with this issue.

Just a quick edit to this. I can get it to work if I change the separator from / to - like @JasonD33 said above. It allows me to work around the issue for now at least. Thanks @JasonD33 !

@Metroslim
Copy link

Hi,
For fr_FR the bug is still there.
I've been able to use the workaround propose by @JasonD33

I'm using the 'date_format': 'd-m-Y' on the field and I've changed the CMB2_JS.php file so that the default date format of the datepicker from jquery-ui use the same format.

Thanks @JasonD33

And thanks CMB devs for the good work.

@ddanielbee
Copy link

This is still happening for German month names. Whenever the short month form doesn't comply to default english form, the date isn't saved. Any help here ?

@nreck
Copy link

nreck commented Jun 30, 2017

Can still confirm that the bug is still there in the newst release of CMB2.

@wpalchemist
Copy link

This bug appears to still be happening. :( I have built a sanitization function to replace slashes with slashes in numerical date formats, but if you have a date format such as F j Y where the month is spelled out, the date will not save.

@tw2113
Copy link
Contributor

tw2113 commented Jan 5, 2018

Is it possible to use the sanitization function conditionally based on the format chosen? Or is that F j Y version not saving regardless of the sanitization function used?

@wpalchemist
Copy link

The problem isn't just the date format - the problem is date format and language. So F j Y works just fine in English, but not in any other language. So in theory, I could have my sanitization function check whether we are in English, and if not, then replace any full-text words with numbers instead, but that defeats the purpose of letting users select their own date format.

@tw2113
Copy link
Contributor

tw2113 commented Jan 5, 2018

Compiling the issue is the fact that javascript and php don't follow the same formats/letters/etc. As the saying goes "dates are hard".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests