Fixed #32018 -- Extracted admin colors into CSS variables.

[matthiask]

https://code.djangoproject.com/ticket/32018#ticket

Note! This pull request contains the following changes which could be seen as not being strictly related to the title of the pull request:

• .object-tools' hover state is not using the primary color anymore. Most hover states use a simple desaturation/darkness effect.
• Almost unreadable text according to accessibility guidelines (contrasts of less than 3) have been replaced by shades of grey with better contrast. This allows reducing the count of CSS variables.
• Some colors with minimal differences have been merged, also with the goal of reducing the count of CSS variables in mind.
• The responsive design had a bug where related inlines would have a 2px border at the bottom if some fields in the inline were hidden (the :last-child selector should have hidden the bottom border but it didn't, because it targets the last child (as it should) and not the last visible child which would be useful but CSS does not support this)
• The space between stacked inlines in responsive was bigger than the space between the last stacked inline and the add-another button. I don't think this was intentional.
• Button and link transitions on hover (0.15s) – it just looks so much better in my opinion, and 0.15s should be short enough to not bother anyone (famous last words, I know)

Separating those changes from the current pull request would be possible. It would be much more work preparing the pull request however, and I'd also argue that reviewing wouldn't be less work because I, as a reviewer, would want to check out the changes locally anyway, especially since Django does not have (and should not have IMHO) automated tests for the admin CSS.

I added a few inline comments to CSS rules which are probably unused. I didn't add comments to rules which are only used in the admindocs application; those could be separated out as well, but maybe someone is relying on their presence. A case of practicality over purity in my book.

I didn't touch the vendorized select2 code, but tested with a dark interface for fun. Some code to test a different color scheme or even a dark mode scheme is here: 95b489f

[ajcerejeira]

I purpose we first define variables for colors, the same way that bootstrap does, so we could end up with something like:

--white
--black

--grey-100
--grey-200
...
--grey-900

--blue-100
...
--blue-900

Then we use this colors to construct the variables for the other elements:

--body-bg: var(--white)
--link-color: var(--blue-200)
--form-bg: var(--gray-100)

---

Also, I used css-color-extractor on all admin CSS files, and this is the list of every color used, we can group the variables based on this:

Some of them look really close to each other (like #f6f6f6 and #f9f9f9) should we merge them into a single variable?

[matthiask]

Thanks!

Re. defining colors first. There's a different accepted issue for adding a dark mode here https://code.djangoproject.com/ticket/31259 – I think defining colors first isn't a big help in this case, except if we started to calculate colors from other colors. I have been experimenting a bit with this approach (see the base.html file) but this may be too much for a single pull request. Maybe colors should be redefined in 3rd party projects anyway, which should be much easier once we get good names down...

... which brings me to the second point: Naming is very hard. And it gets harder the more names we have to define. I experimented with using the same color for borders everywhere but that does not look right on different backgrounds. Other colors can be merged easily and I'd certainly propose we should try to merge those. Otherwise this PR wouldn't help at all with addressing the accessibility issues and with theming.

Fresh eyes on the code are always helpful, thanks again for your inputs!

[matthiask]

Btw, I scrolled through the list of pull requests and found #13409 – merging was straightforward without many conflicts. A merge of the current branch tips is here https://github.com/matthiask/django/tree/mk/admin-css-31986-merge

[knyghty]

I really like this PR, I was going to work on this myself at some point 👍

This may or may not be a good idea, I'm just wondering if you've thought about it:

Have you considered putting the variables definitions in a new file (theme.css, colors.css, or similar) that is loaded before anything else? That way changing to dark theme or users customising colours and so forth can be done very easily. Of course, it's also another request to the server...

I'd also be in favour of moving some of the more similar colours together to improve consistency and maintainability - I think it should be in another ticket / PR though.

[matthiask]

@knyghty Thanks! Yes, I've thought about that. However, I don't think it's necessary since overriding those values also works when it happens when loading the theme CSS last since CSS works that way. Or even like this, with a project-specific templates/admin/base.html override:

{% extends 'admin/base.html' %}

{% block extrahead %}{{ block.super }}
<style>
/* Purple, just as an experiment to see what's missing */
:root {
  --primary: #9774d5;
  --secondary: #785cab;
  --link-fg: #7c449b;
  --link-selected-fg: #8f5bb2;
}
</style>
{% endblock %}
```

[knyghty]

Oh, you mean you can override the CSS variables after they are set? That's very good to know, I had no idea, thanks.

So then a dark or other alternate theme could just implement the same vars in a new file that's loaded whenever?

If that's the case, it sounds good. It might be nice to just have the colours in one place so you know what you can override, but I think having them all together in base CSS file is good enough 👍

[matthiask]

Regarding:

I'd also be in favour of moving some of the more similar colours together to improve consistency and maintainability - I think it should be in another ticket / PR though.

Doing this in a different ticket may be fine but either we first convert everything to use CSS variables (meaning we have to find names for all those colors which we'll drop again anyway) or we standardize colors first. Both sound like good approaches but since contributor and reviewer time isn't an endless resource I'd strongly prefer moving forward with the approach outlined here. Killing two birds with one stone and all that.

So then a dark or other alternate theme could just implement the same vars in a new file that's loaded whenever?

Yes, exactly.

[matthiask]

I've imported https://github.com/matthiask/django/tree/mk/admin-css-31986-merge into https://github.com/matthiask/django-variable-admin and published this to https://pypi.org/project/django-variable-admin/ so that I can use this in projects and find any remaining problems with the pull request.

[mimi89999]

Hello,
I was considering using CSS variables or SASS when I opened #12444. I feared that CSS variables wouldn't be accepted because of a possible (?) performance impact or browser support, but that is not bad: https://caniuse.com/css-variables. SASS had even bigger issues because it would either require compiling it on every change in code or compiling it when uploading to pypi, etc.

I think that using variables makes it much easier to make theme changes in Django and add other themes like the dark theme. I hope that this PR will be merged soon and I will rebase #12444 on it.

Also, I think that dedicated and designed dark themes are nicer than calculated dark themes from light ones.

[carltongibson]

Hi @matthiask.

Thanks for this. Really good.

• Can you squash into a single commit, with the commit message format and so on.
• In commit message, can you make a note of the colours you merged for posterity? (It may be that we want to recover some variations if there's a visual regression we didn't manage to spot.)
• Docs: I think we need at least a release note. Do you think a section on the Admin site docs showing how to add your own theme would be worthwhile? (The example you gave in the discussion is nice.)

I think we should take this. Nice addition, and should allow @mimi89999 to implement a dark mode (which folks seem to like 😀) so 👍

[carltongibson]

Hey @matthiask — just checking in on you, since this is on my long-list for v3.2… 

Did you have any comment on:

Docs: I think we need at least a release note....

Are you on it?

Ta. Merry Christmas! 🎄

[matthiask]

Hi @carltongibson

Thanks for your patience. I have revised the commit message, added a few lines to the docs and a changelog entry.

A happy new year to everyone following here 🥳

[matthiask]

Now a JavaScript-related test is failing. The failure does seem to be related to the changes in this pull request so I'll wait for additional review comments :-)

[carltongibson]

OK @matthiask super, thank you.

[claudep]

\o/ Thanks all!

[matthiask]

Thanks! 🥳🎉

[mimi89999]

Hello,
Sorry. for the late feedback. I started working on updating #12444 and immediately encountered and issue. I want to make the caption dark with a light text as in #12444 (comment), but it's not possible with the current variables. Could you please split the colors a bit more? If not, I could make a separate PR for that. Anyway, using body-bg for things that are not a background seems strange.

[carltongibson]

@mimi89999 No stress. Add an initial commit to your PR adding the variable. (Finding this sort of this is expected, and whole point of the pre-release phase. 😄) Thanks!

[carltongibson]

Looks like pygments doesn't like the variables here.

django/docs/ref/contrib/admin/index.txt:2815: WARNING: Could not lex literal_block as "html+django". Highlighting skipped.

Opened an issue over there. pygments/pygments#1666