Opened 2 months ago

Closed 2 months ago

Last modified 2 months ago

#35456 closed Cleanup/optimization (duplicate)

Fieldset title inside a collapsed StackedInline are `h3` but on errors are shown as `h2`

Reported by: Natalia Bidart Owned by: nobody
Component: contrib.admin Version: 5.0
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: yes

Description

Given the following models:

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)
    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)


class Review(models.Model):
    book = models.ForeignKey(Book, on_delete=models.CASCADE)
    reviewer = models.CharField(max_length=100)
    content = models.TextField()
    creation_date = models.DateField(default=now)

    def __str__(self):
        return f"Review of {self.book.title} by {self.reviewer_name}"

And admin models:

from django.contrib import admin

from .models import Book, Cover, Review


class CoverTabularInlin(admin.TabularInline):
    model = Cover
    fieldsets = (
        ("Cover Details", {
            "fields": ("image",),
            "classes": ("collapse",)
        }),
    )


class ReviewStackedInline(admin.StackedInline):
    model = Review
    fieldsets = (
        (None, {
            "fields": ("reviewer",)
        }),
        ("History", {
            "fields": ("content", "creation_date"),
            "classes": ("collapse",)
        }),
    )



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",)
        }),
    )
    inlines = [
        CoverTabularInlin,
        ReviewStackedInline,
    ]


admin.site.register(Book, BookAdmin)

When visiting the admin for a Book, the ReviewStackedInline shows fieldset heading using an h3 tag, but on errors, it uses an h2 tag (see screenshots attached). I think the correct tag to be used both with and without errors is an h3.

Attachments (2)

image-20240515-150933.png (106.2 KB ) - added by Natalia Bidart 2 months ago.
image-20240515-151005.png (109.2 KB ) - added by Natalia Bidart 2 months ago.

Download all attachments as: .zip

Change History (4)

by Natalia Bidart, 2 months ago

Attachment: image-20240515-150933.png added

by Natalia Bidart, 2 months ago

Attachment: image-20240515-151005.png added

comment:1 by Sarah Boyce, 2 months ago

Resolution: duplicate
Status: newclosed
Summary: Fieldset title inside StackedInline are `h3` but on errors are shown as `h2`Fieldset title inside a collapsed StackedInline are `h3` but on errors are shown as `h2`

Thank you for the models, admin and screenshots!

I think the core issue here you are reporting is that the heading tag should not change between the collapse and expand state. When there is an error, this is in the expanded state but this header tag switching behaviour also happens when clicking on show and hide.

What the header tags should be is also interesting. This article helped me understand: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Heading_Elements
Here are some rules:

  • Have only 1 <h1> (this is "Add book")
  • Don't skip headers
  • Use headers to show subsections

I think these are the issues:

  • the first collapsed inline "Advanced options" is h2 when collapsed but h3 when expanded. It should always be h2 otherwise we skip a header level
  • the next collapsed inline "History" is h2 when collapsed but h3 when expanded. In this case it is subsection of Review: #1 which is a h3 (1. Add book > 2. Reviews > 3. Review #1) and so as a subsection of a h3, it should be a h4 in both collapsed and expanded.

Then basically there are 2 bugs:

  • the header shouldn't change between the collapse and expanded state (this issue is covered by #35189)
  • inline headers do not respect the page hierarchy (this is independent of the collapse and expand feature, it is "touched" upon in #35189 and will likely be fixed in #35189 - we can pull this out to a separate issue if we wish)

I am going to mark this as a duplicate of #35189 as what you have reported I believe is covered and will be fixed there 👍

in reply to:  1 comment:2 by Natalia Bidart, 2 months ago

Replying to Sarah Boyce:

Thank you for the models, admin and screenshots!

I think the core issue here you are reporting is that the heading tag should not change between the collapse and expand state. When there is an error, this is in the expanded state but this header tag switching behaviour also happens when clicking on show and hide.

In general I agree with the triaging of this ticket but please note a small detail: the heading tag does not in fact change. The current behavior (in both main and in the PR fixing #35189) is that on error, the template would not include the "collapsibility" capabilities in the header, it would present the fieldset with error as a fieldset that is not collapsible at all (and there is no way to close it because, on error, the "expand/hide" capabilities are not there). But the headings are in both cases h2, only that they are notoriously differently styled.

What the header tags should be is also interesting. This article helped me understand: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Heading_Elements

Sarah and I had a chat about this (because it's quite confusing) and we agreed that in the referenced ticket/PR, the "Add Foo" page is skipping a header, but the "Change Foo" is not. Given that in main no header is skipped, we'll tweak the PR to use h2 for fieldsets keeping current heading hierarchy.

OTOH, in main there is an issue with fieldsets headers inside inlines, where h2 headings are used for the fieldset title but these are inside h3 headings which are the inline title. This is being fixed along with #35189 by using h4 in these cases.

I am going to mark this as a duplicate of #35189 as what you have reported I believe is covered and will be fixed there 👍

Makes sense, thank you!

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