Issue #1164812 by Niklas Fiekas, tim.plunkett, xjm: Improve UX for machine names for fields.

5 files changed, 97 insertions, 41 deletions

diff --git a/includes/form.inc b/includes/form.inc
index da6e03027..a7803448d 100644
--- a/includes/form.inc
+++ b/includes/form.inc
@@ -3395,6 +3395,9 @@ function form_process_tableselect($element) {
  *       different character, 'replace_pattern' needs to be set accordingly.
  *     - error: (optional) A custom form error message string to show, if the
  *       machine name contains disallowed characters.
+ *     - standalone: (optional) Whether the live preview should stay in its own
+ *       form element rather than in the suffix of the source element. Defaults
+ *       to FALSE.
  *   - #maxlength: (optional) Should be set to the maximum allowed length of the
  *     machine name. Defaults to 64.
  *   - #disabled: (optional) Should be set to TRUE in case an existing machine
@@ -3406,6 +3409,9 @@ function form_process_machine_name($element, &$form_state) {
     '#title' => t('Machine-readable name'),
     '#description' => t('A unique machine-readable name. Can only contain lowercase letters, numbers, and underscores.'),
     '#machine_name' => array(),
+    '#field_prefix' => '',
+    '#field_suffix' => '',
+    '#suffix' => '',
   );
   // A form element that only wants to set one #machine_name property (usually
   // 'source' only) would leave all other properties undefined, if the defaults
@@ -3416,6 +3422,9 @@ function form_process_machine_name($element, &$form_state) {
     'label' => t('Machine name'),
     'replace_pattern' => '[^a-z0-9_]+',
     'replace' => '_',
+    'standalone' => FALSE,
+    'field_prefix' => $element['#field_prefix'],
+    'field_suffix' => $element['#field_suffix'],
   );
 
   // By default, machine names are restricted to Latin alphanumeric characters.
@@ -3431,7 +3440,7 @@ function form_process_machine_name($element, &$form_state) {
   }
 
   // Retrieve the form element containing the human-readable name from the
-  // complete form in $form_state. By reference, because we need to append
+  // complete form in $form_state. By reference, because we may need to append
   // a #field_suffix that will hold the live preview.
   $key_exists = NULL;
   $source = drupal_array_get_nested_value($form_state['complete form'], $element['#machine_name']['source'], $key_exists);
@@ -3439,16 +3448,21 @@ function form_process_machine_name($element, &$form_state) {
     return $element;
   }
 
-  // Append a field suffix to the source form element, which will contain
-  // the live preview of the machine name.
   $suffix_id = $source['#id'] . '-machine-name-suffix';
-  $source += array('#field_suffix' => '');
-  $source['#field_suffix'] .= ' <small id="' . $suffix_id . '">&nbsp;</small>';
+  $element['#machine_name']['suffix'] = '#' . $suffix_id;
 
-  $parents = array_merge($element['#machine_name']['source'], array('#field_suffix'));
-  drupal_array_set_nested_value($form_state['complete form'], $parents, $source['#field_suffix']);
+  if ($element['#machine_name']['standalone']) {
+    $element['#suffix'] .= ' <small id="' . $suffix_id . '">&nbsp;</small>';
+  }
+  else {
+    // Append a field suffix to the source form element, which will contain
+    // the live preview of the machine name.
+    $source += array('#field_suffix' => '');
+    $source['#field_suffix'] .= ' <small id="' . $suffix_id . '">&nbsp;</small>';
 
-  $element['#machine_name']['suffix'] = '#' . $suffix_id;
+    $parents = array_merge($element['#machine_name']['source'], array('#field_suffix'));
+    drupal_array_set_nested_value($form_state['complete form'], $parents, $source['#field_suffix']);
+  }
 
   $js_settings = array(
     'type' => 'setting',
diff --git a/misc/machine-name.js b/misc/machine-name.js
index 3f8e788a2..66eb9d472 100644
--- a/misc/machine-name.js
+++ b/misc/machine-name.js
@@ -19,6 +19,10 @@ Drupal.behaviors.machineName = {
    *     disallowed characters in the machine name; e.g., '[^a-z0-9]+'.
    *   - replace: A character to replace disallowed characters with; e.g., '_'
    *     or '-'.
+   *   - standalone: Whether the preview should stay in its own element rather
+   *     than the suffix of the source element.
+   *   - field_prefix: The #field_prefix of the form element.
+   *   - field_suffix: The #field_suffix of the form element.
    */
   attach: function (context, settings) {
     var self = this;
@@ -49,10 +53,12 @@ Drupal.behaviors.machineName = {
         var machine = self.transliterate($source.val(), options);
       }
       // Append the machine name preview to the source field.
-      var $preview = $('<span class="machine-name-value">' + machine + '</span>');
-      $suffix.empty()
-        .append(' ').append('<span class="machine-name-label">' + options.label + ':</span>')
-        .append(' ').append($preview);
+      var $preview = $('<span class="machine-name-value">' + options.field_prefix + Drupal.checkPlain(machine) + options.field_suffix + '</span>');
+      $suffix.empty();
+      if (options.label) {
+        $suffix.append(' ').append('<span class="machine-name-label">' + options.label + ':</span>');
+      }
+      $suffix.append(' ').append($preview);
 
       // If the machine name cannot be edited, stop further processing.
       if ($target.is(':disabled')) {
@@ -80,7 +86,7 @@ Drupal.behaviors.machineName = {
           if (machine != '') {
             if (machine != options.replace) {
               $target.val(machine);
-              $preview.text(machine);
+              $preview.html(options.field_prefix + Drupal.checkPlain(machine) + options.field_suffix);
             }
             $suffix.show();
           }
diff --git a/modules/field_ui/field_ui.admin.inc b/modules/field_ui/field_ui.admin.inc
index f19429df5..33ce2f05c 100644
--- a/modules/field_ui/field_ui.admin.inc
+++ b/modules/field_ui/field_ui.admin.inc
@@ -316,7 +316,7 @@ function field_ui_field_overview_form($form, &$form_state, $entity_type, $bundle
       t('Label'),
       t('Weight'),
       t('Parent'),
-      t('Name'),
+      t('Machine name'),
       t('Field'),
       t('Widget'),
       array('data' => t('Operations'), 'colspan' => 2),
@@ -498,16 +498,24 @@ function field_ui_field_overview_form($form, &$form_state, $entity_type, $bundle
         ),
       ),
       'field_name' => array(
-        '#type' => 'textfield',
+        '#type' => 'machine_name',
         '#title' => t('New field name'),
         '#title_display' => 'invisible',
         // This field should stay LTR even for RTL languages.
         '#field_prefix' => '<span dir="ltr">field_',
         '#field_suffix' => '</span>&lrm;',
-        '#attributes' => array('dir'=>'ltr'),
-        '#size' => 10,
-        '#description' => t('Field name (a-z, 0-9, _)'),
+        '#size' => 15,
+        '#description' => t('A unique machine-readable name containing letters, numbers, and underscores.'),
+        // 32 characters minus the 'field_' prefix.
+        '#maxlength' => 26,
         '#prefix' => '<div class="add-new-placeholder">&nbsp;</div>',
+        '#machine_name' => array(
+          'source' => array('fields', $name, 'label'),
+          'exists' => '_field_ui_field_name_exists',
+          'standalone' => TRUE,
+          'label' => '',
+        ),
+        '#required' => FALSE,
       ),
       'type' => array(
         '#type' => 'select',
@@ -678,25 +686,8 @@ function _field_ui_field_overview_form_validate_add_new($form, &$form_state) {
       $field_name = $field['field_name'];
 
       // Add the 'field_' prefix.
-      if (substr($field_name, 0, 6) != 'field_') {
-        $field_name = 'field_' . $field_name;
-        form_set_value($form['fields']['_add_new_field']['field_name'], $field_name, $form_state);
-      }
-
-      // Invalid field name.
-      if (!preg_match('!^field_[a-z0-9_]+$!', $field_name)) {
-        form_set_error('fields][_add_new_field][field_name', t('Add new field: the field name %field_name is invalid. The name must include only lowercase unaccentuated letters, numbers, and underscores.', array('%field_name' => $field_name)));
-      }
-      if (strlen($field_name) > 32) {
-        form_set_error('fields][_add_new_field][field_name', t("Add new field: the field name %field_name is too long. The name is limited to 32 characters, including the 'field_' prefix.", array('%field_name' => $field_name)));
-      }
-
-      // Field name already exists. We need to check inactive fields as well, so
-      // we can't use field_info_fields().
-      $fields = field_read_fields(array('field_name' => $field_name), array('include_inactive' => TRUE));
-      if ($fields) {
-        form_set_error('fields][_add_new_field][field_name', t('Add new field: the field name %field_name already exists.', array('%field_name' => $field_name)));
-      }
+      $field_name = 'field_' . $field_name;
+      form_set_value($form['fields']['_add_new_field']['field_name'], $field_name, $form_state);
     }
 
     // Missing field type.
@@ -719,6 +710,24 @@ function _field_ui_field_overview_form_validate_add_new($form, &$form_state) {
 }
 
 /**
+ * Render API callback: Checks if a field machine name is taken.
+ *
+ * @param $value
+ *   The machine name, not prefixed with 'field_'.
+ *
+ * @return
+ *   Whether or not the field machine name is taken.
+ */
+function _field_ui_field_name_exists($value) {
+  // Prefix with 'field_'.
+  $field_name = 'field_' . $value;
+
+  // We need to check inactive fields as well, so we can't use
+  // field_info_fields().
+  return (bool) field_read_fields(array('field_name' => $field_name), array('include_inactive' => TRUE));
+}
+
+/**
  * Validates the 'add existing field' row of field_ui_field_overview_form().
  *
  * @see field_ui_field_overview_form_validate()
diff --git a/modules/field_ui/field_ui.css b/modules/field_ui/field_ui.css
index ae469e207..2184023b7 100644
--- a/modules/field_ui/field_ui.css
+++ b/modules/field_ui/field_ui.css
@@ -12,6 +12,10 @@ table.field-ui-overview tr.add-new .tabledrag-changed {
 }
 table.field-ui-overview tr.add-new .description {
   margin-bottom: 0;
+  max-width: 250px;
+}
+table.field-ui-overview tr.add-new .form-type-machine-name .description {
+  white-space: normal;
 }
 table.field-ui-overview tr.add-new .add-new-placeholder {
   font-weight: bold;
@@ -29,6 +33,10 @@ table.field-ui-overview tr.region-populated {
 table.field-ui-overview tr.region-add-new-title {
   display: none;
 }
+table.field-ui-overview tr.add-new td {
+  vertical-align: top;
+  white-space: nowrap;
+}
 
 /* 'Manage display' overview */
 #field-display-overview .field-formatter-summary-cell {
diff --git a/modules/field_ui/field_ui.test b/modules/field_ui/field_ui.test
index f1e770baa..75a70846f 100644
--- a/modules/field_ui/field_ui.test
+++ b/modules/field_ui/field_ui.test
@@ -172,7 +172,7 @@ class FieldUIManageFieldsTestCase extends FieldUITestCase {
     // Check all table columns.
     $table_headers = array(
       t('Label'),
-      t('Name'),
+      t('Machine name'),
       t('Field'),
       t('Widget'),
       t('Operations'),
@@ -338,7 +338,7 @@ class FieldUIManageFieldsTestCase extends FieldUITestCase {
     $bundle_path1 = 'admin/structure/types/manage/' . $this->hyphen_type;
     $edit1 = array(
       'fields[_add_new_field][label]' => $this->field_label,
-      'fields[_add_new_field][field_name]' => $this->field_name,
+      'fields[_add_new_field][field_name]' => $this->field_name_input,
     );
     $this->fieldUIAddNewField($bundle_path1, $edit1);
 
@@ -426,6 +426,25 @@ class FieldUIManageFieldsTestCase extends FieldUITestCase {
 
     $this->drupalGet('admin/structure/types/manage/' . $hyphen_type2 . '/fields');
   }
+
+  /**
+   * Tests that a duplicate field name is caught by validation.
+   */
+  function testDuplicateFieldName() {
+    // field_tags already exists, so we're expecting an error when trying to
+    // create a new field with the same name.
+    $edit = array(
+      'fields[_add_new_field][field_name]' => 'tags',
+      'fields[_add_new_field][label]' => $this->randomName(),
+      'fields[_add_new_field][type]' => 'taxonomy_term_reference',
+      'fields[_add_new_field][widget_type]' => 'options_select',
+    );
+    $url = 'admin/structure/types/manage/' . $this->hyphen_type . '/fields';
+    $this->drupalPost($url, $edit, t('Save'));
+
+    $this->assertText(t('The machine-readable name is already in use. It must be unique.'));
+    $this->assertUrl($url, array(), 'Stayed on the same page.');
+  }
 }
 
 /**
@@ -454,7 +473,7 @@ class FieldUIManageDisplayTestCase extends FieldUITestCase {
     // Create a field, and a node with some data for the field.
     $edit = array(
       'fields[_add_new_field][label]' => 'Test field',
-      'fields[_add_new_field][field_name]' => 'field_test',
+      'fields[_add_new_field][field_name]' => 'test',
     );
     $this->fieldUIAddNewField($manage_fields, $edit);
 
@@ -499,7 +518,7 @@ class FieldUIManageDisplayTestCase extends FieldUITestCase {
     // Create a field, and a node with some data for the field.
     $edit = array(
       'fields[_add_new_field][label]' => 'Test field',
-      'fields[_add_new_field][field_name]' => 'field_test',
+      'fields[_add_new_field][field_name]' => 'test',
     );
     $this->fieldUIAddNewField('admin/structure/types/manage/' . $this->hyphen_type, $edit);
     // For this test, use a formatter setting value that is an integer unlikely