Opened 10 years ago

Closed 6 years ago

#2723 closed enhancement (invalid)

yes/no option for BooleanField

Reported by: Gary Wilson <gary.wilson@…> Owned by: Remco Wendt
Component: contrib.admin Version: master
Severity: normal Keywords: nfa-someday, sprintdec01, sprint-pycon08
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

It would be nice to have the same sort of yes/no dropdown/radio option for BooleanField that NullBooleanField does (without the "unknown" choice of course). I think there are instances where this sort of interface works better than a checkbox. Adrian likes the radio option. google groups thread.

Attachments (3)

booleanwidget_radio.diff (1.8 KB) - added by David Tulig 9 years ago.
Adds a custom boolean field widget to the admin interface.
booleanwidget_radio_with_tests.2.diff (9.4 KB) - added by Remco Wendt 9 years ago.
Improved patch with tests
booleanwidget_radio_with_tests.diff (9.4 KB) - added by Remco Wendt 9 years ago.
Improved patch with tests

Download all attachments as: .zip

Change History (21)

comment:1 Changed 10 years ago by Michael Radziej <mir@…>

Triage Stage: UnreviewedDesign decision needed

comment:2 Changed 9 years ago by anonymous

Component: Admin interfacedjango-admin.py
Needs documentation: set
Patch needs improvement: set
Resolution: invalid
Status: newclosed
Version: SVN

comment:3 Changed 9 years ago by James Bennett

Component: django-admin.pyAdmin interface
Needs documentation: unset
Patch needs improvement: unset
Resolution: invalid
Status: closedreopened

Reverting spam close.

comment:4 Changed 9 years ago by Chris Beaven

Triage Stage: Design decision neededAccepted

Shouldn't be hard to do in newforms-admin. Adrian likes it. Let's promote to accepted.

Changed 9 years ago by David Tulig

Attachment: booleanwidget_radio.diff added

Adds a custom boolean field widget to the admin interface.

comment:5 Changed 9 years ago by David Tulig

Has patch: set
Keywords: newforms-admin sprintdec01 added
Owner: changed from nobody to David Tulig
Status: reopenednew

comment:6 Changed 9 years ago by Remco Wendt

Owner: changed from David Tulig to Remco Wendt
Status: newassigned

comment:7 Changed 9 years ago by Remco Wendt

Keywords: sprint-pycon08 added
Triage Stage: AcceptedReady for checkin

See attached improved patch + unit test. Unit tests check out and have been run against sqlite (since there is no db specific things going on here I deemed that to be enough)

I've changed the naming to AdminBooleanSelectWidget for the widget itself, to match the naming pattern of other widget.

The widget uses it's own RadioFieldRenderer, this is done to enable the <ul> that will be rendered to use the css plainlist layout, otherwise both radioselects would be rendered with square bullets in front of them in the Admin.

Also added are tests to see whether the hooks in BaseModelAdmin work (those that bind specific widgets to certain fields), to booleanfield is of course tested.

Changed 9 years ago by Remco Wendt

Improved patch with tests

Changed 9 years ago by Remco Wendt

Improved patch with tests

comment:8 Changed 9 years ago by Malcolm Tredinnick

Version: SVNnewforms-admin

comment:9 Changed 8 years ago by anonymous

Keywords: nfa-someday added; newforms-admin removed

comment:10 Changed 8 years ago by Jeff Anderson

milestone: 1.0

comment:11 Changed 8 years ago by peterd12

Why hasn't this been checked in? It seems like it's ready. I'd really like this feature without having to patch Django.

comment:12 Changed 8 years ago by anonymous

Version: newforms-adminSVN

comment:13 Changed 8 years ago by Brian Rosner

milestone: 1.0post-1.0

This can be easily done without patching Django and it really is a new feature and not fixing a bug. Marking post-1.0.

comment:14 Changed 8 years ago by Malcolm Tredinnick

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

This shouldn't be supplying a widget only for the admin interface. Will no other application ever use boolean fields? If we're adding a widget, it's generally going to either be worthy of putting into django/forms/widgets.py or not including at all; there are very few cases when a specialised widget just for admin is appropriate.

So, let's move the widget to a more useful place.

Secondly, the tests here are kind of confusing (all the hooked widget stuff). They seem to be trying to enforce something about admin widget overrides that I'm not sure is actually correct (why can't those things ever change?). Whilst tests are good, those tests aren't testing that this widget behaves correctly; they're trying to test some other incidental behaviour. So let's move those to another patch so that when we resolve how to do better custom widget overrides in admin they can be part of that. At the moment they're distracting a bit from the issue at hand and I'm not sure that if they were to fail it would actually mean there's a problem in Django's public behaviour (they're internal implementation details, as far as I can tell).

Thirdly, it looks unnecessary to have strings like u'1' for the choices. Just use integers there. The point is that there's no need to do all this forcing to strings and it makes the reader wonder why you're doing that. If you used 0 and 1 for the choice values, then you'd simplify a lot of the code, too, since bool(value) would give the right integer -> boolean conversion and int(value) would give the right choice value for both boolean and integer input.

Finally, please don't mark your own patches as "ready for checkin". I suspect this went unreviewed for so long because it was in that state and we hadn't realised it hadn't been through a review.

comment:15 Changed 8 years ago by (none)

milestone: post-1.0

Milestone post-1.0 deleted

comment:16 Changed 7 years ago by beer

Where is difference between TypedChoiceField(coerce=bool, choices=((False, 'No'), (True, 'Yes')), widget=forms.RadioSelect) and this?

comment:17 in reply to:  16 Changed 7 years ago by Tobias McNulty

Replying to beer:

Where is difference between TypedChoiceField(coerce=bool, choices=((False, 'No'), (True, 'Yes')), widget=forms.RadioSelect) and this?

That doesn't work because:

In [3]: bool('True')
Out[3]: True

In [4]: bool('False')
Out[4]: True

And django will literally put str(True) ('True') and str(False) ('False') in your HTML.

And if you use int as the coerce method, Django doesn't seem to properly convert the boolean value in the field to an into for display in the form.

comment:18 Changed 6 years ago by Leo Shklovskii

Resolution: invalid
Status: assignedclosed

@beer was close, and @tobias is correct, you can't just use bool. Try this instead:

TypedChoiceField(coerce=lambda x: x =='True', choices=((False, 'No'), (True, 'Yes')), widget=forms.RadioSelect)

Considering that this is a three year old ticket with a totally out of date patch and there's a one line solution for what it's trying to achieve, I think it's prudent to close this ticket.

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