From e415a3a0fb46eb32eafd9d807a11a0ae31fd8de2 Mon Sep 17 00:00:00 2001 From: Trevor Brown Date: Mon, 9 Nov 2026 15:16:36 -0500 Subject: [PATCH] M2: pin the leading-edge drag reorder at its boundaries, zero delta and several layers (#2142 review) Tests for the -129 leftward boundary, the vertical boundaries both ways, a drag with no move along the flow, and several moving layers using their combined box. Doc comments say whose edge is "leading" and what the caller passes; the parity row names the unchanged cases and the web-app measurement source. Co-Authored-By: Claude Sonnet 5.5 Model-Effort: Opus 5.5 triaged and briefed; Sonnet 5.5 verified and wrote at medium --- crates/engine/src/al_drag_tests.rs | 44 ++++++++++++++++++++++++++++++ crates/engine/src/canvas/drop.rs | 12 ++++---- docs/auto-layout-parity.md | 2 +- 3 files changed, 52 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/al_drag_tests.rs b/crates/engine/src/al_drag_tests.rs index 23f416481..0c5119fc5 100644 --- a/crates/engine/src/al_drag_tests.rs +++ b/crates/engine/src/al_drag_tests.rs @@ -122,3 +122,47 @@ fn a_column_reorders_by_the_bottom_edge() { assert_eq!(dragged_order("vertical", 0, 30.0, 50.0), vec![1, 0, 2]); assert_eq!(dragged_order("vertical", 0, 30.0, 130.0), vec![1, 2, 0]); } + +#[test] +fn a_child_dragged_left_passes_the_second_sibling_one_pixel_later() { + assert_eq!(dragged_order("horizontal", 2, 30.0, -129.0), vec![0, 2, 1]); +} + +#[test] +fn a_column_reorders_at_the_boundaries_both_ways() { + assert_eq!(dragged_order("vertical", 0, 30.0, 49.0), vec![0, 1, 2]); + assert_eq!(dragged_order("vertical", 0, 30.0, 129.0), vec![1, 0, 2]); + assert_eq!(dragged_order("vertical", 2, 30.0, -49.0), vec![0, 1, 2]); + assert_eq!(dragged_order("vertical", 2, 30.0, -50.0), vec![0, 2, 1]); + assert_eq!(dragged_order("vertical", 2, 30.0, -129.0), vec![0, 2, 1]); + assert_eq!(dragged_order("vertical", 2, 30.0, -130.0), vec![2, 0, 1]); +} + +#[test] +fn a_drag_with_no_move_along_the_flow_keeps_the_slot() { + // Back at the start, and a drag across the flow only, with snapping on (the default). + assert_eq!(dragged_order("horizontal", 1, 30.0, 0.0), vec![0, 1, 2]); + assert_eq!(dragged_order("vertical", 1, 30.0, 0.0), vec![0, 1, 2]); + let mut s = Session::new(); + let (f, kids) = three(&mut s, "horizontal"); + let b = s.doc().unwrap().world_bounds(kids[1]).unwrap(); + let from = (b.center().x, b.center().y); + drag(&mut s, from, (from.0, from.1 + 40.0)); + let order = s.doc().unwrap().node(f).unwrap().children.clone(); + assert_eq!(order, kids.to_vec()); +} + +#[test] +fn several_dragged_children_use_their_combined_box() { + // Kids 0 and 1 together span 0..140; moved right by 49 the leading edge is at 189, just short of the middle of kid 2 (190). + for (by, expected) in [(49.0, vec![0, 1, 2]), (50.0, vec![2, 0, 1])] { + let mut s = Session::new(); + let (f, kids) = three(&mut s, "horizontal"); + s.execute("select.set", &json!({"ids": [kids[0], kids[1]]})).unwrap(); + let b = s.doc().unwrap().world_bounds(kids[0]).unwrap(); + let from = (b.center().x, b.center().y); + drag(&mut s, from, (from.0 + by, from.1)); + let order: Vec = s.doc().unwrap().node(f).unwrap().children.iter().map(|c| kids.iter().position(|k| k == c).unwrap()).collect(); + assert_eq!(order, expected, "by {by}"); + } +} diff --git a/crates/engine/src/canvas/drop.rs b/crates/engine/src/canvas/drop.rs index 7bad66ac3..f8c73de3e 100644 --- a/crates/engine/src/canvas/drop.rs +++ b/crates/engine/src/canvas/drop.rs @@ -24,9 +24,10 @@ pub fn auto_layout_slot(doc: &Document, parent: NodeId, p: Point, exclude: &[Nod } /// [`auto_layout_slot`] for layers dragged within their own frame (measured in Figma's web app, -/// 2026-11-09): the slot follows the leading edge of `moved`, the box the layers would fill under -/// the pointer, in the direction of `delta` (the drag along the page). Wrapping layouts keep the -/// pointer rule. +/// 2026-11-09): the slot follows the leading edge of `moved` in the direction of `delta`. The caller +/// passes `moved` as the layers' combined start bounds plus the raw (unsnapped) drag, and `delta` +/// as that same drag on the page; only its sign along the flow axis matters, and a zero delta uses +/// the low edge. Wrapping layouts keep the pointer rule. pub fn auto_layout_slot_within( doc: &Document, parent: NodeId, @@ -62,8 +63,9 @@ fn slot_in( let mid = |c: &NodeId| doc.world_bounds(*c).map(|b| if vertical { b.center().y } else { b.center().x }); let k = if let Some((r, d)) = dragged { let (lo, hi, dd) = if vertical { (r.y0, r.y1, d.y) } else { (r.x0, r.x1, d.x) }; - // Forward: past every sibling whose middle the trailing edge has reached; backward: past - // every one whose middle lies before the leading (low) edge. + // Forward: past every sibling whose middle the leading (high) edge has reached; backward: past + // every one whose middle lies before the leading (low) edge. A zero delta (no move along the + // flow) takes the backward rule, which leaves the layers where they were. flow_ids.iter().position(|c| mid(c).is_some_and(|m| if dd > 0.0 { m > hi } else { m >= lo })).unwrap_or(flow_ids.len()) } else { let pos = if vertical { p.y } else { p.x }; diff --git a/docs/auto-layout-parity.md b/docs/auto-layout-parity.md index 16776ca5e..03b6fd830 100644 --- a/docs/auto-layout-parity.md +++ b/docs/auto-layout-parity.md @@ -88,7 +88,7 @@ Status: **pass** (a case in the suite holds), **fixed** (the suite found a diver | Absolute position (ignore auto layout) with constraints | done | | | Strokes included in layout (inside strokes), canvas stacking | done | | | On-canvas padding and gap handles, incl. wrap row/column gap and grid gaps | done | an automatic gap's handle shows the distance it has; dragging it fixes the gap from there (silent in the documentation) | -| Drag to reorder (wrap-aware, by row or column), into grid cells, across nested frames | done | `al_drag_tests.rs`. A child dragged within its own row or column takes its slot by the dragged box's leading edge, not the pointer: it passes a sibling when the edge facing the drag reaches the sibling's middle (right or bottom edge going forward, left or top edge going back), wherever it was grabbed. Measured in Figma's web app, 2026-11-09 (three 60 x 60 rectangles, gap 20: after B at 50, after C at 130; the mirror for a leftward drag; the same along y). Layers dragged in from outside and wrapping layouts keep the pointer rule (unmeasured). Tests: `a_child_dragged_right_passes_a_sibling_when_its_leading_edge_reaches_the_siblings_middle`, `where_the_child_is_grabbed_does_not_change_its_slot`, `a_child_dragged_left_passes_a_sibling_by_its_left_edge`, `a_column_reorders_by_the_bottom_edge` | +| Drag to reorder (wrap-aware, by row or column), into grid cells, across nested frames | done | `al_drag_tests.rs`. A child dragged within its own row or column takes its slot by the dragged box's leading edge, not the pointer: it passes a sibling when the edge facing the drag reaches the sibling's middle (right or bottom edge going forward, left or top edge going back), wherever it was grabbed. Measured in Figma's web app, 2026-11-09 (three 60 x 60 rectangles, gap 20: after B at 50, after C at 130; the mirror for a leftward drag; the same along y). Several moving layers use their combined box. Layers dragged in from outside, wrapping layouts, grids and a drag out of the frame keep their earlier rules (unmeasured). The measurement was made in the web app; the desktop target was not re-checked. Tests: `a_child_dragged_right_passes_a_sibling_when_its_leading_edge_reaches_the_siblings_middle`, `where_the_child_is_grabbed_does_not_change_its_slot`, `a_child_dragged_left_passes_a_sibling_by_its_left_edge`, `a_column_reorders_by_the_bottom_edge`, `a_column_reorders_at_the_boundaries_both_ways`, `a_drag_with_no_move_along_the_flow_keeps_the_slot`, `several_dragged_children_use_their_combined_box` | | Arrow keys move children one place (flows, rows, columns, grid cells, into neighbours) | done | `al_keys_tests.rs` | | Constraints: left, right, left and right, centre, scale; top/bottom likewise | done | | | Layout grids: columns, rows, grid; stretch/left/centre/right; variables bound | done | |