#18872 closed New feature (fixed)
Add prefix attribute to FormMixin
Reported by: | Owned by: | anonymous | |
---|---|---|---|
Component: | Generic views | Version: | dev |
Severity: | Normal | Keywords: | FormMixin prefix |
Cc: | ibustamante | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | yes |
Needs tests: | yes | Patch needs improvement: | yes |
Easy pickings: | yes | UI/UX: | no |
Description
I just want to suggest adding a "prefix" attribute, and its corresponding get_prefix() method, to the django.views.generix.edit.FormMixin, in order to make it possible to add a prefix to the form.
I'll attach a patch later, in case my suggestion is considered.
Change History (19)
comment:1 by , 12 years ago
Cc: | added |
---|
comment:2 by , 12 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 12 years ago
Has patch: | set |
---|
comment:4 by , 12 years ago
Version: | 1.4 → master |
---|
comment:5 by , 12 years ago
Summary: | Adding prefix attribute to FormMixin → Add prefix attribute to FormMixin |
---|
follow-up: 7 comment:6 by , 12 years ago
Needs tests: | set |
---|---|
Patch needs improvement: | set |
Triage Stage: | Unreviewed → Accepted |
Please submit patches as either diff attachments or links to *pull request*, which makes them easy to review.
comment:7 by , 12 years ago
Created a pull request to add the patch as requested: https://github.com/django/django/pull/477
comment:8 by , 12 years ago
Owner: | changed from | to
---|
comment:9 by , 12 years ago
Needs documentation: | set |
---|
comment:10 by , 12 years ago
Well, just a thought: prefixes are used to discriminate between multiple forms within the same view. Since the FormMixin handles only one form, what's the use case for setting a custom prefix for that form?
comment:11 by , 12 years ago
Yes, but you can always create two (and more) different forms within one page and with prefix field you can avoid inputs name collision.
comment:12 by , 12 years ago
I want to reinforce what @konradhalas said. It's possible to have multiple forms on the same page when you're using AJAX, for example. In this case you would need to have a prefix set, so as to avoid input id collision.
comment:13 by , 12 years ago
Have to agree. There are lots of reasons you might need to use a prefix that don't involve processing more than one form. I ran into this because we had a form on the page that acted as a context switcher, changing it's value caused the form to be processed, changed a session value, and reloaded the page given the new context. This caused an id collision anytime the user was on a page that happened to have a form that had the same field name (really common considering the context was a FK).
comment:14 by , 12 years ago
Owner: | changed from | to
---|
I have tentatively reserved this ticket for first-time committers who take part in the Don't be afraid to commit workshop at the DjangoCon Europe 2013 sprints on 18th and 19th May.
If you want to tackle this ticket before then, please don't let the fact that it's assigned to me stop you. Feel free to re-assign it to yourself and do whatever you like to it.
Note - the pull request https://github.com/django/django/pull/477 for this one has been closed because "it needs docs and tests". You know what to do...
comment:15 by , 12 years ago
Owner: | changed from | to
---|
Grabbing the ticket. I aim to add docs and tests by the end of the weekend June 1-2.
Is there any reason why we shouldn't always include the prefix in the form kwargs?
def get_form_kwargs(self): """ Returns the keyword arguments for instantiating the form. """ kwargs = {'initial': self.get_initial(), 'prefix': self.get_prefix(), }
There shouldn't be any problems doing this, because the default prefix=None
matches the default prefix=None
in the form class. Always including prefix makes it easier to describe in the get_form_kwargs docs ("the prefix argument is set to get_prefix" than in the initial pull request, where prefix is only included if it evaluates to True.
comment:16 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:18 by , 12 years ago
In the release notes, this was added under "Backwards incompatible changes". I believe it should be under "Minor features" instead?
Patch added: https://github.com/ibustama/django/tree/ticket_18872