Opened 4 weeks ago

Last modified 2 weeks ago

#35706 assigned Cleanup/optimization

Improve admin add/change form top level errors for screen readers

Reported by: Sarah Boyce Owned by: Sanjeev Holla S
Component: contrib.admin Version: dev
Severity: Normal Keywords: accessibility
Cc: Marijke Luttekes, Thibaud Colas, Tom Carrick, Sarah Abderemane Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: yes

Description

I was looking into best practices for "top level" form errors and according to the W3C Design system we can make the following improvements...

Prefix the word “Error:” to the document’s <title>

Confirmed that this is the first thing announced by screen readers when the page loads.
I think this is especially useful considering we have a "Save and add another" button, given you were on the add page and submit using "Save and add another" going to the add page is the "correct" state.

Have a summary of the errors at the top of the page

https://design-system.w3.org/styles/form-errors.html has an example of the top level note. This is similar to our "Please correct the errors below"

{% if errors %}
    <p class="errornote">
    {% blocktranslate count counter=errors|length %}Please correct the error below.{% plural %}Please correct the errors below.{% endblocktranslate %}
    </p>
    {{ adminform.form.non_field_errors }}
{% endif %}

We could (should?) update this to:

  • be above the form title
  • have tabindex="-1" to be the first thing read on the page
  • be in a div with role="alert" with an aria-labelledby attribute set for the title
  • also include the individual form errors (which link to the form elements) in this summary box

I hope the accessibility team can confirm if any of these are not necessary or not wanted

This a bit related to #32819

Change History (16)

comment:1 by Sarah Boyce, 4 weeks ago

Type: UncategorizedCleanup/optimization

comment:2 by Natalia Bidart, 4 weeks ago

Triage Stage: UnreviewedAccepted
UI/UX: set
Version: 5.0dev

Thank you Sarah for all the details! Accepting following the advice from the shared links.

comment:3 by tomc, 4 weeks ago

For me:

Yes:

  • title prefix seems reasonable and useful for everyone
  • Using role and aria-labbeledby also seem obvious improvements
  • Summary box - again seems useful and no obvious harm other than needing to scroll more to see the form - but I think this is more than made up for by the jump to field link, which just seems like a very nice UX improvement

Maybe:

  • Moving the form above the title - not sure. If that's what W3C are doing for themselves, I assume this is backed by something and I'm inclined to say, yes, sure.
  • Usually it's not a good idea to mess with tabindex as it breaks people's expectations. I'm not sure if this is a good exception or not, will ask around. Again, inclined to say that W3C have probably done their homework.

comment:4 by Sanjeev Holla S, 3 weeks ago

Owner: set to Sanjeev Holla S
Status: newassigned

comment:5 by Sanjeev Holla S, 3 weeks ago

Hey

I have started to work on this issue. To give an update, I have completed working on

  1. Prefix the word “Error:” to the document’s <title>
  2. Add the error message above form title (below the breadcrumbs).
  3. I have included the all the error messages in the summary box.

Also one of my suggestion is to improve the default error message (which is used when the field is required). As of now the error message shows "This field is required" for any field. But I am thinking of changing this to "Enter a %(field_title)s", so that it will be more informative in the summary box. I have made this change as of now. Let me know if its not required.

Version 0, edited 3 weeks ago by Sanjeev Holla S (next)

in reply to:  5 ; comment:6 by Sarah Boyce, 3 weeks ago

Replying to Sanjeev Holla S:

Also one of my suggestion is to improve the default error message (which is used when the field is required). As of now the error message shows "This field is required" for any field. But I am thinking of changing this to "Enter a %(field_title)s", so that it will be more informative in the summary box. I have made this change as of now. Let me know if its not required.

I think updating the error message might not be the way to go as some people might be relying on it in tests. We probably want something a bit like django/forms/templates/django/forms/errors/dict/ul.html (specifically {% for field, error in errors %}<li>{{ field }}{{ error }}</li>{% endfor %} as it gets a field and error) but probably requires some tweaks

If you have done the bit prefixing the word “Error:” to the document’s <title>, you can add a test and raise a PR with "Refs #35706" as these two bits can be merged seperatedly

comment:7 by Sanjeev Holla S, 3 weeks ago

Has patch: set
Needs tests: set

in reply to:  6 comment:8 by Sanjeev Holla S, 3 weeks ago

Replying to Sarah Boyce:

Replying to Sanjeev Holla S:

Also one of my suggestion is to improve the default error message (which is used when the field is required). As of now the error message shows "This field is required" for any field. But I am thinking of changing this to "Enter a %(field_title)s", so that it will be more informative in the summary box. I have made this change as of now. Let me know if its not required.

I think updating the error message might not be the way to go as some people might be relying on it in tests. We probably want something a bit like django/forms/templates/django/forms/errors/dict/ul.html (specifically {% for field, error in errors %}<li>{{ field }}{{ error }}</li>{% endfor %} as it gets a field and error) but probably requires some tweaks

If you have done the bit prefixing the word “Error:” to the document’s <title>, you can add a test and raise a PR with "Refs #35706" as these two bits can be merged seperatedly

Hey! Got the point. I have created a PR for the first bit. Please check it and suggest changes if required.

in reply to:  6 comment:9 by Sanjeev Holla S, 3 weeks ago

Replying to Sarah Boyce:

Replying to Sanjeev Holla S:

Also one of my suggestion is to improve the default error message (which is used when the field is required). As of now the error message shows "This field is required" for any field. But I am thinking of changing this to "Enter a %(field_title)s", so that it will be more informative in the summary box. I have made this change as of now. Let me know if its not required.

I think updating the error message might not be the way to go as some people might be relying on it in tests. We probably want something a bit like django/forms/templates/django/forms/errors/dict/ul.html (specifically {% for field, error in errors %}<li>{{ field }}{{ error }}</li>{% endfor %} as it gets a field and error) but probably requires some tweaks

If you have done the bit prefixing the word “Error:” to the document’s <title>, you can add a test and raise a PR with "Refs #35706" as these two bits can be merged seperatedly

I will create a PR for the 2nd bit as well. So I will show the list of errors in the summary box in the same way as suggested (without changing the default error message and using {% for field, error in errors %}<li>{{ field }}{{ error }}</li>{% endfor %}). Even this should be added in all the templates wherever the prefix "Error:" was added in the title I guess.

comment:10 by Sarah Boyce <42296566+sarahboyce@…>, 3 weeks ago

In 387475c:

Refs #35706 -- Prefixed 'Error:' to titles of admin pages with form errors.

This improves the screen reader experience.

comment:11 by Sarah Boyce, 3 weeks ago

Has patch: unset
Needs tests: unset

comment:12 by Sanjeev Holla S, 2 weeks ago

I have created a PR for replacing the '_(...)' with translate tag in template. Please check it.
PR - https://github.com/django/django/pull/18533

comment:13 by Sarah Boyce <42296566+sarahboyce@…>, 2 weeks ago

In 4470d1f1:

Refs #35706 -- Replaced template _('...') usages with translate tag.

comment:14 by Sanjeev Holla S, 2 weeks ago

So for showing the list of errors in the summary box, I thought of using field.field.label (To show which field it is) and field.errors to show the errors of that field. Since field.errors includes HTML tags like <ul>, directly displaying it might not look clean (in summary box). Therefore, I thought of using {{ field.errors|striptags }} to remove the HTML tags and present only the plain text error messages, ensuring the summary box looks neat.
Is this fine?

comment:15 by Sarah Boyce, 2 weeks ago

It might be enough/simpler to have something more generic like "Field X has validation errors." (where 'Field X' is a link to take you to the field), or "The following fields have errors:" (and a list of fields with links).

in reply to:  15 comment:16 by Sanjeev Holla S, 2 weeks ago

Replying to Sarah Boyce:

It might be enough/simpler to have something more generic like "Field X has validation errors." (where 'Field X' is a link to take you to the field), or "The following fields have errors:" (and a list of fields with links).

Okay then its fine. I have few doubts regarding this, I have mentioned them in the PR
PR - https://github.com/django/django/pull/18537

Note: See TracTickets for help on using tickets.
Back to Top