Opened 8 months ago

Closed 8 months ago

#35382 closed Cleanup/optimization (fixed)

Remove unused CSS style `.inline-related fieldset.module h3`

Reported by: Sarah Boyce Owned by: Saurabh
Component: contrib.admin Version: 5.0
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description (last modified by Natalia Bidart)

Currently, the CSS definition in forms.css for .inline-related fieldset.module h3 (which is at least 16 years old) is not being used in the Django code base. The only templates using the inline-related CSS class are contrib/admin/templates/admin/edit_inline/tabular.html and contrib/admin/templates/admin/edit_inline/stacked.html, and the former only uses h3 elements inside a div element which does not fulfill the CSS rule, and in the latter there is no usage of h3 at all.

Attachments (2)

Color.png (73.7 KB ) - added by Sarah Boyce 8 months ago.
suggestion.png (105.7 KB ) - added by Sarah Boyce 8 months ago.

Download all attachments as: .zip

Change History (17)

by Sarah Boyce, 8 months ago

Attachment: Color.png added

comment:1 by Sarah Boyce, 8 months ago

Description: modified (diff)

comment:2 by Natalia Bidart, 8 months ago

Component: Uncategorizedcontrib.admin
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

Thank you Sarah for the report, it makes sense. I checked and the background color is at least 16 years old!

Do you have any concrete suggestion for the new font color?

by Sarah Boyce, 8 months ago

Attachment: suggestion.png added

comment:3 by Sarah Boyce, 8 months ago

Black #000?

comment:4 by Natalia Bidart, 8 months ago

Looks good! I checked the base.css and noticed we have a --body-fg: #333 which also seems like a good option. I would remove the bold though!

comment:5 by Natalia Bidart, 8 months ago

Description: modified (diff)
Summary: Low contrast background color for fieldset headings within inline formsetsRemove unused CSS style `.inline-related fieldset.module h3`

During a Discord conversation between Sarah and Ben, they realized that this low-contrast issue was only reproducible in the context of a PR fixing #35189, meaning that the issue is not present in main. So we are re-purposing this ticket to confirm and delete the unused CSS style .inline-related fieldset.module h3.

comment:6 by Natalia Bidart, 8 months ago

Keywords: accessibility removed
Type: BugCleanup/optimization

comment:7 by vinayaka314, 8 months ago

I'm new here and I wanna help. Can anyone guide me, please?

comment:8 by Marijke Luttekes, 8 months ago

Hello, author of the fix for the aforementioned other ticket here.

The light blue color used for inline H3s feels random; it does not fit the Django Admin's color scheme.

Since the CSS hasn't been touched for years and is unused, and the color is not referred to with CSS custom properties, I assume it's an old remnant that wasn't meant to match modern admin styles.

Hence I propose we find a style or color that better fits with the current base colors tint-wise, and find a suitable text color with a WCAG-approved contrasting text color.

I am not a designer, so I would experiment with my PR and see what works. It's not a scientific method. If anyone has had a good thought about this before, let me know!

Edit: I can also make it look like it does currently so you won't notice a visual difference.

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

comment:9 by Natalia Bidart, 8 months ago

Just an FYI, this ticket has bee repurposed to fully remove the unused CSS rule so there is no need to pick new colors. If a new color is needed in a future work that introduces this h3 back, the colors should be discussed in that PR/ticket. Thank you!

comment:10 by Marijke Luttekes, 8 months ago

I have removed this whole selector and contents for .inline-related fieldset.module h3 in my PR for ticket #35189 (PR #17910).

Do you want to have this as a separate PR, as well?

comment:11 by Sarah Boyce, 8 months ago

Do you want to have this as a separate PR, as well?

Yes please, let's get this merged as a distinct and independent cleanup change. Having a ticket to track this is probably overkill, but merging in a separate commit has a couple of advantages and it's quite common we try to pull out commits and merge early when possible/appropriate. 👍

comment:12 by Saurabh , 8 months ago

Owner: changed from nobody to Saurabh
Status: newassigned

comment:13 by Sarah Boyce, 8 months ago

Has patch: set

comment:14 by Sarah Boyce, 8 months ago

Triage Stage: AcceptedReady for checkin

comment:15 by Sarah Boyce <42296566+sarahboyce@…>, 8 months ago

Resolution: fixed
Status: assignedclosed

In 16d0542b:

Fixed #35382 -- Removed unused CSS for admin inline fieldsets.

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