Fixed #35189 -- Render admin collapsible fieldsets with <details>.

[MHLut]

TL;DR: Improve the accessibility of admin fieldsets and collapse them with a native HTML implementation.

• Ticket: #35189
• Proof of concept: Example 2: Details inside (no JS)
• Initial discussion on Discord, accessibility channel

Related tickets and PRs

Related items:

• Ticket #35382; this ticket removed unused CSS temporarily unearthed by this PR.
• Ticket #35380; documentation screenshots aren't being automatically generated.
• Added a high contrast mode to screenshot cases. #18080; add more forced-colors screenshots.

Important facts

• Works with the main model fieldset and stacked inline fieldsets.
• This change removes all JavaScript for collapsible fieldsets.
• I have recycled as much CSS as possible.
• JavaScript used to check if a fieldset could be collapsible. With JS gone, I moved that check to the Fieldset helper class in Python.
• The translations for "Show" and "Hide" are now redundant.
• A new translation "Fields" (context "form fields") was introduced. Changed in second commit.

Known issues

• No existing CSS bugs were fixed.
  • Example: The font size of the fieldset is below 16px/1rem.
  • Example: The fieldset heading is hard to distinguish in forced-colors mode.
  • Context: Style for .collapse. h3 in forms.css was copied from .module h2, .module caption, .inline-group h2 in base.css.
• The <h3> heading in stacked inlines should be a <h4>; however, it was originally a <h2>, so it's still better than before. Fixed in the second commit.

A potential UX issue could be that the clickable area of the fieldset has moved: You click on the details toggle ("fields") instead of the blue fieldset header. Changed in the second commit.

Screenshots

A collapsible fieldset; collapsed

A collapsible fieldset; expanded

A fieldset that cannot be collapsed due to a validation error

A collapsible fieldset inside a stacked inline; collapsed

A collapsible fieldset inside a stacked inline; cannot be collapsed due to a validation error

[carltongibson]

buildbot, test on selenium. I don’t think that works anymore 😅

[sarahboyce]

Hi @MHLut I think this looks really promising thank you ⭐
I've found something I'd like you to look into. I had some screenshots generated from this PR. You can see these here when you download the Artifacts.
These screenshots are taken under a few scenarios we try to support include dark mode and right to left languages (side note we should think about adding forced-colors mode to these).

I can see on your PR that in RTL we have this (notice that 'Collapsible' is on the left)

Previously 'Collapsible' was on the right:

I am also not sure if the collapse icon (> triangle thingy) should be on the left, so before Fields in the right to left case (maybe @sakhawy might be able to tell us 🙏 )

You might need to look into django/contrib/admin/static/admin/css/rtl.css 👍

[sarahboyce]

(on further thought, I think the triangle is in the correct place for RTL languages but Moe is welcome to confirm)

[sakhawy]

@sarahboyce I guess it "feels off" because we're used to seeing dropdowns with the triangle after the text and not before it :D
But I think the triangle here is part of the <details> tag's default style, so nothing to worry about!

One reason for checking the django/contrib/admin/static/admin/css/rtl.css file would be this line:

django/django/contrib/admin/static/admin/css/forms.css

Line 225 in 88274f6

text-align: left;

I think it's the reason 'Collapsible' is on the left when dir="rtl".

[MHLut]

@sarahboyce @sakhawy Thanks for the heads-up. I can't remember why I put the text-align: left in forms.py, but it's gone now. The heading aligns to the right side again in RTL mode.

The triangle is—as Moe mentioned—part of <details> native styling; I'd rather not mess with that. Since Moe seems OK with it, I'll keep that as-is.

FYI: I'm currently doing some free-form CSS experiments for forced-colors mode.

[knyghty]

I didn't look much at the code, or look at it on a screen.

From an accessibility standpoint I think we're good with this approach. The outline seems a little odd still (that there are multiple h3s per link, rather than one h3 and then going to h4 or something similar) but I think this is better than what we currently have and shouldn't block this merge - to be honest I'm not even sure if it's a problem at all, I could be totally wrong.

If I find time before someone else I will also review the code.

[sarahboyce]

@django/triage-review InlineModelAdmin.classes references ModelAdmin.fieldsets which has a versionchanged note, do you think that same version changed note is also required here for InlineModelAdmin.classes?

Also suggestions are welcome around tweaking the release notes/version changed note/docs updates. I've done my best but not quite mastered the art of good docs. ❤️

[MHLut]

I'm also unsure about this. If one source references the other, then it's probably OK to list a versionchanged with the latter only as a single source of truth. But I am unaware of Django's practices here.

[ngnpope]

A few minor thoughts. Looks great though!

[nessita]

Hello @MHLut! Thank you so much for your work on this, for all the investigation and sharing your thoughts in the description and in Discord.

I read thru the Discord thread (I may have missed something), and while I acknowledge that you said the following:

So far, I like Example 2 most, Example 1 is OK, and don't like Example 3.
[...] the third example just feels meh to me. The look and feel is clunky, with a duplicate mention of "Address", where the content feels useless.
Code-wise though; the second example is the cleanest, and allows us to reuse the existing structure of the fieldsets.
The first example has the same perks, but needs JS to get it to work.
The first example also requires CSS to add in the triangle icons which have no specific meaning.

I honestly dislike the "Fields" summary. It feels (to me, I know, very objectively) very unpolished in terms of UI, mainly because the fieldset is supposed to be collapsed and yet Django is "stubbornly" showing just a peak, a little more of it. I think is, to some extent, important to keep the previous "implicit agreement" that a collapsed fieldset would only show the fieldset name and nothing else.

With that in mind, do you think there is a way to have Example 3 working (purely in terms of UI) without duplicating the fieldset name? Basically have Example 3 without the words highlighted in red:

Happy to coordinate a chat or meet if you feel that discussing this IRL(ish) would be easier.

[MHLut]

@nessita Thank you for your feedback. As far as I have seen, there is no good way to do the third option without repeating information. Here is why.

The HTML requires the following items:

• A <details> element.
• A <summary> inside the details.
• A <fieldset> element.
• A <legend> inside the fieldset.
• A heading (<h3> in this case) inside the fieldset or summary.

We can place the heading inside the legend.
However, we cannot place the summary inside anything other than the top level of the details element.
This means that both the heading and the summary need to have some form of descriptive text.

I picked the solution that looks the most streamlined (in my opinion) and has the least replication.

By ensuring that the heading comes first in the structure, we don't have to add the heading contents to any of its child elements. (Since the fieldset name has already been announced)
If we choose the third solution, we must repeat the fieldset name in the summary.

I prefer solution 2 to the old UI, so let's agree to disagree there.
I also think that showing just a hint of the fieldset contents alongside the details triangle icon gives the user a visual hint of an expandable area.

Nb. If desired, "Fields" could be replaced with another text.

[nessita]

The HTML requires the following items:
* A <details> element.
* A <summary> inside the details.
* A <fieldset> element.
* A <legend> inside the fieldset.
* A heading (<h3> in this case) inside the fieldset or summary.

We can place the heading inside the legend. However, we cannot place the summary inside anything other than the top level of the details element. This means that both the heading and the summary need to have some form of descriptive text.

Thanks for these details. To be honest, I'm struggling to understand why all these elements are necessary. Based on the Discord messages I read, it seems there are two primary reasons: firstly, to enable the use of CSS for selectors and to handle the hide/show behavior without JavaScript, and secondly, to ensure proper accessibility and assist screen readers. However, I'm still not entirely clear on why each element is essential. I'm unsure how to experiment with this and comprehend the consequences of removing each item individually.

To try to understand a bit more, I copied the HTML from your codepen for Example 3, and below these I put some questions:

<section>
  <h2>Example 3: Details outside (no JS)</h2>
  <div id="details-example-outside" class="example example--details-outside">
    <details open="">
      <summary>Address fields</summary>
      <fieldset class="module aligned collapse">
        <h3>Address</h3>

        <div class="description">Optional description.</div>
        <div class="form-row field-address_line_1">...</div>
        <div class="form-row field-address_line_2">...</div>
        <div class="form-row field-city">...</div>
        <div class="form-row field-state">...</div>
        <div class="form-row field-zip_code">...</div>
        <div class="form-row field-country">...</div>

      </fieldset>
    </details>
  </div>
</section>

• Could we simply remove the h3? If not, could you please explain why not? It would be easier for me to understand if you could outline what happens if the element is not present.
• If we absolutely cannot remove the h3, could we hide it instead? That way, we could retain the necessary information for screen readers while making it visually invisible.

With the above, and changing "Address fields" -> "Address", I think we would have a nice solution.

By ensuring that the heading comes first in the structure, we don't have to add the heading contents to any of its child elements. (Since the fieldset name has already been announced) If we choose the third solution, we must repeat the fieldset name in the summary.

This I'm not sure what it means, sorry. Could you please help me understand? An example would certainly help me!

I prefer solution 2 to the old UI, so let's agree to disagree there.

Sure.

I also think that showing just a hint of the fieldset contents alongside the details triangle icon gives the user a visual hint of an expandable area.

I agree that the hint about the expandable area is necessary, but I think the triangle in the "fieldset title row" (the blue one with white letters) accomplish that perfectly. There is no need for an extra row IMHO.

Having yet another row (white background in example 2) seems unnecessary (from an UI POV) and doesn't provide any useful information (I know we can change the text, I replied about this further down). I consider this a regression, in that Example 2 makes the interface feel cluttered and can be confusing for users who expect collapsible sections to minimize the amount of visible content, especially when vertical space is limited.

I believe it's unnecessary to append or mention "fields" anywhere, as it seems redundant. This is similar to how the admin doesn't append "Model" everywhere because it's implied.

Nb. If desired, "Fields" could be replaced with another text.

Yes, absolutely, though I can't think of a text to put in there that would justify the extra row. Do you have any ideas?

[knyghty]

@MHLut, @nessita,

I had a (very quick!) look into this.

The <legend> element is not (strictly speaking) necessary. It is useful, but in our case I think the <summary> element achieves the same goal. We do need to make sure we keep it for <fieldset>s that do not use collapse however.

As for the <h3>, I am not sure. We could move it into the <summary> element, this is allowed. I think that we should just do whatever makes the outline more correct here, whether that's having a heading or not.

With removing the <legend> element we should probably test that it makes sense when using a screen reader. I'd guess so, and I'd guess possibly it's even less confusing without the repeated text.

[MHLut]

Notes regarding UI changes to the third proof of concept (link in top description):

I tried to combine the fieldset and details elements, but the Firefox accessibility inspector threw warnings.

The problem is that a fieldset needs a legend to describe its contents to assistive technology, and the legend should be its first child.

The same goes for details: the summary describes its contents and should be the first child.
(If you omit a summary, the collapsible widget's label defaults to "Details" in English.)

When putting the legend inside the summary, the accessibility inspector did not recognize it as a label.

I also tried to attach the fieldset name to the fieldset through aria-label, but the accessibility inspector indicated that this isn't good enough (and once again hinted at using a legend).

I don't see how we can use the third UI solution without sacrificing accessibility.
Unless we could somehow use aria-label or aria-describedby.
(My third proof of concept also lacks a legend, meaning it is an incomplete solution.)

NB. Regarding aria-describedby:
If we'd use that, we need to add an ID to the fieldset's heading element and use the heading as a descriptor.
However, fieldset names are not mandatory, and sometimes there is no heading.
We also lack a property guaranteeing a unique ID output.

[nessita]

@MHLut could we use a "complete" solution for the third option without sacrificing accessibility by hiding the <h3> with a custom CSS class that allows to screen readers to still mention it?

[MHLut]

@nessita The <h3> isn't the problem.
(Edit: we can place the H3 in a legend or summary, so it'll fit in either solution)

We cannot intertwine the <legend> and the <summary> without sacrificing accessibility.
We would need to do that to achieve the look and feel of the third solution without JavaScript or hacks.

I also would prefer to keep the <fieldset> as the outer element so that most of the outline will be identical between collapsible and regular fieldsets.
Solution 2 provides that, but solution 3 doesn't.

If we sacrifice the <legend>, solution 3 would probably be achievable, even retaining <fieldset> as the outer element.
However, adding a legend is one of the original assignments of this ticket (point 3 under "Accessibility bugs").

I have tried several things and don't know what else would be good.
Accessibility-wise, the current solution is probably the best we will get.
Thibaud hasn't looked at this yet, though, and I hope he has ideas for an improvement.

[nessita]

@MHLut Thank you. Could you please, when you have some time, push changes in another branch with what you have for solution 3? Ideally this temporary branch would have a complete solution in terms of accessibility (ie all the required HTML elements for assistive technologies would be present). I'd like to see how it looks and use it a bit, if possible.

[MHLut]

Per @nessita's request, I have updated the PR (in a separate commit for now) with a new UI style.

After a discussion with a blind developer, I think I have found a middle group that works for screen readers without creating a UI regression.

Note: If anyone wants to discuss his suggestions in more detail, we can do that in person or an accessibility meeting (I wouldn't copy/paste a Slack thread here or in Discord).

See these notes I added to the PR:

---

Accessibility notes:

I discussed this solution with a blind developer. He suggested we replace <fieldset> with <div role="group"> if the lack of <legend> bothers the accessibility checkers.

Due to styling issues, I had to change the heading level of inline fieldsets to <h4>. This also fixes a heading bug I mentioned in the original PR.

Other notes:

This solution hinges on adding a unique ID to a fieldset heading so that we can use aria-labelledby on the <fieldset>. Due to the lack of a clean way of generating such an ID in the template, I added another property to the Fieldset class.

[MHLut]

Tagging @jscholes as my aforementioned advisor.

[MHLut]

Apologies for the failed tests; I've added some classes to existing headings, but the tests do not like it. I will fix these later, but you can test this new UI design already!