Datetime element support for #suffix key leads to #suffix being duplicate. Once after date and once after time.
Ex.:
$form['datetime'] = [
'#type' => 'datetime',
'#default_value' => date('Y-m-d H:i:s'),
'#ajax' => array(
'callback' => array($callback_object, 'datetimeCallback'),
),
'#suffix' => '<div id="ajax_datetime_value">No datetime yet selected</div>',
];
Results in :
Insert #prefix part before datetime, and #suffix part after it.
#prefix and #suffix should appear once only.
None
| Comment | File | Size | Author |
|---|---|---|---|
| #16 | interdiff-06-16.txt | 1.61 KB | mpdonadio |
| #16 | 2783617-16.patch | 4.39 KB | mpdonadio |
| #16 | 2783617-16-test-only.patch | 3.86 KB | mpdonadio |
| #6 | avoid_duplicate_prefix_suffix--2783617-6.patch | 3.9 KB | dom. |
| #2 | after_patch.png | 3.54 KB | dom. |
Comments
Comment #2
dom. commentedPatch attached.
Before patch screenshot is in issue summary.
After screenshot:
Comment #3
mpdonadioThanks for working out all of these datetime related FAPI bugs.
Think we need a test here to make sure the #prefix and #suffix appear exactly once to prevent a regression if we rework the wrapper (eg, #1918994: Improve Datetime and Daterange Widget accessibility).
I am also scratching my head here. Not seeing similar unsets for other form elements that have wrappers? What is different about this one?
Comment #4
mpdonadioSpent some time looking at this one. Did some testing with other form and render elements that have wrappers (mainly looked at checkboxes and dropbuttons).
The other elements seem to build up the wrapped contents individually, while this has a blind
in template_preprocess_datetime_form(), which is what is causing the double prefix/suffix. The render engine handling those directly, but the other elements that would be duplicated (like #title) are handled by the templates themselves. So, I think this fix does look like it is correct.
Setting NW, because I think this needs a test to show the problem and to prevent a regression. Test should cover both 'datetime' and 'datelist' elements (it happens with both), that the '#prefix' and '#suffix' appear exactly once. Should also probably test that the '#prefix' and '#suffix' are the the right places.
Comment #6
dom. commentedHere are new patchs containing test (and test-only version).
It check the presence of only one suffix and prefix for both date and datetime elements.
NOTE: the issue occurs only with datetime element, not simple date.
The full patch also includes correction.
The patch is design to merge with : #2782831: Date FAPI element type do not support #attributes meaning that the test form and DateTest file are the same in both.
Comment #8
dom. commentedBack to "need review" : test--only fails as expected, full patch is successfull at test-bot !
Comment #9
dom. commentedAdding issue tag, hoping for review :)
Comment #10
hazaTested the patch, and I can confirm it is working as expected.
On a side note, if we use a
#prefixand a#titlein the datetime element, the title is set to be inline.No sure if we need to deal with that here. (I guess, no)
Comment #13
k4v commentedWorks for me too.
Comment #14
xjmHm the bug is pretty clear but a bugfix that relies on an unset in a preprocess to work around code elsewhere seems... wrong. Since I don't have an actionable suggestion on what else to do I'll ping the frontend framework managers here to see what the think. Thanks!
Comment #15
xjmActually, inaugurating a new tag.
Comment #16
mpdonadioWe need to also add test coverage for the 'datelist' element, too, since that is affected.
Comment #18
tacituseu commented@frontend framework manager:
I feel like this and other issues with complex (multi element/input) widgets stem from lack of generic way of dealing with them in core, which results in:
- custom
'#theme_wrapper's ('datetime_wrapper','checkboxes','radios')- nested
'.form-item's and'.form-wrapper's- changing structure issues (label and description sometime inside
'form-item', sometimes outside)Tried looking for some clear definition of what exactly
'.form-item'and'.form-wrapper'is without any luck.Shouldn't there be one more level to all of this like
'.form-element'(or'.form-input') ?So it would become
'.form-item > .form-element > input', and there can be multiple'.form-element's inside'.form-item', and multiple'input's inside'.form-element', but never nested'.form-item's or'.form-element's.Comment #19
tacituseu commentedNoticed there's also
CompositeFormElementTraitwith'.form-composite', so I guess it's meant to be'.form-composite > .form-item > input'.Comment #21
lauriiiIs this fixed as a results of #2346893: Duplicate AJAX wrapper around a file field?
Comment #22
lauriiiSeems like the test only patch is now passing. Marking as a duplicate for #2346893: Duplicate AJAX wrapper around a file field.
Comment #23
lauriii