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

Dark and light mode #102

Closed
AntoineSoetewey opened this issue Feb 4, 2022 · 9 comments
Closed

Dark and light mode #102

AntoineSoetewey opened this issue Feb 4, 2022 · 9 comments
Labels
bug Something isn't working

Comments

@AntoineSoetewey
Copy link
Contributor

What happened?

In video:

Screen.Recording.2022-02-04.at.00.57.51.mov

My default mode is dark. When I am in light mode (because I forced it by clicking on the sun icon) and then visit another page, the mode goes back to the initial mode (dark in my case). It is as if it doesn't remember that my choice is light mode.

Is it possible to implement the mode toggle such that it "remembers" my choice ?

FYI my repo is https://github.com/AntoineSoetewey/antoinesoetewey.com

Regards,
Antoine

Theme version

v2.0.1

Hugo version

last

What browsers are you seeing the problem on?

Chrome, Safari

Relevant Hugo log output

No response

@AntoineSoetewey AntoineSoetewey added the bug Something isn't working label Feb 4, 2022
@jpanther
Copy link
Owner

jpanther commented Feb 4, 2022

Hmm okay this is definitely a bug. I inspected the local storage on your site and can see that the setting is being persisted correctly. I think the issue is that when the default appearance is set to dark, it's not honouring the user preference.

@AntoineSoetewey
Copy link
Contributor Author

Thanks !

@cbelena
Copy link
Contributor

cbelena commented Feb 6, 2022

Hi @jpanther! I noticed that in head.html there is a parameter that doesn't exist: enableAppearanceSwitching

{{ if .Site.Params.enableAppearanceSwitching | default true }}

I think you meant autoSwitchAppearance or maybe showAppearanceSwitcher.

I don't know if this is related to @AntoineSoetewey issue, but since this is related to dark and light appearance, I didn't want to open another issue.

I also noticed that if you set the appearance to dark, Congo loads first the content in light appearance and then switches to dark. This is done fast, but somewhat noticeable. Check out this page for example: set dark appearance and then refresh the page. Is that behavior expected? Maybe the latest change in baseof.html has something to do?

@jpanther
Copy link
Owner

jpanther commented Feb 6, 2022

Okay well done on picking up on that parameter, @cbelena! That was one I named when I was working out how the new dark mode switching would work. It's technically not required anymore as all the switching is handled in JavaScript and so this should always be true.

My original intention was that if you set default appearance to dark you could then disable the switching altogether and not have the JavaScript included in the final build. However, this very issue forced me to change the behaviour and now dark mode is instantiated in JavaScript rather than pure CSS.

The side effect of this change is this new issue you've mentioned whereby it first loads the light appearance and then switches - because the JavaScript is executed deferred so it only happens once the DOM has completed loading. This is one of the problems with trying to have a dark mode that is so flexible - you can't win!

To deal with the first part, I'll just remove this parameter check because it's pointless now. To fix the second part, I think the cleanest solution is going to be to unbundle appearance.js from the rest of the JavaScript and load it immediately. I still don't understand why everyone requests these toggle switches anyway, they seem completely pointless to me but short of removing them altogether I can't think of another solution.

@jpanther
Copy link
Owner

jpanther commented Feb 6, 2022

Okay this should be fixed now. The appearance preference is now loaded earlier in the DOM cycle.

@cbelena
Copy link
Contributor

cbelena commented Feb 6, 2022

Thank you for your quick fix @jpanther! I've tested it and now works fine 😀

My original intention was that if you set default appearance to dark you could then disable the switching altogether and not have the JavaScript included in the final build. However, this very issue forced me to change the behaviour and now dark mode is instantiated in JavaScript rather than pure CSS.

I've thinking about this. If you set defaultAppearance = "light", autoSwitchAppearance = false and showAppearanceSwitcher = false in params.toml, you could avoid loading appearance.js, right? And if defaultAppearance = "dark" is set instead, you could also set the appearance without JavaScript with something like this in baseof.html1:

class="scroll-smooth {{- if (and (eq (.Site.Params.defaultAppearance | default "light") "dark") (not .Site.Params.autoSwitchAppearance | default true) (not .Site.Params.showAppearanceSwitcher | default false)) }} dark{{ end }}"

And then in head.html, put the modified code inside a conditional like this:

{{ if (or (.Site.Params.autoSwitchAppearance | default true) (.Site.Params.showAppearanceSwitcher | default false)) }}
  {{ $jsAppearance := resources.Get "js/appearance.js" }}
  {{ $jsAppearance = $jsAppearance | resources.Minify | resources.Fingerprint "sha512" }}
  <script type="text/javascript" src="{{ $jsAppearance.RelPermalink }}" integrity="{{ $jsAppearance.Data.Integrity }}"></script>
{{ end }}

This way, someone that only wants a static light or dark theme appearance, without toggle switches or automatic switching, doesn't need JavaScript at all (unless search or copying code is needed!), please correct me if I'm wrong.

Sorry if I'm taking this too far 😅

Footnotes

  1. Yes, something like it was before this commit, but with extra conditionals and in one single line of code because otherwise, if defaultAppearance = "light", it outputs an extra space: class="scroll-smooth ".

@jpanther
Copy link
Owner

jpanther commented Feb 6, 2022

Thanks for letting me know that the fix works. I know what you're saying with the JS exclusion here but it feels like it's really not saving much at this point.

For me the preferred use case is default appearance light with auto switching on. This makes the most sense as it follows the OS preference which is essentially what the user is telling you they want. I don't see the point of having a toggle switch on the site itself; the user can toggle this at an OS level if they really need to. Given this base use case requires the JS to be included, it makes little sense to me to exclude it for only a small segment when it's just going to be adding even more code to validate that configuration.

@cbelena
Copy link
Contributor

cbelena commented Feb 6, 2022

Yes, you are right. It's a pretty rare case and it's not worth it. Thanks!

@coder543
Copy link

coder543 commented Aug 28, 2023

@jpanther

I still don't understand why everyone requests these toggle switches anyway, they seem completely pointless to me but short of removing them altogether I can't think of another solution.

One alternative solution... if showAppearanceSwitcher is set to false, I would much rather the website use a CSS-only solution that respects prefers-color-scheme to detect whether the operating system is in dark mode or light mode.

Not a huge deal, but I was searching the issues here to see if anyone had mentioned it, and I couldn't find any mention of it.

EDIT: to clarify, right now, the javascript solution respects the OS color scheme, but this stops working if the user has disabled JavaScript. (which I admit is a super niche thing)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants