From 922847a1ed3adb854c4718653b39d435d40dc4b0 Mon Sep 17 00:00:00 2001 From: harrisonoconnorhover <130776285+harrisonoconnorhover@users.noreply.github.com> Date: Fri, 9 Oct 2026 21:35:21 -0400 Subject: [PATCH] Add system-following interface appearance (#72) --- apps/gridcraft/src/native_menu.rs | 55 ++++++++- crates/ui-egui/src/control.rs | 2 + crates/ui-egui/src/lib.rs | 49 +++++++- crates/ui-egui/src/ribbon.rs | 18 ++- crates/ui-egui/src/theme.rs | 44 ++++---- crates/ui-egui/tests/system_theme.rs | 163 +++++++++++++++++++++++++++ docs/control-protocol.md | 6 + 7 files changed, 308 insertions(+), 29 deletions(-) create mode 100644 crates/ui-egui/tests/system_theme.rs diff --git a/apps/gridcraft/src/native_menu.rs b/apps/gridcraft/src/native_menu.rs index 43e57a4..260d900 100644 --- a/apps/gridcraft/src/native_menu.rs +++ b/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)> { ("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)> { pub struct NativeMenu { _menu: Menu, events: Receiver, + 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"); } } diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index a80448a..09c046b 100644 --- a/crates/ui-egui/src/control.rs +++ b/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; diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 8eb1eca..a21c779 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -34,7 +34,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, @@ -47,6 +49,7 @@ impl Default for UiState { fn default() -> Self { UiState { dark: false, + system_theme: false, ribbon_tab: "Home".into(), ribbon_collapsed: false, formula_bar: true, @@ -57,6 +60,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 { @@ -104,6 +129,7 @@ pub struct SheetApp { pub perf: Perf, pub fonts_ready: bool, fonts_set: bool, + effective_dark: bool, pub name_box: Option, pub(crate) shots: control::Shots, /// Chart selected on the sheet (id). @@ -133,6 +159,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, @@ -183,9 +210,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(""); @@ -362,9 +402,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)> = self diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index 47c5432..cdbb927 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -1565,11 +1565,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})); + } + } + }); }); } diff --git a/crates/ui-egui/src/theme.rs b/crates/ui-egui/src/theme.rs index 2dbd8df..290e5b3 100644 --- a/crates/ui-egui/src/theme.rs +++ b/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; diff --git a/crates/ui-egui/tests/system_theme.rs b/crates/ui-egui/tests/system_theme.rs new file mode 100644 index 0000000..2a5365a --- /dev/null +++ b/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) { + 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); +} diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 638ec21..9c6c31b 100644 --- a/docs/control-protocol.md +++ b/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).