From 8303cfa56f19fd11c13bb4f6933edd5717c7de03 Mon Sep 17 00:00:00 2001 From: RealtaNua Date: Fri, 16 Oct 2026 22:52:41 +0700 Subject: [PATCH 1/3] fix(engine): Shift-drag on a selected layer moves the selection (#498) Shift on a selected layer took it out of the selection on press, so the drag that followed had nothing to move, and with several layers selected the grabbed one stayed behind. It now leaves the selection on release, and only when the press didn't become a drag. Where no move can start (a version being viewed), it still leaves on press. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../engine/src/selection_behaviour_tests.rs | 89 +++++++++++++++++++ crates/engine/src/tools.rs | 38 +++++--- docs/ui-parity.md | 2 +- 3 files changed, 115 insertions(+), 14 deletions(-) diff --git a/crates/engine/src/selection_behaviour_tests.rs b/crates/engine/src/selection_behaviour_tests.rs index b65b7936..bc961a4a 100644 --- a/crates/engine/src/selection_behaviour_tests.rs +++ b/crates/engine/src/selection_behaviour_tests.rs @@ -275,3 +275,92 @@ fn locked_layers_selected_in_the_layers_panel_stay_put() { ex(&mut s, "object.toggleLock", json!({"ids": [k.g], "locked": true})); assert_eq!(handle_at(s.doc().unwrap(), &[k.a], Point::new(60.0, 60.0), 1.0), None); } + +#[test] +fn shift_drag_moves_the_selection() { + let mut s = Session::new(); + let k = scene(&mut s); + let at = |s: &Session, i: NodeId| s.doc().unwrap().world_bounds(i).unwrap().origin(); + let home = (Point::new(500.0, 0.0), Point::new(600.0, 0.0)); + // (selection before, Shift-drag to (from r at 520,20), selection after, where r and empty end up, derivation) + let rows: Vec<(Vec, (f64, f64), Vec, (Point, Point), &str)> = vec![ + ( + vec![k.r], + (560.0, 22.0), + vec![k.r], + (Point::new(540.0, 0.0), Point::new(600.0, 0.0)), + "Shift-drag on the selected layer moves it along one axis and keeps it selected", + ), + ( + vec![k.r, k.empty], + (560.0, 22.0), + vec![k.r, k.empty], + (Point::new(540.0, 0.0), Point::new(640.0, 0.0)), + "Shift-drag on one of several selected layers moves them all", + ), + ( + vec![k.empty], + (560.0, 22.0), + vec![k.r, k.empty], + (Point::new(540.0, 0.0), Point::new(640.0, 0.0)), + "Shift-drag on an unselected layer adds it and moves the selection", + ), + (vec![k.r, k.empty], (521.0, 21.0), vec![k.empty], home, "a Shift-press that wobbles under the drag threshold is a click: the layer leaves"), + ]; + for (before, to, after, end, why) in rows { + ex(&mut s, "select.set", json!({"ids": before})); + drag(&mut s, (520.0, 20.0), to, SHIFT); + assert_eq!(sorted(s.selection()), sorted(after), "{why}"); + assert_eq!((at(&s, k.r), at(&s, k.empty)), end, "{why}"); + if end != home { + assert!(s.undo().unwrap(), "{why}: one undo step"); + assert_eq!((at(&s, k.r), at(&s, k.empty)), home, "{why}: undone"); + } + } + // Shift let go before the button: still a Shift-click. + ex(&mut s, "select.set", json!({"ids": [k.r, k.empty]})); + s.pointer(&ev(PointerKind::Down, 520.0, 20.0, SHIFT)).unwrap(); + s.pointer(&ev(PointerKind::Up, 520.0, 20.0, NONE)).unwrap(); + assert_eq!(s.selection(), vec![k.empty], "Shift let go before the button still takes the layer out"); +} + +#[test] +fn alt_shift_drag_on_a_selected_layer_duplicates_it() { + let mut s = Session::new(); + let k = scene(&mut s); + let layers = |s: &Session| s.doc().unwrap().node(s.page().unwrap()).unwrap().children.len(); + let n = layers(&s); + let alt_shift = |kind, x, y| { + let mut e = ev(kind, x, y, SHIFT); + e.alt = true; + e + }; + ex(&mut s, "select.set", json!({"ids": [k.r]})); + s.pointer(&alt_shift(PointerKind::Down, 520.0, 20.0)).unwrap(); + s.pointer(&alt_shift(PointerKind::Up, 520.0, 20.0)).unwrap(); + assert_eq!(s.selection(), Vec::::new(), "an Alt+Shift-click is a Shift-click: the layer leaves the selection"); + assert_eq!(layers(&s), n, "and nothing is copied"); + ex(&mut s, "select.set", json!({"ids": [k.r]})); + s.pointer(&alt_shift(PointerKind::Down, 520.0, 20.0)).unwrap(); + s.pointer(&alt_shift(PointerKind::Move, 560.0, 22.0)).unwrap(); + s.pointer(&alt_shift(PointerKind::Up, 560.0, 22.0)).unwrap(); + let sel = s.selection(); + assert_eq!(layers(&s), n + 1, "an Alt+Shift-drag copies the layer"); + assert!(sel.len() == 1 && sel[0] != k.r, "the copy is selected"); + let doc = s.doc().unwrap(); + assert_eq!(doc.world_bounds(sel[0]).unwrap().origin(), Point::new(540.0, 0.0), "the copy moves along one axis"); + assert_eq!(doc.world_bounds(k.r).unwrap().origin(), Point::new(500.0, 0.0), "the original stays"); +} + +#[test] +fn shift_click_takes_a_layer_out_where_nothing_can_move() { + let mut s = Session::new(); + let k = scene(&mut s); + // A version being viewed is read-only: the press can't start a move. + let v = ex(&mut s, "file.saveVersion", json!({"name": "v"})); + ex(&mut s, "version.preview", json!({"id": v["id"]})); + ex(&mut s, "select.set", json!({"ids": [k.r, k.empty]})); + assert!(s.pointer(&ev(PointerKind::Down, 520.0, 20.0, SHIFT)).is_err(), "a version being viewed can't be edited"); + s.pointer(&ev(PointerKind::Up, 520.0, 20.0, SHIFT)).unwrap(); + assert_eq!(s.selection(), vec![k.empty], "Shift-click still takes the layer out"); +} diff --git a/crates/engine/src/tools.rs b/crates/engine/src/tools.rs index 3d3b9acb..d074052f 100644 --- a/crates/engine/src/tools.rs +++ b/crates/engine/src/tools.rs @@ -344,6 +344,9 @@ pub(crate) enum Gesture { targets: Targets, duplicate: bool, moved: bool, + /// A Shift-press on this selected layer: it leaves the selection on release, unless the + /// press became a drag. + deselect: Option, }, Resize { start: Point, @@ -1194,15 +1197,15 @@ impl Session { Some(target) => { // Clicking inside the current selection's box keeps it (drag them all). let in_sel_box = sel.len() > 1 && doc.bounds_of(&sel).is_some_and(|b| b.contains(ev.p())) && !ev.shift; + // Shift on a selected layer takes it out on release if the press doesn't become a + // drag; a drag moves the selection with it. Where no drag can start, it goes now. + let deselect = (ev.shift && sel.contains(&target)).then_some(target); + let without_target = || sel.iter().copied().filter(|x| *x != target).collect::>(); let new_sel = if in_sel_box && (sel.contains(&target) || sel.iter().any(|s| doc.is_ancestor(*s, target))) { sel.clone() - } else if ev.shift { + } else if ev.shift && deselect.is_none() { let mut s2 = sel.clone(); - if let Some(pos) = s2.iter().position(|x| *x == target) { - s2.remove(pos); - } else { - s2.push(target); - } + s2.push(target); s2 } else if sel.contains(&target) { sel.clone() @@ -1210,12 +1213,12 @@ impl Session { vec![target] }; self.set_selection(new_sel.clone()); - if new_sel.is_empty() || doc.node(target).is_some_and(|n| n.locked) { - return Ok(()); - } // Locked layers selected in the layers panel stay put. let ids: Vec = roots(&doc, &new_sel).into_iter().filter(|i| !locked_in(&doc, *i)).collect(); - if ids.is_empty() { + if new_sel.is_empty() || doc.node(target).is_some_and(|n| n.locked) || ids.is_empty() { + if deselect.is_some() { + self.set_selection(without_target()); + } return Ok(()); } let mut targets = Targets::gather(&doc, page, &ids); @@ -1225,8 +1228,13 @@ impl Session { { targets.add_box(b); } - self.begin_interaction(if ev.alt { "Duplicate" } else { "Move" })?; - self.tool.gesture = Gesture::Move { start: ev.p(), ids, targets, duplicate: ev.alt, moved: false }; + if let Err(e) = self.begin_interaction(if ev.alt { "Duplicate" } else { "Move" }) { + if deselect.is_some() { + self.set_selection(without_target()); + } + return Err(e); + } + self.tool.gesture = Gesture::Move { start: ev.p(), ids, targets, duplicate: ev.alt, moved: false, deselect }; self.tool.overlay.cursor = if ev.alt { Cursor::Copy } else { Cursor::Default }; } None => { @@ -1735,7 +1743,7 @@ impl Session { self.tool.overlay.drop_cell = None; let drop_target = self.tool.overlay.drop_target.take(); match g { - Gesture::Move { ids, moved, duplicate, start, .. } => { + Gesture::Move { ids, moved, duplicate, start, deselect, .. } => { if moved { // A free move lands in the frame under the pointer (or the page) where it was // dropped; layers in an auto layout frame already sit in their slot. @@ -1777,6 +1785,10 @@ impl Session { } } else { self.cancel_interaction()?; + if let Some(d) = deselect { + let sel = self.selection().into_iter().filter(|x| *x != d).collect(); + self.set_selection(sel); + } if !(ev.shift || ev.cmd || duplicate) { self.widget_click(start, ev.tol() * 2.0)?; } diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 62001715..a8dfab69 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -48,7 +48,7 @@ interaction feel not yet compared. | Item | Status | Notes | |---|---|---| -| Click, Shift-click, marquee, Shift-marquee toggle | done | a marquee selects what it touches; from a top-level frame's background, the layers inside it; locked and hidden layers are skipped (`selection_behaviour_tests`) | +| Click, Shift-click, marquee, Shift-marquee toggle | done | a marquee selects what it touches; from a top-level frame's background, the layers inside it; locked and hidden layers are skipped; Shift-click takes a selected layer out on release, so a Shift-drag on it moves the selection along one axis instead (`selection_behaviour_tests`) | | Deep select (Cmd/Ctrl-click, Cmd/Ctrl-marquee) | done | | | Select layer menu (every layer under the click) | done | | | Enter / Shift+Enter (children / parent), Tab / Shift+Tab (siblings) | done | Tab renames the next layer while renaming | From 569a3b3669c409a0e90dcad6b2424e1134b35a6c Mon Sep 17 00:00:00 2001 From: RealtaNua Date: Fri, 16 Oct 2026 23:11:00 +0700 Subject: [PATCH 2/3] fix(engine): a Shift-click on a selected widget doesn't also click it (#498) With Shift let go before the button, the release took the layer out of the selection and also ran the widget's click. The take-out now skips the widget click, as a Shift-click always has. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/engine/src/tools.rs | 2 +- crates/engine/src/widgets_tests.rs | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/tools.rs b/crates/engine/src/tools.rs index d074052f..dbda1894 100644 --- a/crates/engine/src/tools.rs +++ b/crates/engine/src/tools.rs @@ -1789,7 +1789,7 @@ impl Session { let sel = self.selection().into_iter().filter(|x| *x != d).collect(); self.set_selection(sel); } - if !(ev.shift || ev.cmd || duplicate) { + if deselect.is_none() && !(ev.shift || ev.cmd || duplicate) { self.widget_click(start, ev.tol() * 2.0)?; } } diff --git a/crates/engine/src/widgets_tests.rs b/crates/engine/src/widgets_tests.rs index 7079905f..f228d35c 100644 --- a/crates/engine/src/widgets_tests.rs +++ b/crates/engine/src/widgets_tests.rs @@ -248,6 +248,19 @@ fn clicking_a_widget_on_the_canvas_calls_its_plugin_back() { assert!(after.x - before.x > 30.0 && state(&mut s, w) == json!({"n": 2}), "{before:?} → {after:?}"); } +#[test] +fn a_shift_click_on_a_selected_widget_only_deselects_it() { + let mut s = session(); + let (w, _) = insert(&mut s); + let target = s.doc().unwrap().world_bounds(node(&s, w).children[1]).unwrap().center(); + let mut down = PointerEvent::at(PointerKind::Down, target.x, target.y, 1.0); + down.shift = true; + // Shift let go before the button: still a Shift-click, not a click on the widget. + s.pointer(&down).unwrap(); + s.pointer(&PointerEvent::at(PointerKind::Up, target.x, target.y, 1.0)).unwrap(); + assert_eq!((s.selection(), state(&mut s, w)), (vec![], json!({"n": 1}))); +} + #[test] fn bad_widgets_and_clicks_are_refused_and_roll_back() { let mut s = session(); From ef684c5df4bbde407400194260aa537c2636cbc5 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Sat, 17 Oct 2026 14:58:06 +0700 Subject: [PATCH 3/3] #691: leave the parity docs to the maintainers' docs PR --- docs/ui-parity.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/ui-parity.md b/docs/ui-parity.md index a8dfab69..62001715 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -48,7 +48,7 @@ interaction feel not yet compared. | Item | Status | Notes | |---|---|---| -| Click, Shift-click, marquee, Shift-marquee toggle | done | a marquee selects what it touches; from a top-level frame's background, the layers inside it; locked and hidden layers are skipped; Shift-click takes a selected layer out on release, so a Shift-drag on it moves the selection along one axis instead (`selection_behaviour_tests`) | +| Click, Shift-click, marquee, Shift-marquee toggle | done | a marquee selects what it touches; from a top-level frame's background, the layers inside it; locked and hidden layers are skipped (`selection_behaviour_tests`) | | Deep select (Cmd/Ctrl-click, Cmd/Ctrl-marquee) | done | | | Select layer menu (every layer under the click) | done | | | Enter / Shift+Enter (children / parent), Tab / Shift+Tab (siblings) | done | Tab renames the next layer while renaming |