Opened 5 months ago

Closed 3 months ago

Last modified 3 months ago

#35464 closed Cleanup/optimization (fixed)

Fieldsets defined for TabularInlines are ignored

Reported by: Natalia Bidart Owned by: Maryam Yusuf
Component: contrib.admin Version: dev
Severity: Normal Keywords:
Cc: Marijke Luttekes, Thibaud Colas, Tom Carrick, Sarah Abderemane, Ryan Cheley 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

Following the review of the PR solving #35189 and the analysis done in #35456, I found out that when defining an admin model with a TabularInline instance which defines fieldsets, those fieldsets are "ignored" in the UI (in terms of actually using a fieldset element, the content is shown).
Furthermore, if the fieldsets include the collapse CSS class, nothing changes in the UI and there is not way of having collapsible fieldsets inside TabularInlines. See attached screenshots and some example models down below.

Regarding how to fix this, I see two options:

  1. Discuss with the accessibility plan whether there is a way to structure HTML to have "collapsible fieldsets" inside a tabular context, or
  2. Explicitly document that fieldsets make no sense for TabularInline. The current docs imply this setup is valid:

InlineModelAdmin.classes
A list or tuple containing extra CSS classes to apply to the fieldset that is rendered for the inlines. Defaults to None. As with classes configured in fieldsets, inlines with a collapse class will be initially collapsed and their header will have a small “show” link.

Example modes and admin models:

  • models.py
    from django.db import models
    from django.utils.timezone import now
    
    
    class Book(models.Model):
        title = models.CharField(max_length=100)
        author = models.CharField(max_length=100)
        publisher = models.CharField(max_length=100)
        creation_date = models.DateField(default=now)
        update_date = models.DateField(default=now)
        publication_date = models.DateField(default=now)
    
        def __str__(self):
            return self.title
    
    
    class Cover(models.Model):
        book = models.ForeignKey(Book, on_delete=models.CASCADE)
        image = models.CharField(max_length=100)
        title = models.CharField(max_length=100)
        description = models.TextField()
        creation_date = models.DateField(default=now)
        update_date = models.DateField(default=now)
        updated_by = models.CharField(max_length=100)
    
  • admin.py
    from django.db import models
    from django.contrib import admin
    
    from .models import Book, Cover
    
    
    class CoverInlineMixin:
        model = Cover
        extra = 2
        fieldsets = (
            (None, {"fields": ("image", "title")}),
            ("Details", {
                "fields": ("description", "creation_date"),
                "classes": ("collapse",),
            }),
            ("Details", {
                "fields": ("update_date", "updated_by"),
                "classes": ("collapse",),
            }),
        )
    
    
    class CoverTabularInline(CoverInlineMixin, admin.TabularInline):
        pass
    
    
    class CoverStackedInline(CoverInlineMixin, admin.StackedInline):
        pass
    
    
    class BookAdmin(admin.ModelAdmin):
        list_display = ("title", "author", "publisher","publication_date")
        search_fields = ("title", "author")
        list_filter = ("author", "publication_date")
        fieldsets = (
            (None, {
                "fields": ("title", "author")
            }),
            ("Advanced options", {
                "classes": ("collapse",),
                "fields": ("publisher", "publication_date",)
            }),
            ("Advanced options", {
                "classes": ("collapse",),
                "fields": ("creation_date", "update_date",)
            }),
        )
        inlines = [
            CoverTabularInline,
            CoverStackedInline,
        ]
    
    
    admin.site.register(Book, BookAdmin)
    

Attachments (1)

image-20240517-114532.png (114.5 KB ) - added by Natalia Bidart 5 months ago.

Download all attachments as: .zip

Change History (13)

by Natalia Bidart, 5 months ago

Attachment: image-20240517-114532.png added

comment:1 by Natalia Bidart, 5 months ago

Cc: Marijke Luttekes Thibaud Colas Tom Carrick Sarah Abderemane added
UI/UX: set

comment:2 by Sarah Boyce, 5 months ago

Triage Stage: UnreviewedAccepted

I think this is a little similar to #21353 where a note was added to "description" but I think this note should be pulled out and generally be stated for TabularInlines
I would vote option 2 and have a note 👍

comment:3 by Marijke Luttekes, 5 months ago

I am still unsure of what the UI would be for fieldsets (collapsible or otherwise) in TabularInlines.

A tabular inline is a table; tables are naturally single-use elements: a table contains cells, an optional caption, and nothing more fancy.
They are also very consistent in that they are just as annoying to customize, style, and make responsive as they were many years ago.

There was a suggestion to make the columns collapsible instead. This is not something that can be achieved without using custom JavaScript.
Per the specs, the HTML collapsible details element is not allowed within a table, nor did it work when one of our Discord members tried it in a proof of concept.

Per the above, I am also in favor of option 2.

comment:4 by Maryam Yusuf, 3 months ago

Owner: changed from nobody to Maryam Yusuf
Status: newassigned

comment:5 by Sarah Boyce, 3 months ago

Has patch: set

comment:6 by Ryan Cheley, 3 months ago

Cc: Ryan Cheley added

comment:7 by Sarah Boyce, 3 months ago

Patch needs improvement: set

comment:8 by Marijke Luttekes, 3 months ago

The following comment is only for future reference; no action is required:

Somewhat related to this ticket: Lullabot created a Drupal (a PHP web framework) plugin for responsive HTML tables and described their entire thought process.

The article does not discuss collapsible fieldsets, but I want to park it here since it relates to Natalia's research following the creation of this ticket:
Responsive HTML Tables: Presenting Data in an Accessible Way

Note: Take the bit where they mandate table <caption> with a grain of salt; I know screen reader users who don't like those because it gets in the way of their screen reader reading the table headings.

Note 2: Ignore the accessibility claims in general; they have not been verified.

Last edited 3 months ago by Marijke Luttekes (previous) (diff)

comment:9 by Sarah Boyce, 3 months ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

Note that we have gone for the documentation approach which aligns with #21353.
To me, updating TabularInlines to respect fieldsets would be a new feature, which we should discuss in the forum to see if people actually want this first (this was discovered in a PR review rather than requested by a user organically).

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

Resolution: fixed
Status: assignedclosed

In b5f4d76:

Fixed #35464 -- Updated docs to note fieldsets have limited impact on TabularInlines.

comment:11 by Sarah Boyce <42296566+sarahboyce@…>, 3 months ago

In 65344f0e:

Refs #35464 -- Added test to cover layout of TabularInline fieldsets.

comment:12 by Sarah Boyce <42296566+sarahboyce@…>, 3 months ago

In bdfcda8c:

[5.1.x] Fixed #35464 -- Updated docs to note fieldsets have limited impact on TabularInlines.

Backport of b5f4d76bc400b9f2017da0a52ee4ff0d7c09be15 from main.

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