[D7] labelvalue

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Here is my review of the module.

Major coding standards / best practice issues

An automated review of your project has found some issues with your code. Please take a close look at the pareview.sh review of your module.

README.txt

Please take a moment to make your README.txt follow the guidelines for in-project documentation. README.txt should include the description of the purpose of the module which is currently lacking.

Return value of labelvalue_field_schema() function

Upon adding a field of type "Label value", I get an error Warning: Invalid argument supplied for foreach() in _field_sql_storage_schema() (line 182 of /Sites/labelvalue/drupal/modules/field/modules/field_sql_storage/field_sql_storage.module). Please see the documentation of _field_sql_storage_schema() function and return table index array elements as arrays:

function labelvalue_field_schema($field) {
  // ..
    'indexes' => array(
      'labelvalue_label' => array('labelvalue_label'),
      'labelvalue_value' => array('labelvalue_value'),
    ),
  // ..
}

Word capitalisation

1. Module name in labelvalue.info file is "Label Value" (please note the extra space).
2. labelvalue_field_info() defines a label for the labelvalue field as "Label Value" (please note the extra space).
3. labelvalue_field_widget_info() defines a label for the widget as "Label Value Default Widget".

While documentation on Writing module .info files suggests to capitalise every word of the module, that fact is that every Drupal core module, that has more than one word, has only the first word capitalised. It would be more consistent with other modules to follow the same pattern.

Variable naming consistency

labelvalue_field_formatter_view() function assigns $instance variable (field instance) to #field theme variable which makes the code harder to read and debug as $field and $instance each has their own content and meaning in Field API system. You should rename theme variable to field_instance or similar name consistent with the content of the variable.

Stylesheet attachment

labelvalue_field_widget_form() function includes module specific stylesheet file using drupal_add_css() function. Consider including stylesheet files using #attached property that is part of Drupal Form API. Modifying widget styling is much more simpler via hook_field_widget_form_alter() rather than other means of CSS file extraction from the output. An example for your module:

/**
 * Implements hook_field_widget_form().
 */
function labelvalue_field_widget_form(&$form, &$form_state, $field, $instance, $langcode, $items, $delta, $element) {
  // .. 
  $element['#attached']['css'][] = drupal_get_path('module', 'labelvalue') . '/labelvalue.css';

  return $element;
}

I really like the simplicity of this module. Thank you for that!

How does this module differ from Double field module which allows to select from a plenty of widgets that have key => value format, including two textfields for label and value? I installed Double field module, created a field and got exatly the same result as the output of labelvalue module.

Have you done a proper research before creating this module? There is, in essence, no added value that it provides.

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.