Opened 13 years ago

Closed 12 years ago

Last modified 11 years ago

#18134 closed Cleanup/optimization (fixed)

BoundField.label_tag should include form.label_suffix

Reported by: Evil Clay <clay.evil@…> 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 Russell Keith-Magee, 13 years ago

Resolution: invalid
Status: newclosed

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.

comment:2 by Evil Clay <clay.evil@…>, 13 years ago

Resolution: invalid
Status: closedreopened

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 Gabe Jackson, 13 years ago

Has patch: set
Keywords: forms added
Needs documentation: set
Owner: changed from nobody to Gabe Jackson
Status: reopenednew
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/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 Gabe Jackson, 13 years ago

Needs documentation: unset
Status: newassigned

Documentation has been changed/added.

https://github.com/django/django/pull/141

Last edited 13 years ago by Gabe Jackson (previous) (diff)

comment:5 by Evil Clay <clay.evil@…>, 13 years ago

Thank you, you're Godlike.

comment:6 by Aymeric Augustin, 12 years ago

Component: UncategorizedForms

comment:7 by Tim Graham, 12 years ago

Cc: timograham@… added
Summary: please add Filed.label_tag_with_suffix methodBoundField.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.

https://github.com/django/django/pull/1252

comment:8 by Tim Graham <timograham@…>, 12 years ago

Resolution: fixed
Status: assignedclosed

In 584bd14dcfdee9585fec7794d53ce120ea73d0bc:

Fixed #18134 -- BoundField.label_tag now includes the form's label_suffix

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.

This is a 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.

comment:9 by Tim Graham <timograham@…>, 11 years ago

In 3632d289dedc2e83cde1976e5a4cd00b08c799ee:

A couple more semicolon -> colon fixes; refs #18134.

comment:10 by Tim Graham <timograham@…>, 11 years ago

In 5ecdf0eb9ccd47c102deb873a242ed40d1cf45cd:

[1.6.x] A couple more semicolon -> colon fixes; refs #18134.

Backport of 3632d289de from master.

comment:11 by Tim Graham <timograham@…>, 11 years ago

In 5eb81ce44532596ecc1c781d93f3421a467a6206:

Fixed #22533 -- Added label_suffix parameter to form fields.

Fields can now receive the label_suffix attribute, which will override
a form's label_suffix.

This enhances the possibility to customize form's label_suffix, allowing
to use such customizations while using shortcuts such as
{{ form.as_p }}.

Note that the field's own customization can be overridden at runtime by
using the label_prefix parameter to BoundField.label_tag().

Refs #18134.

Note: See TracTickets for help on using tickets.
Back to Top