[Django] #33667: Django admin stylesheet ignores --header-branding-color variable.
Django
-------------------------------------+-------------------------------------
Reporter: Mike | Owner: nobody
DeSimone |
Type: Bug | Status: new
Component: | Version: 3.2
contrib.admin | Keywords:
Severity: Normal | css,stylesheet,admin
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------+-------------------------------------
In `contrib/admin/static/admin/css/base.css`, a header branding color
variable is defined at line 20:
{{{
--header-branding-color: var(--accent);
}}}
but it is not used anywhere else. I expected it to be used in the branding
h1 tags starting at line 920:
{{{
#branding h1 {
padding: 0;
margin: 0 20px 0 0;
font-weight: 300;
font-size: 24px;
color: var(--accent);
}
#branding h1, #branding h1 a:link, #branding h1 a:visited {
color: var(--accent);
}
}}}
but as can be seen, `var(--accent)` is used instead. Also the second block
redundantly calls out `#branding h1`. Any idea what it should have been?
`#branding h1 a`?
`--header-branding-color` appears nowhere else in Django. This bug is
found in 3.2, 4.0, and the main branch.
--
Ticket URL: <https://code.djangoproject.com/ticket/33667>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Mike DeSimone | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: css,stylesheet,admin | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Mariusz Felisiak):
* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
Agreed, `--header-branding-color` should be used for `#branding h1`
> Also the second block redundantly calls out `#branding h1`. Any idea
what it should have been? `#branding h1 a`?
As far as I'm aware, `#branding h1` is redundant in the second rule and
can be removed. Would you like to prepare a patch?
--
Ticket URL: <https://code.djangoproject.com/ticket/33667#comment:1>
Django
--------------------------------------+------------------------------------
Reporter: Mike DeSimone | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: css,stylesheet,admin | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Mike DeSimone):
Would the patch get into a 3.2 update? It does me no good if it's only
fixed on `main`.
I might be able to get to it in a week or three; I ran into it on an
airgapped system and would need to recreate it on another machine some
weekend or another. (I already had to jump through internal hoops just to
report this.) I have no problem if someone wants to roll the fix in
themselves.
Also, what else would need to be in the patch? I don't know how to
generate a test case for this.
--
Ticket URL: <https://code.djangoproject.com/ticket/33667#comment:2>
Django
--------------------------------------+------------------------------------
Reporter: Mike DeSimone | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: css,stylesheet,admin | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Mariusz Felisiak):
> Would the patch get into a 3.2 update? It does me no good if it's only
fixed on `main`.
Unfortunately not, it doesn't qualify for a backport.
> Also, what else would need to be in the patch? I don't know how to
generate a test case for this.
IMO a regression test is not needed.
--
Ticket URL: <https://code.djangoproject.com/ticket/33667#comment:3>
Django
--------------------------------------+------------------------------------
Reporter: Mike DeSimone | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: css,stylesheet,admin | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Mike DeSimone):
> Unfortunately not, it doesn't qualify for a backport.
Then making a patch is low priority for me. I'm going to have to override
`#branding h1` directly anyway.
Hmm, it seems `--accent` isn't used anywhere but these two places in
`#branding`. Is this also an oversight? I'm wondering which fix is better:
* use `--header-branding-color` in `#branding h1` and `--accent` in
`#branding h1, #branding h1 a:link, #branding h1 a:visited` (changing the
first `#branding h1` to `#branding h1 a`)
* remove `--header-branding-color` as redundant, or
* use `--accent` elsewhere?
It's hard to say since I'm not the style designer.
--
Ticket URL: <https://code.djangoproject.com/ticket/33667#comment:4>
Django
--------------------------------------+------------------------------------
Reporter: Mike DeSimone | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: css,stylesheet,admin | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Mariusz Felisiak):
I would use `--header-branding-color` for `#branding h1` and leave
`--accent` for `#branding h1 a:link`, `#branding h1 a:visited`.
--
Ticket URL: <https://code.djangoproject.com/ticket/33667#comment:5>
Django
Reporter: Mike DeSimone | Owner:
Type: | Hrushikesh Vaidya
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
css,stylesheet,admin |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Hrushikesh Vaidya):
* owner: nobody => Hrushikesh Vaidya
* status: new => assigned
* has_patch: 0 => 1
Comment:
I've prepared a patch [https://github.com/django/django/pull/15652 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/33667#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Mike DeSimone | Owner:
Type: | Hrushikesh Vaidya
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
css,stylesheet,admin |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"f74d4cae6dd6e64226a10c74f725e649bf736655" f74d4ca]:
{{{
#!CommitTicketReference repository=""
revision="f74d4cae6dd6e64226a10c74f725e649bf736655"
Fixed #33667 -- Used --header-branding-color for branding headers in
admin.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33667#comment:7>