Fixed #4592: Made CheckboxSelectMultiple more like RadioSelect

I refactored RadioSelect and CheckboxSelectMultiple to
make them inherit from a base class, allowing them to share
the behavior of being able to iterate over their subwidgets.

Thanks to Matt McClanahan for the initial patch and to
Claude Paroz for the review.

django/forms/widgets.py

@@ -11,6 +11,7 @@ try:
     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
@@ -585,14 +586,16 @@ class SelectMultiple(Select):
 
 
 @python_2_unicode_compatible
-class RadioInput(SubWidget):
+class ChoiceInput(SubWidget):
     """
-    An object used by RadioFieldRenderer that represents a single
+    An object used by ChoiceFieldRenderer that represents a single
-    <input type='radio'>.
+    <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])
@@ -609,8 +612,7 @@ class RadioInput(SubWidget):
             label_for = format_html(' for="{0}_{1}"', self.attrs['id'], self.index)
         else:
             label_for = ''
-        choice_label = force_text(self.choice_label)
+        return format_html('<label{0}>{1} {2}</label>', label_for, self.tag(), self.choice_label)
-        return format_html('<label{0}>{1} {2}</label>', label_for, self.tag(), choice_label)
 
     def is_checked(self):
         return self.value == self.choice_value
@@ -618,34 +620,69 @@ class RadioInput(SubWidget):
     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,15 +691,25 @@ class RadioFieldRenderer(object):
         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  # subclasses 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):
@@ -670,56 +717,35 @@ class RadioSelect(Select):
 
     def get_renderer(self, name, value, attrs=None, choices=()):
         """Returns an instance of the renderer."""
-        if value is None: value = ''
+        if value is None:
-        str_value = force_text(value) # Normalize to string.
+            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,
+        # Widgets using this RendererMixin are made of a collection of
-        # each of which has a distinct ID. The IDs are made distinct by a "_X"
+        # subwidgets, each with their own <label>, and distinct ID.
-        # suffix, where X is the zero-based index of the radio field. Thus,
+        # The IDs are made distinct by y "_X" suffix, where X is the zero-based
-        # the label for a RadioSelect should reference the first one ('_0').
+        # 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)
+class RadioSelect(RendererMixin, Select):
-            option_value = force_text(option_value)
+    renderer = RadioFieldRenderer
-            rendered_cb = cb.render(name, option_value)
+    _empty_value = ''
-            option_label = force_text(option_label)
+
-            output.append(format_html('<li><label{0}>{1} {2}</label></li>',
+
-                                      label_for, rendered_cb, option_label))
+class CheckboxSelectMultiple(RendererMixin, SelectMultiple):
-        output.append('</ul>')
+    renderer = CheckboxFieldRenderer
-        return mark_safe('\n'.join(output))
+    _empty_value = []
 
-    def id_for_label(self, id_):
-        # See the comment for RadioSelect.id_for_label()
-        if id_:
-            id_ += '_0'
-        return id_
 
 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,9 @@ the widget.
 The outer ``<ul>`` container will now receive the ``id`` attribute defined on
 the widget.
 
+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 import six
 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 @@ beatle J R Ringo False""")
 <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 @@ beatle J R Ringo False""")
         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 @@ beatle J R Ringo False""")
 <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):