Opened 7 years ago

Closed 7 years ago

#29059 closed Bug (wontfix)

ChoiceWidget.optgroups() doesn't group choices without a group together

Reported by: Riccardo Di Virgilio Owned by: Abhishek Gautam
Component: Forms Version: 2.0
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

I'm having problems writing template for custom controls (a bootstrap button bar) using optgroup.
the problem is that optgroup is not grouping choices without groups, this makes very hard to create several button bars that grouped togheter:

from django.forms.widgets import ChoiceWidget

w = ChoiceWidget(choices = [
    ("one", 1),
    ("two", 2), [
        "italian", (
            ("uno", 1),
            ("due", 2)
        )
    ]
])

now see how choices are grouped from optgroup:

import json

print(json.dumps(w.optgroups("name", "one"), indent = 4))
[
    [
        null,
        [
            {
                "name": "name",
                "value": "one",
                "label": 1,
                "selected": true,
                "index": "0",
                "attrs": {
                    "checked": true
                },
                "type": null,
                "template_name": null
            }
        ],
        0
    ],
    [
        null,
        [
            {
                "name": "name",
                "value": "two",
                "label": 2,
                "selected": false,
                "index": "1",
                "attrs": {},
                "type": null,
                "template_name": null
            }
        ],
        1
    ],
    [
        "italian",
        [
            {
                "name": "name",
                "value": "uno",
                "label": 1,
                "selected": false,
                "index": "2_0",
                "attrs": {},
                "type": null,
                "template_name": null
            },
            {
                "name": "name",
                "value": "due",
                "label": 2,
                "selected": false,
                "index": "2_1",
                "attrs": {},
                "type": null,
                "template_name": null
            }
        ],
        2
    ]
]

this means that if choices are flats (no groups) you are getting a number of groups that is the same of the number of choices, and in order to fix this you need to use the grouping filters on the template in order to regroup choices.

I think this is a bug, and flat choices should be grouped together as None by optgroup method

thanks

Change History (5)

comment:1 by Tim Graham, 7 years ago

Summary: Widget optgroup is not grouping choices with no groupChoiceWidget.optgroups() doesn't group choices without a group together
Triage Stage: UnreviewedAccepted

It might be fine -- can you offer a patch?

comment:2 by Abhishek Gautam, 7 years ago

Owner: changed from nobody to Abhishek Gautam
Status: newassigned

comment:3 by Riccardo Di Virgilio, 7 years ago

Sure i'll do this in the next days.
Is it ok to provide just the function?

thanks!

comment:4 by Carlton Gibson, 7 years ago

Has patch: set
Patch needs improvement: set

The suggested fix in 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.)

comment:5 by Tim Graham, 7 years ago

Resolution: wontfix
Status: assignedclosed
Note: See TracTickets for help on using tickets.
Back to Top