Opened 16 years ago

Closed 14 years ago

#5609 closed (fixed)

newforms-admin: input fields do not regard field class

Reported by: anonymous Owned by: Brian Rosner
Component: contrib.admin Version: dev
Severity: Keywords: length input class
Cc: aloysius.prayitno@…, cmawebsite@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I tried the newforms-admin branch, and noticed input field length for a SmallInteger field is wider than the trunk. Below is excerpt from the html source from newforms-admin branch and trunk. The one from newforms-admin is missing class=... etc.

newforms-admin:

<label for="id_treatment_visits" class="required">Treatment visits:</label>
<input type="text" name="treatment_visits" value="19" id="id_treatment_visits" />

trunk:

<label for="id_treatment_visits" class="required">Treatment visits:</label> 
<input type="text" id="id_treatment_visits" class="vSmallIntegerField required" name="treatment_visits" size="5" value="19" maxlength="5" />

Attachments (2)

na2.diff (1.9 KB ) - added by oggie_rob 16 years ago.
Short version - most changes in trunk
trunk2.diff (29.1 KB ) - added by oggie_rob 16 years ago.
newforms changes to support newforms-admin setting up widgets correctly

Download all attachments as: .zip

Change History (17)

comment:1 by awi <aloysius.prayitno@…>, 16 years ago

Cc: aloysius.prayitno@… added

comment:2 by Xian, 16 years ago

Owner: changed from nobody to xian
Triage Stage: UnreviewedAccepted

Thanks for reporting this, please let me know if any other field types are displaying incorrect widgets.

comment:3 by oggie_rob, 16 years ago

Working on this for the sprint. I'll be working off trunk and changing the db.models.fields.formfields() functions and probably some newforms.fields areas also. This will also result in changes to form_for_model/instance that I believe will be preferred (since things like class="required" is currently not being served up via form_for_model).

by oggie_rob, 16 years ago

Attachment: na2.diff added

Short version - most changes in trunk

by oggie_rob, 16 years ago

Attachment: trunk2.diff added

newforms changes to support newforms-admin setting up widgets correctly

comment:4 by oggie_rob, 16 years ago

This code is split two ways. Most of the changes I made were in newforms, so that form_for_model will return an appropriate widget. For example, a CharField model field will return a TextInput with class="required" (if blank=False), length="30" and maxlength according to the model. These changes then support features in newforms-admin pretty easily.

I updated some tests but ran out of time. Hopefully I can get to them later, but if the target branch is in question it would make sense to delay. Certainly there are plenty of tests that fail currently.

Finally one area that was a little funky... forms.Field uses a TextInput as a default widget, however the number of arguments in the __init__ function are rather small. It is a bit strange because typically you may want to set "size" and "maxlength" attributes of a TextField. Anything that extends Field misses out on those, however, and it is better to extend CharField. My feeling is that it would have been cleaner to not have a default widget.

comment:5 by Brian Rosner, 16 years ago

Keywords: nfa-blocker added; newforms-admin removed

This breaks current admin functionality. I am tagging with nfa-blocker.

comment:6 by Alex Gaynor, 16 years ago

Has patch: set

comment:7 by Marc Garcia, 16 years ago

milestone: 1.0 alpha

comment:8 by Brian Rosner, 16 years ago

Marked #7710 as a duplicate. We need to go through and find all the instances where there is a missing CSS class on a widget.

comment:9 by anonymous, 16 years ago

Cc: cmawebsite@… added

comment:10 by Brian Rosner, 16 years ago

Keywords: nfa-blocker removed
milestone: 1.0 alpha1.0 beta
Owner: changed from xian to Brian Rosner
Status: newassigned

This ticket is going to require a bit of work. alpha will have to be released with this as a known issue in the admin. Bumping to 1.0-beta.

comment:11 by Alex Gaynor, 16 years ago

Version: newforms-adminSVN

comment:12 by Jacob, 16 years ago

Resolution: duplicate
Status: assignedclosed

#8163 has a better (looking, mostly) patch.

comment:13 by jacmkno, 15 years ago

Needs tests: set
Resolution: duplicate
Status: closedreopened

I don't think the #8163 has a solution for this that thicked was supposed to be fixed but it still does not work in the current dev version.
This ticket has to be re-opened.

comment:14 by Gary Wilson, 15 years ago

milestone: 1.0 beta1.2

comment:15 by James Bennett, 14 years ago

milestone: 1.2
Resolution: fixed
Status: reopenedclosed

So far as I'm aware, this was fixed a long time ago.

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