Code

Opened 6 years ago

Closed 3 years ago

#6877 closed Bug (wontfix)

"form.label_for tag" should apply "label_suffix"

Reported by: David Piccione <run4yourlives@…> Owned by: nobody
Component: Forms Version: master
Severity: Normal Keywords: label_tag, label_suffix
Cc: ivanov.maxim@…, django@…, wagnerluis1982@…, spinyol Triage Stage: Design decision needed
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When using newforms, the default behavior is is to render a colon within each label entity. This can of course be overridden by using the "label_suffix" option on form creation.

However, "label_tag" is not consistent with this behavior, and in addition does not respond to the "label_suffix" option at all.

>>> print given_field.label_tag 
<label for="id_test">Label Name</label>
>>> new_form = TestForm(label_suffix='|')
>> print new_form.given_field.label_tag
<label for="id_test">Label Name</label>

The label_tag method should obey the "label_suffix" option as is consistent with other form rendering techniques that print labels.


Attachments (1)

dj-label_suffix-fix.patch (1.2 KB) - added by redbaron 6 years ago.
quick fix fot label_tag and label_suffix problem

Download all attachments as: .zip

Change History (13)

comment:1 Changed 6 years ago by programmerq

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Changed 6 years ago by redbaron

quick fix fot label_tag and label_suffix problem

comment:2 Changed 6 years ago by redbaron

  • Cc ivanov.maxim@… added

comment:3 Changed 5 years ago by ssadler

  • Cc django@… added

comment:4 Changed 4 years ago by jsdalton

I was just about to report this issue when I found it had already been reported, accepted, and given a patch.

What needs to be done to assign it to the 1.3 milestone and get the fix implemented?

comment:5 Changed 4 years ago by lrekucki

  • Has patch set
  • Needs documentation set
  • Needs tests set
  • Triage Stage changed from Accepted to Design decision needed

First of all, I'm not entirely sure it's a bug. Current documentation contains examples which explicitly show that label_tag won't contain any suffix (i.e. http://docs.djangoproject.com/en/dev/topics/forms/#looping-over-the-form-s-fields).

Assuming this is a bug, the change will be backwards incompatible with current behavior, so documentation changes should be included (it will also mean that everyone will need to update their code to reflect this change, which is an argument against this). Usually, the patch should also include a regression test.

comment:6 Changed 4 years ago by jsdalton

I'm not sure it's a bug either, but it's a behavioral inconsistency that does cause real problems:

  • When rendering an entire form, the label_suffix appears *within* the label element. When you use label_tag and add a suffix by hand in the template, the suffix appears *after* the label element. This can cause problems on the design side. For example, if you have a style that adds an asterisk at the end of a label, the label will appear after the suffix when rendering the whole form and before the suffix when rendering the label via label_tag.
  • There's logic related to the handing of the label_suffix in the _html_output() method of a Form that suppresses the suffix if the last char of the label is in ':?.!'. You lose that feature if you use label_tag.

To me the benefit of these features related to form field rendering is that you can drop down into a form and easily tweak a few things by hand if for some the default rendering isn't working for you. This one minor inconsistency has actually prevented me from being able to do that on several projects, so minor as it is it has real implications.

comment:7 Changed 3 years ago by wagnerluis1982

Hello,

I'm not so good with english, so I will be quick.

I agree with jsdalton. And I know the fix will be backwards incompatible, but it's a little change in a new version of Django.

Finally, thanks to redbaron for the patch. I'm using it and it's working very nice.

comment:8 Changed 3 years ago by wagnerluis1982

  • Cc wagnerluis1982@… added

comment:9 Changed 3 years ago by spinyol

  • Cc spinyol added

comment:10 Changed 3 years ago by julien

  • Severity set to Normal
  • Type set to Bug

I'd say this is technically a bug (or at least an annoying inconsistency). It might be made redundant by #15667 but I'm leaving this open to ensure this gets fixed at some stage.

comment:11 Changed 3 years ago by julien

Also, the pretty heavy backwards incompatibility issue here means that this might have to wait until 2.0.

comment:12 Changed 3 years ago by carljm

  • Easy pickings unset
  • Resolution set to wontfix
  • Status changed from new to closed
  • UI/UX unset

This is an inconsistency, but it's significantly easier to work around than indicated in previous comments here: label_tag accepts the label contents as an optional argument, so a trivial template filter can perform the few lines of logic from _html_output, pass in the suffixed label contents, and achieve the desired result.

We can't simply make the change and ignore backwards-compatibility: since the default value of label_suffix is non-empty, this would break the layouts of pretty much anyone currently using label_tag according to its current, and documented, suffix-free behavior. And there's no way to mitigate this backwards incompatibility without introducing yet more options/arguments/settings related to label suffixes. This is simply not worth it for such a small and easily-worked-around inconsistency.

In the longer term the goal is to move to template-based rendering for forms and deprecate much of this awkward hardcoded-HTML machinery anyway; yet another reason it isn't worth backwards-compatibility contortions to fix minor inconsistencies.

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.