Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fixed #18134 -- Inconsistent label_tag generation on form fields #141

Closed
wants to merge 2 commits into from

2 participants

@gabejackson

There was an inconsistency between how the label_tag for forms were
generated depending on which method was used: as_p, as_ul and as_table
contained code to append the label_suffix where as label_tag called on a
form field directly did NOT append the label_suffix. The code for
appending the label_suffix has been moved in to the label_tag code of
the field and the HTML generation code for as_p, as_ul and as_table now
calls this code as well. CAUTION: This may be be "backwards incompatible
change" because users who have added the label_suffix manually in their
templates may now get double label_suffix characters in their forms.

Also some test cases regarding the label_tag output were inconsistent.
Some expected Label: and some expected the label_suffix
outside of the tag: Label:
The format has now been unified to keep the label_suffix inside the
tag: Label:. If the label_suffix is not needed,
the form can still be constructed with label_suffix=''.

gabejackson added some commits
@gabejackson gabejackson Fixed #18134 -- Inconsistent label_tag generation on form fields
There was an inconsistency between how the label_tag for forms were
generated depending on which method was used: as_p, as_ul and as_table
contained code to append the label_suffix where as label_tag called on a
form field directly did NOT append the label_suffix. The code for
appending the label_suffix has been moved in to the label_tag code of
the field and the HTML generation code for as_p, as_ul and as_table now
calls this code as well. CAUTION: This may be be "backwards incompatible
change" because users who have added the label_suffix manually in their
templates may now get double label_suffix characters in their forms.

Also some test cases regarding the label_tag output were inconsistent.
Some expected <label>Label:</label> and some expected the label_suffix
outside of the <label> tag: <label>Label</label>:
The format has now been unified to keep the label_suffix inside the
<label> tag: <label>Label:</label>. If the label_suffix is not needed,
the form can still be constructed with label_suffix=''.
8fff02e
@gabejackson gabejackson Documentation for ticket #18134
Removed the extra ':' from the template examples in the forms
documentation. Noted that the label_tag template tag also outputs the
label_suffix.
8c5ef94
@timgraham
Owner

I've added additional docs for this change and updated it to merge cleanly: #1252

@timgraham timgraham closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 8, 2012
  1. @gabejackson

    Fixed #18134 -- Inconsistent label_tag generation on form fields

    gabejackson authored
    There was an inconsistency between how the label_tag for forms were
    generated depending on which method was used: as_p, as_ul and as_table
    contained code to append the label_suffix where as label_tag called on a
    form field directly did NOT append the label_suffix. The code for
    appending the label_suffix has been moved in to the label_tag code of
    the field and the HTML generation code for as_p, as_ul and as_table now
    calls this code as well. CAUTION: This may be be "backwards incompatible
    change" because users who have added the label_suffix manually in their
    templates may now get double label_suffix characters in their forms.
    
    Also some test cases regarding the label_tag output were inconsistent.
    Some expected <label>Label:</label> and some expected the label_suffix
    outside of the <label> tag: <label>Label</label>:
    The format has now been unified to keep the label_suffix inside the
    <label> tag: <label>Label:</label>. If the label_suffix is not needed,
    the form can still be constructed with label_suffix=''.
  2. @gabejackson

    Documentation for ticket #18134

    gabejackson authored
    Removed the extra ':' from the template examples in the forms
    documentation. Noted that the label_tag template tag also outputs the
    label_suffix.
This page is out of date. Refresh to see the latest.
View
9 django/forms/forms.py
@@ -163,11 +163,6 @@ def _html_output(self, normal_row, error_row, row_ender, help_text_html, errors_
if bf.label:
label = conditional_escape(force_unicode(bf.label))
- # Only add the suffix if the label does not end in
- # punctuation.
- if self.label_suffix:
- if label[-1] not in ':?.!':
- label += self.label_suffix
label = bf.label_tag(label) or ''
else:
label = ''
@@ -504,6 +499,10 @@ def label_tag(self, contents=None, attrs=None):
If attrs are given, they're used as HTML attributes on the <label> tag.
"""
contents = contents or conditional_escape(self.label)
+ # Only add the suffix if the label does not end in punctuation.
+ if self.form.label_suffix:
+ if contents[-1] not in ':?.!':
+ contents += self.form.label_suffix
widget = self.field.widget
id_ = widget.attrs.get('id') or self.auto_id
if id_:
View
11 docs/topics/forms/index.txt
@@ -289,7 +289,7 @@ loop::
{% for field in form %}
<div class="fieldWrapper">
{{ field.errors }}
- {{ field.label_tag }}: {{ field }}
+ {{ field.label_tag }} {{ field }}
</div>
{% endfor %}
<p><input type="submit" value="Send message" /></p>
@@ -303,8 +303,9 @@ templates:
The label of the field, e.g. ``Email address``.
``{{ field.label_tag }}``
- The field's label wrapped in the appropriate HTML ``<label>`` tag,
- e.g. ``<label for="id_email">Email address</label>``
+ The field's label wrapped in the appropriate HTML ``<label>`` tag. This
+ includes the form's label_suffix, e.g.
+ ``<label for="id_email">Email address:</label>``
``{{ field.value }}``
The value of the field. e.g ``someone@example.com``
@@ -356,7 +357,7 @@ these two methods::
{% for field in form.visible_fields %}
<div class="fieldWrapper">
{{ field.errors }}
- {{ field.label_tag }}: {{ field }}
+ {{ field.label_tag }} {{ field }}
</div>
{% endfor %}
<p><input type="submit" value="Send message" /></p>
@@ -384,7 +385,7 @@ using the :ttag:`include` tag to reuse it in other templates::
{% for field in form %}
<div class="fieldWrapper">
{{ field.errors }}
- {{ field.label_tag }}: {{ field }}
+ {{ field.label_tag }} {{ field }}
</div>
{% endfor %}
View
2  docs/topics/forms/modelforms.txt
@@ -791,7 +791,7 @@ Third, you can manually render each field::
{{ formset.management_form }}
{% for form in formset %}
{% for field in form %}
- {{ field.label_tag }}: {{ field }}
+ {{ field.label_tag }} {{ field }}
{% endfor %}
{% endfor %}
</form>
View
4 tests/regressiontests/admin_util/tests.py
@@ -272,7 +272,7 @@ class MyForm(forms.Form):
self.assertEqual(helpers.AdminField(form, 'text', is_first=False).label_tag(),
'<label for="id_text" class="required inline"><i>text</i>:</label>')
self.assertEqual(helpers.AdminField(form, 'cb', is_first=False).label_tag(),
- '<label for="id_cb" class="vCheckboxLabel required inline"><i>cb</i></label>')
+ '<label for="id_cb" class="vCheckboxLabel required inline"><i>cb</i>:</label>')
# normal strings needs to be escaped
class MyForm(forms.Form):
@@ -283,4 +283,4 @@ class MyForm(forms.Form):
self.assertEqual(helpers.AdminField(form, 'text', is_first=False).label_tag(),
'<label for="id_text" class="required inline">&amp;text:</label>')
self.assertEqual(helpers.AdminField(form, 'cb', is_first=False).label_tag(),
- '<label for="id_cb" class="vCheckboxLabel required inline">&amp;cb</label>')
+ '<label for="id_cb" class="vCheckboxLabel required inline">&amp;cb:</label>')
View
24 tests/regressiontests/forms/tests/forms.py
@@ -1583,9 +1583,9 @@ def clean(self):
# Recall from above that passing the "auto_id" argument to a Form gives each
# field an "id" attribute.
t = Template('''<form action="">
-<p>{{ form.username.label_tag }}: {{ form.username }}</p>
-<p>{{ form.password1.label_tag }}: {{ form.password1 }}</p>
-<p>{{ form.password2.label_tag }}: {{ form.password2 }}</p>
+<p>{{ form.username.label_tag }} {{ form.username }}</p>
+<p>{{ form.password1.label_tag }} {{ form.password1 }}</p>
+<p>{{ form.password2.label_tag }} {{ form.password2 }}</p>
<input type="submit" />
</form>''')
self.assertHTMLEqual(t.render(Context({'form': UserRegistration(auto_id=False)})), """<form action="">
@@ -1595,18 +1595,18 @@ def clean(self):
<input type="submit" />
</form>""")
self.assertHTMLEqual(t.render(Context({'form': UserRegistration(auto_id='id_%s')})), """<form action="">
-<p><label for="id_username">Username</label>: <input id="id_username" type="text" name="username" maxlength="10" /></p>
-<p><label for="id_password1">Password1</label>: <input type="password" name="password1" id="id_password1" /></p>
-<p><label for="id_password2">Password2</label>: <input type="password" name="password2" id="id_password2" /></p>
+<p><label for="id_username">Username:</label> <input id="id_username" type="text" name="username" maxlength="10" /></p>
+<p><label for="id_password1">Password1:</label> <input type="password" name="password1" id="id_password1" /></p>
+<p><label for="id_password2">Password2:</label> <input type="password" name="password2" id="id_password2" /></p>
<input type="submit" />
</form>""")
# User form.[field].help_text to output a field's help text. If the given field
# does not have help text, nothing will be output.
t = Template('''<form action="">
-<p>{{ form.username.label_tag }}: {{ form.username }}<br />{{ form.username.help_text }}</p>
-<p>{{ form.password1.label_tag }}: {{ form.password1 }}</p>
-<p>{{ form.password2.label_tag }}: {{ form.password2 }}</p>
+<p>{{ form.username.label_tag }} {{ form.username }}<br />{{ form.username.help_text }}</p>
+<p>{{ form.password1.label_tag }} {{ form.password1 }}</p>
+<p>{{ form.password2.label_tag }} {{ form.password2 }}</p>
<input type="submit" />
</form>''')
self.assertHTMLEqual(t.render(Context({'form': UserRegistration(auto_id=False)})), """<form action="">
@@ -1626,9 +1626,9 @@ def clean(self):
form_output.append(bf.label_tag(attrs={'class': 'pretty'}))
expected_form_output = [
- '<label for="id_username" class="pretty">Username</label>',
- '<label for="id_password1" class="pretty">Password1</label>',
- '<label for="id_password2" class="pretty">Password2</label>',
+ '<label for="id_username" class="pretty">Username:</label>',
+ '<label for="id_password1" class="pretty">Password1:</label>',
+ '<label for="id_password2" class="pretty">Password2:</label>',
]
self.assertEqual(len(form_output), len(expected_form_output))
for i in range(len(form_output)):
View
4 tests/regressiontests/forms/tests/regressions.py
@@ -44,8 +44,8 @@ class SomeForm(Form):
field_2 = CharField(max_length=10, label=ugettext_lazy('field_2'), widget=TextInput(attrs={'id': 'field_2_id'}))
f = SomeForm()
- self.assertHTMLEqual(f['field_1'].label_tag(), '<label for="id_field_1">field_1</label>')
- self.assertHTMLEqual(f['field_2'].label_tag(), '<label for="field_2_id">field_2</label>')
+ self.assertHTMLEqual(f['field_1'].label_tag(), '<label for="id_field_1">field_1:</label>')
+ self.assertHTMLEqual(f['field_2'].label_tag(), '<label for="field_2_id">field_2:</label>')
# Unicode decoding problems...
GENDERS = (('\xc5', 'En tied\xe4'), ('\xf8', 'Mies'), ('\xdf', 'Nainen'))
Something went wrong with that request. Please try again.