Opened 10 years ago

Closed 4 years ago

Last modified 4 years ago

#23681 closed Cleanup/optimization (fixed)

Document how to customize NullBooleanSelect choice names

Reported by: benjaoming Owned by: Jacob Walls
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 (17)

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=[("1", "Male and female"),
                               ("2", "Only female"),
                               ("3", "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!

Version 0, edited 10 years ago by benjaoming (next)

comment:4 by Tim Graham, 10 years ago

No, it wasn't a typo. I tested with None, True, and False as the values in the Select choices. NullBooleanField doesn't know anything about "1", "2", "3". Those special strings are only relevant if you are using the NullBooleanSelect widget.

comment:5 by benjaoming, 10 years ago

Sry, I had a copy paste typo (see edited comment above). I did test it with None/True/False, and it works seemingly fine. But there has to be a good reason why NullBooleanSelect has customized methods. If Select works perfectly using the above choices, why is there a NullBooleanSelect? :)

In case we could just replace it with choices=... then the following could get rid of NullBooleanSelect alltogether...

Code highlighting:

class NullBooleanField(BooleanField):
    """
    A field whose valid values are None, True and False. Invalid values are
    cleaned to None.
    """
    # THIS ONE GOES!
    # widget = NullBooleanSelect
    def __init__(self, *args, **kwargs):
        self.widget = Select(choices=[(None, _("Unknown")),
                            (True, _("Yes")),
                            (False, _("No"))])
        super(NullBooleanField, self).__init__(*args, **kwargs)

    def to_python(self, value):
        """
        Explicitly checks for the string 'True' and 'False', which is what a
        hidden field will submit for True and False, and for '1' and '0', which
        is what a RadioField will submit. Unlike the Booleanfield we need to
        explicitly check for True, because we are not using the bool() function
        """
        if value in (True, 'True', '1'):
            return True
        elif value in (False, 'False', '0'):
            return False
        else:
            return None

    def validate(self, value):
        pass

comment:6 by Tim Graham, 10 years ago

Easy pickings: unset

There may be some subtle differences, I'd have to take a closer look to say for sure. You could try making the change and seeing what tests in Django's test suite fail. I'm not sure it's worth the hassle of removing the NullBooleanSelect widget though.

comment:7 by Tim Graham, 10 years ago

Cc: Tim Graham added
Component: FormsDocumentation
Summary: NullBooleanSelect should have choices kwargDocument how to customize NullBooleanSelect choice names
Triage Stage: UnreviewedAccepted

I am in favor of documenting the use of the Select widget as a way to customize the choices (I imagine RadioSelect would work as well). It seems like adding true_label, etc. to the form field would add tighter coupling between the form field and the widget which probably isn't desired, but feel free to continue the discussion or propose an implementation where that's not the case.

comment:8 by Héctor Urbina, 10 years ago

Hello,

I'm using django-inplaceedit to show the value of a NullBooleanField. When the user edits the value, the form is built by django-inplaceedit. I would like to have a way to set the captions for the options to be other than "Unknown, Yes and No".

I tried in the model definition, by writting

calificada = models.NullBooleanField("Venta calificada", default=False, choices = {(None,"No Aplica"),(True,"Sí"),(False,"No")})

but then inplaceedit shows the values as text instead of the nice images that uses in the default case, which are being very useful for visualizing...

Anyways, I can write some javascript to address my problem. Just wanted to point out an use case where one doesn't control the form creation.

comment:9 by Wim Feijen, 9 years ago

I agree with benjaoming that it would be good to replace NullBooleanField's NullBooleanSelect widget by a normal Select widget as proposed. benjaomings solution works for me and is more intuitive and usable when there is need to change the choices' texts.

For backwards compability, would this mean starting a deprecation path for NullBooleanSelect?

comment:10 by Tim Graham, 9 years ago

Correct. See FeatureDeprecationChecklist for tips.

comment:11 by Giovanni Totaro - aka Vanni, 7 years ago

Regarding original @benjaoming request

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

I just tested it with Django 2.0 and it seems to already work fine:

class MyModel(models.Model):
    na_yes_no = models.NullBooleanField(choices=((None, "I don't care"), (True, "'Yes!"), (False, "Ooh no!"))
)

comment:12 by Jacob Walls, 4 years ago

Easy pickings: set
Owner: changed from benjaoming to Jacob Walls
Type: New featureCleanup/optimization

comment:13 by Jacob Walls, 4 years ago

Has patch: set

comment:14 by Jacob Walls, 4 years ago

Easy pickings: unset

comment:15 by Carlton Gibson, 4 years ago

Triage Stage: AcceptedReady for checkin

comment:16 by GitHub <noreply@…>, 4 years ago

Resolution: fixed
Status: assignedclosed

In d976c254:

Fixed #23681, Fixed #27445 -- Doc'd setting choices for NullBooleanField widgets.

Thanks to David Smith for investigation and review.

Co-authored-by: Carlton Gibson <carlton.gibson@…>

comment:17 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In 82bdc51:

[3.1.x] Fixed #23681, Fixed #27445 -- Doc'd setting choices for NullBooleanField widgets.

Thanks to David Smith for investigation and review.

Co-authored-by: Carlton Gibson <carlton.gibson@…>
Backport of d976c254fc76e5f04d81dfd9d142c58e933c9c92 from master

Note: See TracTickets for help on using tickets.
Back to Top