Skip to content
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

combine light & dark themes #476

Merged
merged 2 commits into from
Jan 10, 2024
Merged

combine light & dark themes #476

merged 2 commits into from
Jan 10, 2024

Conversation

mbostock
Copy link
Member

@mbostock mbostock commented Jan 10, 2024

Themes now declare whether they are intended for light mode, dark mode, both, or neither. If multiple themes are combined (as is the case with the default theme: [light, dark]) then dark-only themes are only applied if the user prefers a dark color scheme.

This allows us to get rid of the auto and auto-alt themes (in favor of [light, dark] and [light-alt, dark-alt] respectively), and makes it easier for users to combine other light and dark themes of their choosing in the future.

@mbostock mbostock requested a review from Fil January 10, 2024 02:17
Copy link
Contributor

@Fil Fil left a comment

Choose a reason for hiding this comment

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

awesome!

@mbostock
Copy link
Member Author

Oops, I need to fix the precedence here. Since I made dark themes secondary, you have to say [light, dark]; if you say [dark, light] then the light theme takes precedence (because it doesn’t have (prefers-color-scheme: light). I’ll fix.

@mbostock mbostock enabled auto-merge (squash) January 10, 2024 18:06
@mbostock mbostock merged commit 4d54ec4 into main Jan 10, 2024
2 checks passed
@mbostock mbostock deleted the mbostock/theme-alignment branch January 10, 2024 18:07
@Fil Fil mentioned this pull request Jan 10, 2024
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