Skip to content

[#481] If user hasn't explicitly set a dark mode pref in Silverbullet, fall back to browser pref #1294

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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

Conversation

niels
Copy link

@niels niels commented Mar 16, 2025

This addresses #481.

To differentiate between a preference having been set explicitly (by toggling dark mode on or off within Silverbullet) or not, the darkMode property across the various components can now either be a boolean, or undefined.

The initial view state sets it to undefined.

When loading the editor UI, the client store is inspected to find an explicitly set preference. If one is found, that preference gets applied. Otherwise, the browser setting is inspected (via a media query) and the UI theme gets set accordingly.

If/once an explicit preference has been set, subsequent changes to the browser setting no longer affect Silverbullet.

One caveat: If the user hasn't set a preference within Silverbullet and then changes their browser setting, the change only gets applied upon page reload. (Toggling the preference within Silverbullet itself already triggers a page reload, thus applying the changed setting automatically.)

As this is my first time interacting with this codebase, my approach may be suboptimal. I would be happy to iterate as feedback is received. For now, with my limited understanding, this seemed like the most straightforward path to take.

@niels niels force-pushed the niels/issue-481_dark-mode-browser-pref branch from 86becd4 to a0b4232 Compare March 16, 2025 15:18
@janssen-io
Copy link
Contributor

janssen-io commented Mar 18, 2025

Can we add an explicit 'system' option next to 'light' or 'dark'. Just so that once a user has set something, it's easy to go back to system? Or an easy way (command?) to remove the preference. I think an explicit 'system' option is clearer, though. And also how many other apps do it.

@niels
Copy link
Author

niels commented Mar 24, 2025

I agree that a "theme" setting with string values would, ultimately, be the best approach. We would initially have system, dark and light; and possibly others in the future.

What I struggle with is the current command UI. You can only "Toggle Dark Mode". For that UI, I don't think a "system" setting makes sense. The app loads in either light or dark mode, and toggling should then simply switch to the other. A simple toggle has no inherent concept of "system" vs. "user-set". By it's very nature, once I perform the toggle, I have set a user preference.

So I think we would have to add the new preference to the SETTINGS page instead. When doing so, would we want to retain the toggle? Would the toggle change what's set in SETTINGS? Changing back to the system theme would then be an "advanced feature" requiring the user to modify their SETTINGS manually.

There are a number of fairly closely related issues:

It seems to me that it would make sense to discuss them together to come up with a design that addresses them all.

This PR here would then simply be an iterative improvement of the current functionality, solely resolving the specific issue linked above (#481), rather than a more holistic approach to theming—with the latter shelved for now.

Would that be acceptable?

@janssen-io
Copy link
Contributor

That's an excellent point. If it would be up to me then it still would be great to add an extra command to the core library that clears the setting (instead of having to dig into the client storage myself).

But other than that, it would be awesome to have this as soon as possible.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants