diff --git a/crates/onboarding/src/basics_page.rs b/crates/onboarding/src/basics_page.rs index 198871c9eb55b41c44a5f0db162eb446c1760ba9..ab5d578f7de731aff6be355b4d7ddb2c6cf95d57 100644 --- a/crates/onboarding/src/basics_page.rs +++ b/crates/onboarding/src/basics_page.rs @@ -220,7 +220,7 @@ fn render_theme_section(tab_index: &mut isize, cx: &mut App) -> impl IntoElement }); } else { let appearance = *SystemAppearance::global(cx); - theme::set_theme(settings, theme, appearance); + theme::set_theme(settings, theme, appearance, appearance); } }); } diff --git a/crates/theme/src/settings.rs b/crates/theme/src/settings.rs index a753859d52677974902a36a5d67ea86611e47006..d60d4882a68412eeb10a95ba5d8540f5cbc87421 100644 --- a/crates/theme/src/settings.rs +++ b/crates/theme/src/settings.rs @@ -304,31 +304,50 @@ impl IconThemeSelection { } } -// impl ThemeSettingsContent { /// Sets the theme for the given appearance to the theme with the specified name. +/// +/// The caller should make sure that the [`Appearance`] matches the theme associated with the name. +/// +/// If the current [`ThemeAppearanceMode`] is set to [`System`] and the user's system [`Appearance`] +/// is different than the new theme's [`Appearance`], this function will update the +/// [`ThemeAppearanceMode`] to the new theme's appearance in order to display the new theme. +/// +/// [`System`]: ThemeAppearanceMode::System pub fn set_theme( current: &mut SettingsContent, theme_name: impl Into>, - appearance: Appearance, + theme_appearance: Appearance, + system_appearance: Appearance, ) { - if let Some(selection) = current.theme.theme.as_mut() { - let theme_to_update = match selection { - settings::ThemeSelection::Static(theme) => theme, - settings::ThemeSelection::Dynamic { mode, light, dark } => match mode { - ThemeAppearanceMode::Light => light, - ThemeAppearanceMode::Dark => dark, - ThemeAppearanceMode::System => match appearance { - Appearance::Light => light, - Appearance::Dark => dark, - }, - }, - }; + let theme_name = ThemeName(theme_name.into()); - *theme_to_update = ThemeName(theme_name.into()); - } else { - current.theme.theme = Some(settings::ThemeSelection::Static(ThemeName( - theme_name.into(), - ))); + let Some(selection) = current.theme.theme.as_mut() else { + current.theme.theme = Some(settings::ThemeSelection::Static(theme_name)); + return; + }; + + match selection { + settings::ThemeSelection::Static(theme) => { + *theme = theme_name; + } + settings::ThemeSelection::Dynamic { mode, light, dark } => { + // Update the appropriate theme slot based on appearance. + match theme_appearance { + Appearance::Light => *light = theme_name, + Appearance::Dark => *dark = theme_name, + } + + // Don't update the theme mode if it is set to system and the new theme has the same + // appearance. + let should_update_mode = + !(mode == &ThemeAppearanceMode::System && theme_appearance == system_appearance); + + if should_update_mode { + // Update the mode to the specified appearance (otherwise we might set the theme and + // nothing gets updated because the system specified the other mode appearance). + *mode = ThemeAppearanceMode::from(theme_appearance); + } + } } } diff --git a/crates/theme/src/theme.rs b/crates/theme/src/theme.rs index aef975416f7698bfad0ba50de08269c9000a1dec..c94e0d60bf3fa561c8d49dbaf544f47fe60e7ea9 100644 --- a/crates/theme/src/theme.rs +++ b/crates/theme/src/theme.rs @@ -84,6 +84,15 @@ impl From for Appearance { } } +impl From for ThemeAppearanceMode { + fn from(value: Appearance) -> Self { + match value { + Appearance::Light => Self::Light, + Appearance::Dark => Self::Dark, + } + } +} + /// Which themes should be loaded. This is used primarily for testing. pub enum LoadThemes { /// Only load the base theme. diff --git a/crates/theme_selector/src/theme_selector.rs b/crates/theme_selector/src/theme_selector.rs index 38e7fc33f7b14f198679d0dd541c39cd444a71a3..74b242dd0b7c3a3ddbe6ca76d34a59f03560f14a 100644 --- a/crates/theme_selector/src/theme_selector.rs +++ b/crates/theme_selector/src/theme_selector.rs @@ -9,7 +9,10 @@ use gpui::{ use picker::{Picker, PickerDelegate}; use settings::{Settings, SettingsStore, update_settings_file}; use std::sync::Arc; -use theme::{Appearance, Theme, ThemeMeta, ThemeRegistry, ThemeSettings}; +use theme::{ + Appearance, SystemAppearance, Theme, ThemeAppearanceMode, ThemeMeta, ThemeName, ThemeRegistry, + ThemeSelection, ThemeSettings, +}; use ui::{ListItem, ListItemSpacing, prelude::*, v_flex}; use util::ResultExt; use workspace::{ModalView, Workspace, ui::HighlightedLabel, with_active_or_new_workspace}; @@ -114,7 +117,14 @@ struct ThemeSelectorDelegate { fs: Arc, themes: Vec, matches: Vec, - original_theme: Arc, + /// The theme that was selected before the `ThemeSelector` menu was opened. + /// + /// We use this to return back to theme that was set if the user dismisses the menu. + original_theme_settings: ThemeSettings, + /// The current system appearance. + original_system_appearance: Appearance, + /// The currently selected new theme. + new_theme: Arc, selection_completed: bool, selected_theme: Option>, selected_index: usize, @@ -129,6 +139,8 @@ impl ThemeSelectorDelegate { cx: &mut Context, ) -> Self { let original_theme = cx.theme().clone(); + let original_theme_settings = ThemeSettings::get_global(cx).clone(); + let original_system_appearance = SystemAppearance::global(cx).0; let registry = ThemeRegistry::global(cx); let mut themes = registry @@ -143,13 +155,15 @@ impl ThemeSelectorDelegate { }) .collect::>(); + // Sort by dark vs light, then by name. themes.sort_unstable_by(|a, b| { a.appearance .is_light() .cmp(&b.appearance.is_light()) .then(a.name.cmp(&b.name)) }); - let matches = themes + + let matches: Vec = themes .iter() .map(|meta| StringMatch { candidate_id: 0, @@ -158,19 +172,25 @@ impl ThemeSelectorDelegate { string: meta.name.to_string(), }) .collect(); - let mut this = Self { + + // The current theme is likely in this list, so default to first showing that. + let selected_index = matches + .iter() + .position(|mat| mat.string == original_theme.name) + .unwrap_or(0); + + Self { fs, themes, matches, - original_theme: original_theme.clone(), - selected_index: 0, + original_theme_settings, + original_system_appearance, + new_theme: original_theme, // Start with the original theme. + selected_index, selection_completed: false, selected_theme: None, selector, - }; - - this.select_if_matching(&original_theme.name); - this + } } fn show_selected_theme( @@ -179,9 +199,10 @@ impl ThemeSelectorDelegate { ) -> Option> { if let Some(mat) = self.matches.get(self.selected_index) { let registry = ThemeRegistry::global(cx); + match registry.get(&mat.string) { Ok(theme) => { - Self::set_theme(theme.clone(), cx); + self.set_theme(theme.clone(), cx); Some(theme) } Err(error) => { @@ -194,21 +215,122 @@ impl ThemeSelectorDelegate { } } - fn select_if_matching(&mut self, theme_name: &str) { - self.selected_index = self - .matches - .iter() - .position(|mat| mat.string == theme_name) - .unwrap_or(self.selected_index); - } - - fn set_theme(theme: Arc, cx: &mut App) { + fn set_theme(&mut self, new_theme: Arc, cx: &mut App) { + // Update the global (in-memory) theme settings. SettingsStore::update_global(cx, |store, _| { - let mut theme_settings = store.get::(None).clone(); - let name = theme.as_ref().name.clone().into(); - theme_settings.theme = theme::ThemeSelection::Static(theme::ThemeName(name)); - store.override_global(theme_settings); + override_global_theme( + store, + &new_theme, + &self.original_theme_settings.theme, + self.original_system_appearance, + ) }); + + self.new_theme = new_theme; + } +} + +/// Overrides the global (in-memory) theme settings. +/// +/// Note that this does **not** update the user's `settings.json` file (see the +/// [`ThemeSelectorDelegate::confirm`] method and [`theme::set_theme`] function). +fn override_global_theme( + store: &mut SettingsStore, + new_theme: &Theme, + original_theme: &ThemeSelection, + system_appearance: Appearance, +) { + let theme_name = ThemeName(new_theme.name.clone().into()); + let new_appearance = new_theme.appearance(); + let new_theme_is_light = new_appearance.is_light(); + + let mut curr_theme_settings = store.get::(None).clone(); + + match (original_theme, &curr_theme_settings.theme) { + // Override the currently selected static theme. + (ThemeSelection::Static(_), ThemeSelection::Static(_)) => { + curr_theme_settings.theme = ThemeSelection::Static(theme_name); + } + + // If the current theme selection is dynamic, then only override the global setting for the + // specific mode (light or dark). + ( + ThemeSelection::Dynamic { + mode: original_mode, + light: original_light, + dark: original_dark, + }, + ThemeSelection::Dynamic { .. }, + ) => { + let new_mode = update_mode_if_new_appearance_is_different_from_system( + original_mode, + system_appearance, + new_appearance, + ); + + let updated_theme = retain_original_opposing_theme( + new_theme_is_light, + new_mode, + theme_name, + original_light, + original_dark, + ); + + curr_theme_settings.theme = updated_theme; + } + + // The theme selection mode changed while selecting new themes (someone edited the settings + // file on disk while we had the dialogue open), so don't do anything. + _ => return, + }; + + store.override_global(curr_theme_settings); +} + +/// Helper function for determining the new [`ThemeAppearanceMode`] for the new theme. +/// +/// If the the original theme mode was [`System`] and the new theme's appearance matches the system +/// appearance, we don't need to change the mode setting. +/// +/// Otherwise, we need to change the mode in order to see the new theme. +/// +/// [`System`]: ThemeAppearanceMode::System +fn update_mode_if_new_appearance_is_different_from_system( + original_mode: &ThemeAppearanceMode, + system_appearance: Appearance, + new_appearance: Appearance, +) -> ThemeAppearanceMode { + if original_mode == &ThemeAppearanceMode::System && system_appearance == new_appearance { + ThemeAppearanceMode::System + } else { + ThemeAppearanceMode::from(new_appearance) + } +} + +/// Helper function for updating / displaying the [`ThemeSelection`] while using the theme selector. +/// +/// We want to retain the alternate theme selection of the original settings (before the menu was +/// opened), not the currently selected theme (which likely has changed multiple times while the +/// menu has been open). +fn retain_original_opposing_theme( + new_theme_is_light: bool, + new_mode: ThemeAppearanceMode, + theme_name: ThemeName, + original_light: &ThemeName, + original_dark: &ThemeName, +) -> ThemeSelection { + if new_theme_is_light { + ThemeSelection::Dynamic { + mode: new_mode, + light: theme_name, + dark: original_dark.clone(), + } + } else { + ThemeSelection::Dynamic { + mode: new_mode, + light: original_light.clone(), + dark: theme_name, + } } } @@ -225,19 +347,20 @@ impl PickerDelegate for ThemeSelectorDelegate { fn confirm( &mut self, - _: bool, - window: &mut Window, + _secondary: bool, + _window: &mut Window, cx: &mut Context>, ) { self.selection_completed = true; - let appearance = Appearance::from(window.appearance()); - let theme_name = ThemeSettings::get_global(cx).theme.name(appearance).0; + let theme_name: Arc = self.new_theme.name.as_str().into(); + let theme_appearance = self.new_theme.appearance; + let system_appearance = SystemAppearance::global(cx).0; telemetry::event!("Settings Changed", setting = "theme", value = theme_name); update_settings_file(self.fs.clone(), cx, move |settings, _| { - theme::set_theme(settings, theme_name.to_string(), appearance); + theme::set_theme(settings, theme_name, theme_appearance, system_appearance); }); self.selector @@ -249,7 +372,9 @@ impl PickerDelegate for ThemeSelectorDelegate { fn dismissed(&mut self, _: &mut Window, cx: &mut Context>) { if !self.selection_completed { - Self::set_theme(self.original_theme.clone(), cx); + SettingsStore::update_global(cx, |store, _| { + store.override_global(self.original_theme_settings.clone()); + }); self.selection_completed = true; }