Problem/Motivation

[This section needs to describe which components aren't accessible (datetime widgets and/or datetime render elements), and describe why the current implementation isn't accessible.]

Proposed resolution

Change date + time output to include fieldsets, particularly around select lists #501428-141: Date and time field type in core

The output HTML will look something like this (in pseudocode):

<fieldset>
  <legend id="js_1">Event start</legend>
  <input id="js_2" aria-labelledby="js_1 js_2" aria-label="date" type="date" />
  <input id="js_3" aria-labelledby="js_1 js_3" aria-label="time" type="datetime" />
</fieldset>

We use aria-labelledby on the individual inputs to composite a label, referencing the text of the legend and the input itself (a self-referencing label), yielding an accessible label like: "Event start, date, edit text"

http://webaim.org/techniques/forms/controls
http://www.nomensa.com/blog/2010/three-rules-for-creating-accessible-forms/
http://www.w3.org/TR/2011/WD-html5-author-20110809/spec.html#the-fieldse...

Remaining tasks

TBD.

User interface changes

TBD.

API changes

None.

Data model changes

None.

Files: 
CommentFileSizeAuthor
#147 Screen Shot 2016-08-19 at 8.37.19 PM.png34.79 KBmpdonadio
#147 interdiff-146-147.txt2.95 KBmpdonadio
#147 1918994-147.patch11.23 KBmpdonadio
#146 1918994-146.patch10.26 KBmpdonadio
#142 date-fieldset2.png23.13 KByoroy
#142 date-fieldset.png54.08 KByoroy
#138 Screen Shot 2016-04-14 at 9.03.47 PM.png33.79 KBmpdonadio
#137 datetime-aria-1918994-137.patch10.25 KBvprocessor
#137 datetime-aria-1918994-137.interdiff.txt1.61 KBvprocessor
#131 datetime-aria-1918994-131.patch8.64 KBvprocessor
#131 datetime-aria-1918994-131.interdiff.txt365 bytesvprocessor
#127 datetime-aria-1918994-127.patch8.43 KBrteijeiro
#123 datetime-aria-1918994-107.patch49.22 KBnabiyllin
#98 datetime-aria-1918994-98.patch9.37 KBTim Bozeman
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch datetime-aria-1918994-98.patch. Unable to apply patch. See the log in the details link for more information. View
#97 Screen Shot 2014-08-10 at 11.57.31 AM.png125.04 KBmgifford
#95 Selection_040.png39.53 KBTim Bozeman
#95 Selection_039.png48.93 KBTim Bozeman
#95 interdiff.txt5.62 KBTim Bozeman
#95 datetime-aria-1918994-95.patch9.05 KBTim Bozeman
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 74,972 pass(es). View
#92 Screen Shot 2014-06-20 at 9.04.01 AM.png151.55 KBmgifford
#92 1918994-psr4-reroll-2_1-92.patch4.83 KBmgifford
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 73,041 pass(es). View
#84 1918994-psr4-reroll-2.patch5.83 KBmgifford
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,999 pass(es). View
#81 1918994-psr4-reroll.patch5.86 KBxjm
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 1918994-psr4-reroll.patch. Unable to apply patch. See the log in the details link for more information. View
#80 Date-ARIA.png129.04 KBmgifford
#80 DateTime-Fieldset-ARIA.png138.32 KBmgifford
#79 interdiff.txt712 bytesTim Bozeman
#79 datetime.fieldset.79.patch5.99 KBTim Bozeman
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,614 pass(es). View
#78 Selection_024.png29.5 KBTim Bozeman
#76 interdiff.txt872 bytesTim Bozeman
#76 datetime.fieldset.76.patch5.98 KBTim Bozeman
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,689 pass(es). View
#72 datetime.fieldset.72.patch5.13 KBTim Bozeman
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,686 pass(es). View
#67 interdiff.txt1.32 KBTim Bozeman
#67 datetime.fieldset.67.patch6.13 KBTim Bozeman
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,467 pass(es), 1 fail(s), and 0 exception(s). View
#65 datetime.fieldset.65.patch5.61 KBmgifford
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,319 pass(es), 3 fail(s), and 0 exception(s). View
#63 interdiff.txt2.8 KBsun
#63 datetime.fieldset.63.patch2.24 KBsun
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,393 pass(es). View
#62 date_fieldset-1918994-62.patch3.71 KBmgifford
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,319 pass(es). View
#59 date_fieldset-1918994-59.patch3.65 KBTim Bozeman
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,414 pass(es). View
#59 Selection_017.png79.38 KBTim Bozeman
#58 Selection_016.png50.78 KBTim Bozeman
#56 date_fieldset-1918994-56.patch4.07 KBTim Bozeman
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,308 pass(es), 1 fail(s), and 0 exception(s). View
#54 date_fieldset-1918994-54.patch3.12 KBmgifford
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 25,211 pass(es), 3,267 fail(s), and 3,822 exception(s). View
#52 date_fieldset-1918994-51.patch2.93 KBTim Bozeman
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 1,686 pass(es), 1,074 fail(s), and 4,260 exception(s). View
#49 date_fieldset-1918994-49.patch2.13 KBTim Bozeman
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,271 pass(es), 1 fail(s), and 0 exception(s). View
#48 date_fieldset-1918994-48.patch1.89 KBTim Bozeman
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,402 pass(es). View
#47 double-date.png117.57 KBmgifford
#46 authoring_information_fieldset.png68.32 KBTim Bozeman
#41 date_fieldset-1918994-41.patch2.21 KBTim Bozeman
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 68,242 pass(es), 1 fail(s), and 0 exception(s). View
#40 date_fieldset-1918994-40.patch2.4 KBTim Bozeman
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 68,064 pass(es). View
#39 date_fieldset-1918994-39.patch914 bytesmgifford
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 68,165 pass(es). View
#39 date_fieldset.png78.35 KBmgifford
#38 legend-match-label.png82.17 KBmgifford
#37 date_fieldset-1918994-37.patch1.38 KBmgifford
PASSED: [[SimpleTest]]: [MySQL] 59,787 pass(es). View
#34 Screen Shot 2013-09-21 at 11.36.04 AM.png122.76 KBmgifford
#33 date_fieldset-33.patch1.37 KBmgifford
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch date_fieldset-33.patch. Unable to apply patch. See the log in the details link for more information. View
#30 Date-Without-Patch.png160.35 KBmgifford
#30 Date-With-Patch.png131.86 KBmgifford
#29 date_fieldset-29.patch870 bytesfalcon03
PASSED: [[SimpleTest]]: [MySQL] 58,701 pass(es). View
#22 fieldsets-with-date-1918994-22.patch1.33 KBmgifford
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch fieldsets-with-date-1918994-22.patch. Unable to apply patch. See the log in the details link for more information. View
#15 DateField with Date and Time Widget Before Patch.PNG47.42 KBcwarsaw
#15 DateField with Select List Widget Before Patch.PNG47.05 KBcwarsaw
#15 DateField with Date and Time Widget After Patch.PNG45.73 KBcwarsaw
#15 DateField with Select List Widget After Patch.PNG39.6 KBcwarsaw
#13 FieldSetNotWorking.png249.01 KBmgifford
#12 fieldsets-with-date-1918994-12.patch1.17 KBmgifford
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch fieldsets-with-date-1918994-12.patch. Unable to apply patch. See the log in the details link for more information. View
#8 datetimefieldsetbefore1.png74.76 KBandymartha
#8 datetimefieldsetafter1.png72.47 KBandymartha
#1 fieldsets-with-date-1918994-1.patch1.14 KBmgifford
PASSED: [[SimpleTest]]: [MySQL] 56,160 pass(es). View
#1 fieldset-with-date-needs-css.png157.43 KBmgifford

Comments

mgifford’s picture

Title: Add Fieldsets to Date » Improve Accessibility with Date field by Using Fieldsets
Status: Active » Needs review
FileSize
157.43 KB
1.14 KB
PASSED: [[SimpleTest]]: [MySQL] 56,160 pass(es). View

The CSS definitely needs work, but it's built against Core (now) using the example I provided in comment #141 of the parent issue.

Oh ya, there is another issue looking to remove fieldsets and I'm looking for clarification on that #1467712-97: Make it possible to disable fieldset for date field

So far all that's been described is formatting problems (which can be fixed with CSS).

Status: Needs review » Needs work

The last submitted patch, fieldsets-with-date-1918994-1.patch, failed testing.

swentel’s picture

Component: field system » datetime.module

Moving to right component

mgifford’s picture

Status: Needs work » Needs review

#1: fieldsets-with-date-1918994-1.patch queued for re-testing.

mgifford’s picture

Seems to work fine in simplytest.me.

mgifford’s picture

Issue tags: -date, -accessibility

#1: fieldsets-with-date-1918994-1.patch queued for re-testing.

mgifford’s picture

Issue tags: +date, +accessibility

#1: fieldsets-with-date-1918994-1.patch queued for re-testing.

andymartha’s picture

Ok, so this patch does nothing for the ui (?) but is for accessibility. After applying patch fieldsets-with-date-1918994-1.patch from mgifford in #1 on a fresh install of Drupal 8, it applies a fieldset and legend to date select lists on a content create/edit screen. Correct me if I'm wrong but it seems like it works well. See screenshots below for markup.
datetimefieldsetbefore1.png

datetimefieldsetafter1.png

mgifford’s picture

Thanks @andymartha - that looks good to me, but I wrote the patch.

Not sure what else would be involved for it to be RTBC. Seems like it's already a strong pattern now in D8 to wrap elements like this in fieldsets.

andymartha’s picture

Status: Needs review » Reviewed & tested by the community

I am switching the status to reviewed if the patch is supposed to change the markup for fieldsets, which I confirm it does.

Bojhan’s picture

Status: Reviewed & tested by the community » Needs work

Can we use the invisible fieldset styling here?

mgifford’s picture

FileSize
1.17 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch fieldsets-with-date-1918994-12.patch. Unable to apply patch. See the log in the details link for more information. View

Until this patch gets in Core, this will need to be applied with http://drupal.org/files/cardinality-label-1932074.38.patch

Or, this would need to be added:

+/* Fieldset variant */
+fieldset.fieldset-no-border {
+  border: none;
+  padding: 0;
+}
+fieldset.fieldset-no-border legend {
+  text-transform: none;
+}
mgifford’s picture

Status: Needs work » Needs review
FileSize
249.01 KB

Well, this should work. I get this output:

<fieldset class="fieldset-no-border">
<legend>DAte</legend>
<div id="edit-field-date-und-0-value" class="container-inline">
</fieldset>

And the CSS is there and it's working fine in the related patch.

Image of Field Set Not Working

Unless core/modules/field_ui/field_ui.admin.css isn't loaded when not administering a field....

In which case this CSS should be moved somewhere else.

Bojhan’s picture

Strange, I have no idea why system would overwrite it there. Perhaps system should provide a class for this too?

cwarsaw’s picture

I have reviewed this patch. I verified that before the patch, the fieldset is not there. I verified after the patch is applied, the fieldset is there correctly. I verified this patch with both the Date and Time widget and the Select List widget. See attached images.

I have also confirmed that the border is visible. If the intention is for there to be no border, then this is a separate CSS issue.

Since that doesn't impact the accessibility of this control, I don't believe that should prevent this patch from being committed to core.

mgifford’s picture

That CSS is going to be provided by #504962: Provide a compound form element with accessible labels - so I don't see any reason not to mark this RTBC.

@cwarsaw want to mark this RTBC?

mgifford’s picture

Issue tags: -date, -accessibility

Status: Needs review » Needs work
Issue tags: +date, +accessibility

The last submitted patch, fieldsets-with-date-1918994-12.patch, failed testing.

mgifford’s picture

Issue tags: +fieldset

tagging

mgifford’s picture

Status: Needs work » Needs review
Issue tags: -date, -accessibility, -fieldset

Status: Needs review » Needs work
Issue tags: +date, +accessibility, +fieldset

The last submitted patch, fieldsets-with-date-1918994-12.patch, failed testing.

mgifford’s picture

Status: Needs work » Needs review
FileSize
1.33 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch fieldsets-with-date-1918994-22.patch. Unable to apply patch. See the log in the details link for more information. View

I've rerolled the patch, but is there only one date widget now? Could only see one here:
admin/structure/types/manage/article/fields/node.article.field_date/field

mgifford’s picture

bowersox’s picture

Issue tags: +TwinCities
mgifford’s picture

Issue tags: -date, -accessibility, -fieldset, -TwinCities

Status: Needs review » Needs work
Issue tags: +date, +accessibility, +fieldset, +TwinCities

The last submitted patch, fieldsets-with-date-1918994-22.patch, failed testing.

jessebeach’s picture

nod_’s picture

falcon03’s picture

Status: Needs work » Needs review
FileSize
870 bytes
PASSED: [[SimpleTest]]: [MySQL] 58,701 pass(es). View

Ok, maybe this issue could be fixed with a patch as simple as this one.

I've tested it in Voiceover and it improves accessibility for both the standard widget and the select list widget.

However, we have a problem when multiple values can be entered in the field. The uix for a screen reader user in that case is pretty bad. But, maybe fixing that is out of scope for this issue (the problem is not introduced by this patch).

mgifford’s picture

Status: Needs review » Needs work
FileSize
131.86 KB
160.35 KB

Unfortunately this patch changes the styling.

Without the patch the date/time are right next to each other:
Date Without Patch

But with the patch it's clearly in a fieldset.
Date With Patch

That still needs #1932074: Tags Includes Label which isn't Associated with an Input Form to pull in the CSS to see that this isn't styled.

falcon03’s picture

@mgifford, would leveraging the css class eventually introduced by #2002336: Introduce a CSS class to hide borders of fieldset elements be enough to solve the styling issues?

aaronbauman’s picture

Adding concerns via #1467712: Make it possible to disable fieldset for date field:
If the input is actually one field, it should not be wrapped in a fieldset.

#22 accomplishes this, i think, but #29 always forces the input to be wrapped by a fieldset.

mgifford’s picture

FileSize
1.37 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch date_fieldset-33.patch. Unable to apply patch. See the log in the details link for more information. View

@aaronbauman - Yes, #22 did check for multiple fields before applying the fieldset. I do think this is a lot harder to do though with twig. Not that this doesn't mean it shouldn't be done with twig.

It might just be cleaner if rather than trying to do the logic in twig, we simply have datetime-single-wrapper-html.twig & datetime-multiple-wrapper-html.twig.

I'm not familiar with how to do this. $element['#date_time_element'] & $element['#date_date_element'] are the only elements we need to consider. If they are both there, it should be wrapped in a fieldset.

I think #date_date_element = 'datetime'; produces more than one field mind you.

@falcon03 - I think so. I added it here and am testing on STM. Next post should include a screenshot.

Latest patch just includes the problematic code from #29 with <fieldset class="fieldset-no-border"> plus the CSS to style it.

mgifford’s picture

I didn't notice this before, but this patch also produces a fieldsets inside of fieldsets. That's obviously not what we want.

<div id="edit-field-date-wrapper" class="field-type-datetime field-name-field-date field-widget-datetime-default form-wrapper">
<div id="field-date-add-more-wrapper">
<fieldset class="fieldset-no-border">
<legend class="label"> Date </legend>
<fieldset class="fieldset-no-border">
<div id="edit-field-date-0-value" class="container-inline">
<div class="form-item form-type-date form-item-field-date-0-value-date">
<label class="visually-hidden" for="edit-field-date-0-value-date">Date</label>
<input id="edit-field-date-0-value-date" class="form-date" type="date" size="12" value="" name="field_date[0][value][date]" title="Date (i.e. 2013-09-21)">
</div>
<div class="form-item form-type-date form-item-field-date-0-value-time">
</div>
</fieldset>
<div class="description">This is where you enter the date.</div>
</fieldset>
</div>
</div>

That being said, I think that with the CSS applied, it looks right:

Screen Shot 2013-09-21 at 11.36.04 AM.png

mgifford’s picture

Issue summary: View changes
Status: Needs work » Needs review

Bot didn't seem to run earlier.

Status: Needs review » Needs work

The last submitted patch, 33: date_fieldset-33.patch, failed testing.

mgifford’s picture

Status: Needs work » Needs review
FileSize
1.38 KB
PASSED: [[SimpleTest]]: [MySQL] 59,787 pass(es). View

re-roll.

mgifford’s picture

Status: Needs review » Needs work
Issue tags: +Novice
FileSize
82.17 KB

Legend needs to match labels <legend class="label"> date </legend>

The legend doesn't match the styling of the labels:
Non-bold legend

mgifford’s picture

Status: Needs work » Needs review
Related issues: +#2192419: Use a WCAG-compliant fieldset (fieldgroup) for #type radios/checkboxes
FileSize
78.35 KB
914 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 68,165 pass(es). View

Much simpler...
Properly emphasized date legend.

Tim Bozeman’s picture

FileSize
2.4 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 68,064 pass(es). View

@mgifford I brought the changes from #2244923: The help text for datetime field needs an ID for screen readers. and closed the issue. Thanks again for walking me through #2126761: The body field summary textarea indicates it has a description with aria-describedby attribute, but the DOM id value points to a non-existent node!

The additions to this patch:

  • add the aria-described by attribute to the datefield input
  • associate the description with with the input by adding an ID to to the description
Tim Bozeman’s picture

FileSize
2.21 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 68,242 pass(es), 1 fail(s), and 0 exception(s). View

This change adds the aria-describedby attribute to the fieldset <fieldset class="fieldgroup form-composite"{{ attributes }}> with an else on line 249 of datetime.module, but I'm not sure its the best way to do it.

  else {
    // Add the aria-describedby to the fieldset.
    $variables['attributes']['aria-describedby'] = $element['#id'] . '--description';
  }

Status: Needs review » Needs work

The last submitted patch, 41: date_fieldset-1918994-41.patch, failed testing.

mgifford’s picture

Getting closer! It's giving me some confusing results though. Fieldsets within fieldsets.

No aria-describedby="" in the first one, but there in the second.

<fieldset id="edit-field-date-0--description" class="fieldgroup form-composite">
<legend class="label"> date </legend>
<fieldset class="fieldgroup form-composite" aria-describedby="edit-field-date-0-value--description">
<div id="edit-field-date-0-value" class="container-inline">
<div class="form-item form-type-date form-item-field-date-0-value-date">
<label class="visually-hidden" for="edit-field-date-0-value-date">Date</label>
<input id="edit-field-date-0-value-date" class="form-date" type="date" size="12" value="" name="field_date[0][value][date]" title="Date (i.e. 2014-04-22)">
</div>
<div class="form-item form-type-date form-item-field-date-0-value-time">
</div>
</fieldset>
<div id="edit-field-date-0--description" class="container-inline description">Well, what do we have here?</div>
</fieldset>

I didn't see that duplicate previously, but could have missed it....

Tim Bozeman’s picture

Hmm, I went back to patch #39 and it looks like there was double fieldsets there too. My guess is that both the date and time field are hitting datetime-wrapper.html.twig twice as rendered elements (content).

mgifford’s picture

That makes sense...

We can dig into this a bit further. If we can't get the duplicate resolved we can move the aria-describedby issue back to #2244923: The help text for datetime field needs an ID for screen readers. and reopen it.

Tim Bozeman’s picture

The date in the Authoring Information section on the right isn't in a double fieldset. Why is that one different from the content type field?

mgifford’s picture

FileSize
117.57 KB

@Tim Bozeman I'm guessing that the authoring information doesn't leverage the datetime.module

It would make sense that it does, but apparently........

Also interesting, set up a date field with 2 or more date options in it. It then has the duplicate fieldsets for each row of date fields (4 or more) but only a single description field.

With multiple values the aria-describedby needs to be in the table field-date-values table or maybe <h4 class="label">Date</h4>

Definitely have to dig into the datetime.module more though.

EDIT: The description for multi-part forms comes out in core/modules/system/templates/field-multiple-value-form.html.twig

Tim Bozeman’s picture

FileSize
1.89 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,402 pass(es). View

There doesn't seem to be a way to delete a row of dates...

I added the aria-describedby to the table field-date-values. I'm not sure what to do about the fieldset. Cottser suggested using a general form field wrapper instead of a date-specific wrapper.

Tim Bozeman’s picture

Status: Needs work » Needs review
FileSize
2.13 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,271 pass(es), 1 fail(s), and 0 exception(s). View

Cottser said to use a more general #theme_wrapper in DateTimeDefaultWidget.php The results are pretty magical IMO.

Status: Needs review » Needs work

The last submitted patch, 49: date_fieldset-1918994-49.patch, failed testing.

mgifford’s picture

Looking great for a single date element. It gracefully degrades for multiple date elements. I think you've got it! I think if we can work through the test this should be good. I think we should deal with more complicated multiple date options in a different issue. It's an edge case for the most part.

EDIT: Pretty neat patch btw. Oh ya, we should eliminate the boundary. Also, we should be using the CSS class fieldgroup so that it doesn't have the border.

Tim Bozeman’s picture

Assigned: Unassigned » Tim Bozeman
Status: Needs work » Needs review
FileSize
2.93 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 1,686 pass(es), 1,074 fail(s), and 4,260 exception(s). View

I googled what this line of DateTimeFieldTest.php:101 is doing.
$this->assertFieldByXPath('//*[@id="edit-' . $field_name . '-wrapper"]/h4/span', '*', 'Required markup found');
It seems that Xpath is some xml element grabbing thing and its looking for the associated h4 or span? I removed that line from the test and it seems to pass simple test. I wonder if it's important...

Status: Needs review » Needs work

The last submitted patch, 52: date_fieldset-1918994-51.patch, failed testing.

mgifford’s picture

Status: Needs work » Needs review
FileSize
3.12 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 25,211 pass(es), 3,267 fail(s), and 3,822 exception(s). View

This patch doesn't actually add the fieldgroup with '#attributes' => array('class' => array('fieldgroup')), it really should.

I'm hoping this fixes the bot's concerns by changing where the tests are looking:
$this->assertFieldByXPath('//*[@id="edit-' . $field_name . '-wrapper"]/fieldset/legend/span', '*', 'Required markup found');

Status: Needs review » Needs work

The last submitted patch, 54: date_fieldset-1918994-54.patch, failed testing.

Tim Bozeman’s picture

Status: Needs work » Needs review
FileSize
4.07 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,308 pass(es), 1 fail(s), and 0 exception(s). View

Should I wrap the field-group in an if type == datetime ?

Status: Needs review » Needs work

The last submitted patch, 56: date_fieldset-1918994-56.patch, failed testing.

Tim Bozeman’s picture

FileSize
50.78 KB

hmm...
should be working
looks like it should be working to me :(

Tim Bozeman’s picture

Status: Needs work » Needs review
FileSize
79.38 KB
3.65 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,414 pass(es). View

The test originally said

$this->assertFieldByXPath('//*[@id="edit-' . $field_name . '-wrapper"]/h4/span', '*', 'Required markup found');

It had an extra span tag so I added an extra span too and it passes simple test locally.

$this->assertFieldByXPath('//*[@id="edit-' . $field_name . '-wrapper"]/fieldset/legend/span/span', '*', 'Required markup found');
mgifford’s picture

That's great progress! The tests pass.

Now how do we deal with the styling so that there's no box around it?

fieldgroup removes it, but it also makes the title ALL CAPS.

It's also not clear to me how to pass this attribute through.

Tim Bozeman’s picture

I added the field-group class to the fieldset tag in DateTimeDefaultWidget.php:59 the CSS acting on it is

fieldset {
border: 1px solid #c0c0c0;
...
}

in normalize.css. If you disable it a thicker border shows up. I don't see a good attribute to grab to over-ride the rule.

mgifford’s picture

FileSize
3.71 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,319 pass(es). View

Ok, this works for datetime, but if you choose just date in the Field settings, there shouldn't be a fieldset admin/structure/types/manage/article/fields/node.article.field_date/field

Fieldsets shouldn't be wrapping a single element.

sun’s picture

FileSize
2.24 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,393 pass(es). View
2.8 KB

The theme hook for fieldsets is registered by System module already, and we should be able to re-use the #pre_render callback of #type radios/checkboxes here.

At least I hope so. :-)

Tim Bozeman’s picture

Assigned: Tim Bozeman » Unassigned
Status: Needs review » Needs work

I don't see how to re-use the #pre_render callback of #type radios/checkboxes.

I read through ElementInfoInterface::getInfo and I see that form_pre_render_conditional_form_element adds the fieldset like we want.

How do we only wrap datetime and not just date by itself? I studied how radios and checkboxes work in system_element_info(), but throw in DateTimeDefaultWidget.php and I can't seem to connect the dots.

I want to add $types['date'] to datetime_element_info(), but it starts making duplicate fields.

mgifford’s picture

Status: Needs work » Needs review
FileSize
5.61 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,319 pass(es), 3 fail(s), and 0 exception(s). View

So we've got two use cases here. One needs fieldsets (date & time) one doesn't (date only). Both need to address the aria-describedby issue of linking the help text to the Label or Legend. With the existing patches we've been working with, I think this combination works.

Status: Needs review » Needs work

The last submitted patch, 65: datetime.fieldset.65.patch, failed testing.

Tim Bozeman’s picture

FileSize
6.13 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,467 pass(es), 1 fail(s), and 0 exception(s). View
1.32 KB

@mgifford nice!!
The date only field was missing the aria-describedby attribute so I added it to template_preprocess_container()

  // Add the aria-describedby attribute for date fields.
  if (isset($element['widget']['#attributes']['aria-describedby'])) {
    $element['#attributes']['aria-describedby'] = $element['widget']['#attributes']['aria-describedby'];
  }

Should we add another date only field to DateTimeFieldTest.php?

mgifford’s picture

Yes.. We'd want the "add another date only field to DateTimeFieldTest.php"

Can you take that on?

Glad you found the template_preprocess_container() to add the aria-describedby.

mgifford’s picture

Status: Needs work » Needs review
Tim Bozeman’s picture

Yep, I'll try adding the test field :)

Status: Needs review » Needs work

The last submitted patch, 67: datetime.fieldset.67.patch, failed testing.

Tim Bozeman’s picture

Issue summary: View changes
FileSize
5.13 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,686 pass(es). View

The patch didn't apply so I rerolled it.

I think this patch does what we need it too and passes simpletest, but the test doesn't reflect the changes.

DateTimeFieldTest.php:65 sets the type as 'settings' => array('datetime_type' => 'date'), which I think hits the switch statement DateTimeDefaultWidget.php:61 case DateTimeItem::DATETIME_TYPE_DATE: which sets the theme_wrapper to datetime_wrapper. That is allowing the test:101 line $this->assertFieldByXPath('//*[@id="edit-' . $field_name . '-wrapper"]/h4[contains(@class, "form-required")]', TRUE, 'Required markup found'); to find the label under the h4. Since this test is testing date + time it should be looking under /fieldset/legend/span, but again because the datetime_type is set to date it's hitting the wrong wrapper.

I tried to change datetime_type to datetime, but I get AJAX errors when I run the test.

An AJAX HTTP error occurred.
HTTP Result Code: 500
Debugging information follows.
Path: /batch?id=22&op=do_nojs&op=do
StatusText: Internal Server Error
ResponseText:

I tried adding case DateTimeItem::DATETIME_TYPE_DATETIME: to the widget. Doesn't help...

Tim Bozeman’s picture

Status: Needs work » Needs review
mgifford’s picture

It's looking good from an output perspective.. I do think it's worth adding the display: block; so that the help text starts on a new line:

.form-composite > .fieldset-wrapper > .description, .form-item .description {
    display: block;
    font-size: 0.85em;
}

The aria-describedby looks great and it's all wrapped properly in a fieldset. I wonder who we could get to help improve the tests....

Tim Bozeman’s picture

Assigned: Unassigned » Tim Bozeman

When in doubt Cottser out. I brought it up during office hours and Cottser walked me through simpletesting.

Tim Bozeman’s picture

FileSize
5.98 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,689 pass(es). View
872 bytes

Okay, I guess the test was closer than I thought. ¯\_(ツ)_/¯

I added this to DateTimeFieldTest.php:173 and I think that it covers all the changes from the patch.

$this->assertFieldByXPath('//*[@id="edit-' . $field_name . '-wrapper"]/fieldset/legend/span[contains(@class, "form-required")]', TRUE, 'Required markup found');

EDIT: @mgifford The help text seems to be showing up on a new line. I had to clear caches before to get them to show up right.

mgifford’s picture

Status: Needs review » Needs work

So close... Unfortunately, the aria-describedby property in the Date only implementation (of a date field) doesn't line up.

<div id="edit-field-date2-wrapper" class="field-type-datetime field-name-field-date2 field-widget-datetime-default form-wrapper" aria-describedby="edit-field-date2--description">

<div id="edit-field-date2-0--description" class="container-inline description">helpful date</div>

Tim Bozeman’s picture

FileSize
29.5 KB

Yes, that's not right... Hmm

Tim Bozeman’s picture

Status: Needs work » Needs review
FileSize
5.99 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,614 pass(es). View
712 bytes

For a single date field the output matches now.

<div class="field-type-datetime field-name-field-date field-widget-datetime-default form-wrapper" id="edit-field-date-wrapper" aria-describedby="edit-field-date-0--description">
<div class="container-inline description" id="edit-field-date-0--description">well, what have we here?</div>



and for date + time

<fieldset class="container-inline fieldgroup form-composite form-wrapper form-item" aria-describedby="edit-field-date-0--description" id="edit-field-date-0">
<div class="description" id="edit-field-date-0--description">well, what have we here?</div>
mgifford’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
138.32 KB
129.04 KB

Looks good. I do think we need to address when we have multiple items in a separate issue. For now though, dates are good!

EDIT: Followup issue added here #2273671: Allowed number of values more than 1 needs aria-describedby Support

DateTime-Fieldset-ARIA

Date-ARIA

xjm’s picture

FileSize
5.86 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 1918994-psr4-reroll.patch. Unable to apply patch. See the log in the details link for more information. View

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 81: 1918994-psr4-reroll.patch, failed testing.

mgifford’s picture

Status: Needs work » Needs review
mgifford’s picture

FileSize
5.83 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,999 pass(es). View
mgifford’s picture

I was having trouble posting this today.. I also re-rolled it on the plane. Hopefully the bot likes it better.

Tim Bozeman’s picture

Status: Needs review » Reviewed & tested by the community

Patch applies and the new attributes seem honky dory still.

alexpott’s picture

The changes here seem to be more far reaching than just date - can we confirm that we are only affecting what we think we are.

Tim Bozeman’s picture

The line if (isset($element['widget'][0]['#attributes']['aria-describedby'])) { is true for the title and tags field as well.

Tim Bozeman’s picture

Assigned: Tim Bozeman » Unassigned
Status: Reviewed & tested by the community » Needs work
Tim Bozeman’s picture

Assigned: Unassigned » Tim Bozeman

I don't think the container is the right place to put the ARIA

Tim Bozeman’s picture

Assigned: Tim Bozeman » Unassigned
mgifford’s picture

Status: Needs work » Needs review
FileSize
4.83 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 73,041 pass(es). View
151.55 KB

@Tim Bozeman - to try to move this issue along, I've taken out the essential aria stuff that was showing up where it shouldn't have been.

This is a slightly simpler patch that is really just building on your work. It will leave us in a good place to insert ARIA when we know where we want to insert it. Possibly even in a separate issue.

Here's a screenshot:

Tim Bozeman’s picture

Status: Needs review » Reviewed & tested by the community

Applies and sounds good to me. The patch changes the wrapper template to use a fieldset.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/datetime/src/Plugin/Field/FieldWidget/DateTimeDefaultWidget.php
@@ -69,6 +69,7 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen
+        $element['#pre_render'][] = 'form_pre_render_conditional_form_element';

But this not a conditional form element.

Tim Bozeman’s picture

Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Novice
FileSize
9.05 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 74,972 pass(es). View
5.62 KB
48.93 KB
39.53 KB

I removed the form_pre_render_conditional_form_element and added an extra template and preprocess function for date + time.
Date

Date + Time

mgifford’s picture

Issue tags: -TwinCities +TCDrupal 2014
mgifford’s picture

Status: Needs review » Needs work
FileSize
125.04 KB

I've got this up here for the sprint - http://se46427db0c1bf5b.s2.simplytest.me/

The date field here - http://se46427db0c1bf5b.s2.simplytest.me/node/add/article - shows the outline from the fieldset. We don't want to add this visual change.

Other than that it is working well.

Tim Bozeman’s picture

Status: Needs work » Needs review
FileSize
9.37 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch datetime-aria-1918994-98.patch. Unable to apply patch. See the log in the details link for more information. View
330 bytes

I added a rule to seven's style.css to hide the fieldset border.

.field-type-datetime .form-item {
  border: none;
}
mgifford’s picture

FileSize
7.84 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,160 pass(es), 13 fail(s), and 3 exception(s). View

That didn't apply for me.. Just rerolling with your css.

The last submitted patch, 98: datetime-aria-1918994-98.patch, failed testing.

The last submitted patch, 98: datetime-aria-1918994-98.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 99: datetime-aria-1918994-99.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 99: datetime-aria-1918994-99.patch, failed testing.

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 99: datetime-aria-1918994-99.patch, failed testing.

Tim Bozeman’s picture

Status: Needs work » Needs review
FileSize
9.38 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,850 pass(es). View

Poor testbot.
reroll of #98. I opened up the patch and added /css to the style path. Passes simple test locally.

Status: Needs review » Needs work

The last submitted patch, 106: datetime-aria-1918994-106.patch, failed testing.

Tim Bozeman’s picture

I don't get it... PDO exceptions and stuff? The fails seem like they are unrelated to this patch.

Status: Needs work » Needs review
Tim Bozeman’s picture

Issue summary: View changes
Tim Bozeman’s picture

Issue summary: View changes

The aria-describedby still needs to go on the date only field. Can we add it to the h4 or the div .container-inline? If we add it to #edit-field-date-time-wrapper div we have to go into template_preprocess_container() and, as alexpott pointed out, touch things that we don't intend to.

Or we could (dare I say?) add an extra wrapping div in datetime.wrapper.html.twig

Tim Bozeman’s picture

Issue summary: View changes
Status: Needs review » Needs work

I brought this up at core mentoring. No one likes the idea of adding templates. There's got to be a way to use a more general wrapper in the widget for date + time.

mgifford’s picture

Adding a div to the templates isn't the same as adding a template as far as I'm concerned.

What would it look like if we put in another div in datetime.wrapper.html.twig?

These are crappy examples, but thought I'd put it here to see where it goes:

{{ fieldset-aria-describedby-start }}<fieldset{{ attributes }}>
  {% if legend.title is not empty or required -%}
    {#  Always wrap fieldset legends in a SPAN for CSS positioning. #}
    <legend{{ legend.attributes }}><span class="{{ legend_span.attributes.class }}">{{ legend.title }}{{ required }}</span></legend>
  {%- endif %}
  <div class="fieldset-wrapper">
    {{ content }}
    {% if description.content %}
      <div{{ description.attributes }}>{{ description.content }}</div>
    {% endif %}
  </div>
</fieldset>{{ fieldset-aria-describedby-end }}

In the scheme of things, not having the aria-describedby is much less important than not having the fieldset for the dates.

Tim Bozeman’s picture

Assigned: Unassigned » Tim Bozeman

Makes sense to me!

Tim Bozeman’s picture

Assigned: Tim Bozeman » Unassigned
Issue tags: -TCDrupal 2014

Are we simply changing datetime-wrapper.html.twig to out put fieldsets or are we still trying to conditionally wrap date+time in fieldset and have date in a div like it is now?

It is still difficult to implement conditionally. The template it hit twice for every field rendered.

  1. The Date and time fields hit
  2. Then the title and description

I guess that's the wrapping part? Double hitting the tpl for one element confuses me. I don't see how to get the title and description inside the fieldset with the content.

mgifford’s picture

Version: 8.0.x-dev » 8.1.x-dev
Status: Needs work » Postponed

I think we're going to have to address this in 8.1.

mpdonadio’s picture

Probably. All of these old issues need to be gone through and reevaluated. On my todo list.

mgifford’s picture

Status: Postponed » Needs review
mgifford’s picture

Re-uploading for the bots.

Status: Needs review » Needs work

The last submitted patch, 119: datetime-aria-1918994-106.patch, failed testing.

mpdonadio’s picture

Issue tags: +Needs reroll
mpdonadio’s picture

This may need to be postponed on #2161337: Add a Date Range field type with support for end date and #2632040: Add ability to select a timezone for datetime field..

If it doesn't, I would love to get this going again. Those two patches are going to make the field widget a mess.

nabiyllin’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
49.22 KB

Status: Needs review » Needs work

The last submitted patch, 123: datetime-aria-1918994-107.patch, failed testing.

mpdonadio’s picture

Category: Bug report » Task
Issue tags: +Needs reroll

Looks like a bad re-roll. The patch went from about 9k to 49k.

Also changing this to a task, as there is no real bug to fix here.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

rteijeiro’s picture

Status: Needs work » Needs review
FileSize
8.43 KB

This is an attempt to re-roll this patch. I think it still needs some work. There are things that seem to be wrong but this is the best I can do right now.

Status: Needs review » Needs work

The last submitted patch, 127: datetime-aria-1918994-127.patch, failed testing.

jessebeach’s picture

Patch looks good. Just get it to pass tests and we'll be gtg.

vprocessor’s picture

Assigned: Unassigned » vprocessor
vprocessor’s picture

Assigned: vprocessor » Unassigned
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
365 bytes
8.64 KB

Hello guys,
diff & patch is ready

andypost’s picture

looks there's no BC possible

+++ b/core/includes/theme.inc
@@ -1721,6 +1767,10 @@ function drupal_common_theme() {
+    'datetime_fieldset_wrapper' => array(

+++ b/core/modules/system/templates/datetime-wrapper.html.twig
@@ -8,6 +8,7 @@
+ * - attributes: Attributes for the description field.

@@ -30,4 +31,4 @@
-{{ description }}
+<div{{ attributes }}>{{ description }}</div>

+++ b/core/modules/system/templates/fieldset.html.twig
@@ -16,6 +16,7 @@
+ * - content: The form element to be output.

@@ -52,6 +53,7 @@
+    {{ content }}

that requires change record

Status: Needs review » Needs work

The last submitted patch, 131: datetime-aria-1918994-131.patch, failed testing.

vprocessor’s picture

Assigned: Unassigned » vprocessor

checking error

andypost’s picture

Failed tests

Ensures that Stable overrides all relevant core templates.
+++ b/core/includes/theme.inc
@@ -576,6 +583,45 @@ function template_preprocess_datetime_wrapper(&$variables) {
+ * Prepares variables for datetime fieldset form wrapper templates.
...
+ * Default template: datetime-fieldset-wrapper.html.twig.

this should be copied to Stable theme

vprocessor’s picture

ok, will do that

vprocessor’s picture

Assigned: vprocessor » Unassigned
Status: Needs work » Needs review
FileSize
1.61 KB
10.25 KB

done

mpdonadio’s picture

Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs usability review, +needs accessibility review
FileSize
33.79 KB

I find the fieldset legend + the label on the input weird, but will defer to our UX/A11Y crew.

Also, patch doesn't seem to work with date-only versions of datetime fields.

jessebeach’s picture

Diff looks like it should produced the desired HTML. Could you just post a chunk in a comment for an example fieldset?

My head isn't in the nitty-gritty of Drupal theming code. Can someone who is more familiar with this bit just 'lgtm' on the PHP/Twig changes?

mpdonadio’s picture

#139 screenshot is from Seven on node add page, from a fresh install of 8.2.x.

jessebeach’s picture

From #138: "Also, patch doesn't seem to work with date-only versions of datetime fields."

Needs to be investigated.

yoroy’s picture

The issue summary is lacking a reason why. Linking to long overview articles elsewhere doesn't count :-)

Screenshot in context:

I did make the typo in that description but did not enter it twice. That doesn't happen in my d8 install without this patch so needs work there too.

Similar to #138, I think that fieldset legend + form label duplicating eachother looks bad. Even more when the field is required:

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

xjm’s picture

Title: Improve Accessibility with Date field by Using Fieldsets » Date fields labels are not accessible
Category: Task » Bug report

This is actually an accessibility bug.

mpdonadio’s picture

Priority: Normal » Major
Issue tags: +blocker

Bumping this to Major because it now blocks #2161337: Add a Date Range field type with support for end date.

mpdonadio’s picture

FileSize
10.26 KB

Reroll b/c #2546248: Use consistent style to mention HTML tags in code comments. Minor merge conflict in core/modules/system/templates/fieldset.html.twig

mpdonadio’s picture

Status: Needs work » Needs review
FileSize
11.23 KB
2.95 KB
34.79 KB

Here is a start.

Status: Needs review » Needs work

The last submitted patch, 147: 1918994-147.patch, failed testing.

jessebeach’s picture

@yoroy, I totally agree with your comment in #142. That double labelling is not ideal! There's a way around this. The output HTML will look something like this (in pseudocode):

<fieldset>
  <legend id="js_1">Event start</legend>
  <input id="js_2" aria-labelledby="js_1 js_2" aria-label="date" type="date" />
  <input id="js_3" aria-labelledby="js_1 js_3" aria-label="time" type="datetime" />
</fieldset>

We use aria-labelledby on the individual inputs to composite a label, referencing the text of the legend and the input itself (a self-referencing label), yielding an accessible label like: "Event start, date, edit text"

jhedstrom’s picture

Can somebody please update the issue summary with the exact fix needed? As @yoroy mentioned in #142, linking off to articles doesn't make clear what the proposed solution is.

jessebeach’s picture

Issue summary: View changes
mpdonadio’s picture

Why remove the label elements for the inputs all together instead of having them invisible (like it currently is)? All of the links in the IS reference labels as being important.

mpdonadio’s picture

Issue summary: View changes
Issue tags: +Needs tests

Few more related questions. Are we fixing datetime.module field widgets (the default one and/or the select list) and/or the datetime form element (what DateTimeDefaultWidget builds upon and can also be used in forms in non-field contexts) and/or the datelist form element (what DateTimeDatelistWidget builds upon)?

Are we not doing anything with date-only fields?

With the field widgets, how will these work with multi-valued fields?

IS still needs some TLC: we have a proposed solution but we don't have a clear explanation of what is wrong (why isn't the current implementation accessible?). Not trying to be a pain here, but this issue is hard and soft blocking a lot of other work right now.

mpdonadio’s picture

This is also mentioned in #2788359: datetime_wrapper improperly applied to DateTimeWidgetBase; I am fine with scoping this issue to also address that if we can address this in a timely manner. Review is against #146

  1. +++ b/core/modules/datetime/src/Plugin/Field/FieldWidget/DateTimeDefaultWidget.php
    @@ -64,6 +64,7 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen
           case DateTimeItem::DATETIME_TYPE_DATE:
             $date_type = 'date';
             $time_type = 'none';
    +        $element['#theme_wrappers'][] = 'datetime_wrapper';
             $date_format = $this->dateStorage->load('html_date')->getPattern();
             $time_format = '';
             break;
    @@ -71,6 +72,7 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen
    
    @@ -71,6 +72,7 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen
           default:
             $date_type = 'date';
             $time_type = 'time';
    +        $element['#theme_wrappers'][] = 'datetime_fieldset_wrapper';
             $date_format = $this->dateStorage->load('html_date')->getPattern();
             $time_format = $this->dateStorage->load('html_time')->getPattern();
             break;
    

    These hunks show the basic problem with the current patch; a lot of the other code in the patch is working around this: 'datetime_wrapper' is being applied to both the widgets and the form elements.

    In this HEAD, DateTimeWidgetBase adds 'datetime_wrapper' to the $element. The in the patch, either 'datetime_wrapper' or 'datetime_fieldset_wrapper' will also be added by DateTimeDefaultWidget. Then the form elements themselves will also get a 'datetime_wrapper' from '#type' => 'datetime' (part of HEAD). Then there is code to try to attempt to fix up the problems introduced by all of these wrappers. This get really ugly when the field is configured to allow multiple values.

    In short 'datetime_wrapper' and 'datelist_wrapper' are meant for the form elements and not the widgets. But, I am starting to think that a proper solution means taking care of this and needing proper handlesMultipleValues() support. I would love a Field API maintainer to weigh in here.

  2. +++ b/core/includes/theme.inc
    @@ -1736,6 +1782,10 @@ function drupal_common_theme() {
    +    'datetime_fieldset_wrapper' => array(
    +      'template' => 'datetime-fieldset-wrapper',
    +      'render element' => 'element',
    +    ),
    

    We are introducing a theme implementation system wide and only using it inside a particular module. If we need a new theme implementation to address this problem, then it should be defined by the module.