Skip to content

Refs #2096: The theme selector now controls all colors #2116

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.

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

Merged
merged 3 commits into from
Apr 2, 2025

Conversation

matthiask
Copy link
Member

@matthiask matthiask commented Mar 31, 2025

Description

Previously, if the system preference was dark mode and the user explicitly selected the light theme, the @media block still interferred with the styling. This is fixed by only evaluating the color scheme preference when initializing the toolbar and later only looking at our own selected theme.

Also, this pull request drops the DEFAULT_THEME setting, and instead prefers looking directly at the preferences of users' systems. Removing settings is most often a plus in my book. The main reason for removing it was to make the code easier to follow, because there's one less theme preference source to look at.

Refs #2096

Checklist:

  • I have added the relevant tests for this change.
  • I have added an item to the Pending section of docs/changes.rst.

Previously, if the system preference was dark mode and the user
explicitly selected the light theme, the @media block still interferred
with the styling. This is fixed by only evaluating the color scheme
preference when initializing the toolbar and later only looking at our
own selected theme.

Also, removed the DEFAULT_THEME setting; falling back to system defaults
seems much better to me.
@matthiask matthiask force-pushed the theme-selector-fixup branch from 06dba2b to 7d76af6 Compare March 31, 2025 20:45
Copy link
Member

@tim-schilling tim-schilling left a comment

Choose a reason for hiding this comment

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

I'm good with this change. We'll need to remember this when doing the release to explicitly call it out. Maybe we should cut another one just so we don't forget?

@matthiask
Copy link
Member Author

Yep, I think we should do a release soon-ish, especially since the release notes of 5.1 mentioned that we fixed the dark mode issue (but it has only been fixed for real with #2114 and this change).

@matthiask matthiask merged commit 66361c5 into django-commons:main Apr 2, 2025
25 checks passed
@matthiask matthiask deleted the theme-selector-fixup branch April 2, 2025 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants