Fix #4592: Make CheckboxSelectMultiple more like RadioSelect.

django/forms/widgets.py

@@ -11,6 +11,7 @@
     from urllib.parse import urljoin
 except ImportError:     # Python 2
     from urlparse import urljoin
+import warnings
 
 from django.conf import settings
 from django.forms.util import flatatt, to_current_timezone
@@ -584,15 +585,18 @@ def value_from_datadict(self, data, files, name):
         return data.get(name, None)
 
 
+
 @python_2_unicode_compatible
-class RadioInput(SubWidget):
+class ChoiceInput(SubWidget):
     """
-    An object used by RadioFieldRenderer that represents a single
-    <input type='radio'>.
+    An object used by ChoiceFieldRenderer that represents a single
+    <input type='$input_type'>.
     """
+    input_type = None  # Subclasses must define this
 
     def __init__(self, name, value, attrs, choice, index):
-        self.name, self.value = name, value
+        self.name = name
+        self.value = value
         self.attrs = attrs
         self.choice_value = force_text(choice[0])
         self.choice_label = force_text(choice[1])
@@ -618,34 +622,69 @@ def is_checked(self):
     def tag(self):
         if 'id' in self.attrs:
             self.attrs['id'] = '%s_%s' % (self.attrs['id'], self.index)
-        final_attrs = dict(self.attrs, type='radio', name=self.name, value=self.choice_value)
+        final_attrs = dict(self.attrs, type=self.input_type, name=self.name, value=self.choice_value)
         if self.is_checked():
             final_attrs['checked'] = 'checked'
         return format_html('<input{0} />', flatatt(final_attrs))
 
+
+class RadioChoiceInput(ChoiceInput):
+    input_type = 'radio'
+
+    def __init__(self, *args, **kwargs):
+        super(RadioChoiceInput, self).__init__(*args, **kwargs)
+        self.value = force_text(self.value)
+
+
+class RadioInput(RadioChoiceInput):
+    def __init__(self, *args, **kwargs):
+        msg = "Radioinput has been deprecated. Use RadioChoiceInput instead."
+        warnings.warn(msg, PendingDeprecationWarning, stacklevel=2)
+        super(RadioInput, self).__init__(*args, **kwargs)
+
+
+class CheckboxChoiceInput(ChoiceInput):
+    input_type = 'checkbox'
+
+    def __init__(self, *args, **kwargs):
+        super(CheckboxChoiceInput, self).__init__(*args, **kwargs)
+        self.value = set(force_text(v) for v in self.value)
+
+    def is_checked(self):
+        return self.choice_value in self.value
+
+
 @python_2_unicode_compatible
-class RadioFieldRenderer(object):
+class ChoiceFieldRenderer(object):
     """
     An object used by RadioSelect to enable customization of radio widgets.
     """
 
+    choice_input_class = None
+
     def __init__(self, name, value, attrs, choices):
-        self.name, self.value, self.attrs = name, value, attrs
+        self.name = name
+        self.value = value
+        self.attrs = attrs
         self.choices = choices
 
     def __iter__(self):
         for i, choice in enumerate(self.choices):
-            yield RadioInput(self.name, self.value, self.attrs.copy(), choice, i)
+            yield self.choice_input_class(self.name, self.value, self.attrs.copy(), choice, i)
 
     def __getitem__(self, idx):
         choice = self.choices[idx] # Let the IndexError propogate
-        return RadioInput(self.name, self.value, self.attrs.copy(), choice, idx)
+        return self.choice_input_class(self.name, self.value, self.attrs.copy(), choice, idx)
 
     def __str__(self):
         return self.render()
 
     def render(self):
-        """Outputs a <ul> for this set of radio fields."""
+        """
+        Outputs a <ul> for this set of choice fields.
+        If an id was given to the field, it is applied to the <ul> (each
+        item in the list will get an id of `$id_$i`).
+        """
         id_ = self.attrs.get('id', None)
         start_tag = format_html('<ul id="{0}">', id_) if id_ else '<ul>'
         output = [start_tag]
@@ -654,72 +693,61 @@ def render(self):
         output.append('</ul>')
         return mark_safe('\n'.join(output))
 
-class RadioSelect(Select):
-    renderer = RadioFieldRenderer
+
+class RadioFieldRenderer(ChoiceFieldRenderer):
+    choice_input_class = RadioChoiceInput
+
+
+class CheckboxFieldRenderer(ChoiceFieldRenderer):
+    choice_input_class = CheckboxChoiceInput
+
+
+class RendererMixin(object):
+    renderer = None  # subclassed must define this
+    _empty_value = None
 
     def __init__(self, *args, **kwargs):
         # Override the default renderer if we were passed one.
         renderer = kwargs.pop('renderer', None)
         if renderer:
             self.renderer = renderer
-        super(RadioSelect, self).__init__(*args, **kwargs)
+        super(RendererMixin, self).__init__(*args, **kwargs)
 
     def subwidgets(self, name, value, attrs=None, choices=()):
         for widget in self.get_renderer(name, value, attrs, choices):
             yield widget
 
     def get_renderer(self, name, value, attrs=None, choices=()):
         """Returns an instance of the renderer."""
-        if value is None: value = ''
-        str_value = force_text(value) # Normalize to string.
+        if value is None:
+            value = self._empty_value
         final_attrs = self.build_attrs(attrs)
         choices = list(chain(self.choices, choices))
-        return self.renderer(name, str_value, final_attrs, choices)
+        return self.renderer(name, value, final_attrs, choices)
 
     def render(self, name, value, attrs=None, choices=()):
         return self.get_renderer(name, value, attrs, choices).render()
 
     def id_for_label(self, id_):
-        # RadioSelect is represented by multiple <input type="radio"> fields,
-        # each of which has a distinct ID. The IDs are made distinct by a "_X"
-        # suffix, where X is the zero-based index of the radio field. Thus,
-        # the label for a RadioSelect should reference the first one ('_0').
+        # Widgets using this RendererMixin are made of a collection of
+        # subwidgets, each with their own <label>, and distinct ID.
+        # The IDs are made distinct by y "_X" suffix, where X is the zero-based
+        # index of the choice field. Thus, the label for the main widget should
+        # reference the first subwidget, hence the "_0" suffix.
         if id_:
             id_ += '_0'
         return id_
 
-class CheckboxSelectMultiple(SelectMultiple):
-    def render(self, name, value, attrs=None, choices=()):
-        if value is None: value = []
-        final_attrs = self.build_attrs(attrs, name=name)
-        id_ = final_attrs.get('id', None)
-        start_tag = format_html('<ul id="{0}">', id_) if id_ else '<ul>'
-        output = [start_tag]
-        # Normalize to strings
-        str_values = set([force_text(v) for v in value])
-        for i, (option_value, option_label) in enumerate(chain(self.choices, choices)):
-            # If an ID attribute was given, add a numeric index as a suffix,
-            # so that the checkboxes don't all have the same ID attribute.
-            if id_:
-                final_attrs = dict(final_attrs, id='%s_%s' % (id_, i))
-                label_for = format_html(' for="{0}_{1}"', id_, i)
-            else:
-                label_for = ''
-
-            cb = CheckboxInput(final_attrs, check_test=lambda value: value in str_values)
-            option_value = force_text(option_value)
-            rendered_cb = cb.render(name, option_value)
-            option_label = force_text(option_label)
-            output.append(format_html('<li><label{0}>{1} {2}</label></li>',
-                                      label_for, rendered_cb, option_label))
-        output.append('</ul>')
-        return mark_safe('\n'.join(output))
 
-    def id_for_label(self, id_):
-        # See the comment for RadioSelect.id_for_label()
-        if id_:
-            id_ += '_0'
-        return id_
+class RadioSelect(RendererMixin, Select):
+    renderer = RadioFieldRenderer
+    _empty_value = ''
+
+
+class CheckboxSelectMultiple(RendererMixin, SelectMultiple):
+    renderer = CheckboxFieldRenderer
+    _empty_value = []
+
 
 class MultiWidget(Widget):
     """

docs/internals/deprecation.txt

@@ -372,6 +372,9 @@ these changes.
   - ``django.db.transaction.is_managed()``
   - ``django.db.transaction.managed()``
 
+* ``django.forms.widgets.RadioInput`` will be removed in favor of
+  ``django.forms.widgets.RadioChoiceInput``.
+
 2.0
 ---
 

docs/ref/forms/widgets.txt

@@ -658,6 +658,11 @@ the widget.
 The outer ``<ul>`` container will now receive the ``id`` attribute defined on
 the widget.
 
+.. versionadded:: 1.6
+
+Like :class:`RadioSelect`, you can now loop over the individual checkboxes making
+up the lists. See the documentation of :class:`RadioSelect` for more details.
+
 .. _file-upload-widgets:
 
 File upload widgets

tests/forms_tests/tests/test_widgets.py

@@ -15,7 +15,7 @@
 from django.utils.translation import activate, deactivate
 from django.test import TestCase
 from django.test.utils import override_settings
-from django.utils.encoding import python_2_unicode_compatible
+from django.utils.encoding import python_2_unicode_compatible, force_text
 
 from ..models import Article
 
@@ -656,7 +656,7 @@ class CustomRadioSelect(RadioSelect):
 <label><input checked="checked" type="radio" name="beatle" value="G" /> George</label><br />
 <label><input type="radio" name="beatle" value="R" /> Ringo</label>""")
 
-        # A RadioFieldRenderer object also allows index access to individual RadioInput
+        # A RadioFieldRenderer object also allows index access to individual RadioChoiceInput
         w = RadioSelect()
         r = w.get_renderer('beatle', 'J', choices=(('J', 'John'), ('P', 'Paul'), ('G', 'George'), ('R', 'Ringo')))
         self.assertHTMLEqual(str(r[1]), '<label><input type="radio" name="beatle" value="P" /> Paul</label>')
@@ -665,11 +665,8 @@ class CustomRadioSelect(RadioSelect):
         self.assertFalse(r[1].is_checked())
         self.assertEqual((r[1].name, r[1].value, r[1].choice_value, r[1].choice_label), ('beatle', 'J', 'P', 'Paul'))
 
-        try:
+        with self.assertRaises(IndexError):
             r[10]
-            self.fail("This offset should not exist.")
-        except IndexError:
-            pass
 
         # Choices are escaped correctly
         w = RadioSelect()
@@ -817,6 +814,25 @@ def get_choices():
 <li><label for="abc_2"><input checked="checked" type="checkbox" name="letters" value="c" id="abc_2" /> C</label></li>
 </ul>""")
 
+        w = CheckboxSelectMultiple()
+        r = w.get_renderer('abc', 'b', choices=[(c, c.upper()) for c in 'abc'])
+        # You can iterate over the CheckboxFieldRenderer to get individual elements
+        expected = [
+            '<label><input type="checkbox" name="abc" value="a" /> A</label>',
+            '<label><input checked="checked" type="checkbox" name="abc" value="b" /> B</label>',
+            '<label><input type="checkbox" name="abc" value="c" /> C</label>',
+        ]
+        for output, expected in zip(r, expected):
+            self.assertHTMLEqual(force_text(output), expected)
+
+        # You can access individual elements
+        self.assertHTMLEqual(force_text(r[1]), '<label><input checked="checked" type="checkbox" name="abc" value="b" /> B</label>')
+
+        # Out-of-range errors are propagated
+        with self.assertRaises(IndexError):
+            r[42]
+
+
     def test_multi(self):
         class MyMultiWidget(MultiWidget):
             def decompress(self, value):