Merge pull request #96 from harrisonoconnorhover/codex/system-theme

Add System appearance alongside Light and Dark themes

apps/gridcraft/src/native_menu.rs

@@ -6,7 +6,7 @@ use std::sync::mpsc::{Receiver, channel};
 
 use gridcraft_ui_egui::SheetApp;
 use muda::accelerator::Accelerator;
-use muda::{Menu, MenuEvent, MenuItem, PredefinedMenuItem, Submenu};
+use muda::{CheckMenuItem, Menu, MenuEvent, MenuItem, PredefinedMenuItem, Submenu};
 use serde_json::{Value, json};
 
 type Entry = (&'static str, &'static str, Option<&'static str>);
@@ -78,7 +78,7 @@ fn tree() -> Vec<(&'static str, Vec<Entry>)> {
                 ("Freeze Top Row", "view.freezeTopRow", None),
                 ("Unfreeze Panes", "view.unfreezePanes", None),
                 ("-", "", None),
-                ("Dark Mode", "view.darkMode", None),
+                ("Display Theme", "view.theme", None),
                 ("Collapse Ribbon", "view.collapseRibbon", Some("CMD+ALT+R")),
             ],
         ),
@@ -181,6 +181,18 @@ fn tree() -> Vec<(&'static str, Vec<Entry>)> {
 pub struct NativeMenu {
     _menu: Menu,
     events: Receiver<MenuEvent>,
+    theme_items: Vec<(&'static str, CheckMenuItem)>,
+}
+
+fn select_theme(app: &mut SheetApp, id: &str) -> bool {
+    let mode = match id {
+        "theme:system" => "system",
+        "theme:light" => "light",
+        "theme:dark" => "dark",
+        _ => return false,
+    };
+    app.run_or_alert("view.theme", json!({"mode": mode}));
+    true
 }
 
 impl NativeMenu {
@@ -200,6 +212,7 @@ impl NativeMenu {
             &PredefinedMenuItem::quit(None),
         ]);
         let _ = menu.append(&app_menu);
+        let mut theme_items = Vec::new();
         for (title, entries) in tree() {
             let sub = Submenu::new(title, true);
             for (label, id, acc) in entries {
@@ -207,6 +220,16 @@ impl NativeMenu {
                     let _ = sub.append(&PredefinedMenuItem::separator());
                     continue;
                 }
+                if id == "view.theme" {
+                    let themes = Submenu::new(label, true);
+                    for (mode, label) in [("system", "System"), ("light", "Light"), ("dark", "Dark")] {
+                        let item = CheckMenuItem::with_id(format!("theme:{mode}"), label, true, false, None);
+                        let _ = themes.append(&item);
+                        theme_items.push((mode, item));
+                    }
+                    let _ = sub.append(&themes);
+                    continue;
+                }
                 let accel = acc.and_then(|a| Accelerator::from_str(a).ok());
                 let item = MenuItem::with_id(id, label, true, accel);
                 let _ = sub.append(&item);
@@ -220,12 +243,15 @@ impl NativeMenu {
             let _ = tx.send(e);
             ctx.request_repaint();
         }));
-        NativeMenu { _menu: menu, events: rx }
+        NativeMenu { _menu: menu, events: rx, theme_items }
     }
 
     pub fn poll(&self, app: &mut SheetApp, ctx: &egui::Context) {
         while let Ok(ev) = self.events.try_recv() {
             let id = ev.id.as_ref().to_string();
+            if select_theme(app, &id) {
+                continue;
+            }
             if let Some(url) = id.strip_prefix("url:") {
                 if let Some(open) = &app.services.open_url {
                     open(url);
@@ -252,5 +278,28 @@ impl NativeMenu {
                 app.run_or_alert(&id, json!({}));
             }
         }
+        for (mode, item) in &self.theme_items {
+            let checked = app.ui.theme_mode() == *mode;
+            if item.is_checked() != checked {
+                item.set_checked(checked);
+            }
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn theme_choices_use_the_shared_preference_command() {
+        let mut app = SheetApp::new(gridcraft_engine::Session::new(), Default::default());
+        for mode in ["dark", "system", "light"] {
+            assert!(select_theme(&mut app, &format!("theme:{mode}")));
+            assert_eq!(app.ui.theme_mode(), mode);
+            assert!(app.message.is_none());
+        }
+        assert!(!select_theme(&mut app, "view.formulaBar"));
+        assert_eq!(app.ui.theme_mode(), "light");
     }
 }

crates/ui-egui/src/control.rs

@@ -160,6 +160,7 @@ pub fn handle(app: &mut SheetApp, ctx: &egui::Context, method: &str, p: &Json) -
                 ("view.formulaBar", "Formula Bar"),
                 ("view.collapseRibbon", "Collapse Ribbon"),
                 ("view.darkMode", "Dark Mode"),
+                ("view.theme", "Display Theme"),
                 ("view.zoom100", "100%"),
                 ("ui.dialog", "Open Dialog"),
             ] {
@@ -178,6 +179,7 @@ pub fn handle(app: &mut SheetApp, ctx: &egui::Context, method: &str, p: &Json) -
         "ui.set" => {
             if let Some(v) = p.get("dark").and_then(Json::as_bool) {
                 app.ui.dark = v;
+                app.ui.system_theme = false;
             }
             if let Some(v) = p.get("formulaBar").and_then(Json::as_bool) {
                 app.ui.formula_bar = v;

crates/ui-egui/src/lib.rs

@@ -35,7 +35,9 @@ pub use control::{ControlRequest, ControlResponse};
 #[derive(Clone, Debug, Serialize, Deserialize)]
 #[serde(default, rename_all = "camelCase")]
 pub struct UiState {
+    /// The manual preference, retained when following the system appearance.
     pub dark: bool,
+    pub system_theme: bool,
     pub ribbon_tab: String,
     pub ribbon_collapsed: bool,
     pub formula_bar: bool,
@@ -48,6 +50,7 @@ impl Default for UiState {
     fn default() -> Self {
         UiState {
             dark: false,
+            system_theme: false,
             ribbon_tab: "Home".into(),
             ribbon_collapsed: false,
             formula_bar: true,
@@ -58,6 +61,28 @@ impl Default for UiState {
     }
 }
 
+impl UiState {
+    pub fn theme_preference(&self) -> egui::ThemePreference {
+        if self.system_theme {
+            egui::ThemePreference::System
+        } else if self.dark {
+            egui::ThemePreference::Dark
+        } else {
+            egui::ThemePreference::Light
+        }
+    }
+
+    pub fn theme_mode(&self) -> &'static str {
+        if self.system_theme {
+            "system"
+        } else if self.dark {
+            "dark"
+        } else {
+            "light"
+        }
+    }
+}
+
 /// Per-sheet view state (scroll position in sheet points).
 #[derive(Clone, Copy, Debug, Default, PartialEq)]
 pub struct SheetView {
@@ -105,6 +130,7 @@ pub struct SheetApp {
     pub perf: Perf,
     pub fonts_ready: bool,
     fonts_set: bool,
+    effective_dark: bool,
     pub name_box: Option<String>,
     pub(crate) shots: control::Shots,
     /// Chart selected on the sheet (id).
@@ -134,6 +160,7 @@ impl SheetApp {
             perf: Perf::default(),
             fonts_ready: false,
             fonts_set: false,
+            effective_dark: false,
             name_box: None,
             shots: control::Shots::default(),
             selected_chart: None,
@@ -184,9 +211,22 @@ impl SheetApp {
                 Ok(Json::Null)
             }
             "view.darkMode" => {
-                self.ui.dark = p.get("on").and_then(Json::as_bool).unwrap_or(!self.ui.dark);
+                let current = if self.ui.system_theme { self.effective_dark } else { self.ui.dark };
+                self.ui.dark = p.get("on").and_then(Json::as_bool).unwrap_or(!current);
+                self.ui.system_theme = false;
                 Ok(json!({"on": self.ui.dark}))
             }
+            "view.theme" => {
+                match p.get("mode").and_then(Json::as_str) {
+                    Some("system") => self.ui.system_theme = true,
+                    Some(mode @ ("light" | "dark")) => {
+                        self.ui.dark = mode == "dark";
+                        self.ui.system_theme = false;
+                    }
+                    _ => return Some(Err("theme mode must be system, light, or dark".into())),
+                }
+                Ok(json!({"mode": self.ui.theme_mode()}))
+            }
             "view.zoom100" => return Some(self.session.run("view.zoom", json!({"percent": 100})).inspect(|_| self.after_engine())),
             "ui.dialog" => {
                 let name = p.get("name").and_then(Json::as_str).unwrap_or("");
@@ -363,9 +403,12 @@ impl SheetApp {
                 ctx.request_repaint();
             }
         }
-        if ctx.global_style().visuals.dark_mode != self.ui.dark {
-            theme::apply(ctx, self.ui.dark);
+        let preference = self.ui.theme_preference();
+        if ctx.options(|o| o.theme_preference) != preference {
+            ctx.set_theme(preference);
+            ctx.request_repaint();
         }
+        self.effective_dark = ctx.theme() == egui::Theme::Dark;
         control::poll(self, ctx);
         // Files read asynchronously (web file picker, dropped files).
         let arrived: Vec<(String, Vec<u8>)> = self

crates/ui-egui/src/ribbon.rs

@@ -1593,11 +1593,21 @@ fn view(app: &mut SheetApp, ui: &mut Ui) {
             ],
         );
     });
-    let mut dark = app.ui.dark;
     ui.vertical(|ui| {
-        if ui.checkbox(&mut dark, "Dark Mode").changed() {
-            app.ui.dark = dark;
-        }
+        ui.label("Display theme");
+        let mode = app.ui.theme_mode();
+        let label = match mode {
+            "system" => "System",
+            "dark" => "Dark",
+            _ => "Light",
+        };
+        egui::ComboBox::from_id_salt("display_theme").selected_text(label).width(88.0).show_ui(ui, |ui| {
+            for (value, label) in [("system", "System"), ("light", "Light"), ("dark", "Dark")] {
+                if ui.selectable_label(mode == value, label).clicked() {
+                    act(app, "view.theme", json!({"mode": value}));
+                }
+            }
+        });
     });
 }
 

crates/ui-egui/src/theme.rs

@@ -317,25 +317,31 @@ pub fn font_definitions() -> FontDefinitions {
 }
 
 pub fn apply(ctx: &egui::Context, dark: bool) {
-    let t = if dark { Tokens::dark() } else { Tokens::light() };
-    let mut v = if dark { Visuals::dark() } else { Visuals::light() };
-    v.panel_fill = t.window;
-    v.window_fill = t.menu_bg;
-    v.extreme_bg_color = t.input_bg;
-    v.selection.bg_fill = t.accent_soft;
-    v.selection.stroke = Stroke::new(1.0, t.accent);
-    v.hyperlink_color = t.accent;
-    v.widgets.noninteractive.fg_stroke = Stroke::new(1.0, t.text);
-    v.widgets.inactive.fg_stroke = Stroke::new(1.0, t.text);
-    v.widgets.inactive.weak_bg_fill = Color32::TRANSPARENT;
-    v.widgets.hovered.weak_bg_fill = t.hover;
-    v.widgets.active.weak_bg_fill = t.pressed;
-    v.widgets.inactive.bg_stroke = Stroke::new(1.0, t.input_border);
-    v.window_corner_radius = egui::CornerRadius::same(10);
-    v.menu_corner_radius = egui::CornerRadius::same(8);
-    v.popup_shadow = egui::epaint::Shadow { offset: [0, 4], blur: 16, spread: 0, color: t.shadow };
-    ctx.set_visuals(v);
-    ctx.global_style_mut(|s| {
+    // Install both palettes before System can switch between the style slots.
+    for theme in [egui::Theme::Light, egui::Theme::Dark] {
+        let dark = theme == egui::Theme::Dark;
+        let t = if dark { Tokens::dark() } else { Tokens::light() };
+        let mut v = if dark { Visuals::dark() } else { Visuals::light() };
+        v.panel_fill = t.window;
+        v.window_fill = t.menu_bg;
+        v.extreme_bg_color = t.input_bg;
+        v.selection.bg_fill = t.accent_soft;
+        v.selection.stroke = Stroke::new(1.0, t.accent);
+        v.hyperlink_color = t.accent;
+        v.widgets.noninteractive.fg_stroke = Stroke::new(1.0, t.text);
+        v.widgets.inactive.fg_stroke = Stroke::new(1.0, t.text);
+        v.widgets.inactive.weak_bg_fill = Color32::TRANSPARENT;
+        v.widgets.hovered.weak_bg_fill = t.hover;
+        v.widgets.active.weak_bg_fill = t.pressed;
+        v.widgets.inactive.bg_stroke = Stroke::new(1.0, t.input_border);
+        v.window_corner_radius = egui::CornerRadius::same(10);
+        v.menu_corner_radius = egui::CornerRadius::same(8);
+        v.popup_shadow = egui::epaint::Shadow { offset: [0, 4], blur: 16, spread: 0, color: t.shadow };
+        ctx.set_visuals_of(theme, v);
+    }
+    ctx.options_mut(|o| o.fallback_theme = egui::Theme::Light);
+    ctx.set_theme(if dark { egui::Theme::Dark } else { egui::Theme::Light });
+    ctx.all_styles_mut(|s| {
         s.spacing.item_spacing = egui::vec2(6.0, 4.0);
         s.spacing.button_padding = egui::vec2(6.0, 3.0);
         s.spacing.interact_size.y = 22.0;

crates/ui-egui/tests/system_theme.rs

@@ -0,0 +1,163 @@
+//! Saved appearance preferences and live OS changes through the whole UI.
+
+use egui::{Theme, ThemePreference};
+use egui_kittest::kittest::Queryable as _;
+use gridcraft_engine::Session;
+use gridcraft_ui_egui::{SheetApp, UiState, control, theme::Tokens};
+use serde_json::{Value, json};
+
+fn harness(ui: UiState) -> egui_kittest::Harness<'static, SheetApp> {
+    let mut session = Session::new();
+    session.new_workbook();
+    let mut app = SheetApp::new(session, Default::default());
+    app.ui = ui;
+    let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).with_step_dt(1.0 / 60.0).build_ui_state(
+        |ui, app: &mut SheetApp| {
+            let ctx = ui.ctx().clone();
+            app.logic(&ctx);
+            app.ui(ui);
+        },
+        app,
+    );
+    h.run_steps(4);
+    h
+}
+
+fn preferences(h: &egui_kittest::Harness<'static, SheetApp>) -> Value {
+    serde_json::to_value(&h.state().ui).unwrap()
+}
+
+fn system_theme(h: &mut egui_kittest::Harness<'static, SheetApp>, theme: Option<Theme>) {
+    h.input_mut().system_theme = theme;
+    h.run_steps(2);
+}
+
+fn set_mode(h: &mut egui_kittest::Harness<'static, SheetApp>, mode: &str) {
+    let ctx = h.ctx.clone();
+    let control::Outcome::Done(response) =
+        control::handle(h.state_mut(), &ctx, "engine.execute", &json!({"command": "view.theme", "params": {"mode": mode}}))
+    else {
+        panic!("engine.execute should return a response");
+    };
+    assert_eq!(response["ok"], true, "{response}");
+    h.run_steps(2);
+}
+
+fn select_ribbon_theme(h: &mut egui_kittest::Harness<'static, SheetApp>, label: &str) {
+    h.get_by_role(egui::accesskit::Role::ComboBox).click();
+    h.run_steps(2);
+    h.get_by_role_and_label(egui::accesskit::Role::Button, label).click();
+    h.run_steps(2);
+}
+
+fn assert_appearance(h: &egui_kittest::Harness<'static, SheetApp>, dark: bool) {
+    let visuals = &h.ctx.global_style().visuals;
+    let tokens = if dark { Tokens::dark() } else { Tokens::light() };
+    assert_eq!(visuals.dark_mode, dark);
+    assert_eq!(visuals.panel_fill, tokens.window);
+    assert_eq!(visuals.window_fill, tokens.menu_bg);
+    assert_eq!(visuals.selection.bg_fill, tokens.accent_soft);
+}
+
+#[test]
+fn old_preferences_stay_manual_and_system_preference_survives_reload() {
+    for dark in [false, true] {
+        let ui: UiState = serde_json::from_value(json!({"dark": dark})).unwrap();
+        assert_eq!(ui.dark, dark);
+        let mut h = harness(ui);
+        assert_eq!(preferences(&h)["systemTheme"], false, "old saved settings remain manual");
+        system_theme(&mut h, Some(if dark { Theme::Light } else { Theme::Dark }));
+        assert_appearance(&h, dark);
+    }
+
+    let mut h = harness(UiState::default());
+    set_mode(&mut h, "system");
+    system_theme(&mut h, Some(Theme::Dark));
+    let saved = serde_json::to_string(&h.state().ui).unwrap();
+    let mut restored = harness(serde_json::from_str(&saved).unwrap());
+    assert_eq!(preferences(&restored)["systemTheme"], true);
+    system_theme(&mut restored, Some(Theme::Light));
+    assert_appearance(&restored, false);
+    system_theme(&mut restored, Some(Theme::Dark));
+    assert_appearance(&restored, true);
+}
+
+#[test]
+fn system_tracks_live_os_changes_with_both_gridcraft_palettes_and_light_fallback() {
+    let mut h = harness(UiState::default());
+    set_mode(&mut h, "system");
+    assert_eq!(h.ctx.options(|o| o.theme_preference), ThemePreference::System);
+    // egui switches between these two style slots when the OS appearance changes.
+    for (theme, tokens) in [(Theme::Light, Tokens::light()), (Theme::Dark, Tokens::dark())] {
+        let style = h.ctx.style_of(theme);
+        assert_eq!(style.visuals.panel_fill, tokens.window);
+        assert_eq!(style.visuals.window_fill, tokens.menu_bg);
+        assert_eq!(style.visuals.selection.bg_fill, tokens.accent_soft);
+    }
+    for (reported, dark) in [(Some(Theme::Dark), true), (Some(Theme::Light), false), (Some(Theme::Dark), true), (None, false)] {
+        system_theme(&mut h, reported);
+        assert_appearance(&h, dark);
+        assert_eq!(preferences(&h)["systemTheme"], true);
+        assert_eq!(h.ctx.options(|o| o.theme_preference), ThemePreference::System);
+    }
+}
+
+#[test]
+fn ribbon_selects_system_and_fixed_modes_ignore_subsequent_os_changes() {
+    let mut h = harness(UiState::default());
+    system_theme(&mut h, Some(Theme::Dark));
+    assert_appearance(&h, false);
+    h.state_mut().run("ui.ribbonTab", json!({"tab": "View"})).unwrap();
+    h.run_steps(2);
+    select_ribbon_theme(&mut h, "System");
+    assert_eq!(preferences(&h)["systemTheme"], true);
+    assert_eq!(h.ctx.options(|o| o.theme_preference), ThemePreference::System);
+    assert_appearance(&h, true);
+    for (label, dark, preference) in [("Dark", true, ThemePreference::Dark), ("Light", false, ThemePreference::Light)] {
+        select_ribbon_theme(&mut h, label);
+        assert_eq!(preferences(&h)["systemTheme"], false);
+        assert_eq!(h.ctx.options(|o| o.theme_preference), preference);
+        for os in [Some(Theme::Dark), Some(Theme::Light), None] {
+            system_theme(&mut h, os);
+            assert_appearance(&h, dark);
+        }
+    }
+}
+
+#[test]
+fn invalid_mode_preserves_preferences_and_legacy_overrides_leave_system_mode() {
+    let mut h = harness(UiState::default());
+    set_mode(&mut h, "system");
+    system_theme(&mut h, Some(Theme::Dark));
+    let before = preferences(&h);
+    assert!(h.state_mut().run("view.theme", json!({"mode": "sepia"})).is_err());
+    h.run_steps(2);
+    assert_eq!(preferences(&h), before);
+    assert_appearance(&h, true);
+
+    // The old toggle flips the displayed appearance, even when System chose it.
+    h.state_mut().run("view.darkMode", json!({})).unwrap();
+    h.run_steps(2);
+    assert_eq!(preferences(&h)["systemTheme"], false);
+    assert_appearance(&h, false);
+
+    set_mode(&mut h, "system");
+    system_theme(&mut h, Some(Theme::Light));
+    h.state_mut().run("view.darkMode", json!({"on": true})).unwrap();
+    h.run_steps(2);
+    assert_eq!(preferences(&h)["systemTheme"], false);
+    assert_appearance(&h, true);
+
+    set_mode(&mut h, "system");
+    system_theme(&mut h, Some(Theme::Dark));
+    let ctx = h.ctx.clone();
+    let control::Outcome::Done(response) = control::handle(h.state_mut(), &ctx, "ui.set", &json!({"dark": false})) else {
+        panic!("ui.set should return a response");
+    };
+    assert_eq!(response["ok"], true);
+    h.run_steps(2);
+    assert_eq!(preferences(&h)["systemTheme"], false);
+    assert_appearance(&h, false);
+    system_theme(&mut h, Some(Theme::Dark));
+    assert_appearance(&h, false);
+}

docs/control-protocol.md

@@ -39,5 +39,11 @@ Errors come back as `{"id": …, "ok": false, "error": "…"}`.
 
 Any other method name that is a command id (e.g. `home.bold`) runs that command.
 
+Use `engine.execute` with `{command: "view.theme", params: {mode: "system"}}`
+to choose the display theme; `mode` accepts `"system"`, `"light"`, or `"dark"`.
+System follows live OS appearance changes and falls back to Light when the platform supplies no preference.
+The saved choice remains System; new users still start in Light. The legacy
+`view.darkMode` command and `ui.set {dark}` select a fixed Light or Dark theme.
+
 For a headless equivalent (no window), use `gridcraft-cli mcp` or `gridcraft-cli run`;
 see [`mcp.md`](mcp.md) and [`cli.md`](cli.md).