From 0ff98d038014d5e2dfef02c0261cc25a58a508e2 Mon Sep 17 00:00:00 2001 From: ruchirguitar <113586074+ruchirguitar@users.noreply.github.com> Date: Sat, 14 Nov 2026 09:47:46 +0530 Subject: [PATCH 1/5] feat(layers): implement row context menu and actions - Add right-click context menu on layer, group, and object rows: - Options... / Layer Options... / Options for Selection... - Duplicate / Duplicate Selection - Delete / Delete Selection - Lock Others / Unlock All Layers - Hide Others / Show All Layers - Locate Object - Collect for Export - Select unselected row on right-click; preserve multiselection if already highlighted - Dispatch actions via registered engine commands with Undo/Redo tracking - Add headless egui tests covering selection, dispatch, and state integrity - Update gaps and ui-parity documentation for G12 --- crates/ui-egui/src/panels/layers/rows.rs | 91 ++++++++++++++++++++++- crates/ui-egui/src/panels/layers/tests.rs | 85 +++++++++++++++++++++ docs/gaps.md | 5 +- docs/ui-parity.md | 4 +- 4 files changed, 178 insertions(+), 7 deletions(-) diff --git a/crates/ui-egui/src/panels/layers/rows.rs b/crates/ui-egui/src/panels/layers/rows.rs index 4e5b7eb218..4dd9d645c2 100644 --- a/crates/ui-egui/src/panels/layers/rows.rs +++ b/crates/ui-egui/src/panels/layers/rows.rs @@ -390,12 +390,14 @@ pub(super) fn row(ui: &mut Ui, view: &View, item: &Row, out: &mut Out) { out.actions.push(("ui.layerOptions".into(), json!({"ids": [n.id.0]}))); } } - // Row menu: Collect for Export on this row (or every highlighted row when this is one of them). + // Row menu: context actions on this row (or every highlighted row when this is one of them). let collect_ids: Vec = if view.rows.contains(&n.id) { view.rows.iter().map(|id| id.0).collect() } else { vec![n.id.0] }; + // Right-clicking an unselected row selects it; a highlighted row keeps multiselection. + if resp.secondary_clicked() && !view.rows.contains(&n.id) { + out.actions.push(("layer.setCurrent".into(), json!({"id": n.id.0}))); + } resp.context_menu(|ui| { - collect_for_export_menu(ui, true, |multiple| { - out.actions.push(("assets.add".into(), json!({ "ids": collect_ids.clone(), "multiple": multiple }))); - }); + row_context_menu(ui, view, n, &collect_ids, out); }); // Dragging rows: the highlighted rows when this is one of them, else this row. Only the primary // button drags; the secondary button opens the row menu, and a drag with it moves nothing. @@ -414,6 +416,87 @@ pub(super) fn row(ui: &mut Ui, view: &View, item: &Row, out: &mut Out) { drop_target(ui, view, n, r, x - 16.0, open, &resp, out); } +/// The right-click context menu of row `n`. +fn row_context_menu(ui: &mut Ui, view: &View, n: &Node, collect_ids: &[u64], out: &mut Out) { + let is_multi = collect_ids.len() > 1; + // "Layer Options…" / "Options for Selection…" / "Options…" + let options_label = if is_multi { + "Options for Selection…" + } else if n.is_layer() { + "Layer Options…" + } else { + "Options…" + }; + if widgets::menu_item(ui, options_label, true, false) { + out.actions.push(("ui.layerOptions".into(), json!({ "ids": collect_ids }))); + ui.close(); + } + // "Duplicate """ or "Duplicate Selection" + let display = { + let raw = painted_name(n, &n.display_name(), crate::i18n::current()); + if raw.chars().count() > 20 { + let truncated: String = raw.chars().take(20).collect(); + format!("{truncated}…") + } else { + raw + } + }; + let dup_label = if is_multi { tl!("Duplicate Selection").to_string() } else { format!("{} \"{display}\"", tl!("Duplicate")) }; + if widgets::menu_item_name(ui, &dup_label, true, false) { + out.actions.push(("layer.duplicate".into(), json!({ "ids": collect_ids }))); + ui.close(); + } + // "Delete """ or "Delete Selection" + let del_label = if is_multi { tl!("Delete Selection").to_string() } else { format!("{} \"{display}\"", tl!("Delete")) }; + if widgets::menu_item_name(ui, &del_label, true, false) { + out.actions.push(("layer.delete".into(), json!({ "ids": collect_ids }))); + ui.close(); + } + ui.separator(); + // Lock Others / Unlock All Layers + { + let (lock_cmd, lock_params) = others_action(view.doc, n, false); + let lock_label = if lock_cmd == "layer.lockOthers" { "Lock Others" } else { "Unlock All Layers" }; + let own = view.doc.layer_of(n.id); + let lock_enabled = if lock_cmd == "layer.lockOthers" { + view.doc.layers.iter().any(|l| Some(l.id) != own && !l.locked) + } else { + view.doc.layers.iter().any(|l| l.locked) + }; + if widgets::menu_item(ui, lock_label, lock_enabled, false) { + out.actions.push((lock_cmd, lock_params)); + ui.close(); + } + } + // Hide Others / Show All Layers + { + let (hide_cmd, hide_params) = others_action(view.doc, n, true); + let hide_label = if hide_cmd == "layer.hideOthers" { "Hide Others" } else { "Show All Layers" }; + let own = view.doc.layer_of(n.id); + let hide_enabled = if hide_cmd == "layer.hideOthers" { + view.doc.layers.iter().any(|l| Some(l.id) != own && l.visible) + } else { + view.doc.layers.iter().any(|l| !l.visible) + }; + if widgets::menu_item(ui, hide_label, hide_enabled, false) { + out.actions.push((hide_cmd, hide_params)); + ui.close(); + } + } + ui.separator(); + // Locate Object + if widgets::menu_item(ui, "Locate Object", true, false) { + out.actions.push(("layer.locate".into(), json!({ "id": n.id.0 }))); + ui.close(); + } + ui.separator(); + // Collect for Export + let ids: Vec = collect_ids.to_vec(); + collect_for_export_menu(ui, true, move |multiple| { + out.actions.push(("assets.add".into(), json!({ "ids": ids.clone(), "multiple": multiple }))); + }); +} + /// 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 0fd8586409..b8f605b460 100644 --- a/crates/ui-egui/src/panels/layers/tests.rs +++ b/crates/ui-egui/src/panels/layers/tests.rs @@ -1969,3 +1969,88 @@ fn guide_rows_list_by_index_whatever_the_node_order() { let indexes: Vec = rows.iter().filter_map(|r| r.guide).collect(); assert_eq!(indexes, [2, 1, 0]); } + +/// Right-clicking a row that is not highlighted selects it. +#[test] +fn context_menu_right_click_selects_unselected_row() { + let mut app = VectorcraftApp::new(Session::new(), Default::default()); + app.session.execute("file.new", &json!({"width": 200, "height": 200})).unwrap(); + app.session.execute("layer.new", &json!({})).unwrap(); + app.session.execute("layer.highlight", &json!({"ids": [], "mode": "set"})).unwrap(); + let ctx = egui::Context::default(); + // Rows top-first: Layer 2, Layer 1. Neither is highlighted. + let (circles, _) = frame(&mut app, &ctx, vec![], false); + assert_eq!(circles.len(), 2); + assert!(rows_of(&app).is_empty()); + // Secondary-click Layer 1's row (bottom row). + let pos = circles[1] - vec2(100.0, 0.0); // name area + frame(&mut app, &ctx, vec![egui::Event::PointerMoved(pos)], false); + frame(&mut app, &ctx, vec![button_by(pos, true, egui::PointerButton::Secondary)], false); + frame(&mut app, &ctx, vec![button_by(pos, false, egui::PointerButton::Secondary)], false); + // The row is now highlighted. + let highlighted = rows_of(&app); + let layer1 = app.session.active().unwrap().doc.layers[0].id; + assert!(highlighted.contains(&layer1), "right-click selected the unselected row"); +} + +/// Right-clicking a row that IS highlighted preserves the multiselection. +#[test] +fn context_menu_right_click_preserves_multiselection() { + let mut app = VectorcraftApp::new(Session::new(), Default::default()); + app.session.execute("file.new", &json!({"width": 200, "height": 200})).unwrap(); + app.session.execute("layer.new", &json!({})).unwrap(); + app.session.execute("layer.new", &json!({})).unwrap(); + let doc = &app.session.active().unwrap().doc; + let ids: Vec = doc.layers.iter().map(|l| l.id.0).collect(); + // Highlight all 3 layers. + app.session.execute("layer.highlight", &json!({"ids": ids, "mode": "set"})).unwrap(); + let ctx = egui::Context::default(); + let (circles, _) = frame(&mut app, &ctx, vec![], false); + assert_eq!(circles.len(), 3); + // Secondary-click the middle row. + let pos = circles[1] - vec2(100.0, 0.0); + frame(&mut app, &ctx, vec![egui::Event::PointerMoved(pos)], false); + frame(&mut app, &ctx, vec![button_by(pos, true, egui::PointerButton::Secondary)], false); + frame(&mut app, &ctx, vec![button_by(pos, false, egui::PointerButton::Secondary)], false); + // All 3 must still be highlighted. + assert_eq!(rows_of(&app).len(), 3, "multiselection preserved after right-click on a highlighted row"); +} + +/// layer.delete removes the specified layer. +#[test] +fn context_menu_delete_dispatches_layer_delete() { + let mut app = VectorcraftApp::new(Session::new(), Default::default()); + app.session.execute("file.new", &json!({"width": 200, "height": 200})).unwrap(); + let layer2_id = app.session.execute("layer.new", &json!({})).unwrap()["id"].as_u64().unwrap(); + let before = app.session.active().unwrap().doc.layers.len(); + // Dispatch layer.delete with explicit ids (what the context menu item does). + app.session.execute("layer.delete", &json!({"ids": [layer2_id]})).unwrap(); + let after = app.session.active().unwrap().doc.layers.len(); + assert_eq!(after, before - 1, "layer.delete removes the layer"); +} + +/// layer.duplicate adds a copy. +#[test] +fn context_menu_duplicate_dispatches_layer_duplicate() { + let mut app = VectorcraftApp::new(Session::new(), Default::default()); + app.session.execute("file.new", &json!({"width": 200, "height": 200})).unwrap(); + let layer_id = app.session.active().unwrap().doc.layers[0].id.0; + let before_layers = app.session.active().unwrap().doc.layers.len(); + let before_undo = app.session.active().unwrap().history.undo.len(); + app.session.execute("layer.duplicate", &json!({"ids": [layer_id]})).unwrap(); + let after = &app.session.active().unwrap(); + assert_eq!(after.doc.layers.len(), before_layers + 1, "layer.duplicate adds a layer"); + assert_eq!(after.history.undo.len(), before_undo + 1, "one undo step"); +} + +/// layer.locate locates a specified object id without requiring a prior canvas selection. +#[test] +fn context_menu_locate_object_locates_by_id() { + let (mut app, _, [a, _]) = two_rects(); + app.session.execute("select.none", &json!({})).unwrap(); + assert!(app.session.active().unwrap().selection.objects.is_empty()); + // Context menu passes the node's id directly to layer.locate. + let result = app.session.execute("layer.locate", &json!({"id": a.0})); + assert!(result.is_ok(), "layer.locate with id succeeds without selection: {:?}", result.err()); + assert_eq!(app.session.active().unwrap().highlighted_rows(), vec![a]); +} diff --git a/docs/gaps.md b/docs/gaps.md index d386be463f..d5d5eefeba 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -34,6 +34,7 @@ Artboard Options forwards the active Artboard tool's Scale Artwork with Artboard | Date | Change | Summary | |---|---|---| +| 2026-11-14 | minor | G12: Layers panel row context menu (Options…, Duplicate, Delete, Lock Others/Show All/Hide Others/Unlock All, Locate Object, Collect for Export) with selection handling and undo tracking. Scores and estimates unchanged. | | 2026-11-13 | minor | PDF import: preserve Type 3 font text runs as named groups ``, or as live text if `/ToUnicode` exists on non-colour fonts (#6740); scores and estimates unchanged. | | 2026-11-08 | minor | #3380, deletion only: preserve gradient placement and exact surviving geometry; consume owned subpaths with a shared original-node snapshot; consolidate status/evidence and document command/helper contracts. Compound behavior and the existing M2 status entry are retained. Endpoint highlighting remains open; scores and estimates unchanged. | | 2026-11-07 | minor | #3380 (deletion only): independent surviving path objects; transformed appearance, stacking, selection, cubic handles and Undo/Redo covered by `delete_path_pieces`; compound semantics retained. Endpoint highlighting remains open; scores and estimates unchanged. | @@ -498,12 +499,12 @@ macOS cursor delivery (#2292) is narrowed to a native host adapter using the exi - **Done:** Hyphenation › Default Language is the language of new type, unless Document Setup chose one, and each run hyphenates in its own language (Character panel). English and eleven Indian languages have break points; other languages stay whole apart from the Exceptions list (#394, #4947). +- **Done (2026-11-14):** Layers panel row context menu: right-clicking a layer, group, or object row shows a context menu (Options…, Duplicate, Delete, Lock Others / Unlock All, Hide Others / Show All, Locate Object, Collect for Export) with proper selection behavior and Undo/Redo tracking. Tests in `crates/ui-egui/src/panels/layers/tests.rs`. - **Missing:** 2 Preferences options stored but not acted on (#394), each waiting on a subsystem of its own: Plug-ins & Scratch Disks › Primary and Secondary (documents are kept in memory; nothing pages to disk), and Clipboard Handling › Legacy vector clipboard with its Preserve Paths / Preserve Appearance and Overprints choice (Illustrator's own clipboard flavour, which has no public specification). Also Properties - panel per context, context menus beyond the canvas, Layers panel row menus and Collect for Export, SVG - Interactivity. + panel per context, context menus beyond the canvas, SVG Interactivity. - **Impact:** settings that silently do nothing erode trust. - **Estimate:** 13–20 h. diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 5f10ea4841..fb40a8054e 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -51,6 +51,7 @@ Artboard Options forwards the active Artboard tool's Scale Artwork with Artboard | Date | Change | Summary | |---|---|---| +| 2026-11-14 | minor | Layers panel: right-clicking a layer/group/object row shows a context menu with Options…, Duplicate, Delete, Lock/Hide Others, Locate Object, and Collect for Export — matching standard desktop layer panel conventions. Scores unchanged. | | 2026-11-13 | minor | Swatches: in a panel made taller the swatch list fills the space above the footer instead of scrolling inside 150 or 200 pt (#1638). Scores unchanged. | | 2026-11-08 | minor | Style panels: a double-click on a user character style's row opens its Character Style Options (`ui.charStyleOptions`), and on a paragraph style's row its Paragraph Style Options (rename); the Character Style Options dialog keeps a fixed width and scrolls its sections in a shorter window, so OK and Cancel stay on screen. Tests `panels::text_styles::tests::double_clicking_a_character_style_row_opens_its_options` and `dialogs::char_style_options::tests::the_sections_scroll_in_a_window_shorter_than_the_general_list`. Scores and estimates unchanged. | | 2026-11-08 | minor | #3380, deletion only: preserve gradient placement and exact surviving geometry; consume owned subpaths with a shared original-node snapshot; consolidate status/evidence and document command/helper contracts. Compound behavior and the existing M2 status entry are retained. Endpoint highlighting remains open; scores and estimates unchanged. | @@ -430,7 +431,8 @@ next unit in the order a right-click on a ruler lists them, from Feet back to Po | Pathfinder panel › Pathfinder Options…: Precision, Remove Redundant Points, Divide and Outline Will Remove Unpainted Artwork | done | #3426; `object.pathfinder.options`, kept in the preferences; defaults (0.01 pt, both off) keep the earlier results; [spec](https://helpx.adobe.com/illustrator/desktop/manage-objects/reshape-transform-objects/pathfinder-panel-overview.html) | | Cmd/Ctrl+4 repeats the last successful Shape Mode or Pathfinder operation on the selection | done | Panel operations, direct `object.pathfinder.*` commands and `command.batch` steps (Actions playback) set history; `action.batch`, Trap, compound-shape edits and live effects do not. Repeat uses current Pathfinder Options; [key spec](https://web.archive.org/web/20260726060015/https://helpx.adobe.com/illustrator/using/default-keyboard-shortcuts.html), [menu spec](https://helpx.adobe.com/illustrator/desktop/manage-objects/reshape-transform-objects/pathfinder-panel-overview.html) | | Pathfinder panel: compound shapes (Make, Release and Expand Compound Shape in the panel menu, Alt-click a Shape Mode, a Shape Mode for selected members, the Expand button) | partial | #4522; `object.compoundShape.make`/`setMode`/`release`/`expand`; members stay editable, the area draws, exports and saves; the button of the members' shared mode is lit; blends, envelopes and warped objects are members as they draw; [spec](https://helpx.adobe.com/illustrator/desktop/manage-objects/reshape-transform-objects/create-compound-shapes-with-pathfinder.html); missing: question marks for mixed modes, hit-testing by the area | -| Canvas context menu | done | Layers rows, panels and type have none (gap) | +| Canvas context menu | done | Panels and type have none (gap) | +| Layers panel row context menu | done | Right-clicking a layer/group/object row shows a context menu with Options…, Duplicate, Delete, Lock/Hide Others, Locate Object, and Collect for Export — matching standard desktop layer panel conventions. | | SVG Interactivity panel: JavaScript for each object's events and the document's linked JavaScript files, written to SVG and read back as text | done | `svgInteractivity.*`, `ui.javascriptFilesDialog` | | Retype, Comments, Version History panels | missing | the last two are cloud features (out of scope) | | Blend panel (30.7) | partial | Window › Blend provides Type (Ease In, Ease Out, Ease In Out, Ease Out In), Ramp 0–100% and independent color Shift, alongside the existing blend controls. Easing is disabled without a spine. Original polynomial distributions approximate the publicly documented behavior; on-canvas easing handles remain missing. [Public source](https://helpx.adobe.com/illustrator/desktop/manage-colors/apply-transparency-and-blending/blend-panel-overview.html) | From ea2deffde1e902e399764dd90c747a872157267b Mon Sep 17 00:00:00 2001 From: ruchirguitar <113586074+ruchirguitar@users.noreply.github.com> Date: Sat, 14 Nov 2026 10:28:44 +0530 Subject: [PATCH 2/5] style(svg): format craft-svg delegation in text_within --- crates/svg/src/lib.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/crates/svg/src/lib.rs b/crates/svg/src/lib.rs index 23cc07d3c1..b035e6cd10 100644 --- a/crates/svg/src/lib.rs +++ b/crates/svg/src/lib.rs @@ -491,16 +491,10 @@ pub(crate) fn text_within<'a>(bytes: &'a [u8], unpacked: &std::sync::atomic::Ato match craft_svg::svgz::inflate(bytes, most) { Ok(raw) => { unpacked.fetch_add(raw.len() as u64, Ordering::Relaxed); - craft_svg::svgz::decode(&raw) - .map(|t| std::borrow::Cow::Owned(t.into_owned())) - .map_err(|_| not_utf8()) - } - Err(craft_svg::svgz::Error::TooLarge(_)) => { - Err(SvgError::TooBig(format!("the SVGZ file unpacks to more than {} MB", most >> 20))) - } - Err(craft_svg::svgz::Error::Corrupt(e)) => { - Err(SvgError::Parse(format!("not a valid SVGZ file: {e}"))) + craft_svg::svgz::decode(&raw).map(|t| std::borrow::Cow::Owned(t.into_owned())).map_err(|_| not_utf8()) } + Err(craft_svg::svgz::Error::TooLarge(_)) => Err(SvgError::TooBig(format!("the SVGZ file unpacks to more than {} MB", most >> 20))), + Err(craft_svg::svgz::Error::Corrupt(e)) => Err(SvgError::Parse(format!("not a valid SVGZ file: {e}"))), Err(craft_svg::svgz::Error::Encoding) => Err(not_utf8()), Err(e) => Err(SvgError::Parse(e.to_string())), } From 46a4aa472b4cab6cc41ac010b097f974eda19a10 Mon Sep 17 00:00:00 2001 From: ruchirguitar <113586074+ruchirguitar@users.noreply.github.com> Date: Sat, 14 Nov 2026 11:51:06 +0530 Subject: [PATCH 3/5] fix(xtask): disambiguate wasm packages with workspace versions --- xtask/src/main.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/xtask/src/main.rs b/xtask/src/main.rs index c827a1ef74..74b6e6f65d 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -205,8 +205,11 @@ fn cmd_layers() -> Result<(), String> { /// Workspace packages that must build for wasm32: all L0–L5 crates, the egui /// shell, and the web app (its wasm-only code is compiled by nothing else). -fn wasm_set() -> Result, String> { - let crates = layers::from_metadata(&metadata()?)?; +fn wasm_set() -> Result, String> { + let meta = metadata()?; + let crates = layers::from_metadata(&meta)?; + let pkgs = meta["packages"].as_array().ok_or("no packages")?; + let versions: std::collections::HashMap<&str, &str> = pkgs.iter().filter_map(|p| Some((p["name"].as_str()?, p["version"].as_str()?))).collect(); Ok(crates .into_iter() .filter(|c| match layers::classify(&c.name) { @@ -214,7 +217,10 @@ fn wasm_set() -> Result, String> { Some(layers::Class::Standalone) => true, _ => layers::short_name(&c.name) == "web", }) - .map(|c| c.name) + .map(|c| { + let ver = versions.get(c.name.as_str()).copied().unwrap_or_default(); + (c.name, ver.to_string()) + }) .collect()) } @@ -224,10 +230,12 @@ fn cmd_wasm() -> Result<(), String> { let set = wasm_set()?; let mut c = cargo(); c.args(["check", "--target", "wasm32-unknown-unknown", "--keep-going"]); - for pkg in &set { - c.args(["-p", pkg]); + for (pkg, ver) in &set { + let spec = if ver.is_empty() { pkg.clone() } else { format!("{pkg}@{ver}") }; + c.args(["-p", &spec]); } - run(c, &format!("cargo check --target wasm32-unknown-unknown --keep-going ({} crates: {})", set.len(), set.join(", "))) + let names: Vec<&str> = set.iter().map(|(p, _)| p.as_str()).collect(); + run(c, &format!("cargo check --target wasm32-unknown-unknown --keep-going ({} crates: {})", set.len(), names.join(", "))) } /// Whether `cargo nextest` is installed. From b0379b03092834705269ebacae95c8e895798dbf Mon Sep 17 00:00:00 2001 From: ruchirguitar <113586074+ruchirguitar@users.noreply.github.com> Date: Sat, 14 Nov 2026 13:25:54 +0530 Subject: [PATCH 4/5] fix(layers): resolve review feedback for row key ownership, others actions, and delete gating --- crates/ui-egui/src/panels/layers/rows.rs | 177 +++++++++++++--------- crates/ui-egui/src/panels/layers/tests.rs | 48 ++++++ 2 files changed, 154 insertions(+), 71 deletions(-) diff --git a/crates/ui-egui/src/panels/layers/rows.rs b/crates/ui-egui/src/panels/layers/rows.rs index af657c0887..0e23e3739f 100644 --- a/crates/ui-egui/src/panels/layers/rows.rs +++ b/crates/ui-egui/src/panels/layers/rows.rs @@ -268,6 +268,66 @@ fn disclosure(ui: &Ui, view: &View, n: &Node, (r, x): (egui::Rect, f32), open: b } } +/// The row's name label or inline rename field. Returns (name_end, name_rect). +fn row_name(ui: &mut Ui, view: &View, item: &Row, r: egui::Rect, x: f32, out: &mut Out) -> (f32, egui::Rect) { + let n = &*item.node; + let doc = view.doc; + let t = &view.t; + let xml_id = view.identify_xml_id; + let name = if doc.mask_edit.is_some_and(|m| m.layer == n.id) { "".to_string() } else { n.identify_label(xml_id) }; + let font = egui::FontId::proportional(13.0); + let renaming: Option<(u64, String)> = ui.data(|d| d.get_temp(rename_id())); + let name_rect = egui::Rect::from_min_max(egui::pos2(x - 2.0, r.center().y - 10.0), egui::pos2(r.right() - 44.0, r.center().y + 10.0)); + let name_end = match renaming { + Some((rid, mut buf)) if rid == n.id.0 => { + let field = rename_id().with("field"); + let kept = crate::widgets::own_field_shift_enter(ui, field, &mut buf, &name); + let mut child = ui.new_child(egui::UiBuilder::new().max_rect(name_rect)); + let te = child + .add(egui::TextEdit::singleline(&mut buf).id(field).align(egui::Align2::LEFT_CENTER).desired_width(name_rect.width()).font(font)); + // Focus the field as it opens. Asking every frame took the focus back from Enter, + // Escape or a click elsewhere, so the rename never ended. + if !te.has_focus() && !te.lost_focus() { + te.request_focus(); + } + if te.lost_focus() { + ui.data_mut(|d| d.remove::<(u64, String)>(rename_id())); + if child.input(|i| !i.key_pressed(egui::Key::Escape)) && buf != name { + out.actions.push(("layer.setProps".into(), json!({"ids": [n.id.0], "name": buf}))); + } + } else { + // Shift+Enter renames it and keeps the field open. + if kept && buf != name { + out.actions.push(("layer.setProps".into(), json!({"ids": [n.id.0], "name": buf.clone()}))); + } + ui.data_mut(|d| d.insert_temp(rename_id(), (n.id.0, buf))); + } + name_rect.right() + } + _ => { + let painter = ui.painter().with_clip_rect(name_rect); + // Generated names ("", "") are translated where painted; the stored name stays English. + // XML IDs stay as ids (not translated). + let shown = if doc.mask_edit.is_some_and(|m| m.layer == n.id) { + tl!("").to_string() + } else if xml_id { + name + } else { + painted_name(n, &n.display_name(), crate::i18n::current()) + }; + let text = paint_name(&painter, egui::pos2(x, r.center().y), &shown, font, if item.dim { t.text_dim } else { t.text }, prints_italic(n)); + // A clipping path's name is underlined, a masked object's with a dashed line. + if item.clip_path { + painter.line_segment([text.left_bottom(), text.right_bottom()], Stroke::new(1.0, t.text)); + } else if n.extras().mask.is_some() { + painter.extend(egui::Shape::dashed_line(&[text.left_bottom(), text.right_bottom()], Stroke::new(1.0, t.text), 3.0, 2.0)); + } + text.right() + } + }; + (name_end, name_rect) +} + /// The widgets of row `item`. pub(super) fn row(ui: &mut Ui, view: &View, item: &Row, out: &mut Out) { if let Some(i) = item.guide { @@ -277,8 +337,7 @@ pub(super) fn row(ui: &mut Ui, view: &View, item: &Row, out: &mut Out) { return spine_row(ui, view, item, out); } let n = &*item.node; - let (depth, clip_path, has_children, open, color) = (item.depth, item.clip_path, item.opens, item.open, item.colour); - let doc = view.doc; + let (depth, has_children, open, color) = (item.depth, item.opens, item.open, item.colour); let t = &view.t; let h = view.h; let (r, resp) = ui.allocate_exact_size(vec2(ui.available_width(), h), Sense::click_and_drag()); @@ -309,60 +368,7 @@ pub(super) fn row(ui: &mut Ui, view: &View, item: &Row, out: &mut Out) { th }); // Name (Preferences › Units › Identify Objects By: Object Name or XML ID). - let xml_id = view.identify_xml_id; - let name = if doc.mask_edit.is_some_and(|m| m.layer == n.id) { "".to_string() } else { n.identify_label(xml_id) }; - let font = egui::FontId::proportional(13.0); - let renaming: Option<(u64, String)> = ui.data(|d| d.get_temp(rename_id())); - let name_rect = egui::Rect::from_min_max(egui::pos2(x - 2.0, r.center().y - 10.0), egui::pos2(r.right() - 44.0, r.center().y + 10.0)); - let name_end; - match renaming { - Some((rid, mut buf)) if rid == n.id.0 => { - let field = rename_id().with("field"); - let kept = crate::widgets::own_field_shift_enter(ui, field, &mut buf, &name); - let mut child = ui.new_child(egui::UiBuilder::new().max_rect(name_rect)); - let te = child.add( - egui::TextEdit::singleline(&mut buf).id(field).align(egui::Align2::LEFT_CENTER).desired_width(name_rect.width()).font(font.clone()), - ); - // Focus the field as it opens. Asking every frame took the focus back from Enter, - // Escape or a click elsewhere, so the rename never ended. - if !te.has_focus() && !te.lost_focus() { - te.request_focus(); - } - if te.lost_focus() { - ui.data_mut(|d| d.remove::<(u64, String)>(rename_id())); - if child.input(|i| !i.key_pressed(egui::Key::Escape)) && buf != name { - out.actions.push(("layer.setProps".into(), json!({"ids": [n.id.0], "name": buf}))); - } - } else { - // Shift+Enter renames it and keeps the field open. - if kept && buf != name { - out.actions.push(("layer.setProps".into(), json!({"ids": [n.id.0], "name": buf.clone()}))); - } - ui.data_mut(|d| d.insert_temp(rename_id(), (n.id.0, buf))); - } - name_end = name_rect.right(); - } - _ => { - let painter = ui.painter().with_clip_rect(name_rect); - // Generated names ("", "") are translated where painted; the stored name stays English. - // XML IDs stay as ids (not translated). - let shown = if doc.mask_edit.is_some_and(|m| m.layer == n.id) { - tl!("").to_string() - } else if xml_id { - name.clone() - } else { - painted_name(n, &n.display_name(), crate::i18n::current()) - }; - let text = paint_name(&painter, egui::pos2(x, r.center().y), &shown, font, if item.dim { t.text_dim } else { t.text }, prints_italic(n)); - name_end = text.right(); - // A clipping path's name is underlined, a masked object's with a dashed line. - if clip_path { - painter.line_segment([text.left_bottom(), text.right_bottom()], Stroke::new(1.0, t.text)); - } else if n.extras().mask.is_some() { - painter.extend(egui::Shape::dashed_line(&[text.left_bottom(), text.right_bottom()], Stroke::new(1.0, t.text), 3.0, 2.0)); - } - } - } + let (name_end, name_rect) = row_name(ui, view, item, r, x, out); // Target circle and selection square. let col_x = r.right() - 43.5; ui.painter().line_segment([egui::pos2(col_x, r.top()), egui::pos2(col_x, r.bottom())], Stroke::new(1.0, t.input_border)); @@ -409,6 +415,10 @@ pub(super) fn row(ui: &mut Ui, view: &View, item: &Row, out: &mut Out) { // Right-clicking an unselected row selects it; a highlighted row keeps multiselection. if resp.secondary_clicked() && !view.rows.contains(&n.id) { out.actions.push(("layer.setCurrent".into(), json!({"id": n.id.0}))); + out.rows_after_click = Some(vec![n.id]); + } + if resp.secondary_clicked() { + crate::panels::set_row_keys(ui.ctx(), ROW_KEYS, true); } resp.context_menu(|ui| { row_context_menu(ui, view, n, &collect_ids, out); @@ -462,21 +472,19 @@ fn row_context_menu(ui: &mut Ui, view: &View, n: &Node, collect_ids: &[u64], out } // "Delete """ or "Delete Selection" let del_label = if is_multi { tl!("Delete Selection").to_string() } else { format!("{} \"{display}\"", tl!("Delete")) }; - if widgets::menu_item_name(ui, &del_label, true, false) { + let del_enabled = { + let listed: std::collections::HashSet = collect_ids.iter().map(|&id| NodeId(id)).collect(); + !view.doc.layers.iter().all(|l| listed.contains(&l.id)) + }; + if widgets::menu_item_name(ui, &del_label, del_enabled, false) { out.actions.push(("layer.delete".into(), json!({ "ids": collect_ids }))); ui.close(); } ui.separator(); // Lock Others / Unlock All Layers { - let (lock_cmd, lock_params) = others_action(view.doc, n, false); + let (lock_cmd, lock_params, lock_enabled) = multi_others_action(view.doc, collect_ids, false); let lock_label = if lock_cmd == "layer.lockOthers" { "Lock Others" } else { "Unlock All Layers" }; - let own = view.doc.layer_of(n.id); - let lock_enabled = if lock_cmd == "layer.lockOthers" { - view.doc.layers.iter().any(|l| Some(l.id) != own && !l.locked) - } else { - view.doc.layers.iter().any(|l| l.locked) - }; if widgets::menu_item(ui, lock_label, lock_enabled, false) { out.actions.push((lock_cmd, lock_params)); ui.close(); @@ -484,14 +492,8 @@ fn row_context_menu(ui: &mut Ui, view: &View, n: &Node, collect_ids: &[u64], out } // Hide Others / Show All Layers { - let (hide_cmd, hide_params) = others_action(view.doc, n, true); + let (hide_cmd, hide_params, hide_enabled) = multi_others_action(view.doc, collect_ids, true); let hide_label = if hide_cmd == "layer.hideOthers" { "Hide Others" } else { "Show All Layers" }; - let own = view.doc.layer_of(n.id); - let hide_enabled = if hide_cmd == "layer.hideOthers" { - view.doc.layers.iter().any(|l| Some(l.id) != own && l.visible) - } else { - view.doc.layers.iter().any(|l| !l.visible) - }; if widgets::menu_item(ui, hide_label, hide_enabled, false) { out.actions.push((hide_cmd, hide_params)); ui.close(); @@ -511,6 +513,39 @@ fn row_context_menu(ui: &mut Ui, view: &View, n: &Node, collect_ids: &[u64], out }); } +/// Context menu Others action for `ids`: Lock Others / Unlock All Layers or Hide Others / Show All Layers. +/// 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 has_other_active = if eye { + doc.layers.iter().any(|l| !own_layers.contains(&l.id) && l.visible) + } else { + doc.layers.iter().any(|l| !own_layers.contains(&l.id) && !l.locked) + }; + let cmd = match (eye, has_other_active) { + (true, true) => "layer.hideOthers", + (true, false) => "layer.showAll", + (false, true) => "layer.lockOthers", + (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), + }; + let params = if cmd.ends_with("Others") { json!({ "ids": ids }) } else { json!({}) }; + (cmd.into(), params, enabled) +} + /// 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 6288754aef..4a6c6ec0b6 100644 --- a/crates/ui-egui/src/panels/layers/tests.rs +++ b/crates/ui-egui/src/panels/layers/tests.rs @@ -2462,6 +2462,7 @@ fn context_menu_right_click_selects_unselected_row() { let highlighted = rows_of(&app); let layer1 = app.session.active().unwrap().doc.layers[0].id; assert!(highlighted.contains(&layer1), "right-click selected the unselected row"); + assert!(crate::panels::has_row_keys(&ctx, super::ROW_KEYS), "secondary click claims row keys"); } /// Right-clicking a row that IS highlighted preserves the multiselection. @@ -2525,3 +2526,50 @@ fn context_menu_locate_object_locates_by_id() { assert!(result.is_ok(), "layer.locate with id succeeds without selection: {:?}", result.err()); assert_eq!(app.session.active().unwrap().highlighted_rows(), vec![a]); } + +/// In a 1-layer document, the Delete action in the context menu is disabled when targeting that layer. +#[test] +fn context_menu_delete_disabled_when_every_layer_targeted() { + 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 layer_id = doc.layers[0].id; + + // Targeting every layer disables delete. + let listed: std::collections::HashSet = [layer_id].into_iter().collect(); + let del_enabled = !doc.layers.iter().all(|l| listed.contains(&l.id)); + assert!(!del_enabled, "Delete must be disabled when every layer is targeted"); + + // Add a second layer: delete is now enabled for one layer. + app.session.execute("layer.new", &json!({})).unwrap(); + let doc = &app.session.active().unwrap().doc; + assert_eq!(doc.layers.len(), 2); + let del_enabled = !doc.layers.iter().all(|l| listed.contains(&l.id)); + assert!(del_enabled, "Delete is enabled when at least one layer remains"); +} + +/// Others actions (Hide Others / Lock Others) retain all multiselected layers. +#[test] +fn context_menu_others_actions_preserve_multiselected_layers() { + let mut app = VectorcraftApp::new(Session::new(), Default::default()); + app.session.execute("file.new", &json!({"width": 200, "height": 200})).unwrap(); + app.session.execute("layer.new", &json!({})).unwrap(); + app.session.execute("layer.new", &json!({})).unwrap(); + let doc = &app.session.active().unwrap().doc; + assert_eq!(doc.layers.len(), 3); + let l1 = doc.layers[0].id.0; + let l2 = doc.layers[1].id.0; + + // Hide Others with multiselection [l1, l2] hides only l3. + let (hide_cmd, hide_params, hide_enabled) = super::rows::multi_others_action(doc, &[l1, l2], true); + assert_eq!(hide_cmd, "layer.hideOthers"); + assert!(hide_enabled); + assert_eq!(hide_params["ids"], json!([l1, l2])); + + // Lock Others with multiselection [l1, l2] locks only l3. + let (lock_cmd, lock_params, lock_enabled) = super::rows::multi_others_action(doc, &[l1, l2], false); + assert_eq!(lock_cmd, "layer.lockOthers"); + assert!(lock_enabled); + assert_eq!(lock_params["ids"], json!([l1, l2])); +} 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 5/5] 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"); +}