Some things I noticed while reviewing...

Body settings

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yched’s picture

Title: Clean up Body field UI settings » Clean up UI for 'Text with summary' field settings

It's not a preview, it's a widget to input the field's default value. All field types show this (except when they don't support default values).
My personal opinion is that it belongs to it's own, separate subform.

About the field being 'long': see #513414: User-Test Position of Teaser field above main text field in node form

yched’s picture

Component: field system » field_ui.module
Bojhan’s picture

FileSize
19.02 KB
2.82 KB

cleanupunfinished.png

Ok, started the cleanup - encountered some issues trying to get the new (Edit Summary) pattern to apply to here as well. Would also like to put Default value in a separate fieldset.

sign’s picture

(Edit Summary) pattern applied

added a new div wrapper around textareas with class .text-textarea-with-summary-wrapper, didn't find any better way to do this, as we can't easily add a class to lets say #body-wrapper (field.form.inc line 152)

$element['#prefix'] = '<div class="text-textarea-with-summary-wrapper">';
$element['#suffix'] = '</div>';
Bojhan’s picture

FileSize
15.83 KB
0 bytes

A screenshot of sign his patch. Adjusted it a bit to put default values in its own fieldset to create more visual seperation.

cleanupmorefinished.png

sun’s picture

Status: Active » Needs work
FileSize
1.67 KB

Can you merge this patch into yours, please?

+++ modules/field_ui/field_ui.admin.inc	5 Oct 2009 19:28:47 -0000
@@ -1046,15 +1046,13 @@ function field_ui_field_edit_form($form,
-    '#title' => t('Required'),
+    '#title' => t('Required field'),

Please revert this - it's hard to translate "field" in this context.

+++ modules/field_ui/field_ui.admin.inc	5 Oct 2009 19:28:47 -0000
@@ -1062,7 +1060,7 @@ function field_ui_field_edit_form($form,
-    '#description' => t('Instructions to present to the user below this field on the editing form.<br />Allowed HTML tags: @tags', array('@tags' => _field_filter_xss_display_allowed_tags())),
+    '#description' => t('Help to present to the user below this field on the editing form.<br />Allowed HTML tags: @tags', array('@tags' => _field_filter_xss_display_allowed_tags())),

I think we should revert this, too. I wanted to say

s/Help/Help text/

...but then figured that it's actually a description, so "Instructions" seems to be correct.

I'm on crack. Are you, too?

Bojhan’s picture

Somehow, it turned out empty. I will look into it when I get home.

I do think "Required field" is far more descriptive then, required - then one would guess what required means in this context. I don't think we should avoid increasing the usability of this checkbox over translatability.

Instructions is better indeed, it was an attempt to make that whole help text area - better, but that seems like a hard thing to achieve.

Bojhan’s picture

FileSize
4.96 KB
Bojhan’s picture

Status: Needs work » Needs review

For testing.

lisarex’s picture

Status: Needs review » Reviewed & tested by the community

Looks great. RTBC.

sun’s picture

Status: Reviewed & tested by the community » Needs review
+++ modules/field/modules/text/text.js	5 Oct 2009 19:40:02 -0000
@@ -8,7 +8,7 @@
-      var $fieldset = $(this).closest('#body-wrapper');
+      var $fieldset = $(this).closest('.text-textarea-with-summary-wrapper');
+++ modules/field/modules/text/text.module	5 Oct 2009 19:40:03 -0000
@@ -746,6 +746,9 @@ function text_textarea_with_summary_proc
+  $element['#prefix'] = '<div class="text-textarea-with-summary-wrapper">';
+  $element['#suffix'] = '</div>';
+
   return $element;

This is not the fieldset.

+++ modules/field_ui/field_ui.admin.inc	5 Oct 2009 20:02:42 -0000
@@ -1046,15 +1046,13 @@ function field_ui_field_edit_form($form,
-    '#weight' => -10,
+    '#weight' => 0,

A #weight of 0 doesn't need to be specified, it's the default.

+++ modules/field_ui/field_ui.admin.inc	5 Oct 2009 20:02:42 -0000
@@ -1062,7 +1060,7 @@ function field_ui_field_edit_form($form,
-    '#description' => t('Instructions to present to the user below this field on the editing form.<br />Allowed HTML tags: @tags', array('@tags' => _field_filter_xss_display_allowed_tags())),
+    '#description' => t('Help to present to the user below this field on the editing form.<br />Allowed HTML tags: @tags', array('@tags' => _field_filter_xss_display_allowed_tags())),

This is no help text. Please revert to instructions.

+++ modules/field_ui/field_ui.admin.inc	5 Oct 2009 20:02:42 -0000
@@ -1187,7 +1185,7 @@ function field_ui_field_edit_instance_pr
 function field_ui_default_value_widget($field, $instance, &$form, &$form_state) {
-  $form['instance']['default_value_widget'] = array(
+  $form['default_value_widget'] = array(
@@ -1209,7 +1207,7 @@ function field_ui_default_value_widget($
-  $form['instance']['default_value_widget'] += $widget_form;
+  $form['default_value_widget'] += $widget_form;

Care to explain these changes?

Perhaps we should wait for the Field UI tests patch... ;)

I'm on crack. Are you, too?

MichaelCole’s picture

#8: cleanup.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, cleanup.patch, failed testing.

colan’s picture

The solution is to dump the entire concept. See #1378350: Clean up the "Long text and summary" field for details.