From dd93a6a2a8e692d87c85b0604aee499e1bf3b3eb Mon Sep 17 00:00:00 2001 From: Nicek Date: Wed, 28 Oct 2026 16:36:47 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20=E4=BF=AE=E5=A4=8D=20PR=20#1457=20?= =?UTF-8?q?=E7=9A=84=E9=80=89=E6=8B=A9=E4=B8=8E=E6=8B=96=E6=8B=BD=E8=AF=84?= =?UTF-8?q?=E5=AE=A1=E9=97=AE=E9=A2=98?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 标签在选择边界外也参与收窄，Dev Mode 单击已选图层只选它 - 区分点击命中图层与描边空白处的拖拽回退目标 - 重排圆环按按下时的修饰键判断点击，画布外松手视为拖拽结束 - 修复 clippy unnecessary_get_then_check --- crates/engine/src/canvas/tools/drag.rs | 18 ++++++++++--- crates/engine/src/devmode_behaviour_tests.rs | 13 ++++++++++ crates/engine/src/frame_label_tests.rs | 15 +++++++++++ crates/engine/src/property_copy_tests.rs | 2 +- .../engine/src/selection_behaviour_tests.rs | 15 +++++++++++ .../ui-egui/src/canvas/selection_spacing.rs | 15 +++++++---- crates/ui-egui/src/selection_spacing_tests.rs | 26 +++++++++++++++++++ 7 files changed, 94 insertions(+), 10 deletions(-) diff --git a/crates/engine/src/canvas/tools/drag.rs b/crates/engine/src/canvas/tools/drag.rs index 82772290..97dfc7a5 100644 --- a/crates/engine/src/canvas/tools/drag.rs +++ b/crates/engine/src/canvas/tools/drag.rs @@ -94,6 +94,10 @@ impl Session { let behind_sel = |t: Option| { t.is_none_or(|t| sel.iter().any(|s| *s != t && (doc.is_ancestor(t, *s) || (in_own_box(*s) && paints_below(&doc, t, *s))))) }; + // What the press hit (a label or a painted layer): a click that doesn't become a drag + // selects just it. A press that reaches nothing above the selection uses the selection as + // its drag target instead; that fallback is not a clicked layer. + let clicked = target; let target = match sel.iter().copied().find(|s| !locked_in(&doc, *s)) { Some(first) if label.is_none() && !ev.shift && behind_sel(target) && selection_box(&doc, &sel).is_some_and(|q| in_quad(&q, ev.p())) => { Some(first) @@ -109,7 +113,10 @@ impl Session { // 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 keep = (in_sel_box && !ev.cmd && (sel.contains(&target) || sel.iter().any(|s| doc.is_ancestor(*s, target)))).then_some(target); + // A release without a drag narrows the selection to the layer the press hit. A + // selected frame's name label sits outside the selection bounds, so it counts too. + let keep = clicked + .filter(|c| (label.is_some() || in_sel_box) && !ev.cmd && (sel.contains(c) || sel.iter().any(|s| doc.is_ancestor(*s, *c)))); let new_sel = if keep.is_some() { sel.clone() } else if ev.shift && deselect.is_none() { @@ -122,10 +129,13 @@ impl Session { vec![target] }; self.set_selection(new_sel.clone()); - // Dev Mode only selects (Shift-click still deselects): nothing moves. + // Dev Mode only selects (Shift-click still deselects): nothing moves. No move + // gesture starts, so a click selects the layer it hit now instead of on release. if !edits { if deselect.is_some() { self.set_selection(without_target()); + } else if let Some(k) = keep.filter(|_| sel.len() > 1) { + self.set_selection(vec![k]); } return Ok(()); } @@ -134,9 +144,9 @@ impl Session { 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()); - } else if keep.is_some() { + } else if let Some(k) = keep { // No drag can start here, so the click narrows the selection now. - self.set_selection(vec![target]); + self.set_selection(vec![k]); } return Ok(()); } diff --git a/crates/engine/src/devmode_behaviour_tests.rs b/crates/engine/src/devmode_behaviour_tests.rs index 38d1a9fb..47c33aa2 100644 --- a/crates/engine/src/devmode_behaviour_tests.rs +++ b/crates/engine/src/devmode_behaviour_tests.rs @@ -38,6 +38,19 @@ fn status(s: &mut Session, ids: &[NodeId], st: &str) { ex(s, "node.set", json!({"ids": ids, "props": {"devStatus": st}})); } +#[test] +fn a_dev_mode_click_on_a_selected_layer_selects_just_it() { + let mut s = Session::new(); + let a = idv(&ex(&mut s, "shape.rectangle", json!({"x": 0, "y": 0, "width": 40, "height": 40}))); + let b = idv(&ex(&mut s, "shape.rectangle", json!({"x": 80, "y": 0, "width": 40, "height": 40}))); + s.tool.dev_mode = true; + ex(&mut s, "select.set", json!({"ids": [a, b]})); + // Dev Mode starts no move gesture, so the click narrows the selection at the press itself. + s.pointer(&ev(PointerKind::Down, 20.0, 20.0)).unwrap(); + s.pointer(&ev(PointerKind::Up, 20.0, 20.0)).unwrap(); + assert_eq!(s.selection(), vec![a], "a click on one of several selected layers selects it"); +} + #[test] fn the_dev_mode_canvas_selects_and_pans_but_never_edits() { let mut s = Session::new(); diff --git a/crates/engine/src/frame_label_tests.rs b/crates/engine/src/frame_label_tests.rs index 3da9522f..9a4143ad 100644 --- a/crates/engine/src/frame_label_tests.rs +++ b/crates/engine/src/frame_label_tests.rs @@ -62,6 +62,21 @@ fn pressing_a_rotated_frames_label_selects_and_drags_the_frame() { assert!(s.selection().is_empty()); } +#[test] +fn a_click_on_a_selected_frames_label_selects_only_it() { + let (mut s, f, q) = setup(); + let other = id_of(&ex(&mut s, "shape.rectangle", json!({"x": 500, "y": 500, "width": 40, "height": 40}))["id"]).unwrap(); + ex(&mut s, "select.set", json!({"ids": [f, other]})); + // A point on the label's left edge, clear of the selection box's resize handles, lies outside + // the selection's bounds (the strip starts left of the frame's left edge there). + let p = q[0] + (q[3] - q[0]) * 0.2 + (q[1] - q[0]) * 0.02; + assert!(s.doc().unwrap().bounds_of(&[f, other]).is_some_and(|b| !b.contains(p)), "the label point is outside the selection bounds"); + assert_eq!(label_at(s.doc().unwrap(), s.page().unwrap(), &s.tool.labels, p), Some(f), "the point is on the label"); + s.pointer(&ev(PointerKind::Down, p)).unwrap(); + s.pointer(&ev(PointerKind::Up, p)).unwrap(); + assert_eq!(s.selection(), vec![f], "the label outside the bounds still narrows the selection to its frame"); +} + #[test] fn labels_of_frames_that_cant_be_picked_do_nothing() { let (mut s, f, q) = setup(); diff --git a/crates/engine/src/property_copy_tests.rs b/crates/engine/src/property_copy_tests.rs index 40f2c4b2..2064fdb7 100644 --- a/crates/engine/src/property_copy_tests.rs +++ b/crates/engine/src/property_copy_tests.rs @@ -114,7 +114,7 @@ fn a_copied_image_fill_brings_its_image_to_another_file() { // Another file has no image under that key: the copy brings the data with it. ex(&mut s, "file.new", json!({})); let b = rect(&mut s); - assert!(s.doc().unwrap().images.get("img-key").is_none()); + assert!(!s.doc().unwrap().images.contains_key("img-key")); ex(&mut s, "fill.paste", json!({"ids": [b], "index": 0})); assert_eq!(s.doc().unwrap().images.get("img-key"), Some(&data)); assert!(matches!(node(&s, b).fills[0].kind, figcraft_color::PaintKind::Image(_))); diff --git a/crates/engine/src/selection_behaviour_tests.rs b/crates/engine/src/selection_behaviour_tests.rs index 35c43a4c..c5852728 100644 --- a/crates/engine/src/selection_behaviour_tests.rs +++ b/crates/engine/src/selection_behaviour_tests.rs @@ -456,6 +456,21 @@ fn a_layer_stacked_below_the_selection_doesnt_take_a_drag_from_inside_it() { assert_eq!(at(&s, gap), Point::new(home.x + 10.0, home.y + 10.0)); } +#[test] +fn a_click_where_nothing_is_hit_inside_a_multiple_selection_keeps_it() { + let mut s = Session::new(); + let a = rect(&mut s, 100.0, 300.0, None); + let b = rect(&mut s, 220.0, 300.0, None); + for i in [a, b] { + ex(&mut s, "node.set", json!({"ids": [i], "props": {"fills": [], "strokes": ["#000000"], "strokeWeight": 1}})); + } + ex(&mut s, "select.set", json!({"ids": [a, b]})); + // The press is inside `a` but its unfilled inside hits nothing: `a` is only the drag fallback, + // not a clicked layer, so a click keeps the selection. + click(&mut s, 120.0, 320.0, NONE); + assert_eq!(sorted(s.selection()), sorted(vec![a, b])); +} + #[test] fn alt_shift_drag_on_a_selected_layer_duplicates_it() { let mut s = Session::new(); diff --git a/crates/ui-egui/src/canvas/selection_spacing.rs b/crates/ui-egui/src/canvas/selection_spacing.rs index 94439931..8dc9db62 100644 --- a/crates/ui-egui/src/canvas/selection_spacing.rs +++ b/crates/ui-egui/src/canvas/selection_spacing.rs @@ -88,6 +88,9 @@ struct Drag { uid: u64, page: NodeId, moved: bool, + /// The modifiers at the press: a release is a click only if it was a plain press, however the + /// modifiers stand when the button comes up. + plain: bool, } fn start_drag(app: &mut FigApp, ctx: &egui::Context, ids: Vec, p: Pos2, action: Action) { @@ -95,10 +98,11 @@ fn start_drag(app: &mut FigApp, ctx: &egui::Context, ids: Vec, p: Pos2, Action::Spacing { .. } => tr("Spacing"), Action::Reorder { .. } => tr("Reorder selection"), }; + let plain = ctx.input(|i| !(i.modifiers.shift || i.modifiers.alt || i.modifiers.command || i.modifiers.ctrl)); if app.session.begin_interaction(label).is_ok() && let Ok(ds) = app.session.ds() { - let drag = Drag { ids, action, start: p, uid: ds.uid, page: ds.page, moved: false }; + let drag = Drag { ids, action, start: p, uid: ds.uid, page: ds.page, moved: false, plain }; app.ui.layers_focus = false; app.ui.pages_focus = None; app.ui.key_box = None; @@ -281,11 +285,12 @@ pub fn input(app: &mut FigApp, ctx: &egui::Context, pos: Option, hovered: }); if !down { // A plain press on a reorder ring that didn't become a drag is a click on that layer: - // the selection narrows to it, as a click on the canvas would. With a modifier held - // the ring keeps Figma's meaning (Shift/Alt/Cmd on a selected layer), so it does nothing. - let plain = !ctx.input(|i| i.modifiers.shift || i.modifiers.alt || i.modifiers.command || i.modifiers.ctrl); + // the selection narrows to it, as a click on the canvas would. The press's modifiers + // decide (letting Shift go before the release doesn't make it a plain click), and a + // release away from where it pressed (outside the canvas, say) ends a drag, not a click. + let released = pos.is_some_and(|p| app.canvas.rect.contains(p) && p.distance(drag.start) < 3.0); let clicked = match &drag.action { - Action::Reorder { source, .. } if plain && !drag.moved => Some(*source), + Action::Reorder { source, .. } if drag.plain && !drag.moved && released => Some(*source), _ => None, }; let _ = app.session.commit_interaction(); diff --git a/crates/ui-egui/src/selection_spacing_tests.rs b/crates/ui-egui/src/selection_spacing_tests.rs index 5de2be94..d23f3436 100644 --- a/crates/ui-egui/src/selection_spacing_tests.rs +++ b/crates/ui-egui/src/selection_spacing_tests.rs @@ -241,6 +241,32 @@ fn a_click_on_one_of_several_selected_layers_selects_it() { assert_eq!(u.app.session.selection(), vec![ids[0]], "the click selects the layer whose ring it hit"); } +#[test] +fn the_press_modifiers_decide_whether_a_ring_release_is_a_click() { + let (mut u, ids) = scene(false); + let p = u.app.to_screen(125., 130.); + let shift = Modifiers::SHIFT; + // Shift is down at the press and let go before the release: the ring keeps its Shift meaning. + u.frame_with(vec![Event::PointerMoved(p)], shift); + u.frame_with(vec![Event::PointerButton { pos: p, button: egui::PointerButton::Primary, pressed: true, modifiers: shift }], shift); + u.frame(vec![Event::PointerButton { pos: p, button: egui::PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE }]); + u.frame(vec![]); + assert_eq!(u.app.session.selection(), ids, "letting Shift go before the release doesn't make it a plain click"); +} + +#[test] +fn releasing_a_ring_outside_the_canvas_ends_a_drag_even_without_motion() { + let (mut u, ids) = scene(false); + let p = u.app.to_screen(125., 130.); + u.frame(vec![Event::PointerMoved(p)]); + u.button(p, true); + // The pointer leaves before any in-canvas motion is seen, so `moved` stays false. + u.frame(vec![Event::PointerGone]); + u.button(Pos2::new(-1., -1.), false); + u.frame(vec![]); + assert_eq!(u.app.session.selection(), ids, "a release away from the press ends a drag, not a click"); +} + #[test] fn reorder_ring_escape_click_and_returning_home_make_no_undo_step() { for action in [0, 1, 2] {