-
-
Notifications
You must be signed in to change notification settings - Fork 73
feat(theme): add new light/dark variants and make default themes follow system color scheme #613
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
base: master
Are you sure you want to change the base?
Conversation
|
I have been considering this option for a while now, and I'm glad to see there is demand for it and that you decided to implement this already. I do think this is a complex feature, because not all would want this, so there should indeed be separate light and dark themes, which you took care of nicely. On the other hand, I am not sure about the approach of replacing default with dark for internal code. Shouldn't the theme be reflected in these parts too? Usually the entire reason the default css is loaded is to apply the default colors, which are then possibly overridden by the active theme. So if we change the default theme to adapt to light and dark, I expect the interface to do so in all places, unless I'm missing some issues with this reasoning you encountered while implementing. Overall, I'm glad there's development on this, we can discuss it further here before we decide to merge this in, which will only happen when I start working on the v13 branch anyway, as this is definitely a breaking change. |
d7920be to
0fb3826
Compare
My intention was to maintain the logic which makes other colorschemes inherit from the default one, since now it has changed to |
|
Since this would only be merged as a breaking change anyway in the upcoming v13 branch, I don't mind having only an updated default.css, not a new-default. So basically: I would prefer this PR to show me the ideal end-state diff without migrations or handling of existing features, merely changing the defaults to do this, and have separate themes dark/light that don't. |
|
Assuming I understood correctly, with this latest push there’s only one hurdle left: the stylesheet loaded by default for Vieb pages is the same one that other colorschemes inherit from—namely To fix this, I just need to figure out how to manually trigger the loading of |
|
Currently there is still a lot of code moved from default to dark, and as such to any other built-in theme using these colors. Ideally only the default.css file should be have a lot of changes to achieve this functionality, and have it affect all places as a result automatically. A new dark theme and the current light theme should then simply undo the automatic switching and probably not have much other styling in there. As well as there being little to no changes needed to other themes. |
…ow system color scheme
|
I moved automatic switching into |
Since recently
set nativetheme=systemwas revived, and with it becoming the default in 13.0.0, I thought it would make sense to also dynamically adjust Vieb's own colorscheme based on system preference.I did a naive search and replace for
default.csstodark.cssI doubt it is desireable to leave it as is, but it works as far as I tested.This is a draft mainly because I want feedback about how to handle
default.cssanddark.cssand ask if perhaps it's best to add css importing instead of repeating the light colorscheme everywhere, I didn't make it into an issue because I also want it to be available to anyone who wants to try it out.