#29059: ChoiceWidget.optgroups() doesn't group choices without a group together
-------------------------------------+-------------------------------------
     Reporter:  Riccardo Di          |                    Owner:  Abhishek
  Virgilio                           |  Gautam
         Type:  Bug                  |                   Status:  assigned
    Component:  Forms                |                  Version:  2.0
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  1
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Carlton Gibson):

 * needs_better_patch:  0 => 1
 * has_patch:  0 => 1


Comment:

 The suggested fix in [https://github.com/django/django/pull/9621 PR#9621]
 breaks choice ordering: it will group all ungrouped choices at whatever
 index the first ungrouped happens to have. This isn't something we should
 impose.  (We may want to present ungrouped default/common choices at the
 beginning, some groups, and then some ungrouped "other" choices at the
 end, for one example.)

 How we might preserve current behaviour whilst allowing grouping of
 ungrouped items seems tricky at best. It looks to me as if a better
 approach would be to recommend using a custom widget and overriding
 `optgroups` in order to gather ungrouped choices if that is what's
 required.

 I put this example on the PR. It's just an untested sketch but it should
 demonstrate the idea:


 {{{
 def optgroups(self, name, value, attrs=None):
     """Override optgroups to group ungrouped choices at end."""
     ret = []
     none_choices = []
     max_index = 0
     for group_name, choices, index in super().optgroups(name, value,
 attrs):
         if group_name is None:
             none_choices.append(choices)
         else:
             max_index = max(max_index, index)
             ret.append((group_name, choices, index))

     ret.append((None, none_choices, max_index +1))
     return ret
 }}}

 As such I'd probably be inclined to close this as Won't Fix.

 (We could document the suggestion, but "override methods to customise
 behaviour" is just OOP.)

-- 
Ticket URL: <https://code.djangoproject.com/ticket/29059#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/076.eb1fa8e8e74250782b74dfee890a19ac%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to