Re: [Django] #5986: Custom field order in newforms
Django
-------------------------------------+-------------------------------------
Reporter: emes | Owner: nobody
Type: Uncategorized | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: field order weight | Triage Stage: Design
form newforms | decision needed
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by mbertheau):
* status: closed => new
* severity: => Normal
* type: => Uncategorized
* easy: => 0
* ui_ux: => 0
* resolution: duplicate =>
Comment:
This is not a dupe of #8164 since that only talks about and implements
field ordering for ModelForms.
--
Ticket URL: <https://code.djangoproject.com/ticket/5986#comment:23>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: emes | Owner: nobody
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: field order weight | Triage Stage:
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Design decision needed => Unreviewed
* type: Uncategorized => New feature
Comment:
I haven't tried this myself, but would a syntax like this work?
{{{
class UserProfileForm(UserForm):
first_name = forms.CharField(max_length=30, weight=-1)
last_name = forms.CharField(max_length=30, weight=-1)
username = UserForm.base_fields['username']
password = UserForm.base_fields['password']
password2 = UserForm.base_fields['password2']
jabber_id = forms.EmailField()
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/5986#comment:24>
Django
-------------------------------------+-------------------------------------
Reporter: emes | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
form newforms | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timgraham):
* needs_docs: 1 => 0
* has_patch: 1 => 0
* stage: Unreviewed => Accepted
Comment:
Pre-Django 1.7, the following worked, but relied on internal
implementation details (that `self.fields` was `SortedDict`; now it's
`OrderedDict`):
{{{
class EditForm(forms.Form):
summary = forms.CharField()
description = forms.CharField(widget=forms.TextArea)
class CreateForm(EditForm):
name = forms.CharField()
def __init__(self, *args, **kwargs):
super(CreateForm, self).__init__(*args, **kwargs)
self.fields.keyOrder = ['name', 'summary', 'description']
}}}
Adding an official API for making ordering easier on forms that use
inheritance seems reasonable. I'm not sure if recommending `base_fields`
is the best approach as that's not public API as of now.
--
Ticket URL: <https://code.djangoproject.com/ticket/5986#comment:25>
Django
-------------------------------------+-------------------------------------
Reporter: emes | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: field order weight | Triage Stage: Accepted
form newforms | Needs documentation: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timgraham):
* has_patch: 0 => 1
Comment:
#23936 was a duplicate and included a
[https://github.com/django/django/pull/3652 pull request].
--
Ticket URL: <https://code.djangoproject.com/ticket/5986#comment:26>
Django
-------------------------------------+-------------------------------------
Reporter: emes | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: field order weight | Triage Stage: Accepted
form newforms | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by loic):
Can't say I'm convinced with this ticket, IMO ordering fields belongs in
the templates.
--
Ticket URL: <https://code.djangoproject.com/ticket/5986#comment:27>
Django
-------------------------------------+-------------------------------------
Reporter: emes | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: field order weight | Triage Stage: Accepted
form newforms | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by alasdairnicol):
Replying to [comment:27 loic]:
> Can't say I'm convinced with this ticket, IMO ordering fields belongs in
the templates.
I used `self.field.keyOrder` in previous versions of Django, and would
find an official API useful. If you render the form in the template with
`{{ form }}` or `{{ form.ul }}`, then it's much easier to change the field
order in the form than the template.
The
[https://github.com/django/django/blob/1.7/django/contrib/auth/forms.py#L338
PasswordChangeForm] in the contrib.auth app changes the field order by
changing base_fields. I think it's much better to change it there than to
tell users to put 'old password' before 'new password' in their template.
It would be even better if we used a public API to change the field order.
--
Ticket URL: <https://code.djangoproject.com/ticket/5986#comment:28>
Django
-------------------------------------+-------------------------------------
Reporter: emes | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: field order weight | Triage Stage: Accepted
form newforms | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* cc: charettes (added)
Comment:
Replying to [comment:27 loic]:
> Can't say I'm convinced with this ticket, IMO ordering fields belongs in
the templates.
The thing is field ordering also affects the order of `clean_<field_name>`
calls.
--
Ticket URL: <https://code.djangoproject.com/ticket/5986#comment:29>
Django
-------------------------------------+-------------------------------------
Reporter: emes | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: field order weight | Triage Stage: Accepted
form newforms | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by carljm):
Hmm, I always thought that relying on ordering of `clean_<field_name>`
calls was unsupported; a `clean_<field_name>` method should only deal with
its field and nothing else, if you need to validate multiple fields it
should be done in `clean`.
I think that in most cases forms are better rendered explicitly in the
template, and that's where field-ordering belongs. But there are use cases
(more generic applications such as the admin) where that's not practical.
The fact that we reorder form fields ourselves in Django is a pretty
strong argument that there should be a public API for it.
--
Ticket URL: <https://code.djangoproject.com/ticket/5986#comment:30>
Django
-------------------------------------+-------------------------------------
Reporter: emes | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: field order weight | Triage Stage: Accepted
form newforms | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* cc: loic (added)
Comment:
Replying to [comment:30 carljm]:
> Hmm, I always thought that relying on ordering of `clean_<field_name>`
calls was unsupported; a `clean_<field_name>` method should only deal with
its field and nothing else, if you need to validate multiple fields it
should be done in `clean`.
+1, especially now that `add_error()` makes it very easy to do.
> I think that in most cases forms are better rendered explicitly in the
template, and that's where field-ordering belongs. But there are use cases
(more generic applications such as the admin) where that's not practical.
The fact that we reorder form fields ourselves in Django is a pretty
strong argument that there should be a public API for it.
Quoting from the PR regarding `Form.field_order`: "Fields missing in the
list are removed". I'm worried this adds yet another way of excluding
fields, `ModelForm` is already pretty confusing with both `fields` and
`exclude` (and the possible overlap of the two). There is also the
interaction with `ModelAdmin` that already supports fields ordering
through `fields` and `fieldsets`.
Finally I'd like to point out that the proposed API doesn't play well with
inheritance. We wouldn't be able to use it in `PasswordChangeForm` for
instance as it would break any subclasses with extra fields.
Doesn't sorting `self.fields` in `__init__()` achieves the same result
anyway?
--
Ticket URL: <https://code.djangoproject.com/ticket/5986#comment:31>
Django
-------------------------------------+-------------------------------------
Reporter: emes | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: field order weight | Triage Stage: Accepted
form newforms | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by tanner):
I have revised my PR to make it fully backwards-compatible.
The field_order attribute/argument is used to rearrange existing fields.
If a key of an existing field is missing in the list, the field is
appended to the fields in the original order.
This way new fields in subclasses are just added (as before) if the
subclass does not override field_order.
If a key in field_order refers to a non-existing field, it is simply
ignored.
This make is possible to remove a field in a subclass by overriding it
with None, without overriding field_order.
Again, this won't break existing subclasses.
If you do not define Form.field_order or set it None, the default order is
kept.
There are now three ways to set the field order:
1. set the class variable as default for all instances
2. pass field_order as argument when instantiating a Form, e.g., when
calling the parent constructor
3. use the order_fields() method to rearrange the field order of an
instance, e.g., in a subclass constructor after some extra fields have
been added.
--
Ticket URL: <https://code.djangoproject.com/ticket/5986#comment:32>
Django
-------------------------------------+-------------------------------------
Reporter: emes | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: field order weight | Triage Stage: Accepted
form newforms | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* cc: tanner (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/5986#comment:33>
Django
-------------------------------------+-------------------------------------
Reporter: emes | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: field order weight | Triage Stage: Accepted
form newforms | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by tanner):
anyone willing to review the PR?
https://github.com/django/django/pull/3652
--
Ticket URL: <https://code.djangoproject.com/ticket/5986#comment:34>
Django
-------------------------------------+-------------------------------------
Reporter: emes | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: field order weight | Triage Stage: Accepted
form newforms | Needs documentation: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
Comment:
Thanks. I've reviewed the patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/5986#comment:35>
Django
-------------------------------------+-------------------------------------
Reporter: emes | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: field order weight | Triage Stage: Accepted
form newforms |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by tanner):
* needs_better_patch: 1 => 0
Comment:
thank you. patch has been revised
--
Ticket URL: <https://code.djangoproject.com/ticket/5986#comment:36>
Django
-------------------------------------+-------------------------------------
Reporter: emes | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: field order weight | Triage Stage: Accepted
form newforms |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by berkerpeksag):
> use the order_fields() method to rearrange the field order of an
instance, e.g., in a subclass constructor after some extra fields have
been added.
It would be good if users can avoid to call `self.order_fields()` manually
(see https://github.com/django/django/pull/3652#issuecomment-65085465 for
a use case). Perhaps we can call this method in `self.full_clean()`,
`__call__` or somewhere else?
--
Ticket URL: <https://code.djangoproject.com/ticket/5986#comment:37>
Django
-------------------------------------+-------------------------------------
Reporter: emes | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: field order weight | Triage Stage: Accepted
form newforms |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by tanner):
why? that use case is very rare and I think it's acceptable to call
order_fields explicitly in this case. The field order has an effect on
many other methods and must be set after initialization, long before
validation.
--
Ticket URL: <https://code.djangoproject.com/ticket/5986#comment:38>
Django
-------------------------------------+-------------------------------------
Reporter: emes | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: field order weight | Triage Stage: Accepted
form newforms |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
Comment:
Cosmetic suggests to go. Please bump patch the ticket to "Ready for
checkin" after you make those updates.
--
Ticket URL: <https://code.djangoproject.com/ticket/5986#comment:39>
Django
-------------------------------------+-------------------------------------
Reporter: emes | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
form newforms | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/5986#comment:40>
Django
-------------------------------------+-------------------------------------
Reporter: emes | Owner: nobody
Component: Forms | Version: master
Severity: Normal | Resolution: fixed
Keywords: field order weight | Triage Stage: Ready for
form newforms | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"28986da4ca167ae257abcaf7caea230eca2bcd80" 28986da]:
{{{
#!CommitTicketReference repository=""
revision="28986da4ca167ae257abcaf7caea230eca2bcd80"
Fixed #5986 -- Added ability to customize order of Form fields
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/5986#comment:41>