From b3505f38f699d25f4c46d07ea31d6cc5d45df0ba Mon Sep 17 00:00:00 2001 From: Marijke Luttekes Date: Tue, 17 Dec 2024 00:22:56 +0100 Subject: [PATCH] Fixed #35870 -- Replaced blank select choice with accessible text. Thanks James Scholes for providing feedback on accessible options, Thibaud Colas for running screen reader tests, Carlton Gibson for helping get started. --- django/conf/global_settings.py | 3 ++ django/contrib/admin/options.py | 4 +- django/db/models/fields/__init__.py | 5 +- django/db/models/fields/reverse_related.py | 6 ++- django/forms/fields.py | 2 +- django/forms/models.py | 5 +- docs/ref/forms/fields.txt | 2 +- docs/ref/models/fields.txt | 2 +- docs/ref/settings.txt | 10 ++++ tests/admin_views/test_actions.py | 2 +- .../test_related_object_lookups.py | 4 +- tests/admin_views/tests.py | 17 ++++--- tests/admin_widgets/tests.py | 2 +- .../field_tests/test_typedchoicefield.py | 7 ++- .../widget_tests/test_radioselect.py | 19 +++++--- tests/model_fields/test_booleanfield.py | 6 ++- tests/model_fields/tests.py | 6 ++- tests/model_forms/test_modelchoicefield.py | 18 +++---- tests/model_forms/tests.py | 48 +++++++++++-------- tests/model_formsets/tests.py | 2 +- tests/modeladmin/tests.py | 14 +++--- 21 files changed, 120 insertions(+), 64 deletions(-) diff --git a/django/conf/global_settings.py b/django/conf/global_settings.py index f4535acb09..9632be0447 100644 --- a/django/conf/global_settings.py +++ b/django/conf/global_settings.py @@ -223,6 +223,9 @@ FORM_RENDERER = "django.forms.renderers.DjangoTemplates" # Set to True to assume "https" during the Django 5.x release cycle. FORMS_URLFIELD_ASSUME_HTTPS = False +# ToDo: Documentation string +FORMS_DEFAULT_BLANK_CHOICE_LABEL = gettext_noop("- Select an option -") + # Default email address to use for various automated correspondence from # the site managers. DEFAULT_FROM_EMAIL = "webmaster@localhost" diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 69b0cc0373..ad14504cfc 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -1087,11 +1087,13 @@ class ModelAdmin(BaseModelAdmin): actions = self._filter_actions_by_permissions(request, self._get_base_actions()) return {name: (func, name, desc) for func, name, desc in actions} - def get_action_choices(self, request, default_choices=models.BLANK_CHOICE_DASH): + def get_action_choices(self, request, default_choices=None): """ Return a list of choices for use in a form object. Each choice is a tuple (name, description). """ + if default_choices is None: + default_choices = [("", settings.FORMS_DEFAULT_BLANK_CHOICE_LABEL)] choices = [] + default_choices for func, name, description in self.get_actions(request).values(): choice = (name, description % model_format_dict(self.opts)) diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py index c4a730f47b..43e441fcc3 100644 --- a/django/db/models/fields/__init__.py +++ b/django/db/models/fields/__init__.py @@ -82,6 +82,7 @@ class NOT_PROVIDED: # The values to use for "blank" in SelectFields. Will be appended to the start # of most "choices" lists. +# ToDo: write deprecation notice per Trac issue #35870. BLANK_CHOICE_DASH = [("", "---------")] @@ -1056,7 +1057,7 @@ class Field(RegisterLookupMixin): def get_choices( self, include_blank=True, - blank_choice=BLANK_CHOICE_DASH, + blank_choice=None, limit_choices_to=None, ordering=(), ): @@ -1064,6 +1065,8 @@ class Field(RegisterLookupMixin): Return choices with a default blank choices included, for use as `` widget used by ``ModelChoiceField`` will have an empty choice at the top of the list. You can change the text of this - label (which is ``"---------"`` by default) with the ``empty_label`` + label (which is ``"- Select an option -"`` by default) with the ``empty_label`` attribute, or you can disable the empty label entirely by setting ``empty_label`` to ``None``:: diff --git a/docs/ref/models/fields.txt b/docs/ref/models/fields.txt index 5b0f127c6f..0f70a962c3 100644 --- a/docs/ref/models/fields.txt +++ b/docs/ref/models/fields.txt @@ -234,7 +234,7 @@ documentation. .. _field-choices-blank-label: Unless :attr:`blank=False` is set on the field along with a -:attr:`~Field.default` then a label containing ``"---------"`` will be rendered +:attr:`~Field.default` then a label containing ``"- Select an option -"`` will be rendered with the select box. To override this behavior, add a tuple to ``choices`` containing ``None``; e.g. ``(None, 'Your String For Display')``. Alternatively, you can use an empty string instead of ``None`` where this makes diff --git a/docs/ref/settings.txt b/docs/ref/settings.txt index e3a0f6d32a..47551119f7 100644 --- a/docs/ref/settings.txt +++ b/docs/ref/settings.txt @@ -1697,6 +1697,16 @@ Set this transitional setting to ``True`` to opt into using ``"https"`` as the new default value of :attr:`URLField.assume_scheme ` during the Django 5.x release cycle. +.. setting:: FORMS_DEFAULT_BLANK_CHOICE_LABEL + +``FORMS_DEFAULT_BLANK_CHOICE_LABEL`` +------------------------------------ + +Default: ``'- Select an option -'`` (Translated) + +The default label for the blank choice option used by `` - + diff --git a/tests/admin_views/test_related_object_lookups.py b/tests/admin_views/test_related_object_lookups.py index 968558491c..99580de663 100644 --- a/tests/admin_views/test_related_object_lookups.py +++ b/tests/admin_views/test_related_object_lookups.py @@ -110,7 +110,7 @@ class SeleniumTests(AdminSeleniumTestCase): self.assertHTMLEqual( fk_dropdown.get_attribute("innerHTML"), f""" - + """, ) @@ -155,7 +155,7 @@ class SeleniumTests(AdminSeleniumTestCase): self.assertHTMLEqual( fk_dropdown.get_attribute("innerHTML"), f""" - + """, ) diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index 8e2c55804b..6f58ab9f31 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -7,6 +7,7 @@ from unittest import mock from urllib.parse import parse_qsl, urljoin, urlsplit from django import forms +from django.conf import settings from django.contrib import admin from django.contrib.admin import AdminSite, ModelAdmin from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME @@ -6450,7 +6451,9 @@ class SeleniumTests(AdminSeleniumTestCase): self.selenium.switch_to.window(self.selenium.window_handles[0]) select = Select(self.selenium.find_element(By.ID, "id_parent")) self.assertEqual(ParentWithUUIDPK.objects.count(), 0) - self.assertEqual(select.first_selected_option.text, "---------") + self.assertEqual( + select.first_selected_option.text, settings.FORMS_DEFAULT_BLANK_CHOICE_LABEL + ) self.assertEqual(select.first_selected_option.get_attribute("value"), "") def test_inline_with_popup_cancel_delete(self): @@ -6696,7 +6699,7 @@ class SeleniumTests(AdminSeleniumTestCase): self.assertHTMLEqual( _get_HTML_inside_element_by_id(born_country_select_id), """ - + """, ) @@ -6715,7 +6718,7 @@ class SeleniumTests(AdminSeleniumTestCase): # limit_choices_to. self.assertHTMLEqual( _get_HTML_inside_element_by_id(favorite_country_to_vacation_select_id), - '', + '', ) # Add new Country from the living_country select. @@ -6734,7 +6737,7 @@ class SeleniumTests(AdminSeleniumTestCase): self.assertHTMLEqual( _get_HTML_inside_element_by_id(born_country_select_id), """ - + """, @@ -6756,7 +6759,7 @@ class SeleniumTests(AdminSeleniumTestCase): # limit_choices_to. self.assertHTMLEqual( _get_HTML_inside_element_by_id(favorite_country_to_vacation_select_id), - '', + '', ) # Edit second Country created from living_country select. @@ -6776,7 +6779,7 @@ class SeleniumTests(AdminSeleniumTestCase): self.assertHTMLEqual( _get_HTML_inside_element_by_id(born_country_select_id), """ - + """, @@ -6796,7 +6799,7 @@ class SeleniumTests(AdminSeleniumTestCase): # favorite_country_to_vacation field has no options. self.assertHTMLEqual( _get_HTML_inside_element_by_id(favorite_country_to_vacation_select_id), - '', + '', ) # Add a new Asian country. diff --git a/tests/admin_widgets/tests.py b/tests/admin_widgets/tests.py index 15e11a6d8f..1d162338f7 100644 --- a/tests/admin_widgets/tests.py +++ b/tests/admin_widgets/tests.py @@ -1811,7 +1811,7 @@ class RelatedFieldWidgetSeleniumTests(AdminWidgetSeleniumTestCase): # Chrome and Safari don't update related object links when selecting # the same option as previously submitted. As a consequence, the - # "pencil" and "eye" buttons remain disable, so select "---------" + # "pencil" and "eye" buttons remain disable, so select "- Select an option -" # first. select = Select(self.selenium.find_element(By.ID, "id_user")) select.select_by_index(0) diff --git a/tests/forms_tests/field_tests/test_typedchoicefield.py b/tests/forms_tests/field_tests/test_typedchoicefield.py index 52a83eca37..bc132b2e3c 100644 --- a/tests/forms_tests/field_tests/test_typedchoicefield.py +++ b/tests/forms_tests/field_tests/test_typedchoicefield.py @@ -1,5 +1,6 @@ import decimal +from django.conf import settings from django.core.exceptions import ValidationError from django.forms import TypedChoiceField from django.test import SimpleTestCase @@ -59,7 +60,11 @@ class TypedChoiceFieldTest(SimpleTestCase): self.assertFalse(f.has_changed("1", "1")) f = TypedChoiceField( - choices=[("", "---------"), ("a", "a"), ("b", "b")], + choices=[ + ("", settings.FORMS_DEFAULT_BLANK_CHOICE_LABEL), + ("a", "a"), + ("b", "b"), + ], coerce=str, required=False, initial=None, diff --git a/tests/forms_tests/widget_tests/test_radioselect.py b/tests/forms_tests/widget_tests/test_radioselect.py index be336151ef..c0bab14b9c 100644 --- a/tests/forms_tests/widget_tests/test_radioselect.py +++ b/tests/forms_tests/widget_tests/test_radioselect.py @@ -1,12 +1,13 @@ import datetime +from django.conf import settings from django.forms import ChoiceField, Form, MultiWidget, RadioSelect, TextInput from django.test import override_settings from django.utils.safestring import mark_safe from .test_choicewidget import ChoiceWidgetTest -BLANK_CHOICE_DASH = (("", "------"),) +BLANK_CHOICE = (("", settings.FORMS_DEFAULT_BLANK_CHOICE_LABEL),) class RadioSelectTest(ChoiceWidgetTest): @@ -16,7 +17,9 @@ class RadioSelectTest(ChoiceWidgetTest): html = """
- +
@@ -32,7 +35,7 @@ class RadioSelectTest(ChoiceWidgetTest):
""" - beatles_with_blank = BLANK_CHOICE_DASH + self.beatles + beatles_with_blank = BLANK_CHOICE + self.beatles for choices in (beatles_with_blank, dict(beatles_with_blank)): with self.subTest(choices): self.check_html(self.widget(choices=choices), "beatle", "J", html=html) @@ -83,11 +86,13 @@ class RadioSelectTest(ChoiceWidgetTest): """ If value is None, none of the options are selected. """ - choices = BLANK_CHOICE_DASH + self.beatles + choices = BLANK_CHOICE + self.beatles html = """
- +
@@ -463,11 +468,11 @@ class RadioSelectTest(ChoiceWidgetTest): def test_render_as_subwidget(self): """A RadioSelect as a subwidget of MultiWidget.""" - choices = BLANK_CHOICE_DASH + self.beatles + choices = BLANK_CHOICE + self.beatles html = """
+ - Select an option -
diff --git a/tests/model_fields/test_booleanfield.py b/tests/model_fields/test_booleanfield.py index 30eb009eb7..5d9e8604e9 100644 --- a/tests/model_fields/test_booleanfield.py +++ b/tests/model_fields/test_booleanfield.py @@ -1,4 +1,5 @@ from django import forms +from django.conf import settings from django.core.exceptions import ValidationError from django.db import IntegrityError, models, transaction from django.test import SimpleTestCase, TestCase @@ -48,7 +49,10 @@ class BooleanFieldTests(TestCase): """ choices = [(1, "Si"), (2, "No")] f = models.BooleanField(choices=choices) - self.assertEqual(f.formfield().choices, [("", "---------")] + choices) + self.assertEqual( + f.formfield().choices, + [("", settings.FORMS_DEFAULT_BLANK_CHOICE_LABEL)] + choices, + ) def test_nullbooleanfield_formfield(self): f = models.BooleanField(null=True) diff --git a/tests/model_fields/tests.py b/tests/model_fields/tests.py index 3d856d36c5..eaab9b80e7 100644 --- a/tests/model_fields/tests.py +++ b/tests/model_fields/tests.py @@ -1,6 +1,7 @@ import pickle from django import forms +from django.conf import settings from django.core.exceptions import ValidationError from django.db import models from django.test import SimpleTestCase, TestCase @@ -354,7 +355,10 @@ class GetChoicesTests(SimpleTestCase): def test_lazy_strings_not_evaluated(self): lazy_func = lazy(lambda x: 0 / 0, int) # raises ZeroDivisionError if evaluated. f = models.CharField(choices=[(lazy_func("group"), [("a", "A"), ("b", "B")])]) - self.assertEqual(f.get_choices(include_blank=True)[0], ("", "---------")) + self.assertEqual( + f.get_choices(include_blank=True)[0], + ("", settings.FORMS_DEFAULT_BLANK_CHOICE_LABEL), + ) class GetChoicesOrderingTests(TestCase): diff --git a/tests/model_forms/test_modelchoicefield.py b/tests/model_forms/test_modelchoicefield.py index 83d801768a..ce684a93df 100644 --- a/tests/model_forms/test_modelchoicefield.py +++ b/tests/model_forms/test_modelchoicefield.py @@ -1,6 +1,7 @@ import datetime from django import forms +from django.conf import settings from django.core.exceptions import ValidationError from django.forms.models import ModelChoiceIterator, ModelChoiceIteratorValue from django.forms.widgets import CheckboxSelectMultiple @@ -24,7 +25,7 @@ class ModelChoiceFieldTests(TestCase): self.assertEqual( list(f.choices), [ - ("", "---------"), + ("", settings.FORMS_DEFAULT_BLANK_CHOICE_LABEL), (self.c1.pk, "Entertainment"), (self.c2.pk, "A test"), (self.c3.pk, "Third"), @@ -102,7 +103,7 @@ class ModelChoiceFieldTests(TestCase): self.assertEqual( list(f.choices), [ - ("", "---------"), + ("", settings.FORMS_DEFAULT_BLANK_CHOICE_LABEL), (self.c1.pk, "Entertainment"), (self.c2.pk, "A test"), ], @@ -118,7 +119,7 @@ class ModelChoiceFieldTests(TestCase): self.assertEqual( list(gen_two), [ - ("", "---------"), + ("", settings.FORMS_DEFAULT_BLANK_CHOICE_LABEL), (self.c1.pk, "Entertainment"), (self.c2.pk, "A test"), ], @@ -130,7 +131,7 @@ class ModelChoiceFieldTests(TestCase): self.assertEqual( list(f.choices), [ - ("", "---------"), + ("", settings.FORMS_DEFAULT_BLANK_CHOICE_LABEL), (self.c1.pk, "category Entertainment"), (self.c2.pk, "category A test"), (self.c3.pk, "category Third"), @@ -143,7 +144,7 @@ class ModelChoiceFieldTests(TestCase): self.assertEqual( list(f.choices), [ - ("", "---------"), + ("", settings.FORMS_DEFAULT_BLANK_CHOICE_LABEL), (self.c1.pk, "Entertainment"), (self.c2.pk, "A test"), (self.c3.pk, "Third"), @@ -154,7 +155,7 @@ class ModelChoiceFieldTests(TestCase): self.assertEqual( list(f.choices), [ - ("", "---------"), + ("", settings.FORMS_DEFAULT_BLANK_CHOICE_LABEL), (self.c1.pk, "Entertainment"), (self.c2.pk, "A test"), (self.c3.pk, "Third"), @@ -174,6 +175,7 @@ class ModelChoiceFieldTests(TestCase): self.assertIs(bool(f.choices), True) def test_choices_radio_blank(self): + blank_choice = [("", settings.FORMS_DEFAULT_BLANK_CHOICE_LABEL)] choices = [ (self.c1.pk, "Entertainment"), (self.c2.pk, "A test"), @@ -190,7 +192,7 @@ class ModelChoiceFieldTests(TestCase): ) self.assertEqual( list(f.choices), - [("", "---------")] + choices if blank else choices, + (blank_choice + choices if blank else choices), ) def test_deepcopies_widget(self): @@ -425,7 +427,7 @@ class ModelChoiceFieldTests(TestCase): self.assertCountEqual( list(f.choices), [ - ("", "---------"), + ("", settings.FORMS_DEFAULT_BLANK_CHOICE_LABEL), (self.c1.pk, "Entertainment"), (self.c2.pk, "A test"), (self.c3.pk, "Third"), diff --git a/tests/model_forms/tests.py b/tests/model_forms/tests.py index 4f7fc2c1c0..972e9eeffe 100644 --- a/tests/model_forms/tests.py +++ b/tests/model_forms/tests.py @@ -5,6 +5,7 @@ from decimal import Decimal from unittest import mock, skipUnless from django import forms +from django.conf import settings from django.core.exceptions import ( NON_FIELD_ERRORS, FieldError, @@ -341,7 +342,7 @@ class ModelFormBaseTest(TestCase): self.assertEqual( list(form.fields["author"].choices), [ - ("", "---------"), + ("", settings.FORMS_DEFAULT_BLANK_CHOICE_LABEL), (writer.pk, "Joe Doe"), ], ) @@ -1534,7 +1535,7 @@ class ModelFormBasicTests(TestCase):
  • Slug:
  • Pub date:
  • Writer:
    • @@ -1546,7 +1547,7 @@ class ModelFormBasicTests(TestCase):
  • Status:
  • Writer:
    • @@ -1600,7 +1601,7 @@ class ModelFormBasicTests(TestCase):
  • Status: - + @@ -1750,7 +1751,7 @@ class ModelFormBasicTests(TestCase):
    • Status: @@ -1783,7 +1784,7 @@ class ModelFormBasicTests(TestCase):
  • Pub date:
  • Writer:
    • @@ -1795,7 +1796,7 @@ class ModelFormBasicTests(TestCase):
  • Status:
    • ' '
  • Pub date:
    • ' '
  • Writer:
    • " @@ -1969,7 +1970,7 @@ class ModelFormBasicTests(TestCase): '' " " '
  • Status:
    • ' '
  • Pub date:
    • ' '
  • Writer:
    • " '
  • Status: - +

    @@ -2437,7 +2439,7 @@ class ModelOneToOneFieldTests(TestCase): """

    @@ -2726,7 +2728,8 @@ class FileAndImageFieldTests(TestCase): form = FPForm() self.assertEqual( - [name for _, name in form["path"].field.choices], ["---------", "models.py"] + [name for _, name in form["path"].field.choices], + [settings.FORMS_DEFAULT_BLANK_CHOICE_LABEL, "models.py"], ) @skipUnless(test_images, "Pillow not installed") @@ -3055,7 +3058,7 @@ class OtherModelFormTests(TestCase): self.assertEqual( tuple(field.choices), ( - ("", "---------"), + ("", settings.FORMS_DEFAULT_BLANK_CHOICE_LABEL), (multicolor_item.pk, "blue, red"), (red_item.pk, "red"), ), @@ -3069,14 +3072,19 @@ class OtherModelFormTests(TestCase): field = forms.ModelChoiceField(Inventory.objects.all(), to_field_name="barcode") self.assertEqual( tuple(field.choices), - (("", "---------"), (86, "Apple"), (87, "Core"), (22, "Pear")), + ( + ("", settings.FORMS_DEFAULT_BLANK_CHOICE_LABEL), + (86, "Apple"), + (87, "Core"), + (22, "Pear"), + ), ) form = InventoryForm(instance=core) self.assertHTMLEqual( str(form["parent"]), """' - '' + '' '' '' "

    " diff --git a/tests/modeladmin/tests.py b/tests/modeladmin/tests.py index 062368d94e..0b9846ed6a 100644 --- a/tests/modeladmin/tests.py +++ b/tests/modeladmin/tests.py @@ -1,6 +1,7 @@ from datetime import date from django import forms +from django.conf import settings from django.contrib.admin.models import ADDITION, CHANGE, DELETION, LogEntry from django.contrib.admin.options import ( HORIZONTAL, @@ -664,7 +665,7 @@ class ModelAdminTests(TestCase): '
    ' '
    " % (band2.id, self.band.id), @@ -688,7 +689,7 @@ class ModelAdminTests(TestCase): '
    ' '
    " % self.band.id, ) @@ -736,29 +737,30 @@ class ModelAdminTests(TestCase): # ForeignKey widgets in the admin are wrapped with RelatedFieldWidgetWrapper so # they need to be handled properly when type checking. For Select fields, all of # the choices lists have a first entry of dashes. + blank_option = ("", settings.FORMS_DEFAULT_BLANK_CHOICE_LABEL) cma = ModelAdmin(Concert, self.site) cmafa = cma.get_form(request) self.assertEqual(type(cmafa.base_fields["main_band"].widget.widget), Select) self.assertEqual( list(cmafa.base_fields["main_band"].widget.choices), - [("", "---------"), (self.band.id, "The Doors")], + [blank_option, (self.band.id, "The Doors")], ) self.assertEqual(type(cmafa.base_fields["opening_band"].widget.widget), Select) self.assertEqual( list(cmafa.base_fields["opening_band"].widget.choices), - [("", "---------"), (self.band.id, "The Doors")], + [blank_option, (self.band.id, "The Doors")], ) self.assertEqual(type(cmafa.base_fields["day"].widget), Select) self.assertEqual( list(cmafa.base_fields["day"].widget.choices), - [("", "---------"), (1, "Fri"), (2, "Sat")], + [blank_option, (1, "Fri"), (2, "Sat")], ) self.assertEqual(type(cmafa.base_fields["transport"].widget), Select) self.assertEqual( list(cmafa.base_fields["transport"].widget.choices), - [("", "---------"), (1, "Plane"), (2, "Train"), (3, "Bus")], + [blank_option, (1, "Plane"), (2, "Train"), (3, "Bus")], ) def test_foreign_key_as_radio_field(self):