settings_ui: Fix theme not being configurable in certain scenarios (#46069)

Finn Evers created 6 days ago

Closes https://github.com/zed-industries/zed/issues/44091

The issue here was that we were missing a proper default for
`ThemeSelection`. This is a disadvantage of not having the defaults in
code but actually in the `default.json`, so this here basically copies
the default from the `default.json` into code for the settings UI to
work properly.

Release Notes:

- Fixed an issue where the theme was not configurable from the settings
UI if no theme was configured prior.

Change summary

crates/settings/src/settings_content/theme.rs | 13 +++++++++++++
crates/settings_ui/src/page_data.rs           | 20 +++++++++-----------
crates/theme/src/settings.rs                  | 11 ++++-------
crates/theme/src/theme.rs                     |  1 +
4 files changed, 27 insertions(+), 18 deletions(-)

Detailed changes

crates/settings/src/settings_content/theme.rs

@@ -183,6 +183,19 @@ pub enum ThemeSelection {
     },
 }
 
+pub const DEFAULT_LIGHT_THEME: &'static str = "One Light";
+pub const DEFAULT_DARK_THEME: &'static str = "One Dark";
+
+impl Default for ThemeSelection {
+    fn default() -> Self {
+        Self::Dynamic {
+            mode: ThemeAppearanceMode::default(),
+            light: ThemeName(DEFAULT_LIGHT_THEME.into()),
+            dark: ThemeName(DEFAULT_DARK_THEME.into()),
+        }
+    }
+}
+
 /// Represents the selection of an icon theme, which can be either static or dynamic.
 #[derive(
     Clone,

crates/settings_ui/src/page_data.rs

@@ -313,9 +313,7 @@ pub(crate) fn settings_data(cx: &App) -> Vec<SettingsPage> {
                                     settings_content.theme.theme = None;
                                     return;
                                 };
-                                let settings_value = settings_content.theme.theme.get_or_insert_with(|| {
-                                    settings::ThemeSelection::Static(theme::ThemeName(theme::default_theme(theme::SystemAppearance::default().0).into()))
-                                });
+                                let settings_value = settings_content.theme.theme.get_or_insert_default();
                                 *settings_value = match value {
                                     settings::ThemeSelectionDiscriminants::Static => {
                                         let name = match settings_value {
@@ -371,8 +369,8 @@ pub(crate) fn settings_data(cx: &App) -> Vec<SettingsPage> {
                                             };
                                             match settings_content
                                                 .theme
-                                                .theme.as_mut() {
-                                                    Some(settings::ThemeSelection::Static(theme_name)) => *theme_name = value,
+                                                .theme.get_or_insert_default() {
+                                                    settings::ThemeSelection::Static(theme_name) => *theme_name = value,
                                                     _ => return
                                                 }
                                         },
@@ -399,8 +397,8 @@ pub(crate) fn settings_data(cx: &App) -> Vec<SettingsPage> {
                                             };
                                             match settings_content
                                                 .theme
-                                                .theme.as_mut() {
-                                                    Some(settings::ThemeSelection::Dynamic{ mode, ..}) => *mode = value,
+                                                .theme.get_or_insert_default() {
+                                                    settings::ThemeSelection::Dynamic{ mode, ..} => *mode = value,
                                                     _ => return
                                                 }
                                         },
@@ -425,8 +423,8 @@ pub(crate) fn settings_data(cx: &App) -> Vec<SettingsPage> {
                                             };
                                             match settings_content
                                                 .theme
-                                                .theme.as_mut() {
-                                                    Some(settings::ThemeSelection::Dynamic{ light, ..}) => *light = value,
+                                                .theme.get_or_insert_default() {
+                                                    settings::ThemeSelection::Dynamic{ light, ..} => *light = value,
                                                     _ => return
                                                 }
                                         },
@@ -451,8 +449,8 @@ pub(crate) fn settings_data(cx: &App) -> Vec<SettingsPage> {
                                             };
                                             match settings_content
                                                 .theme
-                                                .theme.as_mut() {
-                                                    Some(settings::ThemeSelection::Dynamic{ dark, ..}) => *dark = value,
+                                                .theme.get_or_insert_default() {
+                                                    settings::ThemeSelection::Dynamic{ dark, ..} => *dark = value,
                                                     _ => return
                                                 }
                                         },

crates/theme/src/settings.rs

@@ -138,14 +138,11 @@ pub struct ThemeSettings {
     pub unnecessary_code_fade: f32,
 }
 
-pub(crate) const DEFAULT_LIGHT_THEME: &'static str = "One Light";
-pub(crate) const DEFAULT_DARK_THEME: &'static str = "One Dark";
-
 /// Returns the name of the default theme for the given [`Appearance`].
 pub fn default_theme(appearance: Appearance) -> &'static str {
     match appearance {
-        Appearance::Light => DEFAULT_LIGHT_THEME,
-        Appearance::Dark => DEFAULT_DARK_THEME,
+        Appearance::Light => settings::DEFAULT_LIGHT_THEME,
+        Appearance::Dark => settings::DEFAULT_DARK_THEME,
     }
 }
 
@@ -400,8 +397,8 @@ pub fn set_mode(content: &mut SettingsContent, mode: ThemeAppearanceMode) {
     } else {
         theme.theme = Some(settings::ThemeSelection::Dynamic {
             mode,
-            light: ThemeName(DEFAULT_LIGHT_THEME.into()),
-            dark: ThemeName(DEFAULT_DARK_THEME.into()),
+            light: ThemeName(settings::DEFAULT_LIGHT_THEME.into()),
+            dark: ThemeName(settings::DEFAULT_DARK_THEME.into()),
         });
     }
 

crates/theme/src/theme.rs

@@ -22,6 +22,7 @@ mod styles;
 use std::path::Path;
 use std::sync::Arc;
 
+use ::settings::DEFAULT_DARK_THEME;
 use ::settings::Settings;
 use ::settings::SettingsStore;
 use anyhow::Result;