#18134 closed Cleanup/optimization (fixed)
BoundField.label_tag should include form.label_suffix
Reported by: | Owned by: | Gabe Jackson | |
---|---|---|---|
Component: | Forms | Version: | 1.4 |
Severity: | Normal | Keywords: | forms |
Cc: | timograham@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
there were some tickets about label_tag not respecting the label_suffix which is not always right. would it be possible to add a label_tag_with_suffix method so all can be happy?
Thank you.
Change History (11)
comment:1 by , 13 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
comment:2 by , 13 years ago
Resolution: | invalid |
---|---|
Status: | closed → reopened |
I'm sorry, my knowledge of djnago is limited unlike yours, i didn't know that this method exists on more places.
I'm referring to https://docs.djangoproject.com/en/1.4/topics/forms/ (Customizing the form template)
{{ field.label_tag }}
The field's label wrapped in the appropriate HTML <label> tag, e.g. <label for="id_email">Email address</label>
and also to Ticket #6877 where it is explained why the field.label_tag doesn't respect the form's label_suffix which i understand.
The problem is that if i use in my project in as many pages as i can the {{ form.as_p }} to respect the DRY, but in some cases you have to add some HTML code in the form, so you have to manually render the form usually like this:
<p>
{{ form.name.errors }}
{{ form.name.label_tag }}{{ form.name }}<span class="helptext">{{ form.name.help_text }}</span>
</p>
which would render the form without the label_suffix (i.e. without the ":" in the label).
so you would have to do it like this:
<p>
{{ form.name.errors }}
<label for="id_name ">{{ form.name.label }}:</label>{{ form.name }}<span class="helptext">{{ form.name.help_text }}</span>
</p>
which forces you to dirty write the "id_name" where the "id_" is as far as i know configurable in forms so this would be an incorrect way of doing it.
my proposal is to add {{ form.name.label_tag_with_suffix }} method which would solve the problem.
comment:3 by , 13 years ago
Has patch: | set |
---|---|
Keywords: | forms added |
Needs documentation: | set |
Owner: | changed from | to
Status: | reopened → new |
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → Cleanup/optimization |
This has been fixed and discussed in https://github.com/django/django/pull/141
A copy of the commit:
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=.
comment:4 by , 13 years ago
Needs documentation: | unset |
---|---|
Status: | new → assigned |
Documentation has been changed/added.
comment:6 by , 12 years ago
Component: | Uncategorized → Forms |
---|
comment:7 by , 12 years ago
Cc: | added |
---|---|
Summary: | please add Filed.label_tag_with_suffix method → BoundField.label_tag should include form.label_suffix |
This change makes sense to me. I've added a note to the "backwards incompatible changes" section of the release notes and updated the request to merge cleanly.
comment:8 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
As a guide for the future -- when opening a ticket, it's helpful to provide details about exactly what it is you're proposing to fix/change. "Some tickets" referring to a random method name isn't very helpful; suggesting a fix without describing the actual problem also isn't helpful.
In this case, it isn't clear if you're referring to "label_tag" on the BoundField of a form, or "label_tag" on an AdminField (neither of which are documented API entry points). Without a specific reference to tickets, it's impossible to know problem what adding "label_tag_with_suffix" would fix. However, given the current operation of "label_tag", I can't see how adding a second method to do the same thing would improve anything.
Closing invalid. If you want to provide more details -- like, for example, the problem you're trying to address, and and example of how label_tag_with_suffix would address that problem -- feel free to reopen.