Opened 9 months ago

Closed 6 months ago

Last modified 6 months ago

#35189 closed Cleanup/optimization (fixed)

Accessibility issues with collapsed fieldsets in admin forms

Reported by: Thibaud Colas Owned by: Marijke Luttekes
Component: contrib.admin Version: dev
Severity: Normal Keywords: accessibility, whcm, forced colors, screen reader, forms, admin
Cc: Tom Carrick, Sarah Abderemane Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: yes

Description (last modified by Thibaud Colas)

There are a number of issues with the "collapsible fieldset" in admin forms. This can be most easily tested on flatpages, with the "Advanced options" fields. Here’s a Change flat page demo page. And a screen recording of the widget:

https://code.djangoproject.com/raw-attachment/ticket/35189/collapsed-fieldset-django.gif

I think there are enough problems that it makes sense to fix it all in one go, though the issues are technically separate.

Accessibility bugs

  1. The toggle should be a <button>, not a link, so screen reader and WHCM users understand what the element is for more easily.
  2. The toggle needs an aria-expanded attribute set to true or false based on its current state, so screen reader users know what the state of the element is. See the APG disclosure pattern for further information.
  3. The <fieldset> element should have a <legend>, so the fields are understood to be grouped for screen reader users.
  4. The toggle must have a minimum size of 24x24, so it’s easier for users with low-precision pointing devices or low mobility to hit it.
  5. The toggle shouldn’t change size based on the length of text within (in English show/hide is very close, in other languages not necessarily).
  6. The toggle’s appearance must be distinguished from other heading text via other means than color, so colorblind users can spot it.
  7. In the active state, it’s impossible to tell the toggle apart from the other header text.
  8. The collapsible heading is a <h2>, even though the whole form itself is titled with an <h2>. It should be an <h3> instead so screen reader users have a more accurate representation of the page’s hierarchy.
  9. In forced colors mode, it’s much harder to spot the location of the collapsible section header/heading, as background colors aren’t shown.

Styling issues

While we’re at it there are also styling issues which I think could be fixed at the same time:

  1. The collapsible section heading’s left edge isn’t aligned with the left edge of other form fields’ labels (8px left padding vs. 10px elsewhere)
  2. The collapsible section heading should have a font size at least as large as the field labels within, so the visual hierarchy is clearer.
  3. In the active state, the collapsible section header’s styles change too drastically. It doesn’t need to call for attention so much, when the user’s focus should be on what was revealed. Ideally the toggle’s styles would be different, as that’s the key UI element that’s in a different state.
  4. It seems odd the collapsible section header looses its rounded corners when in the active state.
  5. In forced colors mode, it’s distracting that the position of the heading / toggle shifts due to the collapsible header area border disappearing.

Solution

I think this is a good candidate for a full rebuild, which could keep some of the visual cues of the existing component but there are enough accessibility issues to address that it could also be more different. The new version should:

  1. Either use a <details> and <summary> element for the collapsing, or implement the APG disclosure pattern to spec.
  2. Preserve any bespoke logic of the current component to show/hide based on form state (don’t collapse if there are form errors within?)
  3. Use a <h3> for the heading and <legend> for the heading (probably summary > h3 > legend or button > h3 > legend but TBC)
  4. Use a fixed-size toggle – possibly with a visual device such as a caret to convey the state without use of color. Ideally the whole section header area would be clickable.
  5. Look and work great for all of the admin’s supported assistive technologies.

There could be some value in using <details> so this all works without JS, though we’d need to make sure to still support any JS-dependent behavior of the collapsible section.

Attachments (1)

collapsed-fieldset-django.gif (19.7 KB ) - added by Thibaud Colas 9 months ago.
Screen recording of the "Advanced options" collapsible fieldset in flatpages change forms

Download all attachments as: .zip

Change History (17)

by Thibaud Colas, 9 months ago

Screen recording of the "Advanced options" collapsible fieldset in flatpages change forms

comment:1 by Thibaud Colas, 9 months ago

Description: modified (diff)

comment:2 by Marijke Luttekes, 9 months ago

I'd like to pick this one up!

Version 0, edited 9 months ago by Marijke Luttekes (next)

comment:3 by Natalia Bidart, 9 months ago

Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization
Version: 5.0dev

Hello Thibaud, thanks for this report and its detailed content.

Accepting following the provided rationale. A few notes:

  • Regarding the (loosing of the) rounded corners, I think we need to consider consistency with the "regular" fieldsets headings which are non rounded. Up to verification, I think we should remove the rounding altogether for this fieldset title. It seems the rounding comes from reusing the style of the action row, and then when active, the style comes from the usual fieldset legend.
  • It's likely that we can get rid of the Javascript performing the toggle by using newest CSS selectors.

comment:4 by Marijke Luttekes, 9 months ago

Owner: changed from nobody to Marijke Luttekes
Status: newassigned

comment:5 by Marijke Luttekes, 9 months ago

Regarding the (loosing of the) rounded corners, I think we need to consider consistency with the "regular" fieldsets headings which are non rounded. Up to verification, I think we should remove the rounding altogether for this fieldset title. It seems the rounding comes from reusing the style of the action row, and then when active, the style comes from the usual fieldset legend.

Agreed. When toggling the fieldset, the element also seems to move a pixel, which is visually unpleasing
I will switch to straight edges for these items.

It's likely that we can get rid of the Javascript performing the toggle by using newest CSS selectors.

My original idea for a fix does use a little JavaScript (no jQuery).
I will check with the accessibility team if they know of a better method, e.g., using a checkbox in HTML and the :checked CSS pseudo-class selector.

comment:6 by Marijke Luttekes, 9 months ago

I have a draft PR out (without tests and release notes): https://github.com/django/django/pull/17910

I'm curious what you all think of this solution. It's based on one of my proofs of concept, which is mentioned in the first message of the PR.

comment:7 by Marijke Luttekes, 9 months ago

Has patch: set

comment:8 by Sarah Boyce, 7 months ago

Patch needs improvement: set

I can see the accessibility team also has this in their review queue: https://github.com/orgs/django/projects/7, so hopefully you will receive another review soon.

comment:9 by Sarah Boyce, 7 months ago

Patch needs improvement: unset

comment:10 by Sarah Abderemane, 7 months ago

Cc: Tom Carrick Sarah Abderemane added

comment:11 by nessita <124304+nessita@…>, 6 months ago

In 6c48eed2:

Refs #35189 -- Moved CSS details' cursor declaration to base.css.

comment:12 by Natalia Bidart, 6 months ago

Triage Stage: AcceptedReady for checkin

I'm setting this ticket as ready for checkin but I will review potential new feedback tomorrow, and after that I'll merge before the feature freeze happening in two days.

comment:13 by nessita <124304+nessita@…>, 6 months ago

In 01ed59f7:

Refs #35189 -- Improved admin fieldset's accessibility by setting aria-labelledby.

Before this change, HTML <fieldset> elements in the admin site did not
have an associated label to describe them. This commit defines a unique
HTML id for the heading labeling a fieldset, and sets its
aria-labelledby property to link the heading with the fieldset.

comment:14 by nessita <124304+nessita@…>, 6 months ago

Resolution: fixed
Status: assignedclosed

In e4a693f5:

Fixed #35189 -- Improved admin collapsible fieldsets by using <details> elements.

This work improves the accessibility of the add and change pages in the
admin site by adding <details> and <summary> elements to the collapsible
fieldsets. This has the nice side effect of no longer requiring custom
JavaScript helpers to implement the fieldsets' show/hide capabilities.

Thanks to James Scholes for the accessibility advice, and to Sarah Boyce
and Tom Carrick for reviews.

Co-authored-by: Natalia <124304+nessita@…>
Co-authored-by: Sarah Boyce <42296566+sarahboyce@…>

comment:15 by nessita <124304+nessita@…>, 6 months ago

In 676060d6:

Refs #35189 -- Updated ModelAdmin.fieldsets screenshot in admin docs.

comment:16 by nessita <124304+nessita@…>, 6 months ago

In 7857507:

Refs #35189 -- Removed no longer necessary JavaScript file collapse.js.

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