Opened 17 years ago
Closed 13 years ago
#6877 closed Bug (wontfix)
"form.label_for tag" should apply "label_suffix"
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | Forms | Version: | dev |
Severity: | Normal | Keywords: | label_tag, label_suffix |
Cc: | ivanov.maxim@…, django@…, wagnerluis1982@…, Salva Pinyol | 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)
Change History (13)
comment:1 by , 17 years ago
Triage Stage: | Unreviewed → Accepted |
---|
by , 16 years ago
Attachment: | dj-label_suffix-fix.patch added |
---|
comment:2 by , 16 years ago
Cc: | added |
---|
comment:3 by , 16 years ago
Cc: | added |
---|
comment:4 by , 14 years ago
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 by , 14 years ago
Has patch: | set |
---|---|
Needs documentation: | set |
Needs tests: | set |
Triage Stage: | Accepted → 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 by , 14 years ago
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 by , 14 years ago
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 by , 14 years ago
Cc: | added |
---|
comment:9 by , 14 years ago
Cc: | added |
---|
comment:10 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → 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 by , 14 years ago
Also, the pretty heavy backwards incompatibility issue here means that this might have to wait until 2.0.
comment:12 by , 13 years ago
Easy pickings: | unset |
---|---|
Resolution: | → wontfix |
Status: | new → 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.
quick fix fot label_tag and label_suffix problem