From ff7494bf79b75c8637f0e1f10b6888db1baef146 Mon Sep 17 00:00:00 2001 From: AceHunterr Date: Tue, 21 Feb 2023 01:43:56 +0530 Subject: [PATCH] Replace ButtonSelect widgets with radio buttons - Instead of a complex and non-accessible JS solution for filter selects, replace with simple radio select fields - Fixes #9838 --- CHANGELOG.txt | 1 + CONTRIBUTORS.rst | 1 + client/scss/components/_button-select.scss | 25 ------ client/scss/core.scss | 1 - client/src/entrypoints/admin/core.js | 3 - client/src/includes/initButtonSelects.test.js | 77 ------------------- client/src/includes/initButtonSelects.ts | 35 --------- docs/releases/5.0.md | 1 + wagtail/admin/filters.py | 12 +-- .../wagtailadmin/widgets/button_select.html | 12 --- .../widgets/button_select_option.html | 1 - wagtail/admin/views/reports/workflows.py | 6 +- wagtail/admin/widgets/__init__.py | 2 +- ...tton_select.py => boolean_radio_select.py} | 14 +--- wagtail/contrib/redirects/filters.py | 4 +- wagtail/snippets/tests/test_snippets.py | 14 ++-- 16 files changed, 23 insertions(+), 186 deletions(-) delete mode 100644 client/scss/components/_button-select.scss delete mode 100644 client/src/includes/initButtonSelects.test.js delete mode 100644 client/src/includes/initButtonSelects.ts delete mode 100644 wagtail/admin/templates/wagtailadmin/widgets/button_select.html delete mode 100644 wagtail/admin/templates/wagtailadmin/widgets/button_select_option.html rename wagtail/admin/widgets/{button_select.py => boolean_radio_select.py} (66%) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index e71b8723a2..7170f052b9 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -39,6 +39,7 @@ Changelog * Fix: Prevent matches from unrelated models from leaking into SQLite FTS searches (Matt Westcott) * Fix: Prevent duplicate addition of StreamField blocks with the new block picker (Deepam Priyadarshi) * Fix: Enable partial search on images and documents index view where available (Mng) + * Fix: Adopt a no-JavaScript and more accessible solution for option selection in reporting, using HTML only `radio` input fields (Mehul Aggarwal) * Docs: Add code block to make it easier to understand contribution docs (Suyash Singh) * Docs: Add new "Icons" page for icons customisation and reuse across the admin interface (Coen van der Kamp) * Docs: Fix broken formatting for MultiFieldPanel / FieldRowPanel permission kwarg docs (Matt Westcott) diff --git a/CONTRIBUTORS.rst b/CONTRIBUTORS.rst index e8101ade17..00c9273725 100644 --- a/CONTRIBUTORS.rst +++ b/CONTRIBUTORS.rst @@ -701,6 +701,7 @@ Contributors * Deepam Priyadarshi * Mng * George Sakkis +* Mehul Aggarwal Translators =========== diff --git a/client/scss/components/_button-select.scss b/client/scss/components/_button-select.scss deleted file mode 100644 index 4e0e74fae5..0000000000 --- a/client/scss/components/_button-select.scss +++ /dev/null @@ -1,25 +0,0 @@ -.button-select { - &__option { - display: block; - width: 100%; - margin-bottom: 10px; - - background-color: $color-white; - border-color: $color-teal; - color: $color-grey-1; - - &--selected { - background-color: $color-teal; - color: $color-white; - } - - @media (forced-colors: active) { - background-color: SelectedItem; - } - } -} - -.button-select .button-select__option { - /* override default margin from horizontally-aligned buttons */ - margin-inline-start: 0; -} diff --git a/client/scss/core.scss b/client/scss/core.scss index 0e1de91ab6..3a52667ff4 100644 --- a/client/scss/core.scss +++ b/client/scss/core.scss @@ -155,7 +155,6 @@ These are classes for components. @import 'components/link.legacy'; @import 'components/indicator'; @import 'components/status-tag'; -@import 'components/button-select'; @import 'components/skiplink'; @import 'components/workflow-tasks'; @import 'components/workflow-timeline'; diff --git a/client/src/entrypoints/admin/core.js b/client/src/entrypoints/admin/core.js index 2236cbb164..192fb065aa 100644 --- a/client/src/entrypoints/admin/core.js +++ b/client/src/entrypoints/admin/core.js @@ -2,7 +2,6 @@ import $ from 'jquery'; import { coreControllerDefinitions } from '../../controllers'; import { escapeHtml } from '../../utils/text'; -import { initButtonSelects } from '../../includes/initButtonSelects'; import { initStimulus } from '../../includes/initStimulus'; import { initTagField } from '../../includes/initTagField'; import { initTooltips } from '../../includes/initTooltips'; @@ -487,6 +486,4 @@ $(document).ready(initDropDowns); wagtail.ui.initDropDowns = initDropDowns; wagtail.ui.DropDownController = DropDownController; -$(document).ready(initButtonSelects); - window.wagtail = wagtail; diff --git a/client/src/includes/initButtonSelects.test.js b/client/src/includes/initButtonSelects.test.js deleted file mode 100644 index 1fff2be6d8..0000000000 --- a/client/src/includes/initButtonSelects.test.js +++ /dev/null @@ -1,77 +0,0 @@ -import { initButtonSelects } from './initButtonSelects'; - -// save our DOM elements to a variable -const testElements = ` -
- - - -
-`; - -describe('initButtonSelects', () => { - const spy = jest.spyOn(document, 'addEventListener'); - - afterEach(() => { - jest.clearAllMocks(); - }); - - it('should do nothing if there is no button-select container', () => { - // Set up our document body - document.body.innerHTML = ` -
- -
`; - initButtonSelects(); - // no event listeners registered - expect(spy).not.toHaveBeenCalled(); - }); - - describe('there is a button-select container present', () => { - it('should add class of button-select__option--selected to button-select__option when clicked', () => { - document.body.innerHTML = testElements; - initButtonSelects(); - document.querySelectorAll('.button-select__option').forEach((button) => { - button.click(); - expect(button.classList.value).toContain( - 'button-select__option--selected', - ); - }); - }); - - it('should remove the class button-select__option--selected when button is not clicked', () => { - document.body.innerHTML = testElements; - initButtonSelects(); - document.querySelectorAll('.button-select__option').forEach((button) => { - button.click(); - document - .querySelector('.button-select') - .querySelectorAll('.button-select__option--selected') - .forEach((selectedButtonElement) => { - selectedButtonElement.classList.remove( - 'button-select__option--selected', - ); - }); - expect(button.classList.value).not.toContain( - 'button-select__option--selected', - ); - }); - }); - it('add the value of the button clicked to the input value', () => { - document.body.innerHTML = testElements; - initButtonSelects(); - const inputElement = document.querySelector('input[type="hidden"]'); - // Checking that the input ellement has no value - expect(inputElement.value).toBeFalsy(); - document.querySelectorAll('.button-select__option').forEach((button) => { - button.click(); - expect(inputElement.value).toEqual(button.value); - }); - }); - }); -}); diff --git a/client/src/includes/initButtonSelects.ts b/client/src/includes/initButtonSelects.ts deleted file mode 100644 index daedebcc27..0000000000 --- a/client/src/includes/initButtonSelects.ts +++ /dev/null @@ -1,35 +0,0 @@ -/** - * Initialise button selectors - */ -const initButtonSelects = () => { - document.querySelectorAll('.button-select').forEach((element) => { - const inputElement = element.querySelector( - 'input[type="hidden"]', - ) as HTMLInputElement; - - if (!inputElement) { - return; - } - - element - .querySelectorAll('.button-select__option') - .forEach((buttonElement) => { - buttonElement.addEventListener('click', (event) => { - event.preventDefault(); - inputElement.value = (buttonElement as HTMLButtonElement).value; - - element - .querySelectorAll('.button-select__option--selected') - .forEach((selectedButtonElement) => { - selectedButtonElement.classList.remove( - 'button-select__option--selected', - ); - }); - - buttonElement.classList.add('button-select__option--selected'); - }); - }); - }); -}; - -export { initButtonSelects }; diff --git a/docs/releases/5.0.md b/docs/releases/5.0.md index 58b480f8ed..997dbcfc46 100644 --- a/docs/releases/5.0.md +++ b/docs/releases/5.0.md @@ -53,6 +53,7 @@ Support for adding custom validation logic to StreamField blocks has been formal * Prevent matches from unrelated models from leaking into SQLite FTS searches (Matt Westcott) * Prevent duplicate addition of StreamField blocks with the new block picker (Deepam Priyadarshi) * Enable partial search on images and documents index view where available (Mng) + * Adopt a no-JavaScript and more accessible solution for option selection in reporting, using HTML only `radio` input fields (Mehul Aggarwal) ### Documentation diff --git a/wagtail/admin/filters.py b/wagtail/admin/filters.py index 259aabbc6d..178ef1c0b9 100644 --- a/wagtail/admin/filters.py +++ b/wagtail/admin/filters.py @@ -1,14 +1,10 @@ import django_filters +from django import forms from django.db import models from django.utils.translation import gettext_lazy as _ from django_filters.widgets import SuffixedMultiWidget -from wagtail.admin.widgets import ( - AdminDateInput, - BooleanButtonSelect, - ButtonSelect, - FilteredSelect, -) +from wagtail.admin.widgets import AdminDateInput, BooleanRadioSelect, FilteredSelect from wagtail.coreutils import get_content_type_label @@ -96,7 +92,7 @@ class WagtailFilterSet(django_filters.FilterSet): filter_class, params = super().filter_for_lookup(field, lookup_type) if filter_class == django_filters.ChoiceFilter: - params.setdefault("widget", ButtonSelect) + params.setdefault("widget", forms.RadioSelect) params.setdefault("empty_label", _("All")) elif filter_class in [django_filters.DateFilter, django_filters.DateTimeFilter]: @@ -106,7 +102,7 @@ class WagtailFilterSet(django_filters.FilterSet): params.setdefault("widget", DateRangePickerWidget) elif filter_class == django_filters.BooleanFilter: - params.setdefault("widget", BooleanButtonSelect) + params.setdefault("widget", BooleanRadioSelect) return filter_class, params diff --git a/wagtail/admin/templates/wagtailadmin/widgets/button_select.html b/wagtail/admin/templates/wagtailadmin/widgets/button_select.html deleted file mode 100644 index e1b14eb58a..0000000000 --- a/wagtail/admin/templates/wagtailadmin/widgets/button_select.html +++ /dev/null @@ -1,12 +0,0 @@ -
- - - {% for group_name, group_choices, group_index in widget.optgroups %} - {% if group_name %} -

{{ group_name }}

- {% endif %} - {% for option in group_choices %} - {% include option.template_name with widget=option %} - {% endfor %} - {% endfor %} -
diff --git a/wagtail/admin/templates/wagtailadmin/widgets/button_select_option.html b/wagtail/admin/templates/wagtailadmin/widgets/button_select_option.html deleted file mode 100644 index e75ab54642..0000000000 --- a/wagtail/admin/templates/wagtailadmin/widgets/button_select_option.html +++ /dev/null @@ -1 +0,0 @@ - diff --git a/wagtail/admin/views/reports/workflows.py b/wagtail/admin/views/reports/workflows.py index ed05749e8a..83ba6761a5 100644 --- a/wagtail/admin/views/reports/workflows.py +++ b/wagtail/admin/views/reports/workflows.py @@ -1,6 +1,7 @@ import datetime import django_filters +from django import forms from django.contrib.auth import get_user_model from django.contrib.contenttypes.models import ContentType from django.db.models import CharField, Q @@ -13,7 +14,6 @@ from wagtail.admin.filters import ( WagtailFilterSet, ) from wagtail.admin.utils import get_latest_str -from wagtail.admin.widgets import ButtonSelect from wagtail.coreutils import get_content_type_label from wagtail.models import ( Task, @@ -61,7 +61,7 @@ class WorkflowReportFilterSet(WagtailFilterSet): method="filter_reviewable", choices=(("true", _("Awaiting my review")),), empty_label=_("All"), - widget=ButtonSelect, + widget=forms.RadioSelect, ) requested_by = django_filters.ModelChoiceFilter( field_name="requested_by", queryset=get_requested_by_queryset @@ -107,7 +107,7 @@ class WorkflowTasksReportFilterSet(WagtailFilterSet): method="filter_reviewable", choices=(("true", _("Awaiting my review")),), empty_label=_("All"), - widget=ButtonSelect, + widget=forms.RadioSelect, ) def filter_reviewable(self, queryset, name, value): diff --git a/wagtail/admin/widgets/__init__.py b/wagtail/admin/widgets/__init__.py index 81c18ad291..047b8ea925 100644 --- a/wagtail/admin/widgets/__init__.py +++ b/wagtail/admin/widgets/__init__.py @@ -1,6 +1,6 @@ from wagtail.admin.widgets.auto_height_text import * # NOQA +from wagtail.admin.widgets.boolean_radio_select import * # NOQA from wagtail.admin.widgets.button import * # NOQA -from wagtail.admin.widgets.button_select import * # NOQA from wagtail.admin.widgets.chooser import * # NOQA from wagtail.admin.widgets.datetime import * # NOQA from wagtail.admin.widgets.filtered_select import * # NOQA diff --git a/wagtail/admin/widgets/button_select.py b/wagtail/admin/widgets/boolean_radio_select.py similarity index 66% rename from wagtail/admin/widgets/button_select.py rename to wagtail/admin/widgets/boolean_radio_select.py index e5a2e83586..5c386806e5 100644 --- a/wagtail/admin/widgets/button_select.py +++ b/wagtail/admin/widgets/boolean_radio_select.py @@ -2,20 +2,12 @@ from django import forms from django.utils.translation import gettext_lazy as _ -class ButtonSelect(forms.Select): +class BooleanRadioSelect(forms.RadioSelect): """ - A select widget for fields with choices. Displays as a list of buttons. + A radio select widget for boolean fields. Displays as three options; "All", "Yes" and "No". """ - input_type = "hidden" - template_name = "wagtailadmin/widgets/button_select.html" - option_template_name = "wagtailadmin/widgets/button_select_option.html" - - -class BooleanButtonSelect(ButtonSelect): - """ - A select widget for boolean fields. Displays as three buttons. "All", "Yes" and "No". - """ + input_type = "radio" def __init__(self, attrs=None): choices = ( diff --git a/wagtail/contrib/redirects/filters.py b/wagtail/contrib/redirects/filters.py index a1a7ec9329..97f3d409af 100644 --- a/wagtail/contrib/redirects/filters.py +++ b/wagtail/contrib/redirects/filters.py @@ -1,8 +1,8 @@ import django_filters +from django import forms from django.utils.translation import gettext as _ from wagtail.admin.filters import WagtailFilterSet -from wagtail.admin.widgets import ButtonSelect from wagtail.contrib.redirects.models import Redirect from wagtail.models import Site @@ -16,7 +16,7 @@ class RedirectsReportFilterSet(WagtailFilterSet): (False, _("Temporary")), ), empty_label=_("All"), - widget=ButtonSelect, + widget=forms.RadioSelect, ) site = django_filters.ModelChoiceFilter( diff --git a/wagtail/snippets/tests/test_snippets.py b/wagtail/snippets/tests/test_snippets.py index 0e67e03656..fd8b997bfa 100644 --- a/wagtail/snippets/tests/test_snippets.py +++ b/wagtail/snippets/tests/test_snippets.py @@ -539,7 +539,7 @@ class TestSnippetListViewWithFilterSet(WagtailTestUtils, TestCase): ) self.assertContains( response, - '', + '', html=True, ) self.assertTemplateUsed(response, "wagtailadmin/shared/filters.html") @@ -553,7 +553,7 @@ class TestSnippetListViewWithFilterSet(WagtailTestUtils, TestCase): self.assertNotContains(response, "There are 2 matches") self.assertContains( response, - '', + '', html=True, ) @@ -566,7 +566,7 @@ class TestSnippetListViewWithFilterSet(WagtailTestUtils, TestCase): self.assertNotContains(response, "There are 2 matches") self.assertContains( response, - '', + '', html=True, ) @@ -577,7 +577,7 @@ class TestSnippetListViewWithFilterSet(WagtailTestUtils, TestCase): self.assertContains(response, "Sorry, no filterable snippets match your query") self.assertContains( response, - '', + '', html=True, ) @@ -589,7 +589,7 @@ class TestSnippetListViewWithFilterSet(WagtailTestUtils, TestCase): self.assertContains(response, "There is 1 match") self.assertContains( response, - '', + '', html=True, ) @@ -600,7 +600,7 @@ class TestSnippetListViewWithFilterSet(WagtailTestUtils, TestCase): self.assertContains(response, "Sorry, no filterable snippets match your query") self.assertContains( response, - '', + '', html=True, ) @@ -612,7 +612,7 @@ class TestSnippetListViewWithFilterSet(WagtailTestUtils, TestCase): self.assertContains(response, "There is 1 match") self.assertContains( response, - '', + '', html=True, )