Code

Opened 2 years ago

Closed 10 months ago

Last modified 10 months ago

#18134 closed Cleanup/optimization (fixed)

BoundField.label_tag should include form.label_suffix

Reported by: Evil Clay <clay.evil@…> Owned by: gabejackson
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.

Attachments (0)

Change History (10)

comment:1 Changed 2 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to invalid
  • Status changed from new to 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.

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

  • Resolution invalid deleted
  • Status changed from closed to 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 Changed 23 months ago by gabejackson

  • Has patch set
  • Keywords forms added
  • Needs documentation set
  • Owner changed from nobody to gabejackson
  • Status changed from reopened to new
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to 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 Changed 23 months ago by gabejackson

  • Needs documentation unset
  • Status changed from new to assigned

Documentation has been changed/added.

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

Last edited 23 months ago by gabejackson (previous) (diff)

comment:5 Changed 23 months ago by Evil Clay <clay.evil@…>

Thank you, you're Godlike.

comment:6 Changed 13 months ago by aaugustin

  • Component changed from Uncategorized to Forms

comment:7 Changed 10 months ago by timo

  • Cc timograham@… added
  • Summary changed from please add Filed.label_tag_with_suffix method to 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.

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

comment:8 Changed 10 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

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 Changed 10 months ago by Tim Graham <timograham@…>

In 3632d289dedc2e83cde1976e5a4cd00b08c799ee:

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

comment:10 Changed 10 months ago by Tim Graham <timograham@…>

In 5ecdf0eb9ccd47c102deb873a242ed40d1cf45cd:

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

Backport of 3632d289de from master.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.