Posted by YesCT on December 29, 2012 at 9:59am
7 followers
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | field_ui.module |
| Category: | task |
| Priority: | minor |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | Fields in Core, string freeze, Usability |
Issue Summary
Follow up for #552604: Adding new fields leads to a confusing "Field settings" form
Problem/Motivation
A like like: "Body has no field settings." shows on the global field settings tab ... when there are settings there.
Proposed resolution
None yet.
Remaining tasks
- Figure out what that line really means or what it was intended for.
- Propose a solution.
- Implement the change, provide patch
- review patch
- A screenshot with the fix would be nice.
User interface changes
Not really.
API changes
No api changes anticipated.
Comments
#1
The creation of the field and editing of the field settings is similar.
#2
Since #552604-99: Adding new fields leads to a confusing "Field settings" form added cardinality setting (to every field?)
@@ -572,7 +573,42 @@ function field_ui_field_settings_form($form, &$form_state, $instance) {// If so, prevent changes to the field settings.
$has_data = field_has_data($field);
if ($has_data) {
- $form['field']['#description'] = '<div class="messages error">' . t('There is data for this field in the database. The field settings can no longer be changed.') . '</div>' . $form['field']['#description'];
+ $form['field']['#prefix'] = '<div class="messages error">' . t('There is data for this field in the database. The field settings can no longer be changed.') . '</div>' . $form['field']['#prefix'];
+ }
+
+ // Build the configurable field values.
+ $cardinality = $field['cardinality'];
+ $form['field']['container'] = array(
+ // We can't use the container element because it doesn't support the title
+ // or description properties.
+ '#type' => 'item',
+ '#field_prefix' => '<div class="container-inline">',
+ '#field_suffix' => '</div>',
+ '#title' => t('Maximum number of values users can enter'),
+ );
I think there are no fields that will appear to have no field settings. (All fields will at least have the cardinality setting.)
Here is the bit that adds the note about no field settings:
// Add settings provided by the field module. The field module is// responsible for not returning settings that cannot be changed if
// the field already has data.
$form['field']['settings'] = array(
'#weight' => 10,
);
$additions = module_invoke($field['module'], 'field_settings_form', $field, $instance, $has_data);
if (is_array($additions)) {
$form['field']['settings'] += $additions;
}
if (!element_children($form['field']['settings'])) {
$form['field']['settings'] += array(
'#markup' => '<p>' . t('%field has no field settings.', array('%field' => $instance['label'])) . '</p>',
);
}
so maybe it's really meaning... "has no field settings, added by the field module, that can be changed" ?
#3
If it's assumed that every field will be selected the number of allowed values (they will all have the cardinality setting)... then I think we can just take out the line that checks for no settings.
If someone can give steps to reproduce adding a field that has no global settings, please add the steps in a comment. :) I could not get one.
How would contrib control the cardinality setting being there or not? Could contrib provide a field or widget where it would not make sense to allow multiple values?
#4
#5
The last submitted patch, drupal-has_no_field_settings-1876134-3.patch, failed testing.
#6
#3: drupal-has_no_field_settings-1876134-3.patch queued for re-testing.
#7
Yes, that looks correct. Could probably use a look-over by someone @yched-ish :-) for any edge cases that I didn't think of, where this sentence still makes sense. It seems absolutely senseless to me, in the current form.
#8
Agreed, displaying this message males no sense now, there will always be something on that screen, unless some module form_alters() the cardinality select out, in which case the UX WTF of an empty screen is that module's business.
#9
as soon as this gets in, #1876122: Missing new line near enable/disable translation on global field settings tab should be closed.
#10
#3: drupal-has_no_field_settings-1876134-3.patch queued for re-testing.
#11
Looks great. Committed/pushed to 8.x.
#12
Automatically closed -- issue fixed for 2 weeks with no activity.