zed
zed copied to clipboard
Auto switching light/dark theme according to system settings
Resolves #4970 Resolves #5326
Checklist
- [x] Added
themesconfiguration - [x] ~~Added an action to toggle between different theme modes~~ Removed as
automode has been implemented - [x] Added appearance observer to GPUI
- [x] Subscribe appearance change and change theme
We require contributors to sign our Contributor License Agreement, and we don't have @Yesterday17 on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.
@cla-bot check
The cla-bot has been summoned, and re-checked this pull request!
When the editor is launched in a mode that is not equal to the system appearance, it won't change themes. Only when the system appearance transition is triggered the theme is switched.
Maybe we can introduce a callback on the launch of the editor that checks for the current system appearance?
When the editor is launched in a mode that is not equal to the system appearance, it won't change themes.
It seems that https://github.com/zed-industries/zed/blob/e4ae67fd6f2e3c9318ccf2770a34f25ca10f7d25/crates/workspace/src/workspace.rs#L626 is not early enough for themes to know the system appearance. Let me check how to fix it.
This PR is awesome! I did a few tiny touch ups and adjusted how the theme is reloaded. As soon as CI passes, I'll merge this. Great work :)
@mikayla-maki Let's hold off on merging this one until I figure out how it works into the theme stuff I'm working on now.
Want to make sure we have a cohesive narrative around theming.
As @maxdeviant has informed me, this PR is blocked on https://github.com/zed-industries/zed/pull/7027
Shoulda looked more carefully at our PRs :D
While the detection on editor startup now works, there is some trouble left with more than one system theme change.
Notably the themes.list() of the ThemeRegistry returns only one theme (One Dark). As follows the .get(name) method also fails on a differing name, resulting in no theme change, even when the system theme changes.
I think I'm going to get this shipped @Yesterday17, thank you for getting the ball rolling :D
I think I'm going to get this shipped @Yesterday17, thank you for getting the ball rolling :D
@mikayla-maki I was thinking it'd be nice to get the GPUI changes landed first, as those seem pretty independent.
And then I'd be down to pair on how we expose this in the settings, as I think there's some refinement we could do there.
@maxdeviant which GPUI changes are you referring to?
@Yesterday17 Thank you for your work on this!
We ended up taking a slightly different approach with #7404, but we appreciate your jumping in and getting the ball rolling on this feature 🙂