onboarding: Expand power of theme selector (#35421)

Closes #ISSUE

The behavior of the theme selector is documented above the function,
copied here for reference:
```rust
/// separates theme "mode" ("dark" | "light" | "system") into two separate states
/// - appearance = "dark" | "light"
/// - "system" true/false
/// when system selected:
///  - toggling between light and dark does not change theme.mode, just which variant will be changed
/// when system not selected:
///  - toggling between light and dark does change theme.mode
/// selecting a theme preview will always change theme.["light" | "dark"] to the selected theme,
///
/// this allows for selecting a dark and light theme option regardless of whether the mode is set to system or not
/// it does not support setting theme to a static value
```

Release Notes:

- N/A *or* Added/Fixed/Improved ...

Cargo.lock

@@ -10925,7 +10925,9 @@ dependencies = [
  "anyhow",
  "client",
  "command_palette_hooks",
+ "component",
  "db",
+ "documented",
  "editor",
  "feature_flags",
  "fs",

crates/onboarding/Cargo.toml

@@ -18,6 +18,8 @@ default = []
 anyhow.workspace = true
 client.workspace = true
 command_palette_hooks.workspace = true
+component.workspace = true
+documented.workspace = true
 db.workspace = true
 editor.workspace = true
 feature_flags.workspace = true

crates/onboarding/src/basics_page.rs

@@ -1,36 +1,228 @@
-use std::sync::Arc;
-
 use client::TelemetrySettings;
 use fs::Fs;
-use gpui::{App, IntoElement};
+use gpui::{App, Entity, IntoElement, Window};
 use settings::{BaseKeymap, Settings, update_settings_file};
-use theme::{Appearance, SystemAppearance, ThemeMode, ThemeSettings};
+use theme::{Appearance, ThemeMode, ThemeName, ThemeRegistry, ThemeSelection, ThemeSettings};
 use ui::{
-    SwitchField, ThemePreviewTile, ToggleButtonGroup, ToggleButtonSimple, ToggleButtonWithIcon,
-    prelude::*,
+    ParentElement as _, StatefulInteractiveElement, SwitchField, ToggleButtonGroup,
+    ToggleButtonSimple, ToggleButtonWithIcon, prelude::*, rems_from_px,
 };
 use vim_mode_setting::VimModeSetting;
 
-use crate::Onboarding;
+use crate::theme_preview::ThemePreviewTile;
 
-fn read_theme_selection(cx: &App) -> (ThemeMode, SharedString) {
-    let settings = ThemeSettings::get_global(cx);
-    (
-        settings
-            .theme_selection
+/// separates theme "mode" ("dark" | "light" | "system") into two separate states
+/// - appearance = "dark" | "light"
+/// - "system" true/false
+/// when system selected:
+///  - toggling between light and dark does not change theme.mode, just which variant will be changed
+/// when system not selected:
+///  - toggling between light and dark does change theme.mode
+/// selecting a theme preview will always change theme.["light" | "dark"] to the selected theme,
+///
+/// this allows for selecting a dark and light theme option regardless of whether the mode is set to system or not
+/// it does not support setting theme to a static value
+fn render_theme_section(window: &mut Window, cx: &mut App) -> impl IntoElement {
+    let theme_selection = ThemeSettings::get_global(cx).theme_selection.clone();
+    let system_appearance = theme::SystemAppearance::global(cx);
+    let appearance_state = window.use_state(cx, |_, _cx| {
+        theme_selection
             .as_ref()
             .and_then(|selection| selection.mode())
-            .unwrap_or_default(),
-        settings.active_theme.name.clone(),
-    )
-}
-
-fn write_theme_selection(theme_mode: ThemeMode, cx: &App) {
-    let fs = <dyn Fs>::global(cx);
-
-    update_settings_file::<ThemeSettings>(fs, cx, move |settings, _| {
-        settings.set_mode(theme_mode);
+            .and_then(|mode| match mode {
+                ThemeMode::System => None,
+                ThemeMode::Light => Some(Appearance::Light),
+                ThemeMode::Dark => Some(Appearance::Dark),
+            })
+            .unwrap_or(*system_appearance)
     });
+    let appearance = *appearance_state.read(cx);
+    let theme_selection = theme_selection.unwrap_or_else(|| ThemeSelection::Dynamic {
+        mode: match *system_appearance {
+            Appearance::Light => ThemeMode::Light,
+            Appearance::Dark => ThemeMode::Dark,
+        },
+        light: ThemeName("One Light".into()),
+        dark: ThemeName("One Dark".into()),
+    });
+    let theme_registry = ThemeRegistry::global(cx);
+
+    let current_theme_name = theme_selection.theme(appearance);
+    let theme_mode = theme_selection.mode();
+
+    let selected_index = match appearance {
+        Appearance::Light => 0,
+        Appearance::Dark => 1,
+    };
+
+    let theme_seed = 0xBEEF as f32;
+
+    const LIGHT_THEMES: [&'static str; 3] = ["One Light", "Ayu Light", "Gruvbox Light"];
+    const DARK_THEMES: [&'static str; 3] = ["One Dark", "Ayu Dark", "Gruvbox Dark"];
+
+    let theme_names = match appearance {
+        Appearance::Light => LIGHT_THEMES,
+        Appearance::Dark => DARK_THEMES,
+    };
+    let themes = theme_names
+        .map(|theme_name| theme_registry.get(theme_name))
+        .map(Result::unwrap);
+
+    let theme_previews = themes.map(|theme| {
+        let is_selected = theme.name == current_theme_name;
+        let name = theme.name.clone();
+        let colors = cx.theme().colors();
+        v_flex()
+            .id(name.clone())
+            .on_click({
+                let theme_name = theme.name.clone();
+                move |_, _, cx| {
+                    let fs = <dyn Fs>::global(cx);
+                    let theme_name = theme_name.clone();
+                    update_settings_file::<ThemeSettings>(fs, cx, move |settings, _| {
+                        settings.set_theme(theme_name, appearance);
+                    });
+                }
+            })
+            .flex_1()
+            .child(
+                div()
+                    .border_2()
+                    .border_color(colors.border_transparent)
+                    .rounded(ThemePreviewTile::CORNER_RADIUS)
+                    .hover(|mut style| {
+                        if !is_selected {
+                            style.border_color = Some(colors.element_hover);
+                        }
+                        style
+                    })
+                    .when(is_selected, |this| {
+                        this.border_color(colors.border_selected)
+                    })
+                    .cursor_pointer()
+                    .child(ThemePreviewTile::new(theme, theme_seed)),
+            )
+            .child(
+                h_flex()
+                    .justify_center()
+                    .items_baseline()
+                    .child(Label::new(name).color(Color::Muted)),
+            )
+    });
+
+    return v_flex()
+        .child(
+            h_flex().justify_between().child(Label::new("Theme")).child(
+                h_flex()
+                    .gap_2()
+                    .child(
+                        ToggleButtonGroup::single_row(
+                            "theme-selector-onboarding-dark-light",
+                            [
+                                ToggleButtonSimple::new("Light", {
+                                    let appearance_state = appearance_state.clone();
+                                    move |_, _, cx| {
+                                        write_appearance_change(
+                                            &appearance_state,
+                                            Appearance::Light,
+                                            cx,
+                                        );
+                                    }
+                                }),
+                                ToggleButtonSimple::new("Dark", {
+                                    let appearance_state = appearance_state.clone();
+                                    move |_, _, cx| {
+                                        write_appearance_change(
+                                            &appearance_state,
+                                            Appearance::Dark,
+                                            cx,
+                                        );
+                                    }
+                                }),
+                            ],
+                        )
+                        .selected_index(selected_index)
+                        .style(ui::ToggleButtonGroupStyle::Outlined)
+                        .button_width(rems_from_px(64.)),
+                    )
+                    .child(
+                        ToggleButtonGroup::single_row(
+                            "theme-selector-onboarding-system",
+                            [ToggleButtonSimple::new("System", {
+                                let theme = theme_selection.clone();
+                                move |_, _, cx| {
+                                    toggle_system_theme_mode(theme.clone(), appearance, cx);
+                                }
+                            })],
+                        )
+                        .selected_index((theme_mode != Some(ThemeMode::System)) as usize)
+                        .style(ui::ToggleButtonGroupStyle::Outlined)
+                        .button_width(rems_from_px(64.)),
+                    ),
+            ),
+        )
+        .child(h_flex().justify_between().children(theme_previews));
+
+    fn write_appearance_change(
+        appearance_state: &Entity<Appearance>,
+        new_appearance: Appearance,
+        cx: &mut App,
+    ) {
+        appearance_state.update(cx, |appearance, _| {
+            *appearance = new_appearance;
+        });
+        let fs = <dyn Fs>::global(cx);
+
+        update_settings_file::<ThemeSettings>(fs, cx, move |settings, _| {
+            if settings.theme.as_ref().and_then(ThemeSelection::mode) == Some(ThemeMode::System) {
+                return;
+            }
+            let new_mode = match new_appearance {
+                Appearance::Light => ThemeMode::Light,
+                Appearance::Dark => ThemeMode::Dark,
+            };
+            settings.set_mode(new_mode);
+        });
+    }
+
+    fn toggle_system_theme_mode(
+        theme_selection: ThemeSelection,
+        appearance: Appearance,
+        cx: &mut App,
+    ) {
+        let fs = <dyn Fs>::global(cx);
+
+        update_settings_file::<ThemeSettings>(fs, cx, move |settings, _| {
+            settings.theme = Some(match theme_selection {
+                ThemeSelection::Static(theme_name) => ThemeSelection::Dynamic {
+                    mode: ThemeMode::System,
+                    light: theme_name.clone(),
+                    dark: theme_name.clone(),
+                },
+                ThemeSelection::Dynamic {
+                    mode: ThemeMode::System,
+                    light,
+                    dark,
+                } => {
+                    let mode = match appearance {
+                        Appearance::Light => ThemeMode::Light,
+                        Appearance::Dark => ThemeMode::Dark,
+                    };
+                    ThemeSelection::Dynamic { mode, light, dark }
+                }
+
+                ThemeSelection::Dynamic {
+                    mode: _,
+                    light,
+                    dark,
+                } => ThemeSelection::Dynamic {
+                    mode: ThemeMode::System,
+                    light,
+                    dark,
+                },
+            });
+        });
+    }
 }
 
 fn write_keymap_base(keymap_base: BaseKeymap, cx: &App) {
@@ -41,35 +233,10 @@ fn write_keymap_base(keymap_base: BaseKeymap, cx: &App) {
     });
 }
 
-fn render_theme_section(theme_mode: ThemeMode) -> impl IntoElement {
-    h_flex().justify_between().child(Label::new("Theme")).child(
-        ToggleButtonGroup::single_row(
-            "theme-selector-onboarding",
-            [
-                ToggleButtonSimple::new("Light", |_, _, cx| {
-                    write_theme_selection(ThemeMode::Light, cx)
-                }),
-                ToggleButtonSimple::new("Dark", |_, _, cx| {
-                    write_theme_selection(ThemeMode::Dark, cx)
-                }),
-                ToggleButtonSimple::new("System", |_, _, cx| {
-                    write_theme_selection(ThemeMode::System, cx)
-                }),
-            ],
-        )
-        .selected_index(match theme_mode {
-            ThemeMode::Light => 0,
-            ThemeMode::Dark => 1,
-            ThemeMode::System => 2,
-        })
-        .style(ui::ToggleButtonGroupStyle::Outlined)
-        .button_width(rems_from_px(64.)),
-    )
-}
+fn render_telemetry_section(cx: &App) -> impl IntoElement {
+    let fs = <dyn Fs>::global(cx);
 
-fn render_telemetry_section(fs: Arc<dyn Fs>, cx: &App) -> impl IntoElement {
     v_flex()
-
         .gap_4()
         .child(Label::new("Telemetry").size(LabelSize::Large))
         .child(SwitchField::new(
@@ -125,17 +292,7 @@ fn render_telemetry_section(fs: Arc<dyn Fs>, cx: &App) -> impl IntoElement {
         ))
 }
 
-pub(crate) fn render_basics_page(onboarding: &Onboarding, cx: &mut App) -> impl IntoElement {
-    let (theme_mode, active_theme_name) = read_theme_selection(cx);
-    let themes = match theme_mode {
-        ThemeMode::Dark => &onboarding.dark_themes,
-        ThemeMode::Light => &onboarding.light_themes,
-        ThemeMode::System => match SystemAppearance::global(cx).0 {
-            Appearance::Light => &onboarding.light_themes,
-            Appearance::Dark => &onboarding.dark_themes,
-        },
-    };
-
+pub(crate) fn render_basics_page(window: &mut Window, cx: &mut App) -> impl IntoElement {
     let base_keymap = match BaseKeymap::get_global(cx) {
         BaseKeymap::VSCode => Some(0),
         BaseKeymap::JetBrains => Some(1),
@@ -148,22 +305,7 @@ pub(crate) fn render_basics_page(onboarding: &Onboarding, cx: &mut App) -> impl
 
     v_flex()
         .gap_6()
-        .child(render_theme_section(theme_mode))
-        .child(h_flex().children(
-            themes.iter().map(|theme| {
-                ThemePreviewTile::new(theme.clone(), active_theme_name == theme.name, 0.48)
-                .on_click({
-                    let theme_name = theme.name.clone();
-                    let fs = onboarding.fs.clone();
-                    move |_, _, cx| {
-                        let theme_name = theme_name.clone();
-                        update_settings_file::<ThemeSettings>(fs.clone(), cx, move |settings, cx| {
-                            settings.set_theme(theme_name.to_string(), SystemAppearance::global(cx).0);
-                        });
-                    }
-                })
-            })
-        ))
+         .child(render_theme_section(window, cx))
         .child(
             v_flex().gap_2().child(Label::new("Base Keymap")).child(
                 ToggleButtonGroup::two_rows(
@@ -206,7 +348,7 @@ pub(crate) fn render_basics_page(onboarding: &Onboarding, cx: &mut App) -> impl
                 ui::ToggleState::Unselected
             },
             {
-                let fs = onboarding.fs.clone();
+                let fs = <dyn Fs>::global(cx);
                 move |selection, _, cx| {
                     let enabled = match selection {
                         ToggleState::Selected => true,
@@ -222,5 +364,5 @@ pub(crate) fn render_basics_page(onboarding: &Onboarding, cx: &mut App) -> impl
                 }
             },
         )))
-        .child(render_telemetry_section(onboarding.fs.clone(), cx))
+        .child(render_telemetry_section(cx))
 }

crates/onboarding/src/onboarding.rs

@@ -13,8 +13,10 @@ use schemars::JsonSchema;
 use serde::Deserialize;
 use settings::{SettingsStore, VsCodeSettingsSource};
 use std::sync::Arc;
-use theme::{Theme, ThemeRegistry};
-use ui::{Avatar, FluentBuilder, KeyBinding, Vector, VectorName, prelude::*, rems_from_px};
+use ui::{
+    Avatar, FluentBuilder, Headline, KeyBinding, ParentElement as _, StatefulInteractiveElement,
+    Vector, VectorName, prelude::*, rems_from_px,
+};
 use workspace::{
     AppState, Workspace, WorkspaceId,
     dock::DockPosition,
@@ -25,6 +27,7 @@ use workspace::{
 
 mod basics_page;
 mod editing_page;
+mod theme_preview;
 mod welcome;
 
 pub struct OnBoardingFeatureFlag {}
@@ -219,11 +222,8 @@ enum SelectedPage {
 
 struct Onboarding {
     workspace: WeakEntity<Workspace>,
-    light_themes: [Arc<Theme>; 3],
-    dark_themes: [Arc<Theme>; 3],
     focus_handle: FocusHandle,
     selected_page: SelectedPage,
-    fs: Arc<dyn Fs>,
     user_store: Entity<UserStore>,
     _settings_subscription: Subscription,
 }
@@ -234,36 +234,11 @@ impl Onboarding {
         user_store: Entity<UserStore>,
         cx: &mut App,
     ) -> Entity<Self> {
-        let theme_registry = ThemeRegistry::global(cx);
-
-        let one_dark = theme_registry
-            .get("One Dark")
-            .expect("Default themes are always present");
-        let ayu_dark = theme_registry
-            .get("Ayu Dark")
-            .expect("Default themes are always present");
-        let gruvbox_dark = theme_registry
-            .get("Gruvbox Dark")
-            .expect("Default themes are always present");
-
-        let one_light = theme_registry
-            .get("One Light")
-            .expect("Default themes are always present");
-        let ayu_light = theme_registry
-            .get("Ayu Light")
-            .expect("Default themes are always present");
-        let gruvbox_light = theme_registry
-            .get("Gruvbox Light")
-            .expect("Default themes are always present");
-
         cx.new(|cx| Self {
             workspace,
             user_store,
             focus_handle: cx.focus_handle(),
-            light_themes: [one_light, ayu_light, gruvbox_light],
-            dark_themes: [one_dark, ayu_dark, gruvbox_dark],
             selected_page: SelectedPage::Basics,
-            fs: <dyn Fs>::global(cx),
             _settings_subscription: cx.observe_global::<SettingsStore>(move |_, cx| cx.notify()),
         })
     }
@@ -411,7 +386,7 @@ impl Onboarding {
     fn render_page(&mut self, window: &mut Window, cx: &mut Context<Self>) -> AnyElement {
         match self.selected_page {
             SelectedPage::Basics => {
-                crate::basics_page::render_basics_page(&self, cx).into_any_element()
+                crate::basics_page::render_basics_page(window, cx).into_any_element()
             }
             SelectedPage::Editing => {
                 crate::editing_page::render_editing_page(window, cx).into_any_element()

crates/onboarding/src/theme_preview.rs

@@ -1,47 +1,32 @@
-use crate::{component_prelude::Documented, prelude::*, utils::inner_corner_radius};
-use gpui::{App, ClickEvent, Hsla, IntoElement, Length, RenderOnce, Window};
-use std::{rc::Rc, sync::Arc};
+#![allow(unused, dead_code)]
+use gpui::{Hsla, Length};
+use std::sync::Arc;
 use theme::{Theme, ThemeRegistry};
+use ui::{
+    IntoElement, RenderOnce, component_prelude::Documented, prelude::*, utils::inner_corner_radius,
+};
 
 /// Shows a preview of a theme as an abstract illustration
 /// of a thumbnail-sized editor.
 #[derive(IntoElement, RegisterComponent, Documented)]
 pub struct ThemePreviewTile {
     theme: Arc<Theme>,
-    selected: bool,
-    on_click: Option<Rc<dyn Fn(&ClickEvent, &mut Window, &mut App)>>,
     seed: f32,
 }
 
 impl ThemePreviewTile {
-    pub fn new(theme: Arc<Theme>, selected: bool, seed: f32) -> Self {
-        Self {
-            theme,
-            seed,
-            selected,
-            on_click: None,
-        }
-    }
+    pub const CORNER_RADIUS: Pixels = px(8.0);
 
-    pub fn selected(mut self, selected: bool) -> Self {
-        self.selected = selected;
-        self
-    }
-
-    pub fn on_click(
-        mut self,
-        listener: impl Fn(&ClickEvent, &mut Window, &mut App) + 'static,
-    ) -> Self {
-        self.on_click = Some(Rc::new(listener));
-        self
+    pub fn new(theme: Arc<Theme>, seed: f32) -> Self {
+        Self { theme, seed }
     }
 }
 
 impl RenderOnce for ThemePreviewTile {
-    fn render(self, _window: &mut Window, _cx: &mut App) -> impl IntoElement {
+    fn render(self, _window: &mut ui::Window, _cx: &mut ui::App) -> impl IntoElement {
         let color = self.theme.colors();
 
-        let root_radius = px(8.0);
+        let root_radius = Self::CORNER_RADIUS;
         let root_border = px(2.0);
         let root_padding = px(2.0);
         let child_border = px(1.0);
@@ -188,21 +173,9 @@ impl RenderOnce for ThemePreviewTile {
         let content = div().size_full().flex().child(sidebar).child(pane);
 
         div()
-            // Note: If two theme preview tiles are rendering the same theme they'll share an ID
-            // this will mean on hover and on click events will be shared between them
-            .id(SharedString::from(self.theme.id.clone()))
-            .when_some(self.on_click.clone(), |this, on_click| {
-                this.on_click(move |event, window, cx| on_click(event, window, cx))
-                    .hover(|style| style.cursor_pointer().border_color(color.element_hover))
-            })
             .size_full()
             .rounded(root_radius)
             .p(root_padding)
-            .border(root_border)
-            .border_color(color.border_transparent)
-            .when(self.selected, |this| {
-                this.border_color(color.border_selected)
-            })
             .child(
                 div()
                     .size_full()
@@ -244,24 +217,14 @@ impl Component for ThemePreviewTile {
                 .p_4()
                 .children({
                     if let Some(one_dark) = one_dark.ok() {
-                        vec![example_group(vec![
-                            single_example(
-                                "Default",
-                                div()
-                                    .w(px(240.))
-                                    .h(px(180.))
-                                    .child(ThemePreviewTile::new(one_dark.clone(), false, 0.42))
-                                    .into_any_element(),
-                            ),
-                            single_example(
-                                "Selected",
-                                div()
-                                    .w(px(240.))
-                                    .h(px(180.))
-                                    .child(ThemePreviewTile::new(one_dark, true, 0.42))
-                                    .into_any_element(),
-                            ),
-                        ])]
+                        vec![example_group(vec![single_example(
+                            "Default",
+                            div()
+                                .w(px(240.))
+                                .h(px(180.))
+                                .child(ThemePreviewTile::new(one_dark.clone(), 0.42))
+                                .into_any_element(),
+                        )])]
                     } else {
                         vec![]
                     }
@@ -276,11 +239,10 @@ impl Component for ThemePreviewTile {
                                     .iter()
                                     .enumerate()
                                     .map(|(_, theme)| {
-                                        div().w(px(200.)).h(px(140.)).child(ThemePreviewTile::new(
-                                            theme.clone(),
-                                            false,
-                                            0.42,
-                                        ))
+                                        div()
+                                            .w(px(200.))
+                                            .h(px(140.))
+                                            .child(ThemePreviewTile::new(theme.clone(), 0.42))
                                     })
                                     .collect::<Vec<_>>(),
                             )

crates/theme/src/settings.rs

@@ -438,7 +438,7 @@ fn default_font_fallbacks() -> Option<FontFallbacks> {
 
 impl ThemeSettingsContent {
     /// Sets the theme for the given appearance to the theme with the specified name.
-    pub fn set_theme(&mut self, theme_name: String, appearance: Appearance) {
+    pub fn set_theme(&mut self, theme_name: impl Into<Arc<str>>, appearance: Appearance) {
         if let Some(selection) = self.theme.as_mut() {
             let theme_to_update = match selection {
                 ThemeSelection::Static(theme) => theme,

crates/ui/src/components.rs

@@ -34,7 +34,6 @@ mod stack;
 mod sticky_items;
 mod tab;
 mod tab_bar;
-mod theme_preview;
 mod toggle;
 mod tooltip;
 
@@ -77,7 +76,6 @@ pub use stack::*;
 pub use sticky_items::*;
 pub use tab::*;
 pub use tab_bar::*;
-pub use theme_preview::*;
 pub use toggle::*;
 pub use tooltip::*;