Skip to content

Support for custom "date_format" and "time_format" arguments#498

Closed
mweimerskirch wants to merge 1 commit into
CMB2:masterfrom
mweimerskirch:datetime-format
Closed

Support for custom "date_format" and "time_format" arguments#498
mweimerskirch wants to merge 1 commit into
CMB2:masterfrom
mweimerskirch:datetime-format

Conversation

@mweimerskirch

Copy link
Copy Markdown

Support for custom "date_format" and "time_format" arguments for the text_date, text_date_timestamp and text_datetime_timestamp fields.

Original fixes by @yivi, based on #318

I removed all the unrelated changes, and added support for the time picker. To make merging easier, the pull request also doesn't include a minified JS file.

…text_date, text_date_timestamp and text_datetime_timestamp fields
@jtsternberg

Copy link
Copy Markdown
Member

Why do you believe your PR is a better option than @yivi's #318? What do you consider to be "unrelated changes"?

@mweimerskirch

Copy link
Copy Markdown
Author

I downloaded the patch file for #318 directly from GitHub because I didn't want to pull in 23 individual commits and there were hundreds of whitespace changes as well as changes in a translation file: https://github.com/WebDevStudios/CMB2/pull/318.patch
But just now after reading your message I noticed that those changes are not visible when viewing the diff online. Seems to be a bug on GitHub, so I take the part about the "unrelated changes" back.

@yivi

yivi commented Oct 19, 2015

Copy link
Copy Markdown
Contributor

yivi here.

I see that in your version you are are back to using DateTime::createFromFormat, whereas we had a workaround for PHP versions < 5.3.

Also, my work continued in #446, on a different branch that @jtsternberg created to carry on this work. I updated it a couple weeks ago, and it does have support for time_picker as well...

I think I messed up with the PRs, by opening a new one instead of continuing work in #318, but I can merge trunk into #446 again to see if everything keeps working as intended.

Regards.

@jtsternberg

Copy link
Copy Markdown
Member

@yivi we're so close. Thanks for your efforts. Let me know which (#318 or #446) I should be testing against. For now, i've merged trunk into the WebDevStudios:yivi-date-time-picker-fixes branch so it is up-to-date.

@mweimerskirch

Copy link
Copy Markdown
Author

@yivi Could you squash the commits for the current pull request? That way I could test it as well and let you know if any of my use cases fail.

@yivi

yivi commented Oct 20, 2015

Copy link
Copy Markdown
Contributor

@jtsternberg , #446 is the current one.
I'll update it to trunk this afternoon.

@jtsternberg

Copy link
Copy Markdown
Member

This should be good in trunk. Huge thanks for @yivi's work. Please test.

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

Labels

None yet

3 participants