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 browser dark/light theme detection #7830

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

pmario
Copy link
Contributor

@pmario pmario commented Nov 1, 2023

@Jermolene -- I think this PR will give users and plugin-authors good flexibility and it is simple.

The configuration is in ControlPanel -> Appearence -> Palette, where it belongs.

  • By default the new configuration can "Enable Light/Dark Mode Handling" to change the setting manually
  • There is a definition for a "Light Palette" and a "Dark Palette"
  • If the configuration should be switched automatically, according to the browser "appearence" setting we need an action-tiddler tagged: $:/tags/DarkLightChangeActions
    • So several plugins can add new actions if needed
  • If the action-tiddler is removed, auto-switching is gone and the menue is for manual switching.
  • All actions tagged: $:/tags/DarkLightChangeActions will be executed
  • There is a docs tiddler + 1 click auto-switching code example
  • updated "change" event listener, because old one is deprecated

These changes are compatible with my PaletteManager preview function and the TiddlyTools PaletteManager (not shown in the screencast below)

light-dark-mode

Copy link

vercel bot commented Nov 1, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
tiddlywiki5 ✅ Ready (Inspect) Visit Preview Mar 3, 2024 7:18pm

@pmario
Copy link
Contributor Author

pmario commented Nov 1, 2023

There is a little problem with all dark themes and the HelloThere page, which should be fixed in a different PR.

@Jermolene
Copy link
Owner

Thanks @pmario. This does seem like a reasonable low level primitive for us to provide. I think there might well be other events on the "window" object that we might want to expose in the same way (see MDN docs), notably devicemotion, languagechange, resize, copy, cut, paste, offline, online, hashchange, pageshow and pagehide. For media-match events, we might want to support other queries such as (max-width: 600px).

However, before merging I'd prefer to wait until we have a coherent overall view for the automation of dark mode handling. It may be that the best way forward is a more radical overhaul of our handling of palettes, which I am certainly open to.

Copy link
Owner

@Jermolene Jermolene left a comment

Choose a reason for hiding this comment

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

Thanks @pmario I noticed some minor docs style issues

@pmario
Copy link
Contributor Author

pmario commented Nov 1, 2023

However, before merging I'd prefer to wait until we have a coherent overall view for the automation of dark mode handling.

I think there is are 3rd party plugins at the moment, which implement manual dark/light "toggle button". But that's about it.

The auto-switching is not possible atm, because TW core does not support it. So it's a chicken/egg problem. If we do not implement something like this PR - nothing will happen.

The views about how the configuration UX should look like are diametrically opposed depending on the author, as we could see with our discussion with my first attempt.

It may be that the best way forward is a more radical overhaul of our handling of palettes, which I am certainly open to.

I did ask for community help with the palette colours over at Talk. There may be some other occasions, which I cannot find atm. -- Nobody did do anything.

I do have slightly improved versions of most palettes at the palette-manager edtion. But they are not finished yet.


[. . .] a more radical overhaul of our handling of palettes [. . .]

Does this mean, you'll want to wait merging this PR until this happens?

@Jermolene
Copy link
Owner

Thank you @pmario I would like to defer this until we've been able to give more consideration to the user interface.

@linonetwo
Copy link
Contributor

Can auto-switching like VSCode happend in this PR? @pmario seems preview site is not created so I can't test it easily.

I recently find that if you want auto-switching, you shouldn't save $:/palette file when it auto-switching, otherwise this file will change a lot (causing lots of backup, even nothing real is changed)

Also there is a https://github.com/linonetwo/tiddlywiki-switch-theme-user-script as workaround before this is merged, I will integrate it into TidGi APP too, until we have this core feature.

@pmario
Copy link
Contributor Author

pmario commented Jan 1, 2024

[. . .] @pmario seems preview site is not created so I can't test it easily.

@linonetwo There is a preview version available now.

@pmario
Copy link
Contributor Author

pmario commented Jan 1, 2024

I recently find that if you want auto-switching, you shouldn't save $:/palette file when it auto-switching, otherwise this file will change a lot (causing lots of backup, even nothing real is changed)

I'm not sure about this. I usually set the default light or dark theme once in a lifetime. So there will be some backups, when the user is experimenting with the new feature. But later on, I do not see why there should be additional switches at all.

IMO it's the same thing as if users set the $:/DefaultTiddlers to [list[$:/StoryList]]. It does not change any content tiddler, but for me $:/StoryList does save an absolutely essential value for the next start of the wiki. So I do not care about backups. It's important that $:/StoryList is saved.

If the number of backups is a problem, IMO the backup strategy has to be changed. Changing the $:/palette tiddler should not cause any problems with backups.

Just my thoughts.

@linonetwo
Copy link
Contributor

Ok, I accept that $:/palette will change on every day/night switch.

I'm currently omit it in wiki templates. And only keep the theme switch config.

https://github.com/tiddly-gittly/Tiddlywiki-NodeJS-Github-Template/blob/76b444c66a81977794044a846003c8d281e2f65d/.gitignore#L16

https://github.com/tiddly-gittly/Modern.TiddlyDev/blob/6a694373f51e6950826f7d839dbfd8403df38567/.gitignore#L136

@linonetwo
Copy link
Contributor

linonetwo commented Jan 25, 2024

But you are using

\define darkPalette() $:/palettes/GruvboxDark
\define lightPalette() $:/palettes/Vanilla

instead of config tiddler. Can you make it config, and create a control panel for it? I think most of users need this, and most of them don't know "wikiscript".

Why not use action like https://github.com/tiddly-gittly/Modern.TiddlyDev/blob/6a694373f51e6950826f7d839dbfd8403df38567/wiki/tiddlers/%24__Modern.TiddlyDev_Startup.tid#L13

<$action-setfield $tiddler="$:/palette" $value={{{ [{$:/info/darkmode}match[yes]then{$:/configs/palettes/dark}else{$:/configs/palettes/light}] }}}/>

that will use config from control panel.

@pmario
Copy link
Contributor Author

pmario commented Jan 26, 2024

@Jermolene -- I did just update the OP #7830 (comment) with the latest info.

Talk discussion is at: https://talk.tiddlywiki.org/t/light-dark-palette-switching-one-more-time/8960

@linonetwo
Copy link
Contributor

linonetwo commented Feb 15, 2024

The new UI is elegant, can't wait to use it! Thank you @pmario


Thank you @pmario I would like to defer this until we've been able to give more consideration to the user interface.

@Jermolene Now there is a user interface, what do you think?

@BurningTreeC
Copy link
Contributor

BurningTreeC commented Feb 15, 2024

Hello @pmario - with these changes, is it possible to detect the light/dark mode in a startup action, too?

Edit: forget my question XD ... it's easily doable, I know

@Jermolene
Copy link
Owner

Hi @pmario I tried a simple test with the preview release of opening the page and then using my OS controls to switch to dark mode. The "save" button turned red, indicating that a tiddler has changed. I do not think that that is acceptable. If the user has elected to track the OS dark/light mode then a change in the mode should not make the wiki dirty.

@pmario
Copy link
Contributor Author

pmario commented Feb 15, 2024

The "save" button turned red, indicating that a tiddler has changed. I do not think that that is acceptable. If the user has elected to track the OS dark/light mode then a change in the mode should not make the wiki dirty.

From my point of view. This PR does exactly, what it is supposed to accomplish.

  • Implement a "low level" core function, that lets us dynamically detect OS or browser theme changes
  • Implement a minimal UI that enables "manual" light / dark mode switching.

I did include 2 interactive examples, that should make it easy for users to test the new behaviour. That's it: Examples


You are right, there may be a problem with the examples, but IMO that's a problem, which cannot be resolved with this PR. IMO it's a "read only mode" core problem.

This PR has 3 modes.

  1. Enable Light / Dark Mode handling.
    • The user can switch between 2 modes manually. No action tiddlers are created
  2. Automatic switching based on a $:/tags/StartupAction/Browser startup action, which does not change trigger "dirty"
    • This mode is important if the theme settings changed, while the wiki was closed.
  3. Dynamic detection while the wiki is active, which does trigger "dirty"
    • This mode is used if the wiki is active and the OS or browser setting is changed.

For case 3, there is no mechanism in the tiddlywiki core atm, which allows us to block dirty mode on changes to the $:/palette tiddler. This is a general problem if someone changes anything at tiddlywiki.com, which sets the dirty state. If we do not want that, we need a new function in the core, which imo is not in the scope of this PR.

If we would add -[[$:/palette]] to $:/core/save/all, we would loose mode 1 (manual palette switching)

@pmario
Copy link
Contributor Author

pmario commented Feb 15, 2024

@Jermolene, There is a new Preview version, which has 2 interactive examples now.

https://tiddlywiki5-git-fork-pmario-generic-dark-light-2f731d-jermolene.vercel.app/#DarkLightChangeActions

@pmario
Copy link
Contributor Author

pmario commented Feb 20, 2024

@Jermolene .. As I wrote, I think changing the $:/palette has to trigger "dirty", since we want to save the $:/palette if the user selects a palette without any "auto-detection" going on.

So the question is: Can we create a new configuration, that allows us to avoid dirty tracking, but still be able to save tiddlers with the next save action or throw away the setting if it is not saved.

eg: $:/config/IgnoreDirty ... which would allow a "titlelist" or a "filter"

What do you think?

@Jermolene
Copy link
Owner

Hi @pmario the UI I would expect would look like this:

  • The default "vanilla" palette would be refactored to have two distinct sets of colours, one for dark mode and one for light mode
  • The <<colour>> procedure would choose choose the dark variant variant if the tiddler $:/state/palette/darkmode is set to yes
  • There would be a default core action invoked when the system changes between dark mode and light mode. The logic of the action would be to inspect the config tiddler $:/config/DarkMode/AutoSwitch and if it is set to yes, then the tiddler $:/state/palette/darkmode is updated to the current setting from $:/info/darkmode

This arrangement would allow users to choose whether or not their wiki is affected by system dark mode changes. If it is not, they can manually set $:/state/palette/darkmode to switch between the variants. This arrangement would not require any change to the saving mechanism.

It would, though, require us to change palette handling in a way that it will not be backwards compatible. The easiest approach might be to use the prefixes @dark- and @light- to indicate variants of colours. The <<colour>> procedure would first look for the colour without the prefix, then fall back to looking for the appropriate prefix. It might also be nice to introduce explicit inheritance:

title: $:/MyHypotheticalNewPalette
name: Vanilla
description: Unobtrusively pale or dark
tags: $:/tags/Palette
type: application/x-tiddler-dictionary
import-dark: $:/MyFavouriteDarkPalette
import-light: $:/MyFavouriteLightPalette

@dark-background: #000000
@light-background: #ffffff
etc....

@linonetwo
Copy link
Contributor

linonetwo commented Feb 20, 2024

Why not make a Vanilla-dark palette, instead of make vanilla palette have @dark-background and @light-background? If a palette can adapt to both dark and light, then this makes color-scheme field of palettes useless.

(I usually switch between Notion and Nord, but my Notion palette use many variable from Vanilla, and is a light only palette. For the dark one, based on the time, I sometime switch to SolarisDark instead)

@Jermolene
Copy link
Owner

Why not make a Vanilla-dark palette, instead of make vanilla palette have @dark-background and @light-background? If a palette can adapt to both dark and light, then this makes color-scheme field of palettes useless.

We could have separate Vanilla-Dark and Vanilla-Light, and switch between them, but we want to retain compatibility with one basic feature: existing colour palette choosers should still work by changing $:/palette. As we've discussed, having separate $:/palette-dark and $:/palette-light tiddlers is not backwards compatible.

(I usually switch between Notion and Nord, but my Notion palette use many variable from Vanilla, and is a light only palette. For the dark one, based on the time, I sometime switch to SolarisDark instead)

It sounds like you'd make a custom palette that inherited from both Notion and Nord, with some customisations.

@pmario
Copy link
Contributor Author

pmario commented Feb 21, 2024

I think, I can live with a "meta palette" configuration as you suggested. A possible "autoswitcher" chooses from the configuration shown below, but additionally of import-dark, import-light I would like to have more field names.

The following usecase needs to be possible: From 6am-11am a palette-6-11 should be active. from 11:01am to 16pm palette-11-16 should be active ... and so on.

So I would rename import-dark and import-light to palette-dark and palette-light and the "palette-selector" knows about these fields. The active palette selector could be a "filter-cascade", so we basically can use any field-name to reference a palette to be active. -- The last filter could use the existing $:/palette tiddler for backwards compatibility reasons.

I personally do not want to use dark-palettes, but I would like to switch between 2 or 3 different light ones, based on time-of-day.

title: $:/palettes/MyPaletteSelector
tags: $:/tags/Palette
name: MakeMeHappy
description: Select a palette based on time of day
type: application/x-tiddler-dictionary
origin: 

palette-dark: $:/palettes/DesertSand
palete-light: $:/palettes/Vanilla
palette-6-10: $:/palettes/MorningBlue
palette-10-16: $:/palettes/DayVanilla
palette-16-23: $:/palettes/LateDark

I'm completely against the mechanism to have 2 values per mode in 1 palette. I think palettes should be combinable in every possible way. Not everyone wants to switch between light / dark even if the browser says it should be dark.

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.

None yet

4 participants