Opened 11 months ago
Closed 11 months ago
#35008 closed Cleanup/optimization (fixed)
Minifiers break django contrib admins UI
Reported by: | Raphaël Stefanini | Owned by: | Mariusz Felisiak |
---|---|---|---|
Component: | contrib.admin | Version: | 4.2 |
Severity: | Normal | Keywords: | UI admin minified css |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | yes |
Description
I have been using a HTML minifier on django's output and it breaks some UI elements in django's shipped admin by stripping some default attributes, mainly by removing type="text"
on inputs, since it is a default html input attribute.
This affect anyone who would use a minifier either as middleware or as CDN feature, and the logic is clear, you don't have to keep default html attribute.
The problem is that django repo CSS rules don't take that in account and it assume for all attributes to be present.
Django would need to have a CSS rule stating input:not([type]), input[type=text]
instead of just input[type=text]
for the minified version to match the original UI.
I have found only three places in django.contrib.admin that would need a CSS update, so I believe it is an easy fix:
// base.css:485 input:not([type]), ... { border: 1px solid var(--border-color); border-radius: 4px; padding: 5px 6px; margin-top: 0; color: var(--body-fg); background-color: var(--body-bg); } // base.css:495 input:not([type]):focus, ... { border-color: var(--body-quiet-color); } // responsive.css:177 .form-row input:not([type]), ... { box-sizing: border-box; margin: 0; padding: 6px 8px; min-height: 2.25rem; font-size: 0.875rem; }
I create this ticket after having discussed the issue here https://github.com/adamchainz/django-minify-html/issues/165
Change History (10)
comment:2 by , 11 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:4 by , 11 months ago
What about re-opening this PR then? https://github.com/django/django/pull/17183
comment:5 by , 11 months ago
Raphaël, feel-free to send a new PR (based on PR17183). We need to at least add a comment to the CSS files.
comment:6 by , 11 months ago
New PR with added comments https://github.com/django/django/pull/17631
comment:7 by , 11 months ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:8 by , 11 months ago
Has patch: | set |
---|---|
Owner: | changed from | to
comment:9 by , 11 months ago
Owner: | changed from | to
---|
It was already proposed, check out comment. I'm pretty sure that we will remove them in some cleanup PR titled "Removed unused CSS rules ...'"'. We normally don't add CSS rules not used by Django itself.