Opened 18 months ago
Closed 12 months ago
#34670 closed Cleanup/optimization (fixed)
Django Admin light theme flickers on dark system
Reported by: | Thomas Feldmann | Owned by: | Nicolas Lupien |
---|---|---|---|
Component: | contrib.admin | Version: | 4.2 |
Severity: | Normal | Keywords: | |
Cc: | Sarah Abderemane | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | yes |
Description
My system is set to dark mode: macOS 13.4 (22F66) with Safari 16.5 (18615.2.9.11.4).
Clicking any link in django admin will result in first showing the dark theme for a short moment and then switching to the light theme.
Attachments (2)
Change History (15)
by , 18 months ago
Attachment: | Django Flickering.mp4 added |
---|
comment:1 by , 18 months ago
Resolution: | → needsinfo |
---|---|
Status: | new → closed |
This is probably related with Django Debug Toolbar. It slows everything down and makes the switch visible. I don't think there is anything that Django could do better here. Please reopen the ticket if you can provide details about why and where Django is at fault.
by , 17 months ago
Attachment: | django-flash(2).mp4 added |
---|
comment:2 by , 17 months ago
Resolution: | needsinfo |
---|---|
Status: | closed → new |
I have been able to replicate this on a fresh install (see latest video)
How to replicate:
- Set OS to dark mode or emulate it via chrome tools - https://dev.to/codepo8/quick-developer-tools-tip-simulating-dark-light-colour-mode-1cpg
- Force Django Admin to light mode
- Click around and see
Why is this happening you may ask. The JS code that forces the theme to dark/light is set to run on page load. https://github.com/django/django/blob/main/django/contrib/admin/static/admin/js/theme.js#L3
Because of this the full page loads for a moment with the dark theme, then the page load event happens and then the js code changes the theme to light.
The initTheme() and setTheme() functions don't really need to be inside the event page load. Nothing in those 2 functions are doing anything that require the entire page to be loaded. It is just using local storage and modify the dataset on the html div. So those functions could be moved outside of the on load event. I don't mind making the merge request for it just wanted to get opinions on it.
comment:3 by , 17 months ago
Cc: | added |
---|---|
Triage Stage: | Unreviewed → Accepted |
Type: | Bug → Cleanup/optimization |
Thanks for details, tentatively accepted.
comment:4 by , 17 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:5 by , 17 months ago
Yes, I changed the code to avoid that on djangoproject.com but I forgot to make the change also on Django for the admin.
Feel free to make the changes, I'm happy to review it :)
comment:8 by , 12 months ago
Has patch: | set |
---|
comment:9 by , 12 months ago
Owner: | changed from | to
---|
comment:10 by , 12 months ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:11 by , 12 months ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Ready for checkin → Accepted |
comment:12 by , 12 months ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
A video of the problem