New feature: Defining fieldsets in form class (Ticket #17301)

[Mikołaj Siedlarek]

Hi,

I've just posted a new ticket with everything the feature proposal needs - motivation, idea and actual implementation.

https://code.djangoproject.com/ticket/17301

It definitely needs some discussion, so please -- ask, discuss, criticize, share some thoughts and one day, hopefully +1. :)

Thanks,

[Daniel Swarbrick]

+1

I've often wondered why fieldsets have been possible in the Django
admin interface for a while now, but not possible with ModelForms.
Looking forward to this.

[Kiril Vladimirov]

+1

[Russell Keith-Magee]

2011/11/27 Mikołaj Siedlarek <miko...@gmail.com>:

Hi Mikołaj,

You won't get any argument from me on this one.

In my ideal world, Django's admin should just be a light customization
of Django's native form capabilities, tied to some nifty model
introspection logic. However, as currently implemented, there are
plenty of features -- like fieldsets -- that are only available to
admin forms. Refactoring this code out of admin, and into base forms
is certainly something worth pursuing.

Also worth keeping in mind is the intersection of this sort of
refactoring with efforts to replace Django's form rendering with a
template-based renderer. This was the subject of a Google Summer of
Code project this year, and while it hasn't been merged to trunk,
there's some obvious potential for overlap with any plans to introduce
fieldsets into base forms.

From an quick inspection of your patch, you look like your on the
right track -- you've certainly got all the right pieces (tests, docs,
and what appears to be a solid initial implementation).

The only thing that I would like to see is an attempt to eat our own
dogfood -- i.e., to try and replace the admin's custom fieldset tools
with this new capabilitiy. Dogfooding is an important step in
demonstrating that you've got the API right; given that this is
something that is being factored out of admin, actually using it in
admin would be a good proof of that concept.

Thanks for the great contribution!

Yours,
Russ Magee %-)

[Mikołaj Siedlarek]

Hi Russel,

My patch obviously was not posted to be merged into trunk in it's current state.

It is meant to be a message "i'd like to do that, seriously". Now that I see general

community support I can perfect it. There several more things I'd like to do, mainly:

1. "Dogfooding" (which is a great phrase I've never heard of :) my concept by
   modyfying admin;
2. `classes` option, just like the one in current admin fieldsets;
3. Several fields in one row, once again like in current admin;
4. Write some more documentation in my poor-English and ask someone nicely
   to translate it to real-English

This features should be complete by the end of the week, and then we'll see.

PS. If there's anyone willing to edit some really bad writing, please contact me.

Thanks,

Mikołaj

[Russell Keith-Magee]

2011/11/28 Mikołaj Siedlarek <miko...@gmail.com>:

> Hi Russel,
> My patch obviously was not posted to be merged into trunk in it's current
> state.
> It is meant to be a message "i'd like to do that, seriously". Now that I see
> general
> community support I can perfect it. There several more things I'd like to
> do, mainly:
>
> "Dogfooding" (which is a great phrase I've never heard of :) my concept by
> modyfying admin;

Apologies for the confusion. For those not familiar with the term:

http://en.wikipedia.org/wiki/Eating_your_own_dog_food

> `classes` option, just like the one in current admin fieldsets;
> Several fields in one row, once again like in current admin;
> Write some more documentation in my poor-English and ask someone nicely
> to translate it to real-English

Don't worry too much about this bit. The hard part of writing
documentation is always the first draft, and coming up with a good set
of examples to use in that draft. If you can get to a first draft that
is roughly understandable (and the first draft you've got in your
patch is certainly good enough), it can get cleaned up on editing when
it is committed.

Yours,
Russ Magee %-)

[Carl Meyer]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Mikołaj,

Thanks for your work on contributing to Django!

I am a bit skeptical about defining form fieldsets in Python code, as
they have no effect on server-side interpretation or validation of form
data, they are purely an HTML/presentational issue. I think that in
general we should be moving form presentation entirely into templates
rather than adding more presentation-related attributes into Python
code. There's already work begun on this via a Google Summer of Code
project (see #15667 and #16922); there are some barriers to overcome
(particularly performance issues with the template engine), but I think
it's still the right approach for the long term. And the more
form-presentation controls we add in Python code, the more difficult
this transition becomes (because we end up with two conflicting methods
of defining a forms layout).

However, I can see some justification for simple fieldset definitions in
the form definition, as a high-level way to group fields. This can allow
more flexibility in defining custom reusable form layouts (via
templates) that depend on having such field groupings. And it's clear
that others want this feature: I won't stand in the way of it if another
core developer wants to champion its inclusion.

However, I feel quite strongly that even if we add this, we should NOT
add the additional "classes" and "fields in a row" features that you
mention. Fieldsets themselves are defensible as logical "structure" of
the form - these additional features are getting into the arena of
trying to fully define HTML layout of a form in Python code, which I
think is fundamentally the wrong approach, and makes it more difficult
for non-Python-programmer template authors to work with Django forms.
Why should forms be an exception to the general rule that HTML and
presentation belong in templates?

I see the admin as the exception here, rather than as a model for how
the forms library should work. The admin's purpose is to create generic
customizable CRUD screens for arbitrary models; this is necessarily
going to involve defining the available axes of presentation
customization, and making it easy to do those particular customizations.
That's a very specific use case; it means trading off flexibility for
convenience in very simple customizations. That's an appropriate
tradeoff for the admin to make in building on top of django.forms; it's
not an appropriate tradeoff for django.forms, IMO.

In sum: +0 to fieldsets in Python code, -1 to classes and rows defined
in Python code.

Carl
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk7TwyAACgkQ8W4rlRKtE2eWiACgnT5afZXt7skGy66pIa4zDTrM
kKkAoOoM/VIvJNPFMk0eaROh2DYT7/gv
=SdfG
-----END PGP SIGNATURE-----

[Mikołaj Siedlarek]

Hi Carl,

To be honest, I think that Django will never achieve complete separation in terms of MVC

model staying this convenient tool. There are many places where Django has a simple way

of doing things, even when it's not full MVC-compilant. Current Form.as_(table|ul|p) methods

are examples. I believe that Django should keep simple things simple.

Template-based forms are great idea (however can't say I've read the whole patch, it's rather

huge), but I don't think that creating template to separate groups of fields is keeping simple

things simple.

Fair point about `classes` and several fields in a row though. I think you're right and decided

not to do that.

Mikołaj