Calculate valid hours and minutes intervals at the field widget level, not the form element [#1913034]

This data is invariant for a widget, there's no need to calculate it a ton of times.

Comment	File	Size	Author
#5	office_hours_1913034.patch	0 bytes	johnv
#5
#3	office_hours-1913034.patch	4.8 KB	jonhattan
#3
#1	office_hours-1913034.patch	5.06 KB	jonhattan
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

jonhattan

Spanish

Plasencia

CreditAttribution: jonhattan commented 9 February 2013 at 20:07

Status:

Active

» Needs review

File	Size
office_hours-1913034.patch	5.06 KB

Another benefit of this change is that form elements are simpler since we send less parameters through.

Comment #2

jonhattan

Spanish

Plasencia

CreditAttribution: jonhattan commented 9 February 2013 at 20:10

Above patch includes an unrelated change in _office_hours_slot_validate():

+++ b/office_hours.elements.inc
@@ -153,22 +148,15 @@ function _office_hours_slot_validate($element, &$form_state) {
-  elseif (!empty($element['#limitstart']) || !empty($element['#limitend'])) {
-    $starthours = (int) substr($item['starthours'], 0, 2);
-    $endhours = (int) substr($item['endhours'], 0, 2);
-    if ( ($starthours && $element['#limitstart'] > $starthours) || ($endhours && $element['#limitend'] < $endhours) ) {
-      $error_text = 'Opening hours are outside limits' . ' (' . $element['#limitstart'] . ' - ' . $element['#limitend'] . ').';
-    }

This is only met if the user hijacks a select element with firebug or similar. Drupal does its own validation for this case.

OTOH:

+++ b/office_hours.module
@@ -586,12 +586,33 @@ function office_hours_field_widget_form(&$form, &$form_state, $field, $instance,
+  $hours = array('' => '');
+  for ($i=$start;$i<=$end;$i++) {
+    $hours[$i] = $i < 10 && ($field['settings']['hoursformat'] == 0) ? "0$i" : $i;
+  }

This is easier than calling date_hours() and filtering results afterwards.

File	Size
office_hours-1913034.patch	4.8 KB

Comment #4

johnv

Dutch

CreditAttribution: johnv commented 6 March 2013 at 20:08

Status:

Needs review

» Needs work

This is not working for two reasons:

You are moving calculations from _process() to _widget_form(), which (minimally) slows down the rendering of the page. In the current situation, the calculation is upon submitting the page, when already all heavy lifting is being executed.
Also _widget_form is executed 28 times. (This might be fixed with your last and final patch for the week-widget)

But above all: it is not working with a 12-hour clock and hours after 12:00, and is breaking existing data when changing from 24-hour clock to 12-hour clock.

Comment #5

johnv

Dutch

CreditAttribution: johnv commented 16 May 2013 at 18:19

Status:

Needs work

» Fixed

File	Size
office_hours_1913034.patch	0 bytes

Forget my previous post. I was confused.
Attached patch is committed here.

I also contains the following:
- changes in css to be able the block element in nomal forms.
- corrected the non-working hours limits when working in 12hr mode.

Comment #6

johnv

Dutch

CreditAttribution: johnv commented 17 May 2013 at 08:30

Added another fix.
I removed the hook_field_presave(), and moved the functionality to the widget_form, since the hook_field_presave() and hook_field_load() are not loaded on the widget settings page, where we now have a multivalue default value.

See #1944678: On field settings form, hook_field_load() and hook_field_presave() are not called. .