For my current project I needed just a simple date-popup field to set the date. Unfortunately the date field is always rendered in the fieldset, what looks very ugly for some simple forms (a fieldset introduced just to keep a single date input). It should be possible to disable this 'fieldset' behaviour from config.

For my own purposes I patched the date field, introducing new 'simple' control. The patch is in attachment.

CommentFileSizeAuthor
#169 date field.png9.94 KBkarimiehsan1819
#168 remove_loc_label_bug_when_using_reg_field.patch679 bytesDuneBL
#163 Modified Date Setup.png65.01 KBshaisamuel
#157 date-disable-fieldset-1467712-157-D7.patch8.14 KBjibran
#151 date-disable-fieldset-1467712-151-D7.patch8.28 KBankur_novatree
#149 date-disable-fieldset-1467712-149-D7.patch8.49 KBankur_novatree
#147 date-disable-fieldset-1467712-142-D7.patch83.25 KBankur_novatree
#144 date-disable-fieldset-1467712-144-D7.patch8.49 KBankur_novatree
#137 date-mod.png46.33 KBkevinsiji
#137 date-unmod.png64.9 KBkevinsiji
#127 Screen Shot 2014-01-26 at 12.20.21.png24.86 KBvijaycs85
#127 Screen Shot 2014-01-26 at 12.20.02.png18.3 KBvijaycs85
#127 Screen Shot 2014-01-22 at 23.02.46.png99.2 KBvijaycs85
#123 interdiff-1467712-120-123.txt5.74 KBMustangGB
#123 date_option_render_as_regular_field-1467712-123.patch8.22 KBMustangGB
#120 date_option_render_as_regular_field-1467712-120.patch6.87 KBflefle
#119 date_option_render_as_regular_field-1467712-118.patch6.84 KBflefle
#108 date_option_render_as_regular_field-1467712-108.patch6.62 KBDuaelFr
#108 interdiff.txt1.62 KBDuaelFr
#63 settings.png41.74 KBSinan Erdem
#60 date_option_render_as_regular_field-1467712-60.patch5.42 KBanrikun
#49 date-field-settings-form-after-patch-1467712-36.png11.75 KBanrikun
#48 field_settings.png124.24 KBloominade
#48 actual-form.png33.6 KBloominade
#36 date_option_render_as_regular_field-1467712-36.patch5.48 KBanrikun
#34 regular-field.png4.04 KBpuddyglum
#33 date_option_render_as_regular_field-1467712-33.patch5.46 KBanrikun
#28 no-fieldset-edit-field.png20.28 KBpuddyglum
#28 no-fieldset-using-field.png7.19 KBpuddyglum
#26 date-n1467712-26.patch2.97 KBpuddyglum
#27 date_field_respecting_standard-1467712-26.patch6.2 KBanrikun
#27 date-fields-node-form-patch-result.png10.11 KBanrikun
#25 Variations.JPG62.04 KBarlinsandbulte
#21 date-field-fieldset.png1.87 KBanrikun
#9 date-n1467712-9.patch2.87 KBpuddyglum
#6 date-n1467712-6.patch2.77 KBpuddyglum
#4 date-n1467712-4.patch4.64 KBpuddyglum
simple-date-control-7.2.1.patch3.03 KBl0coful
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fehin’s picture

This will be great if it's possible.

puddyglum’s picture

This is exactly the problem I'm having. None of our fields have Fieldsets except the date, and it looks pretty clunky.

puddyglum’s picture

I haven't had any problems after applying the patch you used, thanks!

puddyglum’s picture

Status: Active » Needs review
FileSize
4.64 KB

I've cleaned up the code and attached a proper patch.

Status: Needs review » Needs work

The last submitted patch, date-n1467712-4.patch, failed testing.

puddyglum’s picture

FileSize
2.77 KB

Fixed the patch

puddyglum’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, date-n1467712-6.patch, failed testing.

puddyglum’s picture

Status: Needs work » Needs review
FileSize
2.87 KB
Bronislovas’s picture

Thanks jmonkfish it is really good patch!

meladawy’s picture

#9: date-n1467712-9.patch queued for re-testing.

chertzog’s picture

Status: Needs review » Reviewed & tested by the community

Awesome!!! Love this. Hope this gets committed. Looks good on my end.

meladawy’s picture

I don know ,
This patch didn't work for me but i hope that the developers of date module to remove this ugly fieldset :\

puddyglum’s picture

bazo0oka, the patch adds a "Do not use fieldset" option on the field's edit page. You have to check it to make the fieldset go away for that field.

meladawy’s picture

K jmonkfish thanks :)

KarenS’s picture

Status: Reviewed & tested by the community » Postponed (maintainer needs more info)

The fieldset is added in a theme. You can override the theme. If we add a setting for every single thing you might or might no want to see the settings page will be overwhelming.

What is wrong with using the theme system as it was designed?

puddyglum’s picture

Well my angle on this is that the Date module has incorrectly wrapped date fields in a fieldset for a long time. Maybe the correct approach is to remove the fieldset altogether, and add an option: "Wrap the Date field in a fieldset"?

How easy is it for the non-Drupal developer to write a custom theme to eliminate the fieldset? It's quite a bit easier to simply check or un-check a fieldset.

I don't understand why the fieldset is the standard, default, and currently ONLY way to display a date without having to write custom code.

KarenS’s picture

The fieldset is because we do in fact have a collection of fields, so it is not 'wrong' to use a fieldset. Depending on the type of date and its granularity there could be seven or eight floating date elements in each field. The fieldset also is needed to keep the 'All day' and 'End date' checkboxes from escaping. So removing the fieldsets only works if you are not using those and you have a small collection of date parts. If that applies to you you can fix that in your theme. If I make this a setting, I also have to provide solutions for all the things that will now break.

puddyglum’s picture

If I make this a setting, I also have to provide solutions for all the things that will now break.

But since this is an important feature that I think many Drupal newbs will want, I see creating a simple toggle to turn it on or off as ideal.

anrikun’s picture

I hate this fieldset too. By default, date fields should be themed like other core fields, that is, using div instead of fieldset.
Besides, theme_date_combo states:

<?php
/**
 * Returns HTML for a start/end date combination on form.
 */
function theme_date_combo($variables) {
  // [...]

  // Group start/end items together in fieldset.

  // [...]
}
?>

So why is it used even when there is only a single date and not a combination?

anrikun’s picture

FileSize
1.87 KB

Look at the attached screenshot.
You can't tell that there is no problem here:
Whereas other fields are rendered as label/field (as expected), date field has no label but a fieldset with a legend.
This can even be considered as a bug.

chertzog’s picture

I understand where @KarenS is coming from. When you don't use the popup calendar, and you use select widgets for the date, i can see where the fieldset could be desirable. But when you are using just a textfield or the the popup calendar (which i suspect most people are), the fieldset is unneeded. Could we compromise, and have an option to disable the fieldset when using the popup calendar? And leave it in place when using other widgets.

As someone who has used this module on many projects, starting before I even knew how to spell php, the fact that the fieldset is added in a theme, is irrelevant to people who dont understand how to override module provided templates and themes. For some of my earliest projects, just used css to hide all of the extra labels, fieldsets, padding, and margins.

just my $.02

anrikun’s picture

I have decided to file a separate issue for #21 as this is clearly a bug.
#1538072: Date field rendering does not conform to Drupal field standards

KarenS’s picture

I closed the other issue. This is not a bug. As noted on the other issue this field could potentially contain a lot of elements -- the all day checkbox, the end date checkbox, a field for the date, time, year, month, day, hour, minute, second, am/pm, and/or timezone. And all the sub-elements have to be floated to keep the form from being 5 miles long. So the fieldset serves a purpose -- to keep all this contained.

Even if there was an option to eliminate the fieldset no one has addressed the problem of how to manage all these sub-elements that are now going to render badly across themes. You guys only care if it works for your individual situation. I have to care that it is totally portable no matter what elements are included.

Adding in switches for different things to happen depending on how many sub-elements there are makes this code even more complex than it already is. Both the code to implement the alternatives and the field of settings that the admin has to wade through.

You can fix this relatively easily in the theme, which is the standard drupal way of fixing these things (since that related issue talks about 'the Drupal standard'). You want a non-standard solution -- a setting for something you can control in the theme.

I understand that not everyone likes the fieldset but it makes things work and I don't want to be responsible for the things that will break without it.

The one change I would consider is a single switch to eliminate the fieldset in the display. It needs to have a warning that whatever display problems there are from removing the fieldset are not 'bugs' and will not be supported. If you choose to get rid of it you are on your own to fix whatever the fallout is. That will not keep people from posting support questions and bug reports about it, which I will then have to process. So this is going to require more work from me. But if you provide a patch like that I will look at it.

arlinsandbulte’s picture

FileSize
62.04 KB

There are some good points on both sides of the issue here. But first, we need to better evaluate what the current situation is and what (if any) changes should/could be made to improve that situation.

In another duplicte issue, anrikun attached a screenshot of 4 different field settings to try and make a point about the current situation. But, that screenshot does not consider all the various configurations of a date field. That all-inclusive list is very large.
Instead, attached is a list of settings I think are relevant to the discussion.

puddyglum’s picture

Version: 7.x-2.x-dev » 7.x-2.1
FileSize
2.97 KB

Thanks for your time on this! Jotting down what has been discussed so far:

  • Date field is a very common field to use
  • Date field has a variety of different configurations and display types
  • Fieldset is needed in some configurations
  • Fieldset is not needed in all configurations
  • Fieldset makes the Date field look different than most other fields on a form
  • Theming is a common Drupal practice to customize a field to work with individual situations

Solutions so far:

  1. Adjust the module to automatically decide whether or not use a fieldset
    • This would involve a lot of effort in accounting for all possible situations
  2. Create a toggle button for users to turn fieldset on or off
    • This could complicate the field edit screens
    • This might cause problems for some field configurations
    • Maintainer would have to support problems arising from this
    • Maintainer is open to this approach
  3. Do nothing, the field would have to be themed by the site developer
    • Increased effort to develop the site
    • Without a theme, may degrade user experience
    • Required effort might decrease use of Date module, rely on a standard field

Attached a patch for #2 which includes this warning: " WARNING: Some Date field configurations will not display correctly without being wrapped in a fieldset."

anrikun’s picture

Version: 7.x-2.1 » 7.x-2.x-dev
Status: Postponed (maintainer needs more info) » Needs review
FileSize
10.11 KB
6.2 KB

Here is my patch as well.
I will post comments later as I need to return to real life for 2 hours :-)
But I have also attached a screenshot showing the result.

puddyglum’s picture

Version: 7.x-2.1 » 7.x-2.x-dev
FileSize
7.19 KB
20.28 KB

#27 looks awesome! My only concern is that sites that already account for the Fieldset may have a new set of problems if it just disappears suddenly.

Attached are images of #26

KarenS’s picture

The default behavior for any patch like this is that existing fields will be unaltered. I haven't reviewed the patch yet, just noting that.

anrikun’s picture

Let me explain what patch at #27 does. I would have prefered to submit it in the other issue, as it is not just a matter of removing the fieldset.
Anyway, the patch makes a date field look the way a Drupal user would expect a field to look like:
- It brings back theme_form_element so that field has a label (with required mark if necessary, thus fixing this bug), input elements, a description/help text
- When the field is multiple, each item is rendered inside a table cell, so it removes the unecessary fieldset. Patch also fixes the duplicate help text bug displayed both for the field itself and each item.
- input elements are still wrapped inside a container, but a div instead of a fieldset.
- patch detects if inputs inside the container are displayed on several lines, and if so, add an extra class to the div. By default, this extra class adds a border around inputs (see screenshot at #27). When inputs are displayed on only 1 line, there is no border.
- patch seems to work with any variation at #25 + supports date_repeat_field

@KarenS:
I understand your concern. If this patch has a chance to be committed, I'm OK to merge it with jmonkfish's one, so that by default fieldset is still used.

KarenS’s picture

OK, this patch is less invasive than I expected, which is good. We need to merge the ideas so that we have a warning and a default that leaves things alone. You should test that existing fields are unaltered even if no one has touched the field settings.

For the warning message, I might change "WARNING: Some Date field configurations will not display correctly without being wrapped in a fieldset." to "The date field elements are wrapped in a fieldset by default, and may not display well without it."

KarenS’s picture

Status: Needs review » Needs work
anrikun’s picture

Status: Needs work » Needs review
FileSize
5.46 KB

Here is the merged patch:
- added the option to render the date field as a regular field (per instance setting, *unchecked by default*).
- rewrote my previous patch so that there are almost only additions.
- added a separate theme function to render the date field as a regular field. After applying the patch, theme registry must be rebuilt for this function to be detected.

*This patch leaves existing fields unchanged (tested).*

puddyglum’s picture

FileSize
4.04 KB

Patch applied great, and I like the verbiage on the Edit Field screen.

The only problem I have is that there is still an extra label and an extra container with a border on it that doesn't flow well with a bunch of other regular fields and doesn't look like a regular field.

No fieldset, no container, no extra label:
No fieldset

No fieldset, extra container, extra label:
Regular field

Is there a way to remove the extraneous container and label from a Date field that is a single date field?

anrikun’s picture

Maybe I forgot something.
What widget are you using? Date popup?

anrikun’s picture

Try this updated patch.
And edit your date field and set Position of date part labels to None under More settings and values > Advanced settings
Field should display as you expect then.

puddyglum’s picture

Yes, Pop-up calendar. Attached a screenshot of the settings.
Okay, trying the patch

stompersly’s picture

The patch in #36 works great. It took me a minute to see I needed to look under advanced options in the fields settings, but it works great thanks anrikun

puddyglum’s picture

:) #36 works perfectly for me as well. Thanks anrikun!

Edit: actually, when I applied the patch it caused my main navigation to get moved around... I need to figure out if the patch really caused that, seems unrelated.

puddyglum’s picture

I still haven't figured out what's wrong, but after I applied the patch there are some non-ascii characters before the doctype tag and it's causing the browser to go into quirks mode. If I take away the patch the problem goes away.

Update: Patch #36 is working great for me.

puddyglum’s picture

Can anybody leave feedback or identify problems with implementing this patch? At first it sounded a little invasive, but it's looking very safe to apply right now.

loominade’s picture

the new option only removes the label, the fieldset is still there. is it what you wanted?

anrikun’s picture

@loominade: no, the new option allow to render the date field as a regular field instead of a fieldset.
But after applying patch at #36, theme registry must be rebuilt for the new function to be detected.
(I had written this at #33.)

loominade’s picture

@anrikun yes, I flushed all the caches. but still I have a fieldset and a field without label.

I use the following field configuration:

 array(
    'field_config' => array(
      'active' => '1',
      'cardinality' => '1',
      'deleted' => '0',
      'entity_types' => array(),
      'field_name' => 'field_birthday',
      'field_permissions' => array(
        'type' => '0',
      ),
      'foreign keys' => array(),
      'indexes' => array(),
      'module' => 'date',
      'settings' => array(
        'cache_count' => '4',
        'cache_enabled' => 0,
        'granularity' => array(
          'day' => 'day',
          'hour' => 0,
          'minute' => 0,
          'month' => 'month',
          'second' => 0,
          'year' => 'year',
        ),
        'timezone_db' => '',
        'todate' => '',
        'tz_handling' => 'none',
      ),
      'translatable' => '0',
      'type' => 'datestamp',
    ),
    'field_instance' => array(
      'bundle' => 'person',
      'deleted' => '0',
      'description' => 'Birthday',
      'display' => array(
        'default' => array(
          'label' => 'above',
          'module' => 'date',
          'settings' => array(
            'format_type' => 'long',
            'fromto' => 'both',
            'multiple_from' => '',
            'multiple_number' => '',
            'multiple_to' => '',
          ),
          'type' => 'date_default',
          'weight' => 9,
        ),
        'teaser' => array(
          'label' => 'above',
          'settings' => array(),
          'type' => 'hidden',
          'weight' => 0,
        ),
      ),
      'entity_type' => 'node',
      'field_name' => 'field_birthday',
      'label' => 'Birthday',
      'required' => 0,
      'settings' => array(
        'default_value' => 'now',
        'default_value2' => 'same',
        'default_value_code' => '',
        'default_value_code2' => '',
        'user_register_form' => FALSE,
      ),
      'widget' => array(
        'active' => 1,
        'module' => 'date',
        'settings' => array(
          'increment' => '15',
          'input_format' => 'm/d/Y - H:i:s',
          'input_format_custom' => '',
          'label_position' => 'above',
          'text_parts' => array(),
          'year_range' => '1900:+3',
        ),
        'type' => 'date_popup',
        'weight' => '6',
      ),
    ),
  );
anrikun’s picture

Have you checked the new option to render the date field as a regular field?
It doen't seem present in your config.

loominade’s picture

I did. Its just not in the feature (code above)

anrikun’s picture

How strange. Could you post a screenshot of your date field settings form?
Just to make sure that it looks the same as mine.

loominade’s picture

FileSize
33.6 KB
124.24 KB

The field settings
And how it looks in the entity create form

anrikun’s picture

Patch #36 does not seem applied on your site as your form is missing the new setting.
See below:

loominade’s picture

ah, sorry. now it works.

but the label is <label for="edit-field-birthday-und-0">Birthday</label> while the actual id of the input is edit-field-birthday-und-0-value-datepicker-popup-0

anrikun’s picture

I know but this is a different issue.
Whether applying the patch or not, date field breaks accessibility and W3C validation.
I have already posted an issue about this: #600236: Invalid XHTML "label for" when using popup

Current issue is only about allowing to render a date field as a regular field (Accessibility and validation problems should be addressed in the other issue).
If you think that it works, then feel free to mark this issue as RTBC.

puddyglum’s picture

Patch #36 has been working very well for us in a production environment on several content types. We got positive feedback on it as well.

play4quarters’s picture

Hi,

following to see if this gets included in the module.

I appreciate everyone's work here, as the field sets are causing me problems with mobile theming.

thanks much.

anrikun’s picture

@jmonkfish, play4quarters:
I guess it will never get committed if nobody marks it as RTBC first (as I wrote at #51).

puddyglum’s picture

Status: Needs review » Reviewed & tested by the community
KarenS’s picture

Status: Reviewed & tested by the community » Needs work

I tried this patch out and when I check that box the date completely disappears from the form. I cleared the caches several times. I tried all kinds of different date fields and none that use the Date Popup widget worked, all of those completely disappeared from the form. Dates that use the Date Select widget seem to be fine.

I can't get it working so marking this back to needs work.

Also I would like this moved to the 'Advanced settings' section. The Aquia UX team spent a lot of time simplifying the field form to keep it simple, and this makes it more confusing. It will still be available.

anrikun’s picture

@KarenS: thanks for your review and explanations. I will have a look at this problem.

puddyglum’s picture

Date fields with the Date Popup are working for me. I'll re-try from scratch.

anrikun’s picture

@KarenS, jmonkfish: Date Popup fields are working for me too.
KarenS, did you configure some other setting (that I may not have tried)?

anrikun’s picture

Status: Needs work » Needs review
FileSize
5.42 KB

In the meantime, here is a new patch that moves the new setting to the 'Advanced settings' section.
To people who already use path at #36: beware that if you switch to this new patch, we will have to check the setting again for each date field.

Sinan Erdem’s picture

The latest patch doesnt work for me. As KarenS stated on #56, the date field completely disappears if I check the option. It doesnt matter if I use popup or select list.

anrikun’s picture

@etcetera9:
Have you cleared your theme registry cache after applying the patch?
What other settings did you set for your date field?

Sinan Erdem’s picture

FileSize
41.74 KB

Yes, I cleared the caches many times, run the cron also.

Settings of my field is on the attachment.

Sinan Erdem’s picture

I think I found my problem. I am using the field inside a field collection (Field Collection module). Then it doesnt show it. On some normal date field, it works as suggested.

mgifford’s picture

Sounds like this should be reviewed by WAVE, but better by a real person using AT.

mh86’s picture

Currently using patch from #60 and it works well

rbruhn’s picture

Thanks for this patch. Hope something can eventually be put into the module permanently. I was searching for this very solution, and saw KarenS mention doing it in the theme. So spent 3 hours searching the internet on how to do exactly that. Well, trying to find that answer was like having molars pulled without anesthesia.

I received an error when applying the patch in that it could not patch date.theme for some reason. I added the code by hand.
The only issue I have is while the old fieldset title is now showing for the field label, so is the Date label. Meaning, my field now looks like:

Date Collected
Date
[input field is here]

Edit: Nevermind. I see setting "Position of date part labels" to none fixes that.

mrmartin’s picture

#60 works well for me with 7.x-2.6 version.

jday’s picture

Works for me too, (#60 on 7.x-2.6). I would also love a global setting, I have at least 50+ pop-up date fields and clicking through to each fields advanced setting to check that box is not so much fun but I'm glad to be rid of the fieldsets - Thanks anrikun!

arlinsandbulte’s picture

Copied from shane's post at the code karate blog:
http://codekarate.com/blog/removing-fieldset-drupal-7-date-field

This is a relatively easy one, but I found myself scratching my head at this for longer than was necessary. There are times in which you do not want a Drupal 7 date field to be added inside a fieldset. One discussion on Drupal.org (http://drupal.org/node/1467712) mentions applying a patch to the Date module. I think it is much better to just use the theme layer for it's intended purpose... overriding theme-able output. The main problem for a lot of people, is that they are just not as comfortable with PHP or are unsure which functions need to be overridden in your template.php to get the desired results.

So if you are looking for the simplest way to get your Drupal 7 date fields from displaying inside of a Fieldset, drop this code into your theme's template.php file (replacing MYTHEME with the name of your theme).

function MYTHEME_date_combo($variables) {
  return theme('form_element', $variables);
}

Just for reference, the original theme function was called theme_date_combo and is located in the date.theme file of the date module. Here is the original code:

function theme_date_combo($variables) {
  $element = $variables['element'];
  $field = field_info_field($element['#field_name']);
  $instance = field_info_instance($element['#entity_type'], $element['#field_name'], $element['#bundle']);
 
  // Group start/end items together in fieldset.
  $fieldset = array(
    '#title' => t($element['#title']) . ' ' . ($element['#delta'] > 0 ? intval($element['#delta'] + 1) : ''),
    '#value' => '',
    '#description' => !empty($element['#fieldset_description']) ? $element['#fieldset_description'] : '',
    '#attributes' => array(),
    '#children' => $element['#children'],
  );
  return theme('fieldset', array('element' => $fieldset));
}

It is almost too simple... you just need to know which function to override.

yannickoo’s picture

Status: Needs review » Reviewed & tested by the community

Patch from comment #60 works fine, thanks! I think we should commit the patch.

yannickoo’s picture

Status: Reviewed & tested by the community » Needs review

Sorry, forgot to read the other comments. Does this patch has problems with field collection? With nodes it works fine.

Agileware’s picture

#70 works for me, good one thanks for posting arlinsandbulte

MustangGB’s picture

Another @todo for #60, the following margin-bottom should be 0 when not in a fieldset.

.container-inline-date > .form-item {
  margin-bottom: 10px;
}
trapper-1’s picture

I am getting similar results as #56 and #61. When applying patch from #60 and changing field settings to 'render as a regular field' the entire field disappears from the forms (add/edit) completely.

balazswmann’s picture

Patch in #60 works well for me. This is exactly what I need. Nice job! My module version is 7.x-2.6.

Frederic wbase’s picture

i can confirm that the patch from #60 works like a charm. Thanks to everyone for this nice and hard work!

mautumn’s picture

#70 works for me - both within a field collection and on its own. Thanks very much for the tip arlinsandbulte.

m@rtijn’s picture

I decided to override the theme_date_combo function mentioned in #70 to get rid of the fieldset of my basic date field. Can someone point me in the right direction to get rid of some div containers? I would like to accomplish the following:

<div class="form-item form-type-textfield form-item-date">
<label for="edit-date">
<input id="edit-date" class="form-text required" type="text" maxlength="255" size="60" value="" name="">
</div>
KarenS’s picture

I just want to note that removing the fieldset has accessibility implications. See http://drupal.org/node/504962 and http://groups.drupal.org/node/19118.

msti’s picture

Dave.Ingram’s picture

Just came across this and would like to point out that if fieldsets are semantically correct but not pretty, they can be easily styled with css. Here's one approach which cleaned things up nicely on a particular theme I was working in, but should be a close approximation for what most seem to be trying to solve here:

.field-type-datestamp fieldset .fieldset-legend {
  font-size: 14px;
  color: #445668;
  padding: 7px 0 2px;
  font-weight: normal;
}

.field-type-datestamp fieldset {
  border: none;
  background: none;
  padding: 0;
  margin: 7px 0 0 0;
}

.field-type-datestamp fieldset label {
  display: none;
}

.field-type-datestamp fieldset .date-padding {
  padding: 0;
}
KarenS’s picture

Status: Needs review » Needs work

I am not going to add this change. As noted in #70, you can do this by overriding the theme, which is the normal way of handling things like this in Drupal. And as noted in #82, your can also override css to make it less obtrusive.

Making it into a UI option adds lots of overhead to an already complicated module. And the real deal-killer is that removing the fieldsets makes the field inaccessible. I'm working on Date in core for D8 and it looks like the fieldsets will be needed there, and they will definitely not be configurable in D8. Core only ships with accessible code. So you will need to either override the theme or the css in D8.

We should add some documentation somewhere about how to do this 'right' -- either by overriding the theme or the css. Let's change the patch to fix the documentation instead.

modulo’s picture

Status: Needs work » Needs review

simple-date-control-7.2.1.patch queued for re-testing.

puddyglum’s picture

@KarenS, can you explain why default Text fields in Drupal do not need fieldsets, but Date module fields do? Is it for the cases where there is both a "From" and "To" Date?

I see this module as fundamental for most Drupal sites, and the fact that its implementation sticks out like a sore thumb. Every single client says, "It looks great, but why does Date look different than all the other fields?" So it's back to implementing CSS & Template file fixes for all of the fields, when it really should be option to turn off the fieldset... or more realistically an option to turn it on.

Dave.Ingram’s picture

@jmonkfish. Karen addressed this in comment #18. The date field may have quite a few different fields in it and therefore should logically have a fieldset around it.

puddyglum’s picture

Thanks Dave. I give up.

KarenS’s picture

Also read #80 which tells you it is an accessibility issue. There are core patches that will require this fieldset coming.

anrikun’s picture

I seriously doubt that accessibility has ever been a priority of the Date module: bugs such as "label for" to non-existent IDs and duplicate IDs that break W3C validation have been around for years.
I did read #18 and #80. But have you read #30? My patch was not about simply removing the fieldset. It was fixing the missing "required" bug and the duplicate description bugs too. And it was removing the fieldset when date field was multiple, because why wrapping an extra fieldset when each item is already displayed inside a table cell? This patch was not perfect but it was made to be as less invasive as possible.

If you really want to stick to core, then date field should use theme_form_element() like any other field, even if there has to be a fieldset inside.

Anyway, I give up too :-(

eliaspallanzani’s picture

+1 for an option to turn off the fieldset.

peteruithoven’s picture

I would personally suggest publishing a separate small module, something like Simple Date Widget. That would only allow a start date, no end date, no all day, no repeat rules. I imagine that in that case it won't brake accessibility issues? Why would you try to do all the possibilities with one widget anyway?

Sk8erPeter’s picture

@peteruithoven: How is developing a less configurable (dumber) module related to the problem of making fieldset possible to be disabled at all? As it was mentioned earlier, you can override theme_date_combo in your own theme, and do the needed modifications. I think publishing a "separate small module" for the same task is simply useless.

peteruithoven’s picture

Because you can make it without the fieldset to begin with? Without adding another exception to an already complicated widget element.

Sk8erPeter’s picture

@peteruithoven: you have to make this exception in your theme only once if you want to do it for all the widgets, but you can always add an if statement if you realize the fieldset is still needed somewhere... without having to create another module for the very same task.

presleyd’s picture

I hope you will reconsider something like this Karen. Fieldsets ARE important for all the reasons you name but I would like to have control over grouping my fields and not have my date fields have an extra layer of fieldsets. I do not think the default behavior being in a fieldset is ideal for the most (likely) use case of wanting to store a single date. The 'to' date and repeat API features are great extras but are not core to the idea of a date field.

Countzero’s picture

Just wanted to add my vote for simplifying the basic use of a date field : storing and displaying dates as simply as possible. It's called Date, not 'Complex Dates Management Field'.

The current default display just feels like a plain anomaly : everybody in FAPI displays its stuff as simply as possible, except Date, which adds its own very special display.

Most of the time, one doesn't need all the Repeat and End Date stuff (which I was otherwise happy to find in some use cases), but just a plain dumb date. I find it hard to understand why anyonne should have to overwrite a theme function to downgrade a default behaviour.

I bet the vast majority of people using the date stuff will feel the same way. The idea of a simpler module seems sound to me. In fact, I think it should be the default, and the current Date module a valuable extra for people (me included) who sometimes need a more versatile solution.

But well ...

mgifford’s picture

The Issue Summary might need to be revised, but I think the problem described so far is that folks don't like the look of the fieldset around the date input fields. If that's the case, shouldn't we just be looking for a CSS solution?

For accessibility it is important for the semantic relationship between the fields to be maintained. The only way to do this is through fieldsets at this point. Drupal 7 is using FIELDSETS badly and Drupal 8's largely moved to using DETAILS.

I don't see any reason to visually display a FIELDSET around a group of date elements. I also have yet to hear a reason why this markup should be excluded. I posted some links about why FIELDSETS are important #501428-107: Date and time field type in core there's also this issue here #504962: Provide a compound form element with accessible labels .

viadimezzo’s picture

#70 awesome, thanks a lot arlinsandbulte

presleyd’s picture

If there is no To date or repeat options, do we still need the fieldset? Nobody wants to break accessibility I think. #97 is correct that it really is just a display issue though. It's the little things like this that weird out new site builders or people coming in from Wordpress. 'Correct' functionality can't stomp out the user experience and theme overrides should be for doing 'different'. Not for trying to make Date look like every other field.

lipinponmala007’s picture

I some how took it all and chande it it to a table

function zircon_date_combo($vars) {
  // Override the display of the combo element
  $element = $vars['element'];
  $label = $element['#title'];
  $date1= '<div id=datepic>'.render($element['value']['date']).'</div>' ;
  $time1='<div id=timepic >'.render($element['value']['time']).'</div>';
  $date2= '<div id=datepic>'.render($element['value2']['date']).'</div>';
  $time2= '<div id=timepic>'.render($element['value2']['time']).'</div>';
  $header=array();
  if(count($element['value2']['date']) <1){
 	$row[]= array($date1,$time1);
 	 $id = 'nonmaster';
 
  }else{
	  $row[]= array('FROM',$date1,$time1,'TO',$date2,$time2);
	  $id='masterdate';
  }
  $out =theme('table',array('headers' => $header,'rows' => $row,'caption'=>$label,'attributes' => array('id' => $id)));
  return $out;

}
mmilano’s picture

If you need to correct this from within a module, hook_theme_registry_alter() works well. Remember to flush the theme registry.

/**
 * Implements hook_theme_registry_alter().
 */
function MYMODULE_theme_registry_alter(&$theme_registry) {
  if (isset($theme_registry['date_combo'])) {
    $theme_registry['date_combo']['function'] = 'theme_MYMODULE_date_combo';
  }
}

/**
 * Date combo theme override.
 */
function theme_MYMODULE_date_combo($variables) {
  return theme('form_element', $variables['element']);
}
falcon03’s picture

Status: Needs review » Closed (won't fix)
Issue tags: -Needs accessibility review

From an A11Y point of view, this is "won't fix". The date widget is a compound form element, so it should be wrapped in a fieldset.

But I can understand that wrapping the date widget in a fieldset can alter the appearance of the date form. So, I would like to invite anyone to have a look (and eventually help with)
#1918994: Improve Datetime and Daterange Widget accessibility
so that we can add the needed fieldsets without altering the appearance of the date form.

puddyglum’s picture

I'm still not understanding why there can't be an option to disable the fieldset element. Looking back through this issue's discussion it sounds like it became pushed aside when things got complicated.

I think the constructive idea of this issue is to add an option to disable the fieldset. Allowing the solution to be in the Field's UI instead of having to hand-code solutions for each theme/site would improve this module. Which would improve Drupal because there's no other Date module. ;)

Yes, it can be solved with CSS / Theme hooks / Module hooks / Hacking the module. But why not provide an option that can be consistently used and relied upon?

The date widget is a compound form element, so it should be wrapped in a fieldset.

Not always, and, in my case, not very often. Please refer to:

If there is no To date or repeat options, do we still need the fieldset? Nobody wants to break accessibility I think. #97 is correct that it really is just a display issue though. It's the little things like this that weird out new site builders or people coming in from Wordpress. 'Correct' functionality can't stomp out the user experience and theme overrides should be for doing 'different'. Not for trying to make Date look like every other field.

So many other modules rely on and extend the Date module, it's not really feasible to create a new Module called "Simple Date" or "Date-lite".

...so that we can add the needed fieldsets without altering the appearance of the date form.

The whole point of this issue is to alter the appearance of the Date form from how it currently is.

arlinsandbulte’s picture

As I understand the issue, the 'extra' fieldset issue should only be a problem when using a simple, single text area for the date & time value and with no End Date.

In all other cases, the fieldset IS needed to properly group the related fields.

puddyglum’s picture

If there is a built-in option to simply hide the fieldset, that would probably be enough. I'm gathering that the fieldset is needed for accessibility, and that hiding the fieldset is not a problem.

I'll try and put together a solution that simply hides the fieldset when the "Display as a normal field" option is enabled.

trapper-1’s picture

my 2 cents....K.I.S.S.

Drupal core + contrib modules allow me to do anything and everything I want - I can build a learning management system, an ecommerce site, and custom in-house applications. I can do this because core and contrib modules are broken down to their basic components - this Date module (specifically the fieldset issue) goes "against the grain" in regards to core and most other contrib modules are built.

If I just need a date field I should be able to add one without a fieldset - "core" date functionality. Y'all got it right with the repeating dates and all-day functionality - to quote from the Date project page:

Note that the latest code includes two new modules, Date Repeat Field, a module to create date fields that use the Date Repeat API, and Date All Day, a module to manage handling of All Day values, including adding an All day checkbox to the widget. Both of those were formerly included in the basic Date module but have been pulled out to streamline the code and make it possible to disable them

The fieldset that is required is because of the start/end date functionality that is still included as part of the date core, but I highly recommend y'all do the same as you did for repeating and all-day. make into it's own sub-module so that all of us who want a simple single-date field can quit hacking date core and/or writing custom CSS to get around this.

Also, the start/end date functionality is super, but goes against database normalization - storing more than 1 value in a single 'field' (I know the database actually stores this in separate fields). this funcationality makes it easy to implement a request that I am sure a lot of people have, but the 'correct' way to capture start/end dates would be to create a new content type with 2 fields 'start' and 'end', and then add an entity-reference field to your parent content type. the entity-reference field will handle the fieldset required.

how hard would it be to implement something like this?:

if ((allday) || (repeating) || (enddate)) {
add fieldset
} else {
don't
}

tregismoreira’s picture

#60 works perfectly for me too. Thanks a lot! :D

DuaelFr’s picture

#60 works pretty well but I faced an UI issue when I enabled it on a really simple date_popup field with date only and no end date.

The fact is that when you only have one component in the field, you have two labels and the first points to nothing. In this case we should hide the main one and copy its title to the component's.

That is what this patch adds. Just take a look at the interdiff.txt to see how I did it (less than 20 lines).

This patch is part of the #1day1patch initiative.

AaronBauman’s picture

Status: Closed (won't fix) » Needs review

Reopening based on responses to #102

If accessibility is the issue, then:
a) if there's actually only one input field, a fieldset is unnecessary (per #103 and #99)
b) a js/css-based solution could preserve accessibility -- even for compound fields -- while granting finer control to form builders

mgifford’s picture

@aaronbauman This seems reasonable. Certainly for a single field a fieldset isn't needed.

wismbuh’s picture

simple-date-control-7.2.1.patch queued for re-testing.

lazly’s picture

Its sounds like incredible useful! Please commit it (if its working).

DuaelFr’s picture

It is still working :)

lazly’s picture

Than an commit sound like an amazing idea ;)
I hate the independent patches in an armed system, because after it you'll cant upgrade simply, just if you always re-patch your code. And only few mounts, and you will forget it.

podarok’s picture

Assigned: Unassigned » vijaycs85
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

#108 looks good
RTBC

vijaycs85’s picture

Assigned: vijaycs85 » Unassigned
Status: Reviewed & tested by the community » Needs work
  1. +++ b/date_api/date.css
    @@ -186,3 +188,11 @@ div.date-calendar-day span.year {
    +.date-form-element-content {
    +}
    

    Can be removed...

  2. +++ b/date_popup/date_popup.module
    @@ -292,6 +292,17 @@ function date_popup_element_process($element, &$form_state, $form) {
    +      $element['date']['#title'] = $element['#instance']['label'];
    ...
    +      $element['time']['#title'] = $element['#instance']['label'];
    

    Missing check_plain here.

Anonymous’s picture

Against which version should this patch be applied?

flefle’s picture

Had to update the patch changes above the latest version of Date module (dev version after 7.x-2.7).
The interdiff interdiff_7024.txt was already in the /date_option_render_as_regular_field-1467712-108.patch

flefle’s picture

Status: Needs work » Needs review
FileSize
6.84 KB

Had to adjust the line above the latest dev version of Date module from date_option_render_as_regular_field-1467712-108.patch. The interdiff_7024.txt is already included in the mentioned patch.

The patch changes are working like a charm. I had to clear cache to see the remove fieldset checkbox.

flefle’s picture

Found out that a #description field is missing in date_elements.inc so I've updated the patch.

yannickoo’s picture

Please use spaces instead of tabs.

podarok’s picture

Status: Needs review » Needs work

#120 needs work due to drupal code standards https://drupal.org/coding-standards#indenting

MustangGB’s picture

  • Addressed #74, #116, #121 and #122
  • Fixed label not appearing for text widgets in date_text_element_process()
  • Added comment to date_popup_element_process()
MustangGB’s picture

Status: Needs work » Needs review

Whoops

capellic’s picture

I applied the patch, looks good! Thank you!

vijaycs85’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests, +SprintWeekend2014, +LONDON_2014_JANUARY

the patch made the date field disappear in content type. Please find the steps to reproduce:

  1. Apply patch in #123
  2. Enable date module and create a date field in article
  3. Visit the article node add page and can see the date field as configured
  4. Check 'Render as a regular' checkbox under 'MORE SETTINGS AND VALUES' -> Advanced settings
  5. No more date field in article add page

Also adding Needs tests to cover the test case for the presence of a date field with new settings.

vijaycs85’s picture

vijaycs85’s picture

rphillipsfeynman’s picture

I confirm that following the steps in #126 it works like charm. Thanks for this great work guys. Looking forward to see it included in the module.

MustangGB’s picture

#126: For me it re-appears if you clear your cache, or are you saying the patch should do this automatically?

vijaycs85’s picture

#129: @rphillipsfeynman I'm afraid, it is not behaving how it suppose to

@akamustang, yes, it should be automatically updated.

thijsvdanker’s picture

#129: @rphillipsfeynman I'm afraid, it is not behaving how it suppose to

@vijaycs85: Could you elaborate on what's not behaving how it's supposed to?

tregismoreira’s picture

The patch #123 works greatly! Thanks a lot!

It is already committed to -dev branch?

gbirch’s picture

For anyone banging their heads against this problem using the suggestions from comment #70:

1. Yes, you really do have to override theme_date_combo even when you have a simple, one-date field.

2. If you have many different kinds of date fields in your site, and want to override the theming only for the simple date case, you want to copy the theme_date_combo function (from date.theme), rename it, and add a bypass that applies only when the field is simple. By the way, if you go this route, remember to update your code any time the date module maintainers decide to update the theme_date_combo() function (even more fun than keeping track of patches):

function MYTHEME_date_combo($variables) {
  $element = $variables['element'];
  // if there's only one date element present, skip the fieldset and do normal field formatting.
  if ( isset($element['#columns']) && count($element['#columns']) < 2 ) {
    return theme('form_element', $variables);
  }
  $field = field_info_field($element['#field_name']);
....

3. But that override only lets you get rid of the fieldset. There are several layers of extraneous divs added by other functions. For a date popup field, you also need to override theme_date_popup() (found in date_popup/date_popup.module) The same caveats apply, and note that I have not tested this with a field that also gathers time values:

function MYTHEME_date_popup($vars) {
  $element = $vars['element'];
  // skip the two extra layers of divs, and just return the input and label.
  if ( !empty($element['#children']) ) {
     return $element['#children'];
  }
  $attributes = !empty($element['#wrapper_attributes']) ? $element['#wrapper_attributes'] : array('class' => array());
....

4. Finally, to suppress the "Date" label that the module adds to your simple date field, make sure that when configuring the field, you go into "More Settings and Values", "Date Entry", "Advanced Settings" and set the "Position of date part labels" to "None". This will at least hide the extra label, although it will still be there in the markup.

I hope this spares some poor sap some time. And it seems to serve as implicit answer to the observation by KarenS that one could/should just fix the markup "the standard drupal way".

mxlav’s picture

Applied #123, everything checks out, except if you enter some Help text, it doesn't get rendered. Turning off Render as a regular field will show the Help Text.

kevinsiji’s picture

Not trying to defeat the purpose of this issue.

Its easy to manage the date field using pure css. Below is what I've arrived at for a single element date field.

.field-type-datetime fieldset.form-wrapper {
  border: none;
  padding: 0;
}
.field-type-datetime fieldset.form-wrapper legend {
  font-weight: bold;
}
.field-type-datetime fieldset.form-wrapper .date-padding {
  padding: 0;
}

Original
Only local images are allowed.

Fixed in css
Only local images are allowed.

kevinsiji’s picture

FileSize
64.9 KB
46.33 KB

Do not know why my images are not displayed.

Embedding here again:

Original

Fixed in css

nessunluogo’s picture

Thanks everybody for the job on this issue!
Talking about the CSS alternative solution proposed by kevin, in my case I had to add some more CSS for my skeleton based theme. This is the code:

.field-type-datetime fieldset.form-wrapper {
  border: none;
  padding: 0;
  top: 0;
  margin: 0;
}
.field-type-datetime fieldset.form-wrapper legend {
  font-weight: bold;
  background: none;
  border: none;
  text-indent: 0;
  left: 0;
  text-shadow: none;
  top: 0;
}
.field-type-datetime fieldset.form-wrapper .fieldset-wrapper {
  border: none;
  padding: 0;
}
.field-type-datetime fieldset.form-wrapper .date-padding {
  padding: 0;
}
lavamind’s picture

@mxlav, To restore the Help text when using patch #123, add the following line

$element['#description'] = $element['value']['#instance']['description'];

right before

return theme('form_element', $variables);

in the theme_date_form_element function.

lazly’s picture

This is mine CSS code, for this "bug". I'm using it for the "Seven" admin theme.

.field-type-datetime fieldset.form-wrapper {
  border: none;
  padding: 0;
  top: 0;
  margin: 0;
}
.field-type-datetime fieldset.form-wrapper legend {
  font-weight: bold;
  background: none;
  border: none;
  text-indent: 0;
  left: 0;
  text-shadow: none;
  top: 0;
  margin: 0 0 15px; 
}
.field-type-datetime fieldset.form-wrapper .fieldset-legend {
  padding: 0;
  margin: 0;      
  text-transform: none !important;
}
.field-type-datetime fieldset.form-wrapper .fieldset-wrapper {
  border: none;
  padding: 0;
}
.field-type-datetime fieldset.form-wrapper .date-padding {
  padding: 0;
}
MustangGB’s picture

CSS doesn't cut it, the bug is the additional markup.

The last submitted patch, 123: date_option_render_as_regular_field-1467712-123.patch, failed testing.

ankur_novatree’s picture

Patch submitted in #123 had gone stale and required a re-roll. It was an auto-merge. Submitting the re-rolled patch.

ankur_novatree’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 144: date-disable-fieldset-1467712-144-D7.patch, failed testing.

ankur_novatree’s picture

Status: Needs work » Needs review
FileSize
83.25 KB
ankur_novatree’s picture

I am really sorry for this trouble and inconvenience. I am contributing for the first time and learning applying and rerolling patches for the first time. Being a Windows user is making it a little more difficult.

Please ignore the file in #147 and bear with my mistakes for this time.

ankur_novatree’s picture

Status: Needs review » Needs work

The last submitted patch, 149: date-disable-fieldset-1467712-149-D7.patch, failed testing.

ankur_novatree’s picture

Status: Needs work » Needs review
FileSize
8.28 KB
vorapoap’s picture

This is my css code for Open Atrium 2.0 under oa_radix theme (I put it to screen.css) modified from #138
I agree that toggling Date field to Regular Field must be put into core not patching like this.

.field-type-datetime fieldset.form-wrapper {
  border: none;
  padding: 0;
  top: 0;
  margin: 0;
}
.field-type-datetime fieldset.form-wrapper legend {
  font-weight: bold;
  background: none;
  border: none;
  text-indent: 0;
  position:relative;
  left: 0;
  text-shadow: none;
  top: -4px;
  margin-bottom:0px;
}
.field-type-datetime fieldset.form-wrapper legend span {
  font-size:14px;
  font-weight:normal;
}
.field-type-datetime fieldset.form-wrapper .fieldset-wrapper {
  border: none;
  padding: 0;
  position:relative;
  top:-9px;
}
.field-type-datetime fieldset.form-wrapper .description {
  display:none;
}
.field-type-datetime fieldset.form-wrapper .date-padding {
  padding: 0;
}
div.pane-form fieldset.form-wrapper .field-type-datetime {
  margin-bottom: -12px;
}
emjayess’s picture

Wow.

Sometimes my clients justifiably inquire as to why some things are so hard, or seem to take so long and be so obtuse and perplexing in Drupal... to which I just respond, "In Soviet Drupal, fieldset eliminates you". Then I tell them to read issue threads like this one. Then we begin evaluating CMS's that still suck at accessibility, but with which we can create aesthetically pleasing forms in a little less than 2 and a half years.

meecect’s picture

@emjayess no kidding. The sad thing is, I could point you to many other examples that are worse than this. I was just looking at an issue yesterday that had been going on for at least 4 years, about adding an option to show the label (or not) on boolean checkbox fields. I love Drupal, but sometimes I think the tagline should be:

Drupal --making me look stupid in front of my clients since 2004.

meecect’s picture

see also: removing N/A as an option to non-required radio buttons.

milos.kroulik’s picture

Has anyone tested patch in #151, yet?

jibran’s picture

#151 worked just fine. Here is the reroll version of the patch. Other then tests it is RTBC.

  • podarok committed e5e71cd on 7.x-2.x authored by jibran
    Issue #1467712 by ankur_novatree, anrikun, jmonkfish, fledev, MustangGB...
podarok’s picture

Status: Needs review » Fixed

2 years...
Let's make this patch in.
commited. Will be tagged in next upcoming beta release. Thanks for hard work on it.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Anonymous’s picture

JavaScript:

var $date_field = $('#edit-field-deadline');

$date_field.find('label').first().html($date_field.find('.fieldset-legend').html());
$date_field.find('.date-padding').appendTo($date_field);
$date_field.find('fieldset').hide();
shaisamuel’s picture

I don't quite understand. I applied the patch (tried both #151 or #157), even added the css, and still nothing changed.

Can anyone help please, and explain what need to be done?

shaisamuel’s picture

FileSize
65.01 KB

Ok, I found it, its in the field setup form.

Setup the way the field will rendered

Just needs to choose "Render as a regular field", and the magic happened...

Thank you !

Anybody’s picture

It works great and it would be time to have this in a new stable release. Any plan or tag for that?

a.milkovsky’s picture

+1 for new stable release.
Works good for me, a very good feature.

kopeboy’s picture

Status: Closed (fixed) » Needs work

Well.. the "Render as a regular field" cuts the fieldset (thanks!) but won't let the Help text show!

A regular field in Drupal should have the help text visible..

DuneBL’s picture

There is also another problem: when "regular field" is selected, the label of the field isn't localized as it should...

DuneBL’s picture

here is an ugly patch to have the labels translated...

karimiehsan1819’s picture

FileSize
9.94 KB

hi, i want hide or remove border of date field how i can do it ?

Zekvyrin’s picture

I found another bug:

when the date field is required, the "fieldset title" appears (and the field title may or may not be rendered as it works as it should with the "Position of date part labels" setting).

dzy’s picture

When "Position of date part labels" is None , should remove the label
<label class="control-label" for="edit-student-birthday-und-0-value"><span class="form-required" title="This field is required.">*</span></label>

frob’s picture

All these bugs really need to be entered as new issues, this patch/feature was already committed.

This issue should be closed(fixed) and all the bugs need to be entered as new issues.

MustangGB’s picture

Status: Needs work » Closed (fixed)

Agreed, restoring status, bugs/follow-ups should go in new issues.