Merge pull request #1382 from gasman/cleanup/singlefieldpanel-labels

Cleanup - render labels within ObjectList <h2>s

wagtail/wagtailadmin/edit_handlers.py

@@ -235,6 +235,14 @@ class EditHandler(object):
         """
         return ""
 
+    def id_for_label(self):
+        """
+        The ID to be used as the 'for' attribute of any <label> elements that refer
+        to this object but are rendered outside of it. Leave blank if this object does not render
+        as a single input field.
+        """
+        return ""
+
     def render_as_object(self):
         """
         Render this object as it should appear within an ObjectList. Should not
@@ -438,19 +446,21 @@ class BaseFieldPanel(EditHandler):
             classes.append("error")
 
         classes.append(self.field_type())
-        classes.append("single-field")
 
         return classes
 
     def field_type(self):
         return camelcase_to_underscore(self.bound_field.field.__class__.__name__)
 
+    def id_for_label(self):
+        return self.bound_field.id_for_label
+
     object_template = "wagtailadmin/edit_handlers/single_field_panel.html"
 
     def render_as_object(self):
         return mark_safe(render_to_string(self.object_template, {
             'self': self,
-            'field_content': self.render_as_field(),
+            'field': self.bound_field,
         }))
 
     field_template = "wagtailadmin/edit_handlers/field_panel_field.html"
@@ -728,13 +738,6 @@ class BaseStreamFieldPanel(BaseFieldPanel):
         classes = super(BaseStreamFieldPanel, self).classes()
         classes.append("stream-field")
 
-        # BaseFieldPanel is essentially for single fields, which are rendered on the front end
-        # with the assumption that the label (singular) will always be promoted to the full-width
-        # divider bar thing.
-        # This results in all the other labels being promoted similarly, so it's better not to
-        # treat this as a single field, and remove the "single-field" class.
-        classes.remove("single-field")
-
         # In case of a validation error, BlockWidget will take care of outputting the error on the
         # relevant sub-block, so we don't want the stream block as a whole to be wrapped in an 'error' class.
         if 'error' in classes:
@@ -746,6 +749,12 @@ class BaseStreamFieldPanel(BaseFieldPanel):
     def html_declarations(cls):
         return cls.block_def.all_html_declarations()
 
+    def id_for_label(self):
+        # a StreamField may consist of many input fields, so it's not meaningful to
+        # attach the label to any specific one
+        return ""
+
+
 class StreamFieldPanel(object):
     def __init__(self, field_name):
         self.field_name = field_name

wagtail/wagtailadmin/static_src/wagtailadmin/scss/layouts/page-editor.scss

@@ -77,7 +77,7 @@
         opacity:1;
     }
 
-    > h2, &.single-field label{
+    > h2{
         -webkit-font-smoothing: auto;
         background:$color-salmon-light;
         text-transform:uppercase;
@@ -93,7 +93,17 @@
         z-index:1;
         overflow:hidden;
 
-        &:before{
+        label{
+            display: inline;
+            text-transform: inherit;
+            font-weight: inherit;
+            float: none;
+            width: auto;
+            color: inherit;
+            font-size: inherit;
+        }
+
+        &:before, label:before{
             text-shadow:none;
             font-family:wagtail;
             text-transform:none;
@@ -127,18 +137,20 @@
         }
     }
 
-    &.single-field{
-        h2, .object-help{
-            display:none; /* The field label is used instead */
+    &.required{
+        > h2 label:after{
+            content:"*";
+            color:$color-red;
+            font-weight:bold;
+            display:inline-block;
+            margin-left:0.5em;
+            line-height:1em;
+            font-size:13px;
         }
     }
 
     /* Special full-width, one-off fields i.e a single text or textarea input */
     &.full{
-        h2{
-            display:none; /* The field label is used instead */
-        }
-
         .object-help{
             display:block;
         }
@@ -146,10 +158,6 @@
         fieldset{
             display:block;
             float:none;
-
-            .help{
-                display:none;
-            }
         }
         li{
             padding:0;
@@ -542,7 +550,7 @@
     &.empty{
         border-bottom:1px solid white;
 
-        > h2, &.single-field label{
+        > h2{
             margin:0 0;
             border-bottom:1px solid white;
         }
@@ -732,12 +740,5 @@ footer .preview{
         &.empty .add{
             margin:0 0 0 -50px;
         }
-
-        &.single-field label{
-            display: block;
-            float: none;
-            width: auto;
-            padding:auto;
-        }
     }
 }

wagtail/wagtailadmin/templates/wagtailadmin/edit_handlers/object_list.html

@@ -2,7 +2,11 @@
     {% for child in self.children %}
         <li class="object {{ child.classes|join:" " }}">
             {% if child.heading %}
-                <h2>{{ child.heading }}</h2>
+                <h2>
+                    <label {% if child.id_for_label %}for="{{ child.id_for_label }}"{% endif %}>
+                        {{ child.heading }}
+                    </label>
+                </h2>
             {% endif %}
             {% if child.help_text %}
                 <div class="object-help help">{{ child.help_text }}</div>

wagtail/wagtailadmin/templates/wagtailadmin/edit_handlers/single_field_panel.html

@@ -1,6 +1,8 @@
 <fieldset>
     <legend>{{ self.heading }}</legend>
     <ul class="fields">
-        <li>{{ field_content }}</li>
+        <li>
+            {% include "wagtailadmin/shared/field.html" with show_label=False show_help_text=False %}
+        </li>
     </ul>
 </fieldset>

wagtail/wagtailadmin/templates/wagtailadmin/shared/field.html

@@ -1,6 +1,6 @@
 {% load wagtailadmin_tags %}
 <div class="field {{ field|fieldtype }} {{ field|widgettype }} {{ field_classes }}">
-    {{ field.label_tag }}
+    {% if show_label|default_if_none:True %}{{ field.label_tag }}{% endif %}
     <div class="field-content">
         <div class="input {{ input_classes }} ">
             {% block form_field %}
@@ -10,7 +10,7 @@
             {# This span only used on rare occasions by certain types of input #}
             <span></span>
         </div>
-        {% if field.help_text %}
+        {% if show_help_text|default_if_none:True and field.help_text %}
             <p class="help">{{ field.help_text }}</p>
         {% endif %}
 

wagtail/wagtailadmin/tests/test_edit_handlers.py

@@ -247,8 +247,11 @@ class TestObjectList(TestCase):
         # result should contain ObjectList furniture
         self.assertIn('<ul class="objects">', result)
 
-        # result should contain h2 headings for children
-        self.assertIn('<h2>Start date</h2>', result)
+        # result should contain h2 headings (including labels) for children
+        self.assertInHTML('<h2><label for="id_date_from">Start date</label></h2>', result)
+
+        # result should include help text for children
+        self.assertIn('<div class="object-help help">Not required if event is on a single day</div>', result)
 
         # result should contain rendered content from descendants
         self.assertIn('Abergavenny sheepdog trials</textarea>', result)
@@ -278,12 +281,13 @@ class TestFieldPanel(TestCase):
         )
         result = field_panel.render_as_object()
 
-        # check that label appears in the 'object' wrapper as well as the field
+        # check that label appears as a legend in the 'object' wrapper,
+        # but not as a field label (that would be provided by ObjectList instead)
         self.assertIn('<legend>End date</legend>', result)
-        self.assertIn('<label for="id_date_to">End date:</label>', result)
+        self.assertNotIn('<label for="id_date_to">End date:</label>', result)
 
-        # check that help text is included
-        self.assertIn('Not required if event is on a single day', result)
+        # check that help text is not included (it's provided by ObjectList instead)
+        self.assertNotIn('Not required if event is on a single day', result)
 
         # check that the populated form field is included
         self.assertIn('value="2014-07-22"', result)