Fixed #35892 -- Supported Widget.use_fieldset in admin forms and linked help texts aria-describedby for accessibility. #19678
Conversation
sarahboyce
added
selenium
There was a problem hiding this comment.
Thank you for the PR ⭐
The styling of the legend is not right on mobile:
Also the AdminSplitDateTime widget (see django/contrib/admin/templates/admin/widgets/split_datetime.html) should have "Date" and "Time" in label tags associated with the appropriate inputs 👍
| @@ -47,7 +47,7 @@ Requires core.js and SelectBox.js. | |||
| 'p', | |||
| selector_available_title, | |||
| interpolate(gettext('Choose %s by selecting them and then select the "Choose" arrow button.'), [field_name]), | |||
| 'class', 'helptext' | |||
| 'id', `${field_id}_choose_helptext`, 'class', 'helptext' | |||
There was a problem hiding this comment.
This is the help text for the from box rather than the help text for the fieldset, so the from box should have the aria-describedby attribute (not the fieldset). Similar for the to box
There was a problem hiding this comment.
Thank you for pointing out such an important detail.
Currently, when the select element is focused, it’s connected to the title via aria-labelledby, so both the label and a description are provided.
However, it might be better to provide the label via aria-labelledby, and then connect the help text and the instructions for selecting multiple options using aria-describedby.
| warning.classList.add('help', warningClass); | ||
| warning.id = `${field_id}_timezone_warning_helptext`; |
There was a problem hiding this comment.
I think this helptext should also be specific to the input rather than the fieldset
There was a problem hiding this comment.
Yes, I think the same. It seems better to associate it with each input element.
| @@ -282,6 +287,8 @@ def __init__( | |||
| self.can_view_related = not multiple and can_view_related | |||
| # To check if the related object is registered with this AdminSite. | |||
| self.admin_site = admin_site | |||
| self.use_fieldset = True | |||
There was a problem hiding this comment.
We potentially need to add use_fieldset = True to django.forms.widgets.ClearableFileInput which also is used (slightly customized) in the admin
Maybe add in a separate commit prior to this one if you agree with a test for forms
Antoliny0919
force-pushed
the
ticket_35892
branch
2 times, most recently
from
55815b2 to
5b17a6e
Compare
|
New Accessibility Improvement Project 🎉🎉🎉 |
Changed ..
When accessing each input field for 
For fields with filter_horizontal applied, the sub_help_text explaining how to select multiple items is provided in the selection boxes. 
Previously, only the help text for the box type(from, to header) fields was provided. |
…ed help texts aria-describedby for accessibility.
Antoliny0919
force-pushed
the
ticket_35892
branch
2 times, most recently
from
9403679 to
93a290f
Compare
|
After adding the label tag, the color of the Before
After
|
I also think it looks fine. More consistent with other labels 🤷 |
| <label>{{ date_label }}</label> {% with widget=widget.subwidgets.0 %}{% include widget.template_name %}{% endwith %}<br> | ||
| <label>{{ time_label }}</label> {% with widget=widget.subwidgets.1 %}{% include widget.template_name %}{% endwith %} |
There was a problem hiding this comment.
As the inputs are not within the labels, I think you will need a "for" attribute on the label tags to associate them
There was a problem hiding this comment.
I agree, thank you
Antoliny0919
force-pushed
the
ticket_35892
branch
from
93a290f to
878b956
Compare
Antoliny0919
force-pushed
the
ticket_35892
branch
from
878b956 to
8289d96
Compare
Trac ticket number
ticket-35892
Branch description
continues PR..
Thank you to @sai-ganesh-03 for implementing the work, to @sarahboyce for offering guidance on the direction,
and to @knyghty for bringing up the issue 🥰
I updated the admin change form to use
<fieldset>and<legend>tags for fields whenuse_fieldsetis set to True.I also connected additional help texts within widgets using
aria-describedby.Checklist
mainbranch.