#4412 -- Optgroups in newforms select widgets
Russell Keith-Magee
I've been looking at ticket #4412:
http://code.djangoproject.com/ticket/4412
This ticket adds support for optgroups in selects for newforms. I've
uploaded an updated patch that works with trunk and newforms-admin;
I'm looking for feedback and/or approval to commit.
This ticket has been discussed before:
http://groups.google.com/group/django-developers/browse_thread/thread/8333f29bbe6a20cd
in which James and myself were +1, and Malcolm was tentatively +1.
Jacob didn't comment on the thread, but he did add a comment to the
ticket indicating he was +1 once the patch had documentation.
Adrian hasn't weighed in on this ticket specifically, but in:
http://groups.google.com/group/django-developers/browse_frm/thread/b27f19d932c84316
he gave a nod to adding support for optgroups. The ticket in that
thread was ultimately rejected because it introduced 2 new widgets,
rather than adapting the existing Select widgets.
Now; that's a lot of +1's without something being committed - did this
just slip through the cracks, or were there objections that I've
missed?
Also - the patch works well within newforms, but if you start using
grouped choice definitions, oldforms widgets break. Rather than risk
the breakage, do we want to commit this to newforms-admin instead
(where it works fine)?
Yours,
Russ Magee %-)
Malcolm Tredinnick
> Hi all,
>
> I've been looking at ticket #4412:
>
> http://code.djangoproject.com/ticket/4412
>
> This ticket adds support for optgroups in selects for newforms. I've
> uploaded an updated patch that works with trunk and newforms-admin;
> I'm looking for feedback and/or approval to commit.
>
> This ticket has been discussed before:
>
> http://groups.google.com/group/django-developers/browse_thread/thread/8333f29bbe6a20cd
>
> in which James and myself were +1, and Malcolm was tentatively +1.
I was (and remain) -1 on letting it be part of models. It's presentation
information, not semantic. So I'm -1 on the patch as it stands,
particularly because it's backwards incompatible. Look at all the
leaping through hoops that is going on in django/db/models/base.py to
process a list of choices that are only complicated for *presentation*
purposes.
We had a good run going of moving unnecessary stuff out of models, let's
not regress now please.
Add a flattening function that converts the nested structure to a flat
sequence that can be used for model choices and only allow flat choices
in models. That way people who want this functionality won't have to
write their choices out twice but we can still keep things nicely
separated.
[...]
> Also - the patch works well within newforms, but if you start using
> grouped choice definitions, oldforms widgets break. Rather than risk
> the breakage, do we want to commit this to newforms-admin instead
> (where it works fine)?
Oldforms are still going to exist even after newforms-admin has merged.
We've committed to it for another full release. So that plan breaks
backwards compatibility. Just because core won't be using oldforms,
doesn't mean existing code won't.
Malcolm
--
Why can't you be a non-conformist like everyone else?
http://www.pointy-stick.com/blog/
Russell Keith-Magee
>
> On Thu, 2007-08-30 at 21:18 +0800, Russell Keith-Magee wrote:
> > Hi all,
> >
> > I've been looking at ticket #4412:
>
Apologies - I misread your vote as 'prefer it wasn't in the model, but
+1', rather than '+1 as long as it isn't in the model'.
> It's presentation
> information, not semantic. So I'm -1 on the patch as it stands,
> particularly because it's backwards incompatible. Look at all the
> leaping through hoops that is going on in django/db/models/base.py to
> process a list of choices that are only complicated for *presentation*
> purposes.
I'm not sure I see why it is backward incompatible - all existing use
cases should still work. The potential problem I indicated was purely
with the admin view; I'll come back to this later.
I also don't necessarily pay that this isn't something that should be
kept out of the model. In a completely pure sense, I agree that the
groups and group names aren't required in the model. However, in a
practical sense, I'm not convinced it matters:
- The grouping data can be safely ignored by the model;
- It avoids the need for a double representation of the option list; and
- The default newforms widget for the field can be more meaningful.
The only place I can see it being a problem is when there are multiple
groupings. Personally, I don't see this as a common use case, but even
if it is, it's easy to configure a widget to use an alternate
grouping.
> Add a flattening function that converts the nested structure to a flat
This fixes the double representation problem, but not the issue of
making the default widget meaningful. However, I could live with this
solution if the BDFLs deem it so.
> Oldforms are still going to exist even after newforms-admin has merged.
> We've committed to it for another full release. So that plan breaks
> backwards compatibility. Just because core won't be using oldforms,
> doesn't mean existing code won't.
Yes, oldforms will still exist, but oldforms-admin wont. The issue is
really only with oldforms-admin. If you are using oldforms in your
project, you can't use grouped choices, and we can easily document
this fact. However, if you are using newforms for your project,
grouped choices are legal - but the existing oldforms-admin will
break. When we merge newforms-admin, this problem goes away.
Of course, this issue goes away if we take your 'flattening function' proposal.
Any comment from the BDFLs?
Yours,
Russ Magee %-)
Tai Lee
presentation with the model? I don't see why having the model ignore
the groupings would be any worse than having the model ignore the
display text for each choice.
I'd really like to see nested choices / optgroups allowed in the
model, so that auto generated forms can take advantage of it. I've
been using the patches attached to this ticket while I wait for it to
hit trunk. The only issue I've had is that the optgroup doesn't work
in the old admin, which is going away soon.