Merge pull request stephenmcd#206 from zbohm/fix-none-and-placeholder-in-dropdown

Fix None and placeholder in Dropdown input.

Author: stephenmcd

forms_builder/forms/forms.py

@@ -159,9 +159,12 @@ def __init__(self, form, context, *args, **kwargs):
                 field_args["max_length"] = settings.FIELD_MAX_LENGTH
             if "choices" in arg_names:
                 choices = list(field.get_choices())
-                if (field.field_type == fields.SELECT and
-                        field.default not in [c[0] for c in choices]):
-                    choices.insert(0, ("", field.placeholder_text))
+                if field.field_type == fields.SELECT and not (field.required and field.default):
+                    # The first OPTION with attr. value="" display only if...
+                    #   1. ...the field is not required.
+                    #   2. ...the field is required and the default is not set.
+                    text = "" if field.placeholder_text is None else field.placeholder_text
+                    choices.insert(0, ("", text))
                 field_args["choices"] = choices
             if field_widget is not None:
                 field_args["widget"] = field_widget
@@ -201,11 +204,15 @@ def __init__(self, form, context, *args, **kwargs):
             css_class = field_class.__name__.lower()
             if field.required:
                 css_class += " required"
-                if (settings.USE_HTML5 and
-                    field.field_type != fields.CHECKBOX_MULTIPLE):
-                    self.fields[field_key].widget.attrs["required"] = ""
+                if settings.USE_HTML5:
+                    # Except Django version 1.10 this is necessary for all versions from 1.8 to 1.11.
+                    self.fields[field_key].widget.attrs["required"] = "required"
+
             self.fields[field_key].widget.attrs["class"] = css_class
-            if field.placeholder_text and not field.default:
+            if field.placeholder_text and not field.default and field.field_type != fields.SELECT:
+                # Attribute `placeholder` not allowed on element `select` at this point.
+                # See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/select
+                # or check the code in https://validator.w3.org.
                 text = field.placeholder_text
                 self.fields[field_key].widget.attrs["placeholder"] = text
 

forms_builder/forms/tests.py

@@ -4,11 +4,11 @@
 from django.contrib.auth.models import User, AnonymousUser
 from django.contrib.sites.models import Site
 from django.db import IntegrityError
-from django.http import HttpResponseRedirect
+from django.http import HttpResponse, HttpResponseRedirect
 from django.template import Context, RequestContext, Template
 from django.test import TestCase
 
-from forms_builder.forms.fields import NAMES, FILE
+from forms_builder.forms.fields import NAMES, FILE, SELECT
 from forms_builder.forms.forms import FormForForm
 from forms_builder.forms.models import (Form, Field,
                                         STATUS_DRAFT, STATUS_PUBLISHED)
@@ -158,3 +158,76 @@ def test_form_redirect(self):
         self.assertEqual(response["location"], redirect_url)
         response = self.client.post(form_absolute_url, {'field': 'bar'})
         self.assertFalse(isinstance(response, HttpResponseRedirect))
+
+    def test_input_dropdown_not_required(self):
+        form = Form.objects.create(title="Test")
+        form.fields.create(label="Foo", field_type=SELECT, required=False, choices="one, two, three")
+        form_for_form = FormForForm(form, Context())
+        self.assertContains(HttpResponse(form_for_form), """
+            <select name="foo" class="choicefield" id="id_foo">
+                <option value="" selected></option>
+                <option value="one">one</option>
+                <option value="two">two</option>
+                <option value="three">three</option>
+            </select>""", html=True)
+
+    def test_input_dropdown_not_required_with_placeholder(self):
+        form = Form.objects.create(title="Test")
+        form.fields.create(label="Foo", placeholder_text="choose item", field_type=SELECT,
+                           required=False, choices="one, two, three")
+        form_for_form = FormForForm(form, Context())
+        self.assertContains(HttpResponse(form_for_form), """
+            <select name="foo" class="choicefield" id="id_foo">
+                <option value="" selected>choose item</option>
+                <option value="one">one</option>
+                <option value="two">two</option>
+                <option value="three">three</option>
+            </select>""", html=True)
+
+    def test_input_dropdown_required(self):
+        form = Form.objects.create(title="Test")
+        form.fields.create(label="Foo", field_type=SELECT, choices="one, two, three")
+        form_for_form = FormForForm(form, Context())
+        self.assertContains(HttpResponse(form_for_form), """
+            <select name="foo" required class="choicefield required" id="id_foo">
+                <option value="" selected></option>
+                <option value="one">one</option>
+                <option value="two">two</option>
+                <option value="three">three</option>
+            </select>""", html=True)
+
+    def test_input_dropdown_required_with_placeholder(self):
+        form = Form.objects.create(title="Test")
+        form.fields.create(label="Foo", placeholder_text="choose item", field_type=SELECT,
+                           choices="one, two, three")
+        form_for_form = FormForForm(form, Context())
+        self.assertContains(HttpResponse(form_for_form), """
+            <select name="foo" required class="choicefield required" id="id_foo">
+                <option value="" selected>choose item</option>
+                <option value="one">one</option>
+                <option value="two">two</option>
+                <option value="three">three</option>
+            </select>""", html=True)
+
+    def test_input_dropdown_required_with_placeholder_and_default(self):
+        form = Form.objects.create(title="Test")
+        form.fields.create(label="Foo", placeholder_text="choose item", field_type=SELECT,
+                           choices="one, two, three", default="two")
+        form_for_form = FormForForm(form, Context())
+        self.assertContains(HttpResponse(form_for_form), """
+            <select name="foo" required class="choicefield required" id="id_foo">
+                <option value="one">one</option>
+                <option value="two" selected>two</option>
+                <option value="three">three</option>
+            </select>""", html=True)
+
+    def test_input_dropdown_required_with_default(self):
+        form = Form.objects.create(title="Test")
+        form.fields.create(label="Foo", field_type=SELECT, choices="one, two, three", default="two")
+        form_for_form = FormForForm(form, Context())
+        self.assertContains(HttpResponse(form_for_form), """
+            <select name="foo" required class="choicefield required" id="id_foo">
+                <option value="one">one</option>
+                <option value="two" selected>two</option>
+                <option value="three">three</option>
+            </select>""", html=True)