[Django] #23681: NullBooleanSelect should have empty_label or similar
Django
-----------------------------+--------------------
Reporter: benjaoming | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 1 | UI/UX: 0
-----------------------------+--------------------
`NullBooleanSelect` is responsible of making the values 1, 2, and 3 turn
into None, True or False. That's why having custom "choices" kwarg is
perhaps a bit over the top. Well, it's pretty meaningless to have a
utility class if you start passing a choices kwarg that contains half of
the code that makes up NullBooleanSelect.
However, I need one small feature from this widget! Namely, that the
following...
{{{
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)
}}}
...is changed to:
{{{
class NullBooleanSelect(Select):
"""
A Select Widget intended to be used with NullBooleanField.
"""
def __init__(self, empty_label=None, attrs=None):
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):
has_payments = forms.NullBooleanField(
label="",
required=False,
widget=NullBooleanSelect(empty_label=_(u"Has previous payments?"))
help_text=_(u"Only show subscriptions that have previously been
charged"),
)
}}}
Even more preferable, would be to place the `empty_label` kwarg in
`NullBooleanField`, as that would match the options for ModelChoiceField.
--
Ticket URL: <https://code.djangoproject.com/ticket/23681>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-----------------------------+--------------------------------------
Reporter: benjaoming | Owner: benjaoming
Type: New feature | Status: assigned
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by benjaoming):
* status: new => assigned
* needs_better_patch: => 0
* needs_tests: => 0
* owner: nobody => benjaoming
* needs_docs: => 0
Old description:
> ...is changed to:
>
New description:
`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=_(u"Only show subscriptions that have previously been
charged"),
)
}}}
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.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/23681#comment:1>
Django
-----------------------------+--------------------------------------
Reporter: benjaoming | Owner: benjaoming
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by timgraham):
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/23681#comment:2>
Django
-----------------------------+--------------------------------------
Reporter: benjaoming | Owner: benjaoming
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Old description:
> `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=_(u"Only show subscriptions that have previously been
> charged"),
> )
>
> }}}
>
> 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.
New description:
`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.
--
Comment (by benjaoming):
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!
--
Ticket URL: <https://code.djangoproject.com/ticket/23681#comment:3>
Django
-----------------------------+--------------------------------------
Reporter: benjaoming | Owner: benjaoming
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by timgraham):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/23681#comment:4>
Django
-----------------------------+--------------------------------------
Reporter: benjaoming | Owner: benjaoming
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by benjaoming):
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...
{{{
#!div style="font-size: 80%"
Code highlighting:
{{{#!python
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
}}}
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23681#comment:5>
Django
-----------------------------+--------------------------------------
Reporter: benjaoming | Owner: benjaoming
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-----------------------------+--------------------------------------
Changes (by timgraham):
* easy: 1 => 0
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/23681#comment:6>
Django
-------------------------------+--------------------------------------
Reporter: benjaoming | Owner: benjaoming
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* cc: timgraham (added)
* stage: Unreviewed => Accepted
* component: Forms => Documentation
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/23681#comment:7>
Django
-------------------------------+--------------------------------------
Reporter: benjaoming | Owner: benjaoming
Type: New feature | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Comment (by ajendrex):
Hello,
I'm using [https://github.com/Yaco-Sistemas/django-inplaceedit/ 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
{{{#!python
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/23681#comment:8>
Django
-------------------------------+--------------------------------------
Reporter: benjaoming | Owner: benjaoming
Type: New feature | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Comment (by wimfeijen):
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/23681#comment:9>
Django
-------------------------------+--------------------------------------
Reporter: benjaoming | Owner: benjaoming
Type: New feature | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Comment (by timgraham):
Correct. See FeatureDeprecationChecklist for tips.
--
Ticket URL: <https://code.djangoproject.com/ticket/23681#comment:10>
Django
-------------------------------+--------------------------------------
Reporter: benjaoming | Owner: benjaoming
Type: New feature | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Comment (by Giovanni Totaro - aka Vanni):
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!"))
)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23681#comment:11>
Django
Reporter: benjaoming | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Jacob Walls):
* owner: benjaoming => Jacob Walls
* type: New feature => Cleanup/optimization
* easy: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/23681#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: benjaoming | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Jacob Walls):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/13437 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/23681#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: benjaoming | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):
* easy: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/23681#comment:14>