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 support for additional theme values #66

Closed
wesharper opened this issue Mar 12, 2024 · 10 comments · Fixed by #83
Closed

Add support for additional theme values #66

wesharper opened this issue Mar 12, 2024 · 10 comments · Fixed by #83
Labels
enhancement New feature or request v1 roadmap

Comments

@wesharper
Copy link

wesharper commented Mar 12, 2024

Describe the feature in detail (code, mocks, or screenshots encouraged)

'light', 'dark', and 'system' are great, but I'm building a fun hobby project where I want to enable other themes like 'unicorn-puke'. It would be great if I could update the expected type and use setMode to set it to any of the types in that union.

What type of pull request would this be?

New Feature

Provide relevant links or additional information.

No response

@wesharper wesharper added the enhancement New feature or request label Mar 12, 2024
@huntabyte
Copy link
Member

huntabyte commented Mar 12, 2024

Hey @wesharper, this is something we will consider before publishing a v1 of the package.

The only thing I foresee is that we will still need to have light/dark/system as the primaries since those are relevant to native scrollbars and UI elements using the color-scheme: style attribute. So this would likely be in addition to that, possibly via a data attribute similar to how DaisyUI does it.

@wesharper
Copy link
Author

Could be interesting to enforce setting a color-scheme for custom theme values with some good jsdoc comments and docs about why it's necessary

@huntabyte
Copy link
Member

But let's say I have a Dracula theme that has both a light and dark variant. I'd probably want to just use the user's system preferences to adapt the theme accordingly.

@ollema
Copy link
Collaborator

ollema commented Mar 19, 2024

are you suggesting that:

  1. mode-watcher should be able to handle variations of the existing dark/light themes, such as dracula-dark/dracula-light? if so, I don't understand the feature request - this should be solved with tailwind/css configuration
  2. mode-watcher should be able to handle additional theme values other than system, dark and light? such as system, dark, light, unicorn-puke, dracula-dark, dracula-light?

if 2), then we would have to figure out:

  • how to deal with toggleMode. I have no idea what it means to toggle unicorn-puke. it does make more sense to toggle dracula-light into dracula-dark but then we need an API that allows for passing pairs of themes to mode-watcher
  • how to track system preferences -> the correct theme, see [1].

I wonder if it would make sense to separate mode (as in dark mode, light mode, system mode) vs. theme (as in default, dracula...) and allow mode + theme to be managed by mode-watcher.

[1] this might even be needed if we want to support tracking system preferences with a non-default theme active. right?

I also wonder how much should be configured by mode-watcher vs. tailwind/css configuration

@huntabyte
Copy link
Member

@ollema, the scope would be limited to simply applying a data attribute at the same time we apply the color-scheme / class.

See how DaisyUI handles themes: https://daisyui.com/docs/themes/

The themes have a dark and a light variant, which is based on the system in our case based on mode-watcher, it's just a matter of applying such theme early enough to prevent FOUC and storing it in local storage. This won't replace the three modes - system, light, dark - rather an in addition to.

I was hesitant at first until i realized you basically have to reinvent mode-watcher to add this functionality to your app if the user is able to select from a list of different themes (like DaisyUI allows).


As for implementation, we'd likely just expose a setTheme(theme: string) and track it in a similar fashion that we track the mode. This is why I've been avoiding using the word "theme" internally and instead prefer "mode", as the two can coexist :)

@ollema
Copy link
Collaborator

ollema commented Mar 19, 2024

ah, thanks for the clarification. I think I got confused by:

It would be great if I could update the expected type and use setMode to set it to any of the types in that union.

in the original post. your setTheme suggestion makes more sense

@huntabyte
Copy link
Member

https://x.com/adamwathan/status/1770979061721919598?s=20

Here's more reasoning to do so :)

@wesharper
Copy link
Author

Reading thru the comments here. The ask is indeed to support additional themes, with the caveat that those themes can and should also specify what their mode should be for browser-rendered UI elements that need to coexist.

@shyakadavis
Copy link
Contributor

This indeed would be very much appreciated if it could land in the lib.

x.com/adamwathan/status/1770979061721919598?s=20
Here's more reasoning to do so :)

@huntabyte, @ollema is it safe to assume that this might land some time after Tailwind CSS V.4 lands? No pressure from me, just want to grab some context. 🙂

@jeffjose
Copy link

Any way we can prioritize this one? In addition to dark, light and system, it'll be nice to have custom theme support likesolarized, dracula etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request v1 roadmap
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants