Opened 18 months ago
Closed 18 months ago
#34705 closed Bug (fixed)
BoundField.as_widget() ignores aria-describedby in attrs argument
Reported by: | Sage Abdullah | Owned by: | Sage Abdullah |
---|---|---|---|
Component: | Forms | Version: | dev |
Severity: | Release blocker | Keywords: | |
Cc: | Gregor Jerše, David Smith, Thibaud Colas | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
BoundField.as_widget()
ignores aria-describedby
that is passed in the attrs
argument.
Use case:
In Wagtail, we have our own mechanism for rendering form fields (called "Panels") that is built on top of Django's Forms API. We use BoundField.as_widget()
in our rendering process to render only the form field's widget, while its help text element is rendered separately via our Panels API with a custom markup (including HTML id
) that conforms to our design system. We've been passing additional attributes to BoundField.as_widget()
to improve accessibility, including aria-describedby
, to establish the relationship between the field widget rendered by Django and our help text element.
As of 966ecdd482167f3f6b08b00f484936c837751cb9, Django automatically adds aria-describedby
to form fields with a help_text
. The logic in build_widget_attrs()
(used by as_widget()
) checks for an existing aria-describedby
in the field's widget's attrs
before automatically generating one. However, it does not take into account the possibility of an existing aria-describedby
in the attrs
argument.
A workaround on Wagtail's side could be to modify the widget's attrs before using as_widget()
, but this is not ideal as the widget is customisable by the developer and we would have to copy the widget instance to avoid modifying the existing widget instance (which might be shared across form fields).
I believe Django should check for aria-describedby
in attrs
first, before falling back to widget.attrs
and the auto-generated value.
Test case:
class BoundFieldTests(SimpleTestCase): def test_as_widget_with_custom_aria_describedby(self): class TestForm(Form): data = CharField(help_text="Some help text") form = TestForm({"data": "some value"}) self.assertHTMLEqual( form["data"].as_widget(attrs={"aria-describedby": "custom_help_text_id"}), """ <input type="text" name="data" value="some value" aria-describedby="custom_help_text_id" required id="id_data"> """, )
Patch:
diff --git a/django/forms/boundfield.py b/django/forms/boundfield.py index deba739329..e4261d5e50 100644 --- a/django/forms/boundfield.py +++ b/django/forms/boundfield.py @@ -290,10 +290,11 @@ class BoundField(RenderableFieldMixin): # If a custom aria-describedby attribute is given and help_text is # used, the custom aria-described by is preserved so user can set the # desired order. - if custom_aria_described_by_id := widget.attrs.get("aria-describedby"): - attrs["aria-describedby"] = custom_aria_described_by_id - elif self.field.help_text and self.id_for_label: - attrs["aria-describedby"] = f"{self.id_for_label}_helptext" + if not attrs.get("aria-describedby"): + if custom_aria_described_by_id := widget.attrs.get("aria-describedby"): + attrs["aria-describedby"] = custom_aria_described_by_id + elif self.field.help_text and self.id_for_label: + attrs["aria-describedby"] = f"{self.id_for_label}_helptext" return attrs @property
Happy to submit a PR if this is accepted.
Attachments (1)
Change History (7)
by , 18 months ago
Attachment: | boundfield-as_widget-aria_describedby.patch added |
---|
comment:1 by , 18 months ago
Description: | modified (diff) |
---|
Updated the patch in the description for better readability.
comment:2 by , 18 months ago
Description: | modified (diff) |
---|
comment:3 by , 18 months ago
Cc: | added |
---|---|
Has patch: | unset |
Severity: | Normal → Release blocker |
Triage Stage: | Unreviewed → Accepted |
Thanks for the report, good catch! Bug in 966ecdd482167f3f6b08b00f484936c837751cb9. I'd prefer a solution from the ticket description, it's more readable.
comment:4 by , 18 months ago
Has patch: | set |
---|
PR.
Slightly adjusted the logic to make the logic closer to the old behaviour.
comment:5 by , 18 months ago
Triage Stage: | Accepted → Ready for checkin |
---|
Patch to fix the issue