Feature/transportpulje form #8
Conversation
…aering-forloeb/ulf into feature/transportpulje-form
…aering-forloeb/ulf into feature/transportpulje-form
| 'status' => 1, | ||
| 'theme' => 'ulf_viborg', | ||
| 'weight' => 0, | ||
| ), |
There was a problem hiding this comment.
Why have this footer been removed ?
There was a problem hiding this comment.
Should not be part of block features as it never reverts correctly
| @@ -169,6 +169,19 @@ function secure_permissions_data_secure_permissions($role) { | |||
| 'view user map', | |||
| 'schedule publishing of nodes', | |||
| 'view scheduled content', | |||
| 'administer custom settings', | |||
| ), | |||
|
|
|||
There was a problem hiding this comment.
Why this empty line. It's not between the other roles in this array.
| #transportpulje-form-add-application > div { | ||
| width: 100%; | ||
| display: -webkit-flex; /* Safari */ | ||
| -webkit-flex-wrap: wrap; /* Safari 6.1+ */ |
There was a problem hiding this comment.
Please remove the old safari lines. Stats says that it's less than 0,5 precent.
| @@ -0,0 +1,133 @@ | |||
| #transportpulje-form-add-application > div { | |||
There was a problem hiding this comment.
The file should be named transportpulje_form.css so it's easy to figure out where it's coming from in the theme layer,
| @@ -0,0 +1,47 @@ | |||
| /** | |||
There was a problem hiding this comment.
Would be nice to have a better name for the JS files. Eg. transportpulje-course-form-alter.js to is would be easier to see where this comes from when debugging the site.
| unset($form_state['values']['distance_calc_failed']); | ||
|
|
||
| foreach ($form_state['values'] as $key => $value) { | ||
| if(empty($value) && $value != '0') { |
There was a problem hiding this comment.
Empty => A variable is considered empty if it does not exist or if its value equals FALSE
0 is the same an FALSE or is this a string with '0' in ?
There was a problem hiding this comment.
I'm looking for sting '0'
| /** | ||
| * Validate expenses for numeric. | ||
| * | ||
| * @param $form_state |
There was a problem hiding this comment.
Missing parameter description!
| /** | ||
| * Validate email fields / Email repeat check. | ||
| * | ||
| * @param $form_state |
There was a problem hiding this comment.
Missing parameter description!
| /** | ||
| * Validate email fields / Email repeat check. | ||
| * | ||
| * @param $form_state |
There was a problem hiding this comment.
Missing parameter description!
| * @return bool | ||
| */ | ||
| function _tpf_validate_email_address($form_state) { | ||
| return(filter_var($form_state['values']['email'], FILTER_VALIDATE_EMAIL)) ? FALSE : TRUE; |
There was a problem hiding this comment.
Could be rewritten:
return filter_var($form_state['values']['email'], FILTER_VALIDATE_EMAIL);
There was a problem hiding this comment.
Why the statement is just extra code thats not needed?
| @@ -216,6 +216,9 @@ function ulf_default_preprocess_node(&$variables) { | |||
| $variables['profile_postal_code'] = $account->location['postal_code']; | |||
| $variables['profile_city'] = $account->location['city']; | |||
| } | |||
|
|
|||
| // Transport application form | |||
| $variables['transport_form'] = module_exists('transportpulje_form') ? TRUE : FALSE; | |||
There was a problem hiding this comment.
Could be re-written:
$variables['transport_form'] = module_exists('transportpulje_form');
| 'description' => t('Access administration tasks for customized settings.'), | ||
| ), | ||
| ); | ||
| } |
| if ('tpf_school' === $form_values['institution_type'] && 'grade_6' <= $form_values['institution_grade']) { | ||
| $distance = _tpf_get_distance($form_values); | ||
|
|
||
| if (is_array($distance) && array_key_exists('scalar', $distance)) { |
There was a problem hiding this comment.
Could be re-written to:
if (isset($distance['scalar']))
| $distance = _tpf_get_distance($form_values); | ||
|
|
||
| if (is_array($distance) && array_key_exists('scalar', $distance)) { | ||
| if (6 >= $distance['scalar']) { |
There was a problem hiding this comment.
Drupal do not use yoda notation!
There was a problem hiding this comment.
Wont' fix! All Hail Yoda!
| // Get the institutions address/location. | ||
| $tpf_institution = taxonomy_term_load($form_values['institution_name']); | ||
| $tpf_from = field_get_items('taxonomy_term', $tpf_institution, 'field_field_tpf_geo_location'); | ||
| $pos_from = empty($tpf_from[0]) ? NULL : array('lat' => $tpf_from[0]['latitude'], 'lon' => $tpf_from[0]['longitude']); |
| * True if input is not an integer. | ||
| */ | ||
| function _tpf_validate_participants(array $form_state) { | ||
| return(filter_var($form_state['values']['participants'], FILTER_VALIDATE_INT)) ? FALSE : TRUE; |
There was a problem hiding this comment.
Could be re-written:
return filter_var($form_state['values']['participants'], FILTER_VALIDATE_INT);
There was a problem hiding this comment.
I'm retuning the reverse
| function _tpf_modify_course_dropdown(array $courses) { | ||
| // Cache the dropdown list. | ||
| if (!isset($courses)) { | ||
| if ($cache = cache_get('tpf_dropdown_data')) { |
There was a problem hiding this comment.
You need to check if the cache has exipred.
if ($cache = cache_get('tpf_dropdown_data') && REQUEST_TIME < $cache->expire)
| ->condition('bundle', 'course') | ||
| ->condition('entity_id', array_keys($courses['course']), 'IN') | ||
| ->execute(); | ||
| foreach ($results as $result) { |
There was a problem hiding this comment.
Would like an empty line before foreach.
| $form_state['values']['course_dropdown']; | ||
|
|
||
| // Set address from either selected node or user provided values. | ||
| $form_address = 'ERROR - NO VALID ADDRESS FOUND'; |
There was a problem hiding this comment.
Should this be translatable!
| * @return bool | ||
| * True if course is selected and street, postal code and city vary from node. | ||
| */ | ||
| function _tpf_address_variation(array $values) { |
There was a problem hiding this comment.
Maybe use an $ret = FALSE and change it to TRUE and return $ret;
No description provided.