Opened 10 years ago

Last modified 4 years ago

#23681 closed Cleanup/optimization

NullBooleanSelect should have choices kwarg — at Version 3

Reported by: benjaoming Owned by: benjaoming
Component: Documentation Version: dev
Severity: Normal Keywords:
Cc: Tim Graham Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by benjaoming)

NullBooleanSelect is responsible of making the values 1, 2, and 3 turn into None, True or False. That's very nice of it, however it does not allow to customize the texts of the choices.

I'm not sure if exposing the internal 1, 2, 3 representation is a good idea, but it would seem okay since it follows the convention of other Select widgets. Ideally, I would like to see this...

class NullBooleanSelect(Select):
    """
    A Select Widget intended to be used with NullBooleanField.
    """
    def __init__(self, attrs=None):
        choices = (('1', ugettext_lazy('Unknown')),
                   ('2', ugettext_lazy('Yes')),
                   ('3', ugettext_lazy('No')))
        super(NullBooleanSelect, self).__init__(attrs, choices)

...changed to:

class NullBooleanSelect(Select):
    """
    A Select Widget intended to be used with NullBooleanField.
    """
    def __init__(self, choices=None, attrs=None):
        if not choices:
            choices = (('1', empty_label or ugettext_lazy('Unknown')),
                       ('2', ugettext_lazy('Yes')),
                       ('3', ugettext_lazy('No')))
        super(NullBooleanSelect, self).__init__(attrs, choices)

The motivation is that I often leave out labels to have them put as the default first option of the Select. An example use:

class MyForm(forms.Form):
    gender = forms.NullBooleanField(
        label="",
        required=False,
        widget=NullBooleanSelect(choices=[("1", "Male and female"),
                                          ("2", "Only female"),
                                          ("3", "Only Male")])
        help_text="Choose gender",
    )

Even more preferable, would be to place the choices kwarg in NullBooleanField, as that would match the options for ChoiceField.

<b>Updated</b> In the original issue report, I put empty_label but realized that when selecting "Yes", it was impossible for the user to see what "Yes" was the answer to.

Change History (3)

comment:1 by benjaoming, 10 years ago

Description: modified (diff)
Owner: changed from nobody to benjaoming
Status: newassigned
Summary: NullBooleanSelect should have empty_label or similarNullBooleanSelect should have choices kwarg

comment:2 by Tim Graham, 10 years ago

As far as I can tell, you can use the Select widget instead of NullBooleanSelect to achieve this (with NullBooleanField form field). widget=forms.Select(choices=[(None, "..."), (True, "..."), (False, "...")]). Assuming that works for you, we can document this tip. Would you like to submit a patch?

comment:3 by benjaoming, 10 years ago

Description: modified (diff)

Hi timgraham! I think what you are hinting at is this:

class MyForm(forms.Form):
    gender = forms.NullBooleanField(
        label="",
        required=False,
        widget=Select(choices=[(None, "Male and female"),
                               (True, "Only female"),
                               (False, "Only Male")])
        help_text="Choose gender",
    )

It will work, but not perfectly. NullBooleanSelect has special methods render, value_from_datadict that are tailored for NullBooleanField (I'm not sure why _has_changed has gone, it used to also be customized). As I understand, they are there to ensure that the None value can be deliberately extracted from the value '1'.

I'm totally open for suggestions... I would like to be able to achieve custom labels in the choices of NullBooleanField, because I think it's an essential option that can keep us from creating one-trick sub classes. And I would probably often like to use other words than Unknown, Yes, and No.

Seeing that NullBooleanField always returns None, False, and True, it might make sense to put them as explicit kwargs, like how empty_label is used. This is perhaps better than using the widget....

class MyForm(forms.Form):
    gender = forms.NullBooleanField(
        label="",
        required=False,
        empty_label="Male and female",
        true_label="Only female",
        false_label="Only Male",
        help_text="Choose gender",
    )

To me, that seems nice, clean, explicit, and useful :) And yes, I can write a patch!

Last edited 10 years ago by benjaoming (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top