Opened 8 years ago

Closed 6 years ago

#8426 closed (fixed)

Wrap help_text in "<span class="helptext"></span>", like errors in "<ul class="errorlist"></ul>" in HTML output

Reported by: ErikW Owned by: Karen Tracey
Component: Forms Version: master
Severity: Keywords: help_text, as_p, as_ul, forms
Cc: randallpbarlow@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

form.as_p() and form.as_ul() should both either have a div (with class="help"), span, or br around the help_text. form.as_table() already has a br tag before the helptext. So this should be added to improve consistency. This will really aid in styling the form with CSS!

Attachments (4)

wrap_help_text_in_css_class.diff (1.1 KB) - added by mrts 8 years ago.
wrap_help_text_in_css_class-with_tests.diff (8.2 KB) - added by mrts 8 years ago.
Add tests and fix as_p()
wrap_help_text_in_css_class-with_tests-r10116.diff (8.2 KB) - added by mrts 8 years ago.
Update to r10116
wrap_help_text_in_css_class-with_tests-r11855.diff (9.3 KB) - added by Randy Barlow 7 years ago.
Here's an updated patch.

Download all attachments as: .zip

Change History (28)

comment:1 Changed 8 years ago by Malcolm Tredinnick

milestone: 1.0post-1.0
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedDesign decision needed

This could well be wontfix, since you can easily write your own output methods to do whatever you like in forms. We can make a decision after 1.0.

comment:2 Changed 8 years ago by Gergely Kontra

Is it the right time for a design decision on this ticket?

Note, that error messages has custom ul class, so why not have it for help_text?
And the implementation is fairly trivial.

What is against accepting it? (well, I can write arbitrary output method, but why cannot be the default ones perfect?)

comment:3 Changed 8 years ago by mrts

Triage Stage: Design decision neededAccepted

Pushing to accepted as per jacob on Version1.1Features:

  • Wrap help_text in "<span class="helptext"></span>", like errors in "<ul class="errorlist"></ul>" in HTML output. (needs ticket)

comment:4 Changed 8 years ago by mrts

That should of course have been Version1.1Features.

Changed 8 years ago by mrts

comment:5 Changed 8 years ago by mrts

Has patch: set
Needs tests: set
Patch needs improvement: set

Changed 8 years ago by mrts

Add tests and fix as_p()

comment:6 Changed 8 years ago by mrts

Needs tests: unset
Patch needs improvement: unset

Not sure if documentation is needed though.

comment:7 Changed 8 years ago by anonymous

Summary: Add "div" tag around help_text html output in nowforms myform.as_p()Wrap help_text in "<span class="helptext"></span>", like errors in "<ul class="errorlist"></ul>" in HTML output

comment:8 Changed 8 years ago by (none)

milestone: post-1.0

Milestone post-1.0 deleted

comment:9 Changed 8 years ago by mrts

milestone: 1.1 beta

Tentatively pushing to 1.1 beta, assuming that this was just missed (it seems that Jacob agrees it's needed). As it's a feature, it should be either committed before 1.1 beta or deferred to 1.2.

Changed 8 years ago by mrts

Update to r10116

comment:10 Changed 8 years ago by Jacob

milestone: 1.1 beta1.1

Markup changes don't count as features; this can wait and go in with the other bug fixes.

comment:11 Changed 7 years ago by Jacob

milestone: 1.11.2

comment:12 Changed 7 years ago by coulix

The path always add help_text even when no help_text is set in the form

comment:13 Changed 7 years ago by André Cruz

Is there a reason for this not being included in trunk?

comment:14 Changed 7 years ago by Randy Barlow

Owner: changed from nobody to Randy Barlow
Status: newassigned

comment:15 Changed 7 years ago by Randy Barlow

Patch needs improvement: set

Changed 7 years ago by Randy Barlow

Here's an updated patch.

comment:16 Changed 7 years ago by Randy Barlow

Owner: changed from Randy Barlow to Karen Tracey
Patch needs improvement: unset
Status: assignednew

Hey Karen, this one's ready to go in!

comment:17 Changed 7 years ago by Randy Barlow

Cc: randallpbarlow@… added

comment:18 Changed 7 years ago by James Bennett

milestone: 1.2

1.2 is feature-frozen, moving this feature request off the milestone.

comment:19 Changed 7 years ago by Gergely Kontra

Definitely not happy. Simple bug, simple patch, more than one year old.

comment:20 in reply to:  19 Changed 7 years ago by James Bennett

Replying to pihentagy:

Definitely not happy. Simple bug, simple patch, more than one year old.

Like it or not, this is a feature request, and the process for feature requests getting into new releases is documented. The biggest factor for a small request is that someone motivated -- perhaps you, if you feel strongly about this ticket -- needs to bring it up during the feature-development phase of a release cycle and make sure there's some discussion about it and a decision as to whether it's worth adding. If no-one does that (and no-one did for the 1.2 release cycle), it's more or less inevitable that the request will simply be punted until someone does. And since 1.2 is feature-frozen there's no way to do that for 1.2 now; if you'd like to see this make it into Django, then make a note of it and pop onto the django-dev list when feature discussion for 1.3 opens up in a month or so.

comment:21 Changed 6 years ago by holms

oh please God... why this must hang for 2 years =( it's really REALLY inconvinient... is it takes too much time to include this fix into release? I need it die hard!

comment:22 Changed 6 years ago by Luke Plant

Resolution: fixed
Status: newclosed

(In [13519]) Fixed #8426 - added 'helptext' CSS class to help text in forms to aid in styling

Thanks to erikcw for report, mrts and rpbarlow for patch

comment:23 Changed 6 years ago by Matt McClanahan

Resolution: fixed
Status: closedreopened

[13519] appears to be just the tests, not the forms/forms.py fix.

comment:24 Changed 6 years ago by Luke Plant

Resolution: fixed
Status: reopenedclosed

(In [13522]) Fixed #8426 - 'helptext' CSS class - hopefully for real this time.

This file was missing from [13519]

Thanks to mattmcc for the catch.

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