From 1db3698719e723236f2f8896ae7cde32f5bdea1c Mon Sep 17 00:00:00 2001 From: BFlat Date: Wed, 7 Oct 2026 20:16:29 +0700 Subject: [PATCH] =?UTF-8?q?Window:=20saved=20workspaces=20are=20listed=20i?= =?UTF-8?q?n=20Window=20=E2=96=B8=20Workspace=20(#191)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Save as New Workspace stored the layout and `window.workspace` could switch to it, but the Workspace submenu was a fixed list of the built-in workspaces, so a saved one never appeared and could not be chosen from the menu. The submenu now has a `savedWorkspaces` dynamic node after the built-ins (DynCtx carries the saved names), in the in-window and native menus, with a check mark on the current one as the built-ins have. The in-window submenu was also too short to show it. egui lays out a new submenu in a default 400 pt area, and the scroll area inside took that height and kept it, so Window ▸ Workspace (514 pt) opened with its last rows (Undocked Panels and below, where saved workspaces go) scrolled out of view. Submenus now open at their full height and scroll only when taller than the window. Tests: saved_workspace_is_listed_in_the_workspace_menu (picks it from the menu bar), saved_workspaces_are_in_the_native_workspace_menu. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/menus.rs | 9 ++++-- crates/ui-egui/src/lib.rs | 8 ++++- crates/ui-egui/src/menus.rs | 22 ++++++++------ crates/ui-egui/src/native_menu.rs | 23 +++++++++++--- crates/ui-egui/tests/ui_panels.rs | 50 +++++++++++++++++++++++++++++++ 5 files changed, 96 insertions(+), 16 deletions(-) diff --git a/crates/engine/src/menus.rs b/crates/engine/src/menus.rs index 76b92efe..c5830828 100644 --- a/crates/engine/src/menus.rs +++ b/crates/engine/src/menus.rs @@ -52,6 +52,8 @@ pub enum MenuNode { pub struct DynCtx<'a> { /// The current workspace (Window ▸ Assign Shortcut to "…" Workspace). pub workspace: Option<&'a str>, + /// Workspaces the user saved (Save as New Workspace), listed in Window ▸ Workspace. + pub saved_workspaces: &'a [String], } fn file_name(p: &str) -> String { @@ -70,8 +72,9 @@ fn holder_of(s: &Session, keys: &str) -> Option { /// The entries of a `@dynamic:` node, and the disabled placeholder shown when there are /// none: `recentProjects`, `recentFootage`, `recentPresets`, `history` (undo steps, newest -/// first), `view3dShortcuts`, `workspaceShortcuts` and `openViewers` (open compositions other -/// than the active one), `scripts` (File ▸ Scripts) and `scriptPanels` (ScriptUI panels). +/// first), `view3dShortcuts`, `workspaceShortcuts`, `savedWorkspaces` (Window ▸ Workspace) and +/// `openViewers` (open compositions other than the active one), `scripts` (File ▸ Scripts) and +/// `scriptPanels` (ScriptUI panels). pub fn dynamic(s: &Session, name: &str, cx: &DynCtx) -> (Vec, Option<&'static str>) { use serde_json::json; let indexed = |list: &[String], cmd: &str, stem: bool| -> Vec { @@ -122,6 +125,7 @@ pub fn dynamic(s: &Session, name: &str, cx: &DynCtx) -> (Vec, Option< .collect(), None, ), + "savedWorkspaces" => (cx.saved_workspaces.iter().map(|w| dyn_entry(w.clone(), "window.workspace", json!({"name": w}))).collect(), None), "openViewers" => ( s.state .open_comps @@ -1151,6 +1155,7 @@ Window Paint | window.workspace {"name":"Paint"} Text | window.workspace {"name":"Text"} Undocked Panels | window.workspace {"name":"Undocked Panels"} + @dynamic:savedWorkspaces --- Reset to Saved Layout | window.resetWorkspace Save Changes to this Workspace | window.saveWorkspace diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index e3cbf26c..49b2e310 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -386,10 +386,16 @@ impl EffectcraftApp { /// Built-in workspaces followed by the saved ones (Window ▸ Workspace ▸ Save as New Workspace). pub fn workspace_names(&self) -> Vec { let mut v: Vec = dock::WORKSPACES.iter().map(|s| s.to_string()).collect(); - v.extend(self.ui.saved_workspaces.keys().filter(|k| !dock::WORKSPACES.contains(&k.as_str())).cloned()); + v.extend(self.saved_workspace_names()); v } + /// The workspaces the user saved under a new name (built-ins whose changes were saved are + /// not repeated), in name order. + pub fn saved_workspace_names(&self) -> Vec { + self.ui.saved_workspaces.keys().filter(|k| !dock::WORKSPACES.contains(&k.as_str())).cloned().collect() + } + /// Show panel `p` (opening it in its usual place if it is closed), bring it to the front and /// give it the focus: Window ▸ . pub fn show_panel(&mut self, p: PanelKind) { diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 28e28835..6fe8a6a3 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -1500,9 +1500,11 @@ fn menu_nodes(app: &mut EffectcraftApp, ui: &mut egui::Ui, nodes: &[MenuNode], c ui.separator(); } MenuNode::Dynamic { name } => { - // Recent projects / footage / presets, undo history, shortcut slots, viewers. + // Recent projects / footage / presets, undo history, shortcut slots, viewers, + // saved workspaces. let ws = app.ui.workspace.clone(); - let (entries, empty) = effectcraft_engine::menus::dynamic(&app.session, name, &dyn_ctx(&ws)); + let saved = app.saved_workspace_names(); + let (entries, empty) = effectcraft_engine::menus::dynamic(&app.session, name, &dyn_ctx(&ws, &saved)); if entries.is_empty() && let Some(e) = empty { @@ -1515,7 +1517,7 @@ fn menu_nodes(app: &mut EffectcraftApp, ui: &mut egui::Ui, nodes: &[MenuNode], c "recentPresets" => app.session.prefs.recent_presets.get(i).cloned(), _ => None, }; - let r = ui.add_enabled(entry_enabled(app, e), egui::Button::new((gutter(false), e.label.as_str()))); + let r = ui.add_enabled(entry_enabled(app, e), egui::Button::new((gutter(entry_checked(app, e) == Some(true)), e.label.as_str()))); let r = match &tip { Some(t) => r.on_hover_text(t), None => r, @@ -1529,13 +1531,15 @@ fn menu_nodes(app: &mut EffectcraftApp, ui: &mut egui::Ui, nodes: &[MenuNode], c } MenuNode::Submenu { label, children } => { let ws = app.ui.workspace.clone(); - let shown = crate::i18n::submenu(app, label, effectcraft_engine::menus::submenu_label(&app.session, label, &dyn_ctx(&ws))); + let shown = crate::i18n::submenu(app, label, effectcraft_engine::menus::submenu_label(&app.session, label, &dyn_ctx(&ws, &[]))); ui.menu_button((gutter(false), shown.as_str()), |ui| { ui.set_min_width(if children.len() > 30 { 200.0 } else { 240.0 }); // Long submenus (Blending Mode, effect categories) scroll instead of running - // off the screen. + // off the screen. Others show whole: egui sizes a new submenu from a default + // 400 pt area, so without a minimum a longer one (Window ▸ Workspace) got + // stuck at that height with its last entries scrolled out of view (#191). let max_h = ui.ctx().content_rect().height() - 40.0; - egui::ScrollArea::vertical().max_height(max_h).show(ui, |ui| menu_nodes(app, ui, children, clicked)); + egui::ScrollArea::vertical().max_height(max_h).min_scrolled_height(max_h).show(ui, |ui| menu_nodes(app, ui, children, clicked)); }); } MenuNode::Item(e) => { @@ -1548,9 +1552,9 @@ fn menu_nodes(app: &mut EffectcraftApp, ui: &mut egui::Ui, nodes: &[MenuNode], c } } -/// Frontend state for dynamic menus (the current workspace). -pub(crate) fn dyn_ctx(workspace: &str) -> effectcraft_engine::menus::DynCtx<'_> { - effectcraft_engine::menus::DynCtx { workspace: Some(workspace) } +/// Frontend state for dynamic menus (the current workspace and the saved ones). +pub(crate) fn dyn_ctx<'a>(workspace: &'a str, saved_workspaces: &'a [String]) -> effectcraft_engine::menus::DynCtx<'a> { + effectcraft_engine::menus::DynCtx { workspace: Some(workspace), saved_workspaces } } /// The check-mark column every menu row reserves (like macOS / After Effects menus). diff --git a/crates/ui-egui/src/native_menu.rs b/crates/ui-egui/src/native_menu.rs index 0fc33ad5..f7332aa5 100644 --- a/crates/ui-egui/src/native_menu.rs +++ b/crates/ui-egui/src/native_menu.rs @@ -135,8 +135,10 @@ pub fn build(app: &EffectcraftApp) -> NativeMenu { match n { MenuNode::Separator => out.push(NativeNode::Separator), MenuNode::Dynamic { name } => { - // Recent projects / footage / presets, undo history, shortcut slots, viewers. - let (entries, empty) = effectcraft_engine::menus::dynamic(&app.session, name, &crate::menus::dyn_ctx(&app.ui.workspace)); + // Recent projects / footage / presets, undo history, shortcut slots, viewers, + // saved workspaces. + let saved = app.saved_workspace_names(); + let (entries, empty) = effectcraft_engine::menus::dynamic(&app.session, name, &crate::menus::dyn_ctx(&app.ui.workspace, &saved)); if entries.is_empty() && let Some(e) = empty { @@ -160,14 +162,14 @@ pub fn build(app: &EffectcraftApp) -> NativeMenu { shortcut: None, accelerator: None, enabled: crate::menus::entry_enabled(app, e), - checked: None, + checked: crate::menus::entry_checked(app, e), })); } } MenuNode::Submenu { label, children } => { let mut kids = vec![]; rec(app, children, &id, &mut kids); - let shown = effectcraft_engine::menus::submenu_label(&app.session, label, &crate::menus::dyn_ctx(&app.ui.workspace)); + let shown = effectcraft_engine::menus::submenu_label(&app.session, label, &crate::menus::dyn_ctx(&app.ui.workspace, &[])); out.push(NativeNode::Submenu { label: crate::i18n::submenu(app, label, shown), children: kids }); } MenuNode::Item(e) => { @@ -394,6 +396,19 @@ mod tests { assert!(v["menus"].as_array().is_some_and(|a| a.len() >= 9)); } + /// Issue #191: saved workspaces are in the native Window ▸ Workspace menu, checked while current. + #[test] + fn saved_workspaces_are_in_the_native_workspace_menu() { + let mut app = app(); + let ctx = egui::Context::default(); + crate::menus::invoke(&mut app, &ctx, "window.saveWorkspaceAs", json!({"name": "My Layout"})).unwrap(); + let entry = |app: &EffectcraftApp| build(app).items().into_iter().find(|i| i.label == "My Layout").cloned(); + let it = entry(&app).expect("the saved workspace is listed"); + assert_eq!((it.command.as_str(), &it.params, it.checked), ("window.workspace", &json!({"name": "My Layout"}), Some(true))); + crate::menus::invoke(&mut app, &ctx, "window.workspace", json!({"name": "Default"})).unwrap(); + assert_eq!(entry(&app).and_then(|i| i.checked), Some(false)); + } + #[test] fn hide_items_are_predefined_on_macos() { let spec = build(&app()); diff --git a/crates/ui-egui/tests/ui_panels.rs b/crates/ui-egui/tests/ui_panels.rs index ee027daa..471e47bf 100644 --- a/crates/ui-egui/tests/ui_panels.rs +++ b/crates/ui-egui/tests/ui_panels.rs @@ -7,6 +7,7 @@ use effectcraft_ui_egui::EffectcraftApp; use effectcraft_ui_egui::dock::PanelKind; use egui::{Event, Pos2, pos2}; use egui_kittest::Harness; +use egui_kittest::kittest::Queryable; use serde_json::json; fn harness() -> Harness<'static, EffectcraftApp> { @@ -103,6 +104,55 @@ fn progress_panel_cancels_a_job() { assert_eq!(h.state().session.job_log.last().unwrap().status, "cancelled"); } +fn click_at(h: &mut Harness<'_, EffectcraftApp>, p: Pos2) { + h.input_mut().events.push(Event::PointerMoved(p)); + h.input_mut().events.push(Event::PointerButton { pos: p, button: egui::PointerButton::Primary, pressed: true, modifiers: Default::default() }); + h.step(); + h.input_mut().events.push(Event::PointerButton { pos: p, button: egui::PointerButton::Primary, pressed: false, modifiers: Default::default() }); + h.run_steps(2); +} + +fn hover(h: &mut Harness<'_, EffectcraftApp>, p: Pos2) { + h.input_mut().events.push(Event::PointerMoved(p)); + h.run_steps(3); +} + +/// Issue #191: a workspace saved with Save as New Workspace is listed in Window ▸ Workspace +/// (below the built-ins, where the submenu used to be cut off) and choosing it there brings its +/// layout back. +#[test] +fn saved_workspace_is_listed_in_the_workspace_menu() { + let mut h = harness(); + let ctx = h.ctx.clone(); + effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, "window.workspace", json!({"name": "Minimal"})).unwrap(); + effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, "window.saveWorkspaceAs", json!({"name": "My Layout"})).unwrap(); + h.run_steps(3); + let saved = h.state().ui.dock.clone(); + assert_eq!(h.state().saved_workspace_names(), ["My Layout"]); + let cx = effectcraft_engine::menus::DynCtx { workspace: Some("My Layout"), saved_workspaces: &["My Layout".to_string()] }; + let (entries, _) = effectcraft_engine::menus::dynamic(&h.state().session, "savedWorkspaces", &cx); + assert_eq!(entries.len(), 1); + assert_eq!((entries[0].label.as_str(), entries[0].command.as_str(), &entries[0].params), ("My Layout", "window.workspace", &json!({"name": "My Layout"}))); + // Leave it, then pick it from the in-window menu bar. + effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, "window.workspace", json!({"name": "Default"})).unwrap(); + h.run_steps(3); + assert_ne!(h.state().ui.dock, saved); + click(&mut h, "menu.Window"); + let ws = h.query_by_label(" Workspace ⏵").expect("Window ▸ Workspace").rect(); + hover(&mut h, ws.center()); + let entry = h.state().auto.find("menu.savedWorkspaces.0").expect("the saved workspace is listed").clone(); + assert_eq!(entry.label, "My Layout"); + let at = h.query_by_label(" My Layout").expect("the saved workspace entry").rect().center(); + // Into the submenu, then down it (as a pointer moves). + hover(&mut h, pos2(at.x, ws.center().y)); + for k in 1..=10 { + hover(&mut h, pos2(at.x, ws.center().y + (at.y - ws.center().y) * k as f32 / 10.0)); + } + click_at(&mut h, at); + assert_eq!(h.state().ui.workspace, "My Layout"); + assert_eq!(h.state().ui.dock, saved, "its layout came back"); +} + /// Headless look at the panels (wgpu offscreen; needs a GPU adapter). Run with /// `PANELS_SNAPSHOT=/abs/dir cargo test -p effectcraft-ui-egui --test ui_panels -- --ignored`. #[test]