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

Add defaultTheme option to override OS choice #302

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

abyss
Copy link

@abyss abyss commented May 17, 2021

Introduction

This is a PR that re-adds the (optional!) ability to have a defaultTheme, as before the new theming rework.

Behavior

If defaultTheme is not set, the behavior is unchanged from the current master branch.
If defaultTheme is set to "light" or "dark", that will be used before the OS theme.

Order of priority:

  • User Choice (if themeToggle is on, and they've clicked it at least once)
  • defaultTheme (if set)
  • OS theme (as current)

Documentation

I've added documentation to both exampleSite's config and updated the primary README to account for this feature.

Bonus

As a side effect, the Table of Contents was regenerated to be fully accurate to the README by my tooling, and I threw in a bonus spelling fix on License.

Compatibility

This should be fully compatible with current configurations with no changes, with one caveat: if a user still has "defaultTheme" in their configuration, it will begin to work again.

Breaking Changes

There is one very minor feature that was dropped because I'm not quite sure the best way to implement it.

  window
    .matchMedia("(prefers-color-scheme: dark)")
    .addEventListener("change", (e) => e.matches && detectOSColorTheme());
  window
    .matchMedia("(prefers-color-scheme: light)")
    .addEventListener("change", (e) => e.matches && detectOSColorTheme());

These two listeners are now gone - my understanding is these would only update the theme if the OS Color theme changed while the site was open. This seems like a fairly rare edge case, and given it's a static site I'm not sure users would expect the website's theme to update dynamically.

Of course, this could be edited back in, if you can think of an elegant way to implement it (the problem is I can't think of a good check for if the current theme is set by the site config, user, or OS - this code should only be present in the last case).

Demo

This is in use at https://abyss.dev if you want to demo - it is set to have a defaultTheme of "dark", with themeToggle set to true.

Conclusion

Of course, this is a fairly substantial change, so feel free to tweak the commits or give feedback as needed. :)

@bradthornborrow
Copy link

Hey, what's the status on this pull request? I think this in combination with the themeToggle option would be a great option. Even though I have my local system set to light mode, there are some sites that just look better in dark mode...

@chvancooten
Copy link

Also related to issue #353
It's a shame this simple and non-destructive PR isn't being merged :(

@chvancooten chvancooten mentioned this pull request May 5, 2022
@L1ghtn1ng
Copy link

I have added this PR to my fork https://github.com/L1ghtn1ng/hugo-theme-hello-friend-ng-cloudflare

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.

4 participants