From f9ab6fcbac428e15654a22eaae15df823a7f50b8 Mon Sep 17 00:00:00 2001 From: ruchirguitar <113586074+ruchirguitar@users.noreply.github.com> Date: Sat, 14 Nov 2026 14:13:47 +0530 Subject: [PATCH] fix(layers): resolve holding layer for object rows in others actions and check sublayer state --- crates/ui-egui/src/panels/layers/rows.rs | 26 +++++----- crates/ui-egui/src/panels/layers/tests.rs | 59 +++++++++++++++++++++++ 2 files changed, 73 insertions(+), 12 deletions(-) diff --git a/crates/ui-egui/src/panels/layers/rows.rs b/crates/ui-egui/src/panels/layers/rows.rs index 0e23e3739f..b898ce8bf8 100644 --- a/crates/ui-egui/src/panels/layers/rows.rs +++ b/crates/ui-egui/src/panels/layers/rows.rs @@ -517,14 +517,7 @@ fn row_context_menu(ui: &mut Ui, view: &View, n: &Node, collect_ids: &[u64], out /// When any other layer is unlocked (visible), returns `lockOthers` (`hideOthers`) preserving all layers holding /// `ids`. When all other layers are locked (hidden) already, returns `unlockAll` (`showAll`). pub(crate) fn multi_others_action(doc: &Document, ids: &[u64], eye: bool) -> (String, Value, bool) { - let own_layers: std::collections::HashSet = ids - .iter() - .filter_map(|&id| { - let nid = NodeId(id); - let own = doc.layer_of(nid); - doc.node(nid).map(|n| layer_row(n, own)) - }) - .collect(); + let own_layers: std::collections::HashSet = ids.iter().filter_map(|&id| doc.layer_of(NodeId(id))).collect(); let has_other_active = if eye { doc.layers.iter().any(|l| !own_layers.contains(&l.id) && l.visible) } else { @@ -537,15 +530,24 @@ pub(crate) fn multi_others_action(doc: &Document, ids: &[u64], eye: bool) -> (St (false, false) => "layer.unlockAll", }; let enabled = match (eye, has_other_active) { - (true, true) => true, - (true, false) => doc.layers.iter().any(|l| !l.visible), - (false, true) => true, - (false, false) => doc.layers.iter().any(|l| l.locked), + (true, true) | (false, true) => true, + (true, false) => any_layer_state(doc, |n| !n.visible), + (false, false) => any_layer_state(doc, |n| n.locked), }; let params = if cmd.ends_with("Others") { json!({ "ids": ids }) } else { json!({}) }; (cmd.into(), params, enabled) } +fn any_layer_state(doc: &Document, pred: impl Fn(&Node) -> bool) -> bool { + let mut found = false; + doc.walk(|n| { + if !found && n.is_layer() && pred(n) { + found = true; + } + }); + found +} + /// Alt-clicking row `n`'s eye (`eye`) or lock: Hide Others or Lock Others, every top-level layer /// but the one holding the row; when they all are hidden (locked) already, Show All Layers (Unlock /// All Layers). Each is one undo step. diff --git a/crates/ui-egui/src/panels/layers/tests.rs b/crates/ui-egui/src/panels/layers/tests.rs index 4a6c6ec0b6..c072a45828 100644 --- a/crates/ui-egui/src/panels/layers/tests.rs +++ b/crates/ui-egui/src/panels/layers/tests.rs @@ -2573,3 +2573,62 @@ fn context_menu_others_actions_preserve_multiselected_layers() { assert!(lock_enabled); assert_eq!(lock_params["ids"], json!([l1, l2])); } + +/// Context menu Others action resolves an object row to its holding top-level layer. +#[test] +fn context_menu_others_actions_object_row_resolves_to_holding_layer() { + let mut app = VectorcraftApp::new(Session::new(), Default::default()); + app.session.execute("file.new", &json!({"width": 200, "height": 200})).unwrap(); + // Layer 1 (current) holding a rectangle. + let rect = app.session.execute("shape.rectangle", &json!({"x": 10, "y": 10, "width": 50, "height": 50})).unwrap(); + let rect_id = rect["id"].as_u64().unwrap(); + + // Create Layer 2 and hide it. + let l2 = app.session.execute("layer.new", &json!({})).unwrap()["id"].as_u64().unwrap(); + app.session.execute("layer.setProps", &json!({"ids": [l2], "visible": false})).unwrap(); + + let doc = &app.session.active().unwrap().doc; + assert_eq!(doc.layers.len(), 2); + + // Right-clicking the object in Layer 1: Layer 1 is visible, Layer 2 is hidden. + // There are no other visible layers, so the action should be Show All Layers, enabled. + let (hide_cmd, _hide_params, hide_enabled) = super::rows::multi_others_action(doc, &[rect_id], true); + assert_eq!(hide_cmd, "layer.showAll"); + assert!(hide_enabled, "Show All Layers is enabled because Layer 2 is hidden"); + + // Now make Layer 2 visible, but lock it. + app.session.execute("layer.setProps", &json!({"ids": [l2], "visible": true, "locked": true})).unwrap(); + let doc = &app.session.active().unwrap().doc; + // Right-clicking the object in Layer 1: Layer 1 is unlocked, Layer 2 is locked. + // There are no other unlocked layers, so the action should be Unlock All Layers, enabled. + let (lock_cmd, _lock_params, lock_enabled) = super::rows::multi_others_action(doc, &[rect_id], false); + assert_eq!(lock_cmd, "layer.unlockAll"); + assert!(lock_enabled, "Unlock All Layers is enabled because Layer 2 is locked"); +} + +/// Context menu Show/Unlock All Layers are enabled when sublayers are hidden or locked. +#[test] +fn context_menu_others_actions_sublayer_state_enables_all_commands() { + let mut app = VectorcraftApp::new(Session::new(), Default::default()); + app.session.execute("file.new", &json!({"width": 200, "height": 200})).unwrap(); + let doc = &app.session.active().unwrap().doc; + assert_eq!(doc.layers.len(), 1); + let l1 = doc.layers[0].id.0; + + // Create a sublayer in Layer 1. + let sub = app.session.execute("layer.newSublayer", &json!({"parent": l1})).unwrap()["id"].as_u64().unwrap(); + + // Case 1: Sublayer is hidden while top layer is visible. + app.session.execute("layer.setProps", &json!({"ids": [sub], "visible": false})).unwrap(); + let doc = &app.session.active().unwrap().doc; + let (hide_cmd, _, hide_enabled) = super::rows::multi_others_action(doc, &[l1], true); + assert_eq!(hide_cmd, "layer.showAll"); + assert!(hide_enabled, "hidden sublayer enables Show All Layers"); + + // Case 2: Sublayer is visible, but locked while top layer is unlocked. + app.session.execute("layer.setProps", &json!({"ids": [sub], "visible": true, "locked": true})).unwrap(); + let doc = &app.session.active().unwrap().doc; + let (lock_cmd, _, lock_enabled) = super::rows::multi_others_action(doc, &[l1], false); + assert_eq!(lock_cmd, "layer.unlockAll"); + assert!(lock_enabled, "locked sublayer enables Unlock All Layers"); +}