Opened 13 years ago
Closed 10 years ago
#17301 closed New feature (duplicate)
Defining fieldsets in a form class
Reported by: | msiedlarek | Owned by: | nobody |
---|---|---|---|
Component: | Forms | Version: | dev |
Severity: | Normal | Keywords: | form, forms, fieldsets |
Cc: | msiedlarek, riccardo.magliocchetti@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | yes |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Motivation
For a large form grouping of fields is not just the pretty-rendering and template-related issue and I believe there should be way to logically group fields a form class definition. It would also be convenient to have Django automatically render those group of fields, just as easy as it is now to do {{ form.as_p }}
.
Idea
The idea is to use existing and well-known conventions. Example:
class PersonForm(forms.Form): home_phone = CharField() cell_phone = CharField() first_name = CharField() last_name = CharField() class Meta: fieldsets = ( (None, { 'fields': ('first_name', 'last_name'), }), ("Phone numbers", { 'fields': ('cell_phone', 'home_phone'), }), )
Note usage of conventions already known to many users -- Meta
class for options (used in models and ModelForm
already) and fieldsets
tuple, just like one in ModelAdmin
options.
The idea is also not to break anything already working meaning being backward compatible. Forms without fieldsets explicitly defined by user should render normally.
Having a form defined as above user can easily use it in a template. Example:
<form action="" method="post"> {{ form.as_table }} <input type="submit" value="Send" /> </form>
Renders to:
<form action="" method="post"> <fieldset> <table> <tr> <th><label for="id_first_name">First name:</label></th> <td><input type="text" name="first_name" id="id_first_name" /></td> </tr> <tr> <th><label for="id_last_name">Last name:</label></th> <td><input type="text" name="last_name" id="id_last_name" /></td> </tr> </table> </fieldset> <fieldset> <legend>Phone numbers</legend> <table> <tr> <th><label for="id_cell_phone">Cell phone:</label></th> <td><input type="text" name="cell_phone" id="id_cell_phone" /></td> </tr> <tr> <th><label for="id_home_phone">Home phone:</label></th> <td><input type="text" name="home_phone" id="id_home_phone" /></td> </tr> </table> </fieldset> <input type="submit" value="Send" /> </form>
Which I believe is a good default behavior. Other examples:
<form action="" method="post"> {% for fieldset in form.fieldsets %} <fieldset> {{ fieldset.legend_tag }} <table> {{ fieldset.as_table }} </table> </fieldset> {% endfor %} <input type="submit" value="Send" /> </form>
<form action="" method="post"> {% for fieldset in form.fieldsets %} <fieldset> {{ fieldset.legend_tag }} <ul> {% for field in fieldset %} <li> {{ field.label_tag }} {{ field }} </li> {% endfor %} </ul> </fieldset> {% endfor %} <input type="submit" value="Send" /> </form>
<form action="" method="post"> {% for fieldset in form.fieldsets %} <fieldset> {{ fieldset.legend_tag }} <ul> {% for field in fieldset.visible_fields %} <li> {{ field.label_tag }} {{ field }} </li> {% endfor %} <li class="super-hidden"> {% for field in fieldset.hidden_fields %} {{ field }} {% endfor %} </li> </ul> </fieldset> {% endfor %} <input type="submit" value="Send" /> </form>
Implementation
Yeah, there's a patch. Short description of implementation for those afraid of reading the diff:
- Creating
BaseFormOptions
class, similar to this already present inModelForm
class (for defining model for form).ModelFormOptions
now subclasses it.BaseFormMetaclass
now handles options-related stuff. (note that this behavior may be used in future for other form options) - Separating all rendering from
Form
class to a new one -Fieldset
. Form not having any fieldsets defined uses the single, dummy fieldset for all its fields. - No editing any of already existing tests. Everything should be backward compatible.
- Providing some tests for new behavior - form options and fieldsets.
- Providing some documentation. Unfortunately my English skills are, as you see, not that impressive, so it definitely need some touch. Now it's more like set of examples.
Patch
Patch is attached. You can read it on Trac here or on my Django github fork: https://github.com/mikoskay/django/compare/master...form-fieldsets
Attachments (3)
Change History (12)
by , 13 years ago
Attachment: | ticket17301-version1.diff added |
---|
comment:1 by , 13 years ago
TODO
There's one more feature I'd like to add - classes
fieldset option, similar to this in
ModelAdmin
.
But I pre-wrote enough already - give me some +1s for encouragement. :)
comment:2 by , 13 years ago
Triage Stage: | Unreviewed → Accepted |
---|
Just a quick question: Would it be possible to design this in a way where you have something more generic which Fieldset bases upon? I just ask this because it might be nice to have TabGroups
for tabular forms (where each tab could perhaps have fieldsets...). A possible idea would be that forms can have FieldGroups
, which can have fields or more FieldGroups
. Fieldset would be a special case of a FieldGroup
.
I don't know if implementing the feature in more generic terms is easy or not. If it would make this overly complicated, feel free to ignore this comment.
I am marking this accepted based on this django-developers thread.
comment:3 by , 13 years ago
Patch needs improvement: | set |
---|
This is actually a great idea! It's possible in current state of the patch by subclassing both Form and Fieldset, but it probably can be made easier for developer. I'll certainly think about it.
comment:4 by , 13 years ago
Needs documentation: | set |
---|---|
Patch needs improvement: | unset |
Given some thought on Carl Meyer's voice in fieldsets discussion (https://groups.google.com/forum/?_escaped_fragment_=msg/django-developers/dCQAmD1BBzY/tMesCJk96qEJ#!msg/django-developers/dCQAmD1BBzY/tMesCJk96qEJ) I've decided not to implement any presentation-layer features into basic form fieldsets and leave it as a feature for logical organization of fields. Note, that fieldsets' classes
, or using tags other than <fieldset>
can easily be achieved using templates and I believe that's where those features belong.
Patch can be considered checkin-ready now (including tests). There's only one problem - documentation desperately needs touch of some better English speaker than myself.
comment:5 by , 13 years ago
Could you make modelform_factory accept fieldsets as an optional parameter, much like fields and excludes? It likes like it would be a pretty small addition but it means the admin site could leverage the same functionality quite easily.
comment:6 by , 13 years ago
I found two odd things in your patch:
One was partially already in Django:
try: parents = [b for b in bases if issubclass(b, ModelForm)] except NameError: # We are defining ModelForm itself. parents = None
Your patch changes ModelForm
to BaseModelForm
which makes the "try: except:
" look silly, because it will never trigger. It's also now duplicated in BaseFormMetaclass
.
Second, is the doubled call to BaseFormOptions.__init__
.
I fixed the second by adding make_options
method on the metaclass, which can be then overriden, and the first by using isinstance(b, cls)
instead of issubclass
. This has a small caveat, that it will consider any class using this metaclass as a parent of the form (not just subclasses of BaseForm/BaseModelForm), but I'm pretty sure that anyone doing this, really knows the implications.
comment:7 by , 13 years ago
I was actually aware of the second problem, but somehow no good solution came to my mind. What you say sounds good, thanks!
comment:8 by , 11 years ago
Cc: | added |
---|
comment:9 by , 10 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
Closing as a duplicate of #6630, which has more discussion and a similar approach
Patch version 1