Skip to content

Fixed #36553 -- Improved semantic structure of admin widgets. #19735

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fixed #36553 -- Improved semantic structure of admin widgets. #19735

wants to merge 1 commit into from

Conversation

Antoliny0919
Copy link
Contributor

Trac ticket number

ticket-36553

Branch description

Improve the semantic structure of widgets used in the admin.

Checklist

  • This PR targets the main branch.
  • The commit message is written in past tense, mentions the ticket number, and ends with a period.
  • I have checked the "Has patch" ticket flag in the Trac system.
  • I have added or updated relevant tests.
  • I have added or updated relevant docs, including release notes if applicable.
  • I have attached screenshots in both light and dark modes for any UI changes.

Antoliny0919
Antoliny0919 commented Aug 16, 2025
View reviewed changes
Copy link
Contributor Author

@Antoliny0919 Antoliny0919 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ve made some style adjustments compared to the original and left comments on the changes.

django/contrib/admin/static/admin/css/responsive.css
Comment on lines -525 to -529
span.clearable-file-input {
margin-left: 15px;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2025-08-16 at 9 08 26 PM

Add the checkbox margin in the clearable file widget apply regardless of screen size. Previously, the margin was applied only on mobile screens and not at all in RTL mode.

django/contrib/admin/static/admin/css/widgets.css
margin: 0;
padding: 0;
color: var(--body-quiet-color);
font-size: 0.6875rem;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I adjusted the font size to 0.8175rem. I felt the previous size was too small and saw no need to make it any smaller.

Before

Screenshot 2025-08-16 at 9 21 36 PM

After

Screenshot 2025-08-16 at 9 22 08 PM

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is most noticeable in mobile view, but I think this change means that the url is quite a bit smaller than the label
Screenshot from 2025-09-26 13-47-57

I think my preference would be either to leave the sizing alone in this change, or to have something consistent

Copy link
Contributor Author

@Antoliny0919 Antoliny0919 Sep 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. I think it’s better to stick with the 0.8125rem, 0.6875 we were using before.

django/contrib/admin/static/admin/css/widgets.css
.datetime span {
white-space: nowrap;
font-weight: normal;
font-size: 0.6875rem;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0.6875rem was applied to links(Today, Now) in the datetime field, causing the date and time field links to have different sizes. It has now been updated to 0.8175rem, making them the same size.

Before

Screenshot 2025-08-16 at 9 27 02 PM

After

Screenshot 2025-08-16 at 9 27 26 PM

@Antoliny0919 Antoliny0919 force-pushed the ticket_36553 branch 3 times, most recently from e70bdaf to de7e008 Compare August 20, 2025 03:03
@Antoliny0919
Copy link
Contributor Author

Please add it to the accessibility improvement project board 🙏

@sarahboyce sarahboyce added selenium Apply to have Selenium tests run on a PR screenshots 🖼️ labels Aug 22, 2025
sarahboyce
sarahboyce reviewed Aug 26, 2025
View reviewed changes
@Antoliny0919 Antoliny0919 mentioned this pull request Aug 27, 2025
Open
6 tasks
@thibaudcolas thibaudcolas moved this from New to To Review in django accessibility improvements Aug 27, 2025
@Antoliny0919 Antoliny0919 force-pushed the ticket_36553 branch 3 times, most recently from cec1446 to 9d08cc7 Compare August 30, 2025 10:40
@Antoliny0919 Antoliny0919 force-pushed the ticket_36553 branch 2 times, most recently from 5d971b0 to 4a3f793 Compare September 28, 2025 10:08
@Antoliny0919
Copy link
Contributor Author

I’m not sure why the Selenium test is failing 🤔 it works fine locally.

@sarahboyce
Copy link
Contributor

I’m not sure why the Selenium test is failing 🤔 it works fine locally.

It's also passing for me locally but it's consistently failing in CI
Some things you can look into would be to take a screenshot and see if in CI the checkbox is somehow not visible/overlapped. You could switch the order so that it is checked before the input is cleared. You could try adding some kinda wait until clickable

@Antoliny0919 Antoliny0919 force-pushed the ticket_36553 branch 4 times, most recently from 8e569f0 to 5582517 Compare September 30, 2025 10:30
@Antoliny0919
Copy link
Contributor Author

It's also passing for me locally but it's consistently failing in CI Some things you can look into would be to take a screenshot and see if in CI the checkbox is somehow not visible/overlapped. You could switch the order so that it is checked before the input is cleared. You could try adding some kinda wait until clickable

Thank you, Sarah.
It seems that the checkbox was not clickable in the CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sarahboyce sarahboyce sarahboyce left review comments

Assignees

No one assigned

Labels

screenshots 🖼️ selenium Apply to have Selenium tests run on a PR

Projects

django accessibility improvements
Status: To Review

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Antoliny0919 @sarahboyce