Opened 17 years ago
Closed 9 years ago
#6630 closed New feature (wontfix)
Fieldsets for forms
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | Forms | Version: | dev |
Severity: | Normal | Keywords: | feature |
Cc: | Florian Apolloner, Adrian Ribao, Carl Meyer, Oliver Beattie, mmitar@…, net147, django@…, bmispelon@…, riccardo.magliocchetti@…, Jason Lingohr | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | yes |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | yes |
Description
This patch and ticket try to add support for simple and flexible definitions of fieldsets in newforms. They would be defined through inner class "Meta" (as in models and ModelForm). There is an side effect - more flexible forms without fieldsets (for example #3111 could be fixed).
Form as a pool of fields
I see form as a pool of fields declared as class attributes in the form and its bases - in the reversed order from mro. In a model form fields from the last model (the first class in mro with a option "model" set) are before all declared fields.
From the pool some fields are selected - through options "fieldsets", "fields" and "exclude". There is a simple rule: If the option "fieldsets" is set, use it. Otherwise if the option "fields" is set, used it. Otherwise if the option "exclude" is set, used it. Otherwise use all fields from the pool.
If no option is set, fields are in the order from the pool. If the option "exclude" is set, all fields not in the option (which is a list of field names) are used in the order from the pool. If the option "fields" is set, all fields in the option (which is a list of field names) are used in the order from the option.
The option "fieldsets" is a dictionary with "fields" (which is a list of field names), "legend" (optional, text for the legend tag in the generated html), "attrs" (optional, html attributes for the fieldset tag in the generated html) and "html_output_method" (optional, described later). If the option is set, all fields from "fields" are used in the order from the value.
Inner class "Meta"
Forms can have inner class "Meta" with some options. Usual forms can have options "fieldsets", "fields" and "exclude", model forms can have all options from usual forms and also "model" and "formfield_for_dbfield".
Option "formfield_for_dbfield" can be function or method (it is normalized as a function) with two arguments - options and a dbfield. It should return a formfield for the dbfield (or nothing, if the dbfield should not be used). The default value is "lambda self, dbfield: dbfield.formfield()". It is called during the construction of base fields for the form.
In the last week there was a new documentation commit for model forms inheritance. I have decided for another approach (it was before it) so I would try to describe the difference.
I would prefer the "DRY" approach - it would be more simple to have one base form with non-default options that many forms inherits from it. But it is no problem to change it to the "Explicit" approach, other parts of the API and of the patch don't depend on it.
"Explicit" approach
class FormOne(forms.Form): a = forms.CharField() class Meta: alpha = 'an option' class FormTwo(forms.Form): b = forms.IntegerField() class Meta: beta = 'another option' class FormThree(FormOne, FormTwo): a = forms.IntegerField() class Meta(FormOne.Meta, FormTwo.Meta): beta = 'last option'
"DRY" approach
class FormOne(forms.Form): a = forms.CharField() class Meta: alpha = 'an option' class FormTwo(forms.Form): b = forms.IntegerField() class Meta: beta = 'another option' class FormThree(FormOne, FormTwo): a = forms.IntegerField() class Meta: beta = 'last option'
Important changes in BaseForm
- Methods html_output, as_table, as_ul and as_p are changed to generate html described by options.
- There are three new methods for simplification of the html_output method (they can be overwrites by a subclass) - top_errors_html_output, fieldset_html_output, hidden_fields_html_output. The method fieldset_html_output could be replaced by a "html_output_method" from the option "fieldsets" - it would be not possible to have different generated html for different fieldsets without it.
- There are three new methods for template authors - has_fieldsets, first_fieldset_attributes and first_fieldset_legend_tag. They are necessary for general templates - they can't be parts of generated html because tags for the first table and first fieldset are not generated.
Important changes in Widget
- There is a new attribute row_attrs - it describes attributes which would be not used by a widget directly, but could be used as row attributes.
- There are new methods label_tag, for_table, for_ul and for_p - for flexibility of generated html and for simplification.
Important changes in BoundField
- There are new properties - label, help_text and row_attrs. Method label_tag is changed to be a property and to be more powerful and less flexible. These properties are used by BaseForm.html_output and they call new widget methods sometimes.
Backward-incompatibilities
There are some backward-incompatible changes. It seems to be a long list - but it should be almost no difference for most developers.
- Semantics of ModelForm with options "fields" and "exclude" is changed - the options "exclude" is ignored now.
- There can be changes of fields ordering in forms with multiply inheritance - it should be more logical now.
- Inheritance for inner class "Meta" is changed - but it can be reverted.
- DeclarativeFieldsMetaclass is renamed to FormMetaclass.
- Argument formfield_callback from ModelFormMetaclass is replaced by the option "formfield_for_dbfield".
- Widgets with positional arguments could be broken - row_attrs are the second positional arguments now.
- Hidden fields have their own row - it is not possible to know where is a place for the hidden fields in the last row if a widget returns custom html.
- There is no "\n" between errors and paragraphs in the html generated by as_p methods - it is more consistent with the other output methods.
- Method BoundField.label_tag is changed so it could break code which used it directly - it simplifies code, method label_tag is complex and not documented.
Base variants
I dislike BaseForm and BaseModelForm. Why they are necessary? In the past there were no fields in model forms - but it is not true now. What if Form and ModelForm have metaclasses and BaseForm and BaseModelForm would be only deprecated names for them?
More fields in a row
In newforms-admin it is possible to have more fields in a row. But I don't know how to describe syntax (attributes for the row and attributes for a field) and I don't know what to generate. There is a more general possibility in the patch - through methods for_table, for_ul and for_p from Widget.
Attachments (11)
Change History (38)
by , 17 years ago
Attachment: | 00-newforms-fieldsets.diff added |
---|
by , 17 years ago
Attachment: | 00-newforms-fieldsets.2.diff added |
---|
comment:1 by , 17 years ago
by , 17 years ago
Attachment: | 00-newforms-fieldsets.3.diff added |
---|
comment:2 by , 17 years ago
- I had some problems with merging - so I am not sure if previous patch really works. Therefore I have uploaded new one - there are no real changes.
- Parts of the patch are in the #6160 now.
comment:3 by , 16 years ago
Cc: | added |
---|
comment:4 by , 16 years ago
Keywords: | feature added |
---|---|
Needs documentation: | set |
Patch needs improvement: | set |
Triage Stage: | Unreviewed → Design decision needed |
I guess the patch is not up to date.
by , 16 years ago
Attachment: | 00-forms-fieldsets.diff added |
---|
comment:5 by , 16 years ago
Changed to be compatible with the new trunk. Row_attrs are now the last parameter of the widget (for backward-compatibility).
by , 16 years ago
Attachment: | 00-forms-fieldsets.2.diff added |
---|
by , 16 years ago
Attachment: | 00-fieldsets.diff added |
---|
comment:6 by , 15 years ago
Cc: | added |
---|
comment:7 by , 15 years ago
Cc: | added |
---|
comment:8 by , 15 years ago
Cc: | added |
---|
by , 14 years ago
Attachment: | 01-move-fields-and-exclude.diff added |
---|
by , 14 years ago
Attachment: | 02-add-fieldsets.diff added |
---|
by , 14 years ago
Attachment: | 03-use-core-fieldsets-in-admin.diff added |
---|
comment:9 by , 14 years ago
Added new series of patches. They are based on previous ones, description on wiki page, forms-utils and discussion in django-developers:
- http://code.djangoproject.com/wiki/ImprovementsForDjangoForms#Fieldsets
- http://bitbucket.org/carljm/django-form-utils/src/tip/form_utils/forms.py
- http://groups.google.com/group/django-developers/browse_thread/thread/37e1e8b8313c38de
Description of individual patches follows:
Patch 01-move-fields-and-exclude.diff: This patch moves meta attributes "fields" and "exclude" from ModelForm to Form. It is the most complex patch.
There are some changes which could be bug fixes or backward incompatibilities:
- Attributes "fields" and "exclude" apply also for fields defined on ModelForm. It solves (accepted) ticket #8620.
- With multiply inheritance fields are sorted by method resolution order, not by a custom way. I think it is the right approach (and it simplifies code) but I am prepared to change it back.
I also do not know if some classes and methods are part of public API. I reworked django.forms.forms.get_declared_fields and renamed django.forms.forms.DeclarativeFieldsMetaclass to FormMetaclass (it makes more things after the change). I also stop using parameters "fields" and "exclude" of django.forms.models.fields_for_model but this method is exported from this module so I do not change it.
follow-up: 12 comment:10 by , 14 years ago
Patch 02-add-fieldsets.diff: This patch adds support for fieldsets. Syntax is inspired by django-forms-utils (and different from previous proposal - so it is possible to use names for fieldsets):
class FieldsetsTestForm(TestForm): class Meta: fieldsets = ( ('main', { 'fields': ('field3', 'field8', 'field2'), 'legend': 'Main fields', 'description': 'You should really fill these fields.', }), ('other', { 'fields': ('field5', 'field11', 'field14'), 'legend': 'Other fields', 'attrs': {'class': 'other'}, }), )
Parameter fieldsets is list of items - each item has name and options. There is one required option "fields" giving list of fields. There are three optional options "description", "legend" and "attrs" which can be used for useful information.
Methods Form.as_ul, Form.as_table and Form.as_p are not changed - they are only for simple forms and they continue to work also with fieldsets as plain lists. But there are new methods - has_fieldsets (boolean property) and fieldsets. Fieldsets is method giving collection of fieldsets, each fieldset is collection of fields - it is possible to iterate or use names of fieldsets or fields in fieldsets.
Changes compared to discussion and django-forms-utils:
- There is "attrs" option - it is the most general option. But it is a design decision which could be changed without any problem, it means changes only in two places. I am not really sure if options "legend", "description" and "attrs" are the right choice (so I sometimes change my opinion.)
- Forms without fieldset meta attribute have one dummy fieldset but has_fieldsets property is False - so it is possible to identify form without fieldsets and also to use general templates which want to use fieldsets only.
- There is no length of fieldset collection - is it really useful? For example, there is no length of form as collection of fields.
- There is no caching of fieldsets - it does not seem to be necessary, fieldsets are used only once in more cases. And I try to create patch so simple as possible.
Patch 03-use-core-fieldsets-in-admin.diff: This patch adds support for using of fieldsets in admin. It is similar to current situation with "fields" and "exclude" - if ModelAdmin has "fieldsets" or "fields", they are used. Otherwise meta parameters "fields" and "exclude" from form are de facto respected - this patch is explicit but with similar effects, meta parameter "fieldsets" is respected.
There was one expected requirement - there should be only one implementation of fieldsets in Django, core fieldsets should be reused in admin. I absolutely agree that it seems to be logical requirement - but I really think that requirements for core fieldsets and admin fieldsets are totally different, there is only one common thing - using of HTML fieldset tag. Admin fieldsets are collection of fieldlines (each fieldline has fields), core fieldsets are collection of fields. Core fieldsets are only very light iterators without any specific feature (with exception of options - but they are only saved for using in templates, they are not used in core) - admin fieldsets support for example javascript for collapsing of fields, dependency between fields (for slug fields), not editable fields and more. Please - compare my very simple patch (02-add-fieldsets.diff) and very complex implementation in django.contrib.admin.helpers.
This third patch is without tests - I am prepared to create them if the idea to have two different implementation of fieldsets is accepted. (But I tested it with my code and it works without problems.)
comment:11 by , 14 years ago
Cc: | added |
---|
comment:12 by , 14 years ago
Replying to Petr Marhoun <petr.marhoun@gmail.com>:
Please - compare my very simple patch (02-add-fieldsets.diff) and very complex implementation in django.contrib.admin.helpers.
I have even a simpler approach. Simply add to every field an attribute fieldset
which describes in which fieldset this field is and then use ifchanged
template tag to render fieldsets. I am attaching code for that which populates this attribute accordingly.
And then you can simply do:
... {% for field in form.visible_fields %} {% ifchanged field.fieldset.name %} {% if not forloop.first %} </ul> </fieldset> {% endif %} <fieldset> {% if field.fieldset.name %}<legend>{{ field.fieldset.name }}</legend>{% endif %} <ul> {% endifchanged %} ...
I am also attaching a simple form template which uses this.
by , 14 years ago
A simple approach to fieldsets – example form template
comment:13 by , 14 years ago
Ah, yes. This code can currently be used as a mixin. So you just do:
class MyFieldsetForm(metaforms.FieldsetFormMixin, MyForm): fieldset = ... # same structure as for django.contrib.admin.ModelAdmin
comment:14 by , 14 years ago
Type: | → New feature |
---|
comment:15 by , 14 years ago
Severity: | → Normal |
---|
comment:16 by , 13 years ago
Cc: | added |
---|---|
Easy pickings: | unset |
UI/UX: | unset |
comment:17 by , 13 years ago
UI/UX: | set |
---|
comment:18 by , 13 years ago
Triage Stage: | Design decision needed → Accepted |
---|
This feature is accepted, its never really been DDN (although there are certainly design decisions to be made in the implementation).
This feature needs to remain pretty barebones and structural. Its useful to have information in Python code about the fieldsets structure of a form, so it can be used with generic reusable rendering templates. Any more in-depth presentation-related tweaking should be done in templates, though, not via a complicated Python data structure.
comment:19 by , 13 years ago
Cc: | added |
---|
comment:20 by , 12 years ago
Cc: | added |
---|
I just closed #8272 as a duplicate of this one.
It had a very lightweight approach (with a patch), similar to the one described in comment 12.
comment:21 by , 11 years ago
Cc: | added |
---|
comment:23 by , 10 years ago
Summary: | Fieldsets for newforms → Fieldsets for forms |
---|
comment:24 by , 9 years ago
Hi, forgive me, but I cannot work out of this has been implemented or not? I see carljm's comment, but it isn't clear if this is going anywhere, or if we have to patch manually?
comment:25 by , 9 years ago
Cc: | added |
---|
comment:26 by , 9 years ago
There has been no progress on this ticket that I know of.
Personally I'm no longer interested in it, as I've migrated to the view that even formsets are too much display logic in Python code for most cases (unless you are implementing a generic form-generation system like the admin, in which case you can do it as a custom system like the admin or a reusable third-party extension). For the common case, formsets are something that belong in templates, not Python code.
So I'd personally be happy to just wontfix this ticket, unless some other core developer steps up to champion it.
comment:27 by , 9 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
On that note, going ahead and wontfixing. Any core dev who feels otherwise, feel free to reopen and I won't care :-)
Changes are partly based on a comment from mailing list (patch would be much more worse without it) - http://groups.google.com/group/django-developers/tree/browse_frm/thread/6ec46682fdc298d7/1d6ee0f4099bf9c7?rnum=1&_done=%2Fgroup%2Fdjango-developers%2Fbrowse_frm%2Fthread%2F6ec46682fdc298d7%3Ftvc%3D1%26#doc_93084b127febeb15.
Changes in metaclassing
Motivation for changes in generating of HTML code
Forms with fieldsets could be very complex. And I think that they have to be customizable to be really useful. Now I can have everything or nothing - I can use prepared methods (as_table, as_ul and as_p) or do everything myself (through new methods or in templates). I think patch for adding of fieldsets has to answer three questions - what shall I do when I need to change html code for only one widget, for only one row or for only one fieldset? And there should be two kinds of answers - simple (usual situation - how to mark them for css or javascript) and complex (complete customatization).
First kind of answers: Use argument "attrs" of Widget.init for customization of one widget; Use argument "row_attrs" of Widget.init for customization of one row; Use option "attrs" of fieldset (an item in fieldset dictionary) for customization of one fieldsets. It is not changed in the patch.
Second kind of answers: Rewrite method Widget.render for customization of one widget; rewrite method Form._row_html_output for customization of one row (test row through bf.name or something similar); rewrite method Form._fieldset_html_output for customization of one fieldset (test fieldset dictionary through an item).
Changes in generating of HTML code
There are more methods for generating of html in BaseForm - _label_tag_html_output, _row_html_output, _help_text_html_output, _top_errors_html_output, _fieldset_html_output, _hidden_fields_html_output and _html_output. They have complete responsibility for generating of html code - so it is not necessary to touch any widget for new output method.
There are no backward incompatibilities in bf.label and bf.label_tag in the new patch.