Problem/Motivation
I am trying to implement a datetime-local element but I can't get it to work
All my tests led me to the catch (\Exception $e) in Datetime::valueCallback()
with the error "The date cannot be created from a format."
Steps to reproduce
Build a custom form with the following method in it:
/**
* {@inheritdoc}
*/
public function buildForm(array $form, FormStateInterface $form_state) {
$form['datetimepicker'] = [
'#type' => 'datetime',
'#date_date_element' => 'datetime-local',
'#date_time_element' => 'none',
];
$form['submit'] = [
'#type' => 'submit',
'#value' => 'Submit',
];
return $form;
}
Then access your form page, fill the input and submit the form.
Proposed resolution
- Alter
Datetime::valueCallback() to provide a valid format (ie. $date_format = 'Y-m-d'; and $time_format = 'H:i:s';) to \DateTime::createFromFormat() when '#date_date_element' is 'datetime-local'
- Also alter
Datetime::processDatetime() to fill $element['#value'] from a 'Y-m-d\TH:i:s' submitted value after the form has been processed.
Remaining tasks
- Choose whether or not we have to provide a fix when
'#date_date_element' is 'datetime' since it's also using an invalid format. If not open a dedicated new issue to fix it
- Provide tests covering those changes
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Comments
Comment #2
macsim commentedComment #3
quietone commentedComment #4
cilefen commentedThe code throwing the exception is:
It is in DateTimePlus::createFromFormat. This code returns `false` in the PHP REPL:
So I think your time format string is invalid to the \DateTime library. I hope this helps.
Comment #5
cilefen commentedActually, 'now' can never work. It's just the case that the format has to match the input. What is the input in the cases where this doesn't work?
This works:
\DateTime::createFromFormat('Y-m-d H:i', '2008-06-01 12:50')I think
\Tis totally unsupported. Anyway, this is PHP's standard library, not Drupal.Comment #6
macsim commentedWell then it seems that there's a bug somewhere
In
Datetime::processDatetime()doc says:And in
DateFormatterInterface::format():No matter what the value I give to that
"#date_date_format"("long","short","medium",'html_datetime',"d/m/Y H:i","Y-m-d H:i",'d/m/Y',Y-m-d, etc.) - and I've got the same result if I don't set that"#date_date_format"(which leads to aY-m-ddefault value) :Datetime::valueCallback()with the following values:$input=$date_input="2025-02-09T12:00"$time_input=""$date_format="Y-m-d\TH:i:sO"$time_format="""The date cannot be created from a format."Comment #7
macsim commentedComment #8
quietone commentedChanges are made on on 11.x (our main development branch) first, and are then back ported as needed according to the Core change policies.
Comment #9
macsim commentedSorry quietone #8 I didn't realised I changed the version
Ok now I undestand why I've got this behavior no matter what I pass to
'#date_date_format'It's because of the
Datetime::valueCallback()variables attributions made just before the try/catch:$date_format = $element['#date_date_format'] != 'none' ? static::getHtml5DateFormat($element) : '';and as cilefen said in #4,
'html_datetime'(ie."Y-m-d\TH:i:sO") is not a valid format to pass to\DateTime::createFromFormat()Comment #11
macsim commentedMR provides a fix for the
Datetime::ValueCallback()method.Few thoughts:
- We should maybe find a way to fix it in a less-dirty way (ie. not with that ugly "if" condition)
- It only fixes the problem for the
datetime-localelement butdatetimeelement also uses the'html_datetime'format and would break the same way-
doneDatetime::processDatetime()needs to be updated as well,$element['#value']is now not correct to display the submitted value in the input- We should introduce tests
Comment #12
macsim commentedComment #13
smustgrave commentedAssuming NR for feedback for approach? Could the issue summary be filled out to clear up for proposed solution. Easier for reviews
I'm 50/50 that this should be fixed here too or could be it's own issue. Guess it depends if the code is the same spot.
100% thanks for tagging for that!
Comment #14
macsim commented#13 Yes I wanted some feedback, thank you.
I updated the issue summary ; hopefully it is clear enough and will help resolving the issue.
Comment #15
macsim commentedComment #16
macsim commentedComment #17
macsim commentedAdded kernel tests for datetime-local
Comment #18
smustgrave commentedLeft comments on MR.
Comment #19
macsim commentedThanks smustgrave.
I applied your suggestions except for the first one since it's how it's done on other core KernelTests.
IMHO it will be easier to maintain the "form + tests" mix rather than to declare the form in another location even if it sounds prettier.
Comment #20
smustgrave commentedThanks, yea personally think it's messy and think the current examples of it aren't good standards. So I'll leave in review for others to take a look.
Comment #21
macsim commentedComment #22
smustgrave commentedWon't hold it up longer, will see what the committers think.
Comment #25
catchAgreed with the form being defined in the kernel test - it keeps everything in one place vs. split between the test and the test module. It's not possible to do this for functional tests, but for kernel/unit tests the more that is kept together the better IMO.
Committed/pushed to 11.x, thanks!