Code

Opened 6 years ago

Closed 4 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: erikcw Owned by: kmtracey
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 6 years ago.
wrap_help_text_in_css_class-with_tests.diff (8.2 KB) - added by mrts 6 years ago.
Add tests and fix as_p()
wrap_help_text_in_css_class-with_tests-r10116.diff (8.2 KB) - added by mrts 5 years ago.
Update to r10116
wrap_help_text_in_css_class-with_tests-r11855.diff (9.3 KB) - added by rpbarlow 5 years ago.
Here's an updated patch.

Download all attachments as: .zip

Change History (28)

comment:1 Changed 6 years ago by mtredinnick

  • milestone changed from 1.0 to post-1.0
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design 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 6 years ago by pihentagy

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 6 years ago by mrts

  • Triage Stage changed from Design decision needed to Accepted

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 6 years ago by mrts

That should of course have been Version1.1Features.

Changed 6 years ago by mrts

comment:5 Changed 6 years ago by mrts

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

Changed 6 years ago by mrts

Add tests and fix as_p()

comment:6 Changed 6 years ago by mrts

  • Needs tests unset
  • Patch needs improvement unset

Not sure if documentation is needed though.

comment:7 Changed 6 years ago by anonymous

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

comment:8 Changed 5 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:9 Changed 5 years ago by mrts

  • milestone set to 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 5 years ago by mrts

Update to r10116

comment:10 Changed 5 years ago by jacob

  • milestone changed from 1.1 beta to 1.1

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

comment:11 Changed 5 years ago by jacob

  • milestone changed from 1.1 to 1.2

comment:12 Changed 5 years ago by coulix

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

comment:13 Changed 5 years ago by edevil

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

comment:14 Changed 5 years ago by rpbarlow

  • Owner changed from nobody to rpbarlow
  • Status changed from new to assigned

comment:15 Changed 5 years ago by rpbarlow

  • Patch needs improvement set

Changed 5 years ago by rpbarlow

Here's an updated patch.

comment:16 Changed 5 years ago by rpbarlow

  • Owner changed from rpbarlow to kmtracey
  • Patch needs improvement unset
  • Status changed from assigned to new

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

comment:17 Changed 5 years ago by rpbarlow

  • Cc randallpbarlow@… added

comment:18 Changed 4 years ago by ubernostrum

  • milestone 1.2 deleted

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

comment:19 follow-up: Changed 4 years ago by pihentagy

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

comment:20 in reply to: ↑ 19 Changed 4 years ago by ubernostrum

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 4 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 4 years ago by lukeplant

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

(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 4 years ago by mattmcc

  • Resolution fixed deleted
  • Status changed from closed to reopened

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

comment:24 Changed 4 years ago by lukeplant

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

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

This file was missing from [13519]

Thanks to mattmcc for the catch.

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.