Does it seem reasonable to anyone else that the core form generation tools should also include time?

For example the form generation tools will spit out pulldowns for the date. But what if someone needed to specify a particular hour within the date? How would a developer accomplish such a task?

I would like to suggest that we provide an option for extending the date fields so that a developer can also specify time. It might look something like this:

 $fields[$category][$field->name] = array(
  '#type' => 'date',
  '#title' => check_plain($field->title),
  '#default_value' => $edit[$field->name], description,
  '#time' => TRUE,
  '#description' => _profile_form_explanation($field),
  '#required' => $field->required
);

In other words if the developer specifies '#time' => TRUE, then the date form will also include drop downs for the time. Taking things one step further you could add options for 12 and 24 hour time.

 $fields[$category][$field->name] = array(
  '#type' => 'date',
  '#title' => check_plain($field->title),
  '#default_value' => $edit[$field->name], description,
  '#time_12' => TRUE,
  '#description' => _profile_form_explanation($field),
  '#required' => $field->required
);

The form generation tools are awesome for developing modules. I would really like to see something like this. I've looked at forms.inc, but I'm not sure how I would add time to this. Does anyone out there have a suggestion on how to add time to forms?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

strayhand’s picture

I took a stab at adding the "time" (hours and minutes) to all date forms by making some changes to the forms.inc file. Basically I went in and added hours and minutes to the arrays. My changes output a working form, however when I try to edit the form I run into issues, and I get the following error:

Fatal error: Cannot create references to/from string offsets nor overloaded objects in /home/fatstudi/public_html/beta/includes/form.inc on line 277

If you have any thoughts on how I might tweak this I'm all ears. Thanks. Here's the code that I've changed in the form.inc file.

/**
 * Roll out a single date element.
 */
function expand_date($element) {
  // Default to current date
  if (!isset($element['#value'])) {
    $element['#value'] = array('minute' => format_date(time(), 'custom', 'i'),
							'hour' => format_date(time(), 'custom', 'H'),
							'day' => format_date(time(), 'custom', 'j'),
                            'month' => format_date(time(), 'custom', 'n'),
                            'year' => format_date(time(), 'custom', 'Y'));
  }

  $element['#tree'] = TRUE;

  // Determine the order of day, month, year in the site's chosen date format.
  $format = variable_get('date_format_short', 'm/d/Y - H:i');
  $sort = array();
  $sort['minute'] = strpos($format, 'i');
  $sort['hour'] = max(strpos($format, 'H'), strpos($format, 'G'));
  $sort['day'] = max(strpos($format, 'd'), strpos($format, 'j'));
  $sort['month'] = max(strpos($format, 'm'), strpos($format, 'M'));
  $sort['year'] = strpos($format, 'Y');
  asort($sort);
  $order = array_keys($sort);

  // Output multi-selector for date
  foreach ($order as $type) {
    switch ($type) {
      case 'minute':
        $options = drupal_map_assoc(range(0, 59), 'map_minute');
        break;
      case 'hour':
        $options = drupal_map_assoc(range(0, 23), 'map_hour');
        break;
	  case 'day':
        $options = drupal_map_assoc(range(1, 31));
        break;
      case 'month':
        $options = drupal_map_assoc(range(1, 12), 'map_month');
        break;
      case 'year':
        $options = drupal_map_assoc(range(1900, 2050));
        break;
    }
    $parents = $element['#parents'];
    $parents[] = $type;
    $element[$type] = array(
      '#type' => 'select',
      '#value' => $element['#value'][$type],
      '#attributes' => $element['#attributes'],
      '#options' => $options,
    );
  }

  return $element;
}

/**
 * Helper function for usage with drupal_map_assoc to display month names.
 */
function map_month($month) {
  return format_date(gmmktime(0, 0, 0, $month, 2, 1970), 'custom', 'M', 0);
}

/**
 * Helper function for usage with drupal_map_assoc to display hours.
 */
function map_hour($hour) {
  return format_date(gmmktime($hour, 0, 0, 1, 2, 1970), 'custom', 'h', 0);
}

/**
 * Helper function for usage with drupal_map_assoc to display minutes.
 */
function map_minute($minute) {
  return format_date(gmmktime(0, $minute, 0, 1, 2, 1970), 'custom', 'i', 0);
}
chx’s picture

i am not willing to review this until you provide a patch. http;//drupal.org/diffandpatch a patch would show us what have you changed -- and because we know what have now works, but what you have done does not work, the problem is in the difference.

chx’s picture

oh and make sure that you do not get these errors with the original form.inc sometimes contrib modules cause these.

strayhand’s picture

Sounds reasonable. I don't know jack about CVS, or generating diff/patches. But I'll figure it out and post a patch here. Thanks.

strayhand’s picture

FileSize
1.23 KB

I downloaded ExamDiff and compaired my modified file against the form.inc file provided with 4.7 Beta 4. It gave me the option of saving the differences in a unix style diff file, which I have attached here. Please take a look at this patch and let me know if there's something obviously wrong with my implementation. I'm trying to add the hour to all form generated date elements. Thanks.

strayhand’s picture

FileSize
2.3 KB

Unified format? I tried WinMerge and it had an option for unified format. I'm going to include this patch as well.

Justin Freeman’s picture

Title: Adding "time" to form generation » Adding "time" to form generation, pls add this to Drupal core

Can we please get either this patch or at least the "idea" of having a time field type into the core Drupal form code. I believe this is critical, to ensure that there is a consistent method used in Drupal to display and store time values. Otherwise module developers will just come up with their own implementations.

Thanks for listening!

pfaocle’s picture

Version: 4.7.0-beta4 » x.y.z
hickory’s picture

Version: x.y.z » 6.x-dev
StevenPatz’s picture

Status: Active » Needs review

Make sure you set a patch to 'needs review' so developers know to review it.

Freso’s picture

This strikes me as something that ought to wait for Drupal 7, due to code freeze of D6.
Please do correct me if I am wrong on this - but if you, on the other hand, agree with me, please set the version appropriately.

chx’s picture

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

We will revisit datetime handling totally in Drupal 7 with the new features PHP5 gives us. D6 is indeed frozen.

Dave Cohen’s picture

Version: 6.x-dev » 7.x-dev
Status: Closed (won't fix) » Needs work

I've run into the same problem, wanting a time selector, so I'm bringing this issue back to life.

However, I'm not a fan of the proposed changes, because I need a time selector that doesn't care about date. That is, I just want the hour and minute. So my approach is to create a brand new element, just for time. I realize this is not a one size fits all scenario.

But I'm glad I found this issue, because I used the code here to write a module that provides the element I need. I'm sharing it here, but note its a 5.x module.

http://cvs.drupal.org/viewvc.py/drupal/contributions/sandbox/yogadex/mod...

I hope that's useful to anyone else who needs it sooner rather than later.

RobLoach’s picture

Title: Adding "time" to form generation, pls add this to Drupal core » Add a "datetime" Form API element
Issue tags: +form API, +datetime
FileSize
5.23 KB

This patch attempts to introduce a "datetime" form element and then uses it on the node form for the "Authored on" field. I went with creating a new field for it as that's what is pretty much already done on both the node form and the comment form with those textfields. We don't really have to worry about the upgrade path since it uses the timestamp.

Thoughts? There are still errors with it, but it's a good first step.......

RobLoach’s picture

Status: Needs work » Needs review
FileSize
22.03 KB
41.7 KB

This patch does two things:

  1. Adds the datetime Form API element
  2. Uses jQuery UI Date Picker as well as the jQuery UI Time Picker to make it easy to use
  3. Applies it to the "Authored on" field in both the node and comment form

Attached is the patch and a screenshot.

Status: Needs review » Needs work

The last submitted patch failed testing.

RobLoach’s picture

Status: Needs work » Needs review
FileSize
22.42 KB

This fixes a couple things in it.............

Status: Needs review » Needs work

The last submitted patch failed testing.

cwgordon7’s picture

-return FALSE;
+//return FALSE;

probably shouldn't be in the patch.

mfer’s picture

subscribe.

mfer’s picture

This would be very slick to get into drupal, but....

+  // jQuery UI Timepicker
+  $libraries['timepicker'] = array(
+    'title' => 'jQuery UI: Timepicker',
+    'website' => 'http://milesich.com/timepicker/',
+    'version' => '0.2.1',
+    'js' => array(
+      'misc/timepicker.js' => array(),
+    ),
+    'dependencies' => array(
+      array('system', 'ui.datepicker'),
+      array('system', 'ui.slider'),
+    ),
+  );

This is not part of jQuery UI and should not be noted as such in the comment or title.

-        return FALSE;
+        //return FALSE;

This should be fixed.

+function theme_datetime($element) {
+  $blah = drupal_add_library('system', 'timepicker');
+  drupal_add_js('misc/form.datetime.js');
+  return drupal_render_children($element);
+}

A variable name of $blah is not something we should be using.

+  // Default to current date
+  if (empty($element['#value'])) {
+    $element['#value'] = REQUEST_TIME;
+  }

Why do we assume if the value is empty the current time should be used? This may work for node forms but there may be other cases where the date is left blank and we don't want a value filled in. Filling in a default value should be done per specific forms and not at the field definition level.

On the website for the timepicker it says "It is very early release and it is buggy." Should we include something in drupal core the developer considers buggy?

Additionally, the timepicker is said to be copyright it's owner with no mention of a GPL or GPL compatible license. Without that we cannot add it to drupal core.

klonos’s picture

Ping? Any news on this? I mean its been almost a year.

Damien Tournoud’s picture

Version: 7.x-dev » 8.x-dev

Drupal 7 is long feature frozen now.

klonos’s picture

...thought so.

klonos’s picture

I did some googling around and posted most of my findings in this Node expire issue: #366475: Date and time instead of just date? (look in posts #10, #11 and #12).

webkenny’s picture

I think it makes the most sense for this to be a contributed module for Drupal 7 and a required feature of Drupal 8. However, more discussion is needed here. How do we envision timezones being handled? Do we use the site timezone? User timezone?

I wonder if we use the standard PHP masks though for determining whether time displays. In other words, given hh:mm tt or any representation of time, it makes the fields react. Seems more intuitive to the language.

Eric_A’s picture

#504338: Extend Form API's date field with jQuery UI's Date Picker.
#504524: Extend Authored on field with jQuery UI Date Picker.
The Date project provides element types date_select, date_text, date_timezone, date_combo and last but not least date_popup. See also #647390: Rework core CCK widgets to FAPI elements that have Field counterparts.

webkenny’s picture

@Eric_A: I saw those issues but none of the core widgets support time as it is and the Date Picker is JQuery UI. Is the intention that a normal form element (say on a mobile device which might not have JS enabled) would be skipped? My opinion is that we shouldn't need a contributed module to have time involved in date storage. it's a natural element of the date object anyway.

I can move the conversation (and contribution) anywhere. I just want to make sure we're talking about the right thing in the right place. :)

Eric_A’s picture

#501428: Date and time field type in core is also about adding more Form API element types to core, but takes the Field API angle.

andypost’s picture

swentel’s picture

Status: Needs work » Closed (fixed)

The date and field type module is indeed in core, but hook_element_info exposes it as a form api element, so we're good here.

sanghamitra.swain’s picture

For Drupal 8, the following piece of code works:

// Form element inside buildForm().

$form['start_date'] = [
      '#type' => 'datetime',
      '#title' => t('Start Date'),
      '#default_value' => DrupalDateTime::createFromTimestamp(strtotime($config->get('start_date')))
    ];

// Inside submitForm().
->set('start_date', (string) $form_state->getValue('start_date'))