-
Notifications
You must be signed in to change notification settings - Fork 136
WIP feat: Theme applet #1166
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?
WIP feat: Theme applet #1166
Conversation
mmstick
left a comment
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.
Not sure if we want this at the moment, but these would need to be fixed.
| core: cosmic::app::Core, | ||
| icon_name: String, | ||
| popup: Option<window::Id>, | ||
| is_dark: bool, |
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.
This property isn't needed because it's managed by libcosmic
| fn subscription(&self) -> cosmic::iced::Subscription<Message> { | ||
| cosmic::iced::Subscription::batch(vec![ | ||
| theme_subscription::theme_subscription(self.theme_file.clone()) | ||
| .map(Message::ThemeUpdate), |
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.
A subscription for theme changes is not necessary since you can get that from libcosmic directly.
| let toggle = cosmic::widget::button::custom( | ||
| widget::container( | ||
| row![ | ||
| icon::from_name(if self.is_dark { |
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.
You can use cosmic::theme::is_dark() instead.
| enum Message { | ||
| Action(ThemeAction), | ||
| Closed(window::Id), | ||
| ThemeUpdate(theme_subscription::ThemeUpdate), |
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.
This variant can be removed because libcosmic is already subscribed to theme changes on startup
| } | ||
| fs::write(&theme_file, is_dark.to_string()) | ||
| .expect("Failed to write initial theme state"); | ||
| } |
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.
Shouldn't need to write this file, or assume that dark is the system default.
| "weather-sunny-symbolic" | ||
| } | ||
| .to_string(); | ||
| Task::none() |
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.
It would be better to toggle it through libcosmic
use cosmic::cosmic_config::ConfigSet;
use cosmic::cosmic_theme::ThemeMode;ToggleTheme => {
if let Ok(config) = ThemeMode::config() {
config.set::<bool>("is_dark", !cosmic::theme::is_dark());
}
}| "weather-clear-night-symbolic" | ||
| } else { | ||
| "weather-sunny-symbolic" | ||
| } |
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.
Better to cache the icon handle instead of the name
widget::icon::from_name("icon-name").handle()| ThemeAction::ToggleTheme => { | ||
| self.is_dark = !self.is_dark; | ||
| fs::write(&self.theme_file, self.is_dark.to_string()) | ||
| .expect("Failed to write theme state"); |
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.
Shouldn't write to files directly here. See cosmic_config below.
| Changed(bool), // is_dark | ||
| } | ||
|
|
||
| pub fn theme_subscription(theme_file: PathBuf) -> Subscription<ThemeUpdate> { |
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.
This file can be removed. In the future you'd want to use the application's core to subscribe to a cosmic_config interface.
self.core.watch_config::<MyAppConfig>("com.github.MyAppName")
Applet for switching Light/Dark theme
Fixing #739