#34705: BoundField.as_widget() ignores aria-describedby in attrs argument
-------------------------------+-----------------------------------------
Reporter: Sage Abdullah | Owner: Sage Abdullah
Type: Bug | Status: assigned
Component: Forms | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-----------------------------------------
Description changed by Sage Abdullah:
Old description:
> `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.
New description:
`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.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/34705#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/0107018944c078a6-9e3c22a1-9e97-4776-8177-e98335cb6476-000000%40eu-central-1.amazonses.com.
