-
-
Notifications
You must be signed in to change notification settings - Fork 295
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 new theming system #2833
Add new theming system #2833
Conversation
Not a fan of the new system from 2 angles:
|
The reason i can't just do that is because chakra has a lot of internal things that rely on the data-theme setting just being "dark" or "light". If i change that to "charcoal-light" it all breaks. though we could probably do something with the way that setting dropdown works to set the separate settings depending on which mode the theme is, so maybe that isn't really a concern. Anyway, I thought that this would just be a bit of a convenience thing to get both color modes per theme, but i can redo this in way where each theme is individually crafted all the time with lots of code duplication if you prefer it that way. |
Also, in terms of making it difficult/harder, due to the way I've set this up right now we would not need to test everything in every theme. Which colors are used for what is only determined by the dark and light modes. The colors themselves are chosen by the theme. So long as you use a standard array of shades for each theme, you shouldn't have any issues. Doing what you suggest would turn it into the concern you have about overcomplicating everything, since there would thereby be no standard for what colors map to what. |
Uhmmm, is that really the extent of the new system? I thought that you just made a super simple system to get started. If the theming system only allows themes to define the shades of gray used, then all themes are essentially just gray with a slight tint towards some color. So yes, this does solve the problem of having to design for many themes, but only because there will be only one theme, just with a different tint each time. This theming system wouldn't even allow themes to define accent colors, right? |
You can add individual color overrides to each theme, sure (though, I'm not actually sure if they would override the dark/light mode ones... I thought they would but I haven't tested it yet). But right now the extent of this is just having tinted gray, yes. I would like to build out a more extensive theming system in the future, but right now this simple one should allow us to include basic themes for those that want to use them without much hassle. |
If i were to combine the themes and just have Default (Dark), Default (Light), Charcoal (Dark), and Charcoal (Light), how would the "system" setting work? It's not a theme, so what theme would it pick for light mode or dark mode? Default? What if i want it to follow my system theme but but i want to use charcoal? The way I have it now would allow that |
Actually, we should just copy how vscode handles it: https://code.visualstudio.com/docs/getstarted/themes#_automatically-switch-based-on-os-color-scheme |
This theming system actually does allow overrides per theme, so that's cool |
So, I think all the concerns from this are lifted now. Dark and light themes are selected separately. I spent some time experimenting with what is possible under this system by making a few more themes. Overriding more than just the "gray" colors is possible after all, which led to me being able to make some interesting themes. Obviously it will require a larger refactor to make this more powerful, but I think this is a good starting point. Let me know if you think we should change or remove any of the themes I made. Also, we could hide this under the experimental features toggle if we don't want to expose this by default quite yet |
No, we should not. These settings are useful for when you share the same VSCode settings across multiples machines/people. Nobody uses them when just setting the theme of their machine. You completely overengineered this. The UX is also pretty bad too. Try explaining to a new user what all 3 settings do, when they just wanted to change the theme. And only one of those 2 theme settings actually does anything at a time. A simple dropdown would have been enough. You know, kinda like the one VSCode has: |
Ok, I'll just remove the "system" color mode option then, since that would not work with this |
Sounds good. Be sure to migrate settings though. |
I was being sarcastic but ok, sure. |
How exactly do I migrate settings under the new settings system? |
Co-authored-by: Michael Schmidt <[email protected]>
Co-authored-by: Michael Schmidt <[email protected]>
Co-authored-by: Michael Schmidt <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Right now themes are just based on the 0-900 token-based color scheming, but there's also no reason you couldn't add overrides for specific things in the [data={theme}] attributes if you wanted for a theme.
issues:
Example of new theme:

