Merge pull request #3279 from gorkematlyn/upstream-ui7

UI.7: workspaces keep their layout changes

apps/vectorcraft/src/main.rs

@@ -247,10 +247,13 @@ fn decode_saved_prefs(bytes: &[u8]) -> Option<vectorcraft_ui_egui::UiState> {
     let mut value: Value = serde_json::from_slice(bytes).ok()?;
     let object = value.as_object_mut()?;
     recover(object, "docking_hidden");
-    if let Some(Value::Array(workspaces)) = object.get_mut("custom_workspaces") {
-        for workspace in workspaces {
-            if let Some(object) = workspace.as_object_mut() {
-                recover(object, "dockingHidden");
+    // Saved workspaces and the layout changes kept for each workspace carry the same layouts.
+    for key in ["custom_workspaces", "workspace_changes"] {
+        if let Some(Value::Array(workspaces)) = object.get_mut(key) {
+            for workspace in workspaces {
+                if let Some(object) = workspace.as_object_mut() {
+                    recover(object, "dockingHidden");
+                }
             }
         }
     }
@@ -1065,6 +1068,7 @@ mod tests {
         app.ui.engine_prefs = json!({"interfaceLanguage":"fr", "uiBrightness":"light"});
         app.ui.custom_workspaces =
             vec![vectorcraft_ui_egui::workspaces::capture(&app.ui, "Future"), vectorcraft_ui_egui::workspaces::capture(&app.ui, "Valid")];
+        app.ui.workspace_changes = vec![vectorcraft_ui_egui::workspaces::capture(&app.ui, "Future")];
         let original = serde_json::to_value(&app.ui).unwrap();
         let valid_location = original["docking_hidden"]["layers"].clone();
         assert!(valid_location.is_object());
@@ -1075,6 +1079,8 @@ mod tests {
                 raw["docking_hidden"] = invalid_hidden.clone();
                 raw["custom_workspaces"][0]["docking"] = invalid_layout.clone();
                 raw["custom_workspaces"][0]["dockingHidden"] = invalid_hidden.clone();
+                raw["workspace_changes"][0]["docking"] = invalid_layout.clone();
+                raw["workspace_changes"][0]["dockingHidden"] = invalid_hidden.clone();
                 assert!(serde_json::from_value::<vectorcraft_ui_egui::UiState>(raw.clone()).is_err(), "runtime serde stays strict");
                 let loaded = decode_saved_prefs(&serde_json::to_vec(&raw).unwrap()).unwrap();
                 assert_eq!(loaded.engine_prefs, app.ui.engine_prefs);
@@ -1091,6 +1097,8 @@ mod tests {
                 assert!(loaded.custom_workspaces[0].docking.is_none());
                 assert!(!loaded.custom_workspaces[0].docking_hidden.contains_key("layers"));
                 assert_eq!(serde_json::to_value(&loaded.custom_workspaces[1]).unwrap(), original["custom_workspaces"][1]);
+                assert_eq!(loaded.workspace_changes.len(), 1, "a remembered layout is recovered, not the whole file lost");
+                assert!(loaded.workspace_changes[0].docking.is_none());
             }
         }
         let roundtrip = decode_saved_prefs(&serde_json::to_vec(&original).unwrap()).unwrap();

crates/ui-egui/src/menus.rs

@@ -478,8 +478,18 @@ pub const UI_COMMANDS: &[(&str, &str, &str, &str)] = &[
         "{collapsed?: bool} collapse the dock's Properties | Layers | Libraries group to icons (true), expand it (false) or toggle (omitted), as the double arrow at the top of the dock does; returns the new state",
     ),
     ("window.brightness", "UI Brightness", "", "{brightness: dark|mediumDark|mediumLight|light}"),
-    ("window.workspace", "Workspace", "", "{name} switch workspace (Essentials, Essentials Classic, Painting, …)"),
-    ("window.workspace.reset", "Reset Essentials", "", "{} reset the current workspace"),
+    (
+        "window.workspace",
+        "Workspace",
+        "",
+        "{name} switch workspace (Essentials, Essentials Classic, Painting, …): the one left keeps its layout changes, and the one switched to comes back as it was left (as saved if never changed)",
+    ),
+    (
+        "window.workspace.reset",
+        "Reset Essentials",
+        "",
+        "{} reset the current workspace to the way it was saved, dropping the layout changes made in it",
+    ),
     ("window.workspace.new", "New Workspace…", "", "{name?} save the current layout"),
     ("window.workspace.manage", "Manage Workspaces…", "", "{}"),
     ("window.workspace.delete", "Delete Workspace", "", "{name}"),

crates/ui-egui/src/state.rs

@@ -428,6 +428,10 @@ pub struct UiState {
     /// User-saved workspaces (New Workspace…).
     #[serde(default)]
     pub custom_workspaces: Vec<crate::workspaces::Workspace>,
+    /// Layout changes made in a workspace since it was saved (or last reset), by workspace name:
+    /// switching back to it brings them back; Reset <name> drops them.
+    #[serde(default, skip_serializing_if = "Vec::is_empty")]
+    pub workspace_changes: Vec<crate::workspaces::Workspace>,
     /// File → Open Recent Files, most recent first.
     #[serde(default)]
     pub recent_files: Vec<String>,
@@ -771,6 +775,17 @@ impl UiState {
         crate::edit_toolbar::sanitize(&mut self.toolbar_edits);
         self.docking = self.docking.filter(crate::panel_docking::valid);
         self.docking_icons.retain(|id| ICON_PANELS.iter().any(|entry| entry.0 == id));
+        // Changes kept for workspaces that still exist, under their saved spelling and one per
+        // workspace whatever the case (a hand-edited file).
+        let mut seen = std::collections::BTreeSet::new();
+        let saved: Vec<String> =
+            self.custom_workspaces.iter().map(|w| w.name.clone()).chain(crate::workspaces::builtins().into_iter().map(|w| w.name)).collect();
+        self.workspace_changes.retain_mut(|w| {
+            let Some(name) = saved.iter().find(|n| n.eq_ignore_ascii_case(&w.name)) else { return false };
+            w.name = name.clone();
+            seen.insert(name.to_ascii_lowercase())
+        });
+        self.workspace_changes.truncate(64);
         // Overrides that can't fire (modifier-only chords recorded by older versions, #487) give
         // the default back.
         self.shortcut_overrides.retain(|_, c| c.is_empty() || crate::shortcut_editor::normalize(c).is_some());
@@ -829,6 +844,7 @@ impl Default for UiState {
             workspace: default_workspace(),
             touch_workspace_restore: None,
             custom_workspaces: vec![],
+            workspace_changes: vec![],
             recent_files: vec![],
             recent_fonts: vec![],
             favorite_fonts: vec![],

crates/ui-egui/src/workspaces.rs

@@ -1,6 +1,11 @@
 //! Window → Workspace: saved panel/bar layouts (Essentials, Essentials Classic, Painting, …),
 //! Reset, New Workspace…, Manage Workspaces…. A workspace is a snapshot of `UiState` layout
 //! flags; UI brightness and everything else is untouched when switching.
+//!
+//! As in Illustrator, a workspace remembers the changes made in it: switching to another one keeps
+//! the layout as it was left (`UiState::workspace_changes`, kept with the preferences), switching
+//! back brings it back, and Reset <name> returns to the workspace as saved. New Workspace… with
+//! the name of a user workspace saves the current layout as it.
 
 use std::sync::atomic::Ordering;
 
@@ -209,8 +214,37 @@ pub fn apply(ui: &mut UiState, w: &Workspace) {
     ui.workspace = w.name.clone();
 }
 
-/// A workspace by name (user workspaces shadow nothing: names are unique across both lists).
-pub fn find(ui: &UiState, name: &str) -> Option<Workspace> {
+/// Keep the layout of the current workspace as it is now, so switching back brings it back.
+fn remember(ui: &mut UiState) {
+    // Kept under the saved workspace's spelling, which `find_saved` matches whatever the case.
+    let Some(saved) = find_saved(ui, &ui.workspace.clone()) else { return };
+    let w = capture(ui, &saved.name);
+    ui.workspace_changes.retain(|x| !x.name.eq_ignore_ascii_case(&saved.name));
+    ui.workspace_changes.push(w);
+    // Over the cap, the oldest go: the layout just kept is the newest.
+    let over = ui.workspace_changes.len().saturating_sub(64);
+    ui.workspace_changes.drain(..over);
+}
+
+/// Switch to workspace `name` as it was left (its changes since it was saved), else as saved.
+pub fn switch(ui: &mut UiState, name: &str) -> Result<String, String> {
+    let saved = find_saved(ui, name).ok_or_else(|| format!("unknown workspace (one of {})", names(ui).join(", ")))?;
+    remember(ui);
+    let w = ui.workspace_changes.iter().find(|w| w.name.eq_ignore_ascii_case(&saved.name)).cloned().unwrap_or(saved);
+    apply(ui, &w);
+    Ok(w.name)
+}
+
+/// Reset the current workspace to the way it was saved, dropping its changes.
+pub fn reset(ui: &mut UiState) -> String {
+    let w = find_saved(ui, &ui.workspace.clone()).unwrap_or_default();
+    ui.workspace_changes.retain(|x| !x.name.eq_ignore_ascii_case(&w.name));
+    apply(ui, &w);
+    w.name
+}
+
+/// A workspace by name as saved (user workspaces shadow nothing: names are unique across both lists).
+pub fn find_saved(ui: &UiState, name: &str) -> Option<Workspace> {
     ui.custom_workspaces
         .iter()
         .find(|w| w.name.eq_ignore_ascii_case(name))
@@ -218,6 +252,11 @@ pub fn find(ui: &UiState, name: &str) -> Option<Workspace> {
         .or_else(|| builtins().into_iter().find(|w| w.name.eq_ignore_ascii_case(name)))
 }
 
+/// A workspace by name as saved.
+pub fn find(ui: &UiState, name: &str) -> Option<Workspace> {
+    find_saved(ui, name)
+}
+
 /// Every workspace name: built-ins then user workspaces.
 pub fn names(ui: &UiState) -> Vec<String> {
     builtins().into_iter().map(|w| w.name).chain(ui.custom_workspaces.iter().map(|w| w.name.clone())).collect()
@@ -232,11 +271,15 @@ pub fn save_as(ui: &mut UiState, name: &str) -> Result<(), String> {
     if is_builtin(name) {
         return Err(format!("`{name}` is a built-in workspace; choose another name"));
     }
+    // The changes go into the workspace saved, which starts with none; the one left keeps the
+    // changes it had (none of these).
+    // A user workspace of that name in another case is replaced, as `find_saved` would find it.
     let w = capture(ui, name);
-    match ui.custom_workspaces.iter_mut().find(|x| x.name == name) {
+    match ui.custom_workspaces.iter_mut().find(|x| x.name.eq_ignore_ascii_case(name)) {
         Some(x) => *x = w,
         None => ui.custom_workspaces.push(w),
     }
+    ui.workspace_changes.retain(|x| !x.name.eq_ignore_ascii_case(name));
     ui.workspace = name.into();
     Ok(())
 }
@@ -247,8 +290,13 @@ pub fn delete(ui: &mut UiState, name: &str) -> Result<(), String> {
     if ui.custom_workspaces.len() == before {
         return Err(if is_builtin(name) { format!("`{name}` is built in and can't be deleted") } else { format!("no workspace `{name}`") });
     }
+    ui.workspace_changes.retain(|w| w.name != name);
+    // The current workspace deleted: Essentials (as it was left) takes over, rather than its layout
+    // staying on screen under Essentials's name, to be remembered as Essentials's changes.
     if ui.workspace == name {
-        ui.workspace = ESSENTIALS.into();
+        let essentials =
+            ui.workspace_changes.iter().find(|w| w.name == ESSENTIALS).cloned().or_else(|| find_saved(ui, ESSENTIALS)).unwrap_or_default();
+        apply(ui, &essentials);
     }
     Ok(())
 }
@@ -260,6 +308,9 @@ pub fn rename(ui: &mut UiState, from: &str, to: &str) -> Result<(), String> {
     }
     let w = ui.custom_workspaces.iter_mut().find(|w| w.name == from).ok_or_else(|| format!("no user workspace `{from}`"))?;
     w.name = to.into();
+    for w in ui.workspace_changes.iter_mut().filter(|w| w.name == from) {
+        w.name = to.into();
+    }
     if ui.workspace == from {
         ui.workspace = to.into();
     }
@@ -337,18 +388,8 @@ pub fn popup(app: &mut VectorcraftApp, ui: &mut egui::Ui) {
 pub fn run_command(app: &mut VectorcraftApp, id: &str, p: &Value) -> Option<Result<Value, String>> {
     let s = |k: &str| p.get(k).and_then(Value::as_str).map(str::to_string);
     let r = match id {
-        "window.workspace" => match s("name").and_then(|n| find(&app.ui, &n)) {
-            Some(w) => {
-                apply(&mut app.ui, &w);
-                Ok(json!({"workspace": w.name}))
-            }
-            None => Err(format!("unknown workspace (one of {})", names(&app.ui).join(", "))),
-        },
-        "window.workspace.reset" => {
-            let w = find(&app.ui, &app.ui.workspace.clone()).unwrap_or_default();
-            apply(&mut app.ui, &w);
-            Ok(json!({"workspace": w.name}))
-        }
+        "window.workspace" => switch(&mut app.ui, &s("name").unwrap_or_default()).map(|name| json!({"workspace": name})),
+        "window.workspace.reset" => Ok(json!({"workspace": reset(&mut app.ui)})),
         "window.workspace.new" => match s("name") {
             Some(n) => save_as(&mut app.ui, &n).map(|_| json!({"workspace": n})),
             None => {
@@ -509,6 +550,124 @@ mod tests {
         assert_eq!(back.workspace, "Mine");
     }
 
+    /// Changes made in a workspace come back after switching away and back, and after a restart, until Reset.
+    #[test]
+    fn a_workspace_keeps_its_changes_across_switches_until_reset() {
+        let mut ui = UiState::default();
+        save_as(&mut ui, "Görkem").unwrap();
+        // Changes made in it after saving (a panel moved, the control bar shown)…
+        ui.control_bar = true;
+        ui.dock_tab = DockTab::Layers;
+        ui.docking_icons.insert("color".into());
+        // …are still there after switching away and back.
+        switch(&mut ui, ESSENTIALS).unwrap();
+        assert!(!ui.control_bar && ui.docking_icons.is_empty(), "Essentials comes as saved");
+        switch(&mut ui, "Görkem").unwrap();
+        assert!(ui.control_bar && ui.dock_tab == DockTab::Layers && ui.docking_icons.contains("color"));
+        // Built-in workspaces remember theirs too.
+        switch(&mut ui, ESSENTIALS).unwrap();
+        ui.status_bar = false;
+        switch(&mut ui, "Görkem").unwrap();
+        switch(&mut ui, ESSENTIALS).unwrap();
+        assert!(!ui.status_bar);
+        // They survive a restart (the preferences' JSON round trip).
+        let mut ui: UiState = serde_json::from_value(serde_json::to_value(&ui).unwrap()).unwrap();
+        ui = ui.sanitized();
+        switch(&mut ui, "Görkem").unwrap();
+        assert!(ui.control_bar);
+        // Reset Görkem: back to the way it was saved.
+        assert_eq!(reset(&mut ui), "Görkem");
+        assert!(!ui.control_bar && ui.docking_icons.is_empty());
+        switch(&mut ui, ESSENTIALS).unwrap();
+        switch(&mut ui, "Görkem").unwrap();
+        assert!(!ui.control_bar, "a reset workspace stays reset");
+        // Renamed, it keeps its changes; deleted, they go.
+        ui.control_bar = true;
+        switch(&mut ui, ESSENTIALS).unwrap();
+        rename(&mut ui, "Görkem", "G").unwrap();
+        switch(&mut ui, "G").unwrap();
+        assert!(ui.control_bar);
+        switch(&mut ui, ESSENTIALS).unwrap();
+        delete(&mut ui, "G").unwrap();
+        assert!(ui.workspace_changes.iter().all(|w| w.name != "G"));
+    }
+
+    /// Deleting the current workspace shows Essentials as it was left, not the deleted one's layout.
+    #[test]
+    fn deleting_the_current_workspace_switches_to_essentials_without_leaking_its_layout() {
+        let mut ui = UiState::default();
+        save_as(&mut ui, "Tmp").unwrap();
+        ui.control_bar = true;
+        delete(&mut ui, "Tmp").unwrap();
+        assert_eq!(ui.workspace, ESSENTIALS);
+        assert!(!ui.control_bar, "Essentials's layout is shown, not the deleted one's");
+        save_as(&mut ui, "Other").unwrap();
+        switch(&mut ui, ESSENTIALS).unwrap();
+        assert!(!ui.control_bar, "Essentials didn't remember the deleted workspace's layout");
+    }
+
+    /// With 64 layouts remembered, the oldest goes and the one just left is kept.
+    #[test]
+    fn a_full_list_of_remembered_layouts_drops_the_oldest_not_the_newest() {
+        let mut ui = UiState::default();
+        let names: Vec<String> = (0..70).map(|i| format!("W{i}")).collect();
+        for name in &names {
+            save_as(&mut ui, name).unwrap();
+        }
+        for name in &names {
+            switch(&mut ui, name).unwrap();
+            ui.control_bar = true;
+        }
+        switch(&mut ui, ESSENTIALS).unwrap();
+        assert_eq!(ui.workspace_changes.len(), 64);
+        assert!(ui.workspace_changes.iter().all(|w| w.name != "W0"), "the oldest went");
+        switch(&mut ui, "W69").unwrap();
+        assert!(ui.control_bar, "the layout just left is kept");
+    }
+
+    /// Workspace names match whatever their case, as `find` does: "Mine" saved over "mine" replaces
+    /// it, and changes made under one spelling come back under the other.
+    #[test]
+    fn workspace_names_match_whatever_their_case() {
+        let mut ui = UiState::default();
+        save_as(&mut ui, "mine").unwrap();
+        save_as(&mut ui, "Mine").unwrap();
+        assert_eq!(ui.custom_workspaces.len(), 1, "one workspace, not two spellings");
+        ui.control_bar = true;
+        switch(&mut ui, ESSENTIALS).unwrap();
+        assert_eq!(switch(&mut ui, "MINE").unwrap(), "Mine");
+        assert!(ui.control_bar, "its changes came back");
+        // A preferences file with a remembered layout under another spelling, twice: it maps to the
+        // saved one, once.
+        ui.control_bar = false;
+        switch(&mut ui, ESSENTIALS).unwrap();
+        let mut odd = capture(&ui, "MINE");
+        odd.control_bar = true;
+        ui.workspace_changes = vec![odd.clone(), Workspace { name: "mine".into(), ..odd }];
+        let mut ui = ui.sanitized();
+        assert_eq!(ui.workspace_changes.iter().map(|w| w.name.as_str()).collect::<Vec<_>>(), ["Mine"]);
+        switch(&mut ui, "Mine").unwrap();
+        assert!(ui.control_bar);
+    }
+
+    /// Saving a workspace takes the current layout into it, which then starts with no changes.
+    #[test]
+    fn saving_a_workspace_moves_the_changes_into_it() {
+        let mut ui = UiState { control_bar: true, ..Default::default() };
+        save_as(&mut ui, "Mine").unwrap();
+        switch(&mut ui, ESSENTIALS).unwrap();
+        assert!(!ui.control_bar, "Essentials stays as saved: the changes went into Mine");
+        switch(&mut ui, "Mine").unwrap();
+        assert!(ui.control_bar);
+        // Saving again under the same name takes the current layout and starts with no changes.
+        ui.status_bar = false;
+        save_as(&mut ui, "Mine").unwrap();
+        assert!(ui.workspace_changes.iter().all(|w| w.name != "Mine"));
+        ui.status_bar = true;
+        assert_eq!(reset(&mut ui), "Mine");
+        assert!(!ui.status_bar);
+    }
+
     #[test]
     fn builtin_names_are_protected() {
         let mut ui = UiState::default();