Merge pull request #1661 from marcoarnulfo/fix/align-key-object

Align: a click on one of several selected objects makes it the key object

crates/engine/src/cmd/edit.rs

@@ -262,12 +262,29 @@ pub fn specs() -> Vec<CommandSpec> {
             st.selection.cells = None;
             if let Some(i) = st.selection.items.iter().position(|x| *x == id) {
                 st.selection.items.remove(i);
+                // A key object needs another object to align to it.
+                if st.selection.key == Some(id) || st.selection.items.len() < 2 {
+                    st.selection.key = None;
+                }
             } else {
                 st.selection.items.push(id);
             }
             st.bump_revision();
             ok()
         }),
+        cmd!(noundo "selection.key", "Key Object", [], None, "{id} — make a selected item the key object for Align; the current key object drops the key", has_doc, |s, p| {
+            let id = super::id_param(p, "id").ok_or_else(|| super::bad("selection.key", "missing id"))?;
+            let st = s.doc_mut()?;
+            if !st.selection.items.contains(&id) {
+                return Err(super::bad("selection.key", "the key object must be selected"));
+            }
+            if st.selection.items.len() < 2 {
+                return Err(super::bad("selection.key", "a key object needs at least two selected objects"));
+            }
+            st.selection.key = if st.selection.key == Some(id) { None } else { Some(id) };
+            st.bump_revision();
+            ok()
+        }),
         cmd!(noundo "edit.selectAll", "Select All", ["Edit"], Some("Cmd+A"), "{} — all items on the active spreads, or all text in the story", has_doc, |s, _| {
             let st = s.doc_mut()?;
             if let Some(t) = st.selection.text {

crates/engine/src/cmd/object.rs

@@ -686,7 +686,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Align",
             ["Window", "Object & Layout", "Align"],
             None,
-            "{edge: left|hcenter|right|top|vcenter|bottom, to?: selection|keyObject|margins|page|spread, ids?}",
+            "{edge: left|hcenter|right|top|vcenter|bottom, to?: selection|keyObject|margins|page|spread (default: keyObject while a key object is set), ids?}",
             has_selection,
             align
         ),
@@ -3074,8 +3074,18 @@ fn align_reference(d: &Document, ids: &[ItemId], to: &str, key: Option<ItemId>)
 fn align(s: &mut Session, p: &Value) -> Result<Value> {
     let ids = targets(s, p)?;
     let edge = str_param(p, "edge").unwrap_or("left").to_string();
-    let to = str_param(p, "to").map(str::to_string).unwrap_or_else(|| if ids.len() > 1 { "selection".into() } else { "page".into() });
-    let key = s.doc()?.selection.key;
+    // A key object counts only while it is one of several items aligned. With one, InDesign's
+    // Align To switches to Key Object, so it is also the default target.
+    let key = s.doc()?.selection.key.filter(|k| ids.len() > 1 && ids.contains(k));
+    let to = str_param(p, "to").map(str::to_string).unwrap_or_else(|| {
+        if key.is_some() {
+            "keyObject".into()
+        } else if ids.len() > 1 {
+            "selection".into()
+        } else {
+            "page".into()
+        }
+    });
     s.edit(|d, _| {
         let r = align_reference(d, &ids, &to, key).ok_or_else(|| bad("object.align", "nothing to align"))?;
         for id in &ids {

crates/engine/src/tests.rs

@@ -1651,3 +1651,108 @@ fn a_line_fits_when_its_baseline_does() {
     assert!(!overset(&mut s, 78.5), "the line is set; its descenders hang below the frame");
     assert!(overset(&mut s, 50.0), "a baseline below the frame still oversets the line");
 }
+
+/// Issue #1637: with several objects selected, a click on one of them (no drag) makes it the key
+/// object, and Align to Key Object leaves it in place. A second click on it drops the key.
+#[test]
+fn a_click_in_the_selection_makes_the_key_object() {
+    use designcraft_tools::{PointerEvent, PointerKind};
+    let mut s = session();
+    let a = s.execute("frame.create", &json!({"rect": [50.0, 100.0, 100.0, 150.0], "content": "unassigned"})).unwrap()["id"].as_u64().unwrap();
+    let b = s.execute("frame.create", &json!({"rect": [200.0, 120.0, 300.0, 170.0], "content": "unassigned"})).unwrap()["id"].as_u64().unwrap();
+    s.execute("selection.set", &json!({"ids": [a, b]})).unwrap();
+    s.set_tool("selection");
+    let v = ViewInfo::at_zoom(1.0);
+    // On the part of the key object that the aligned object never covers, away from the handles.
+    let click = |s: &mut Session| {
+        s.pointer(&PointerEvent::new(PointerKind::Down, 280.0, 160.0), v).unwrap();
+        s.pointer(&PointerEvent::new(PointerKind::Up, 280.0, 160.0), v).unwrap();
+    };
+    click(&mut s);
+    assert_eq!(s.doc().unwrap().selection.items.len(), 2, "the click keeps both selected");
+    // The key is the right-hand object, so aligning left edges to the selection would move it.
+    s.execute("object.align", &json!({"edge": "left", "to": "keyObject"})).unwrap();
+    let x0 = |s: &Session, id: u64| s.doc().unwrap().doc.item(designcraft_doc::ItemId(id)).unwrap().bounds().x0;
+    assert!((x0(&s, b) - 200.0).abs() < 1e-6, "the key object moved to {}", x0(&s, b));
+    assert!((x0(&s, a) - 200.0).abs() < 1e-6, "the other object is at {}", x0(&s, a));
+    // The Align panel and the Control panel send no target: the key object is the default one.
+    s.execute("object.align", &json!({"edge": "top"})).unwrap();
+    let y0 = |s: &Session, id: u64| s.doc().unwrap().doc.item(designcraft_doc::ItemId(id)).unwrap().bounds().y0;
+    assert!((y0(&s, b) - 120.0).abs() < 1e-6, "the key object moved to {}", y0(&s, b));
+    assert!((y0(&s, a) - 120.0).abs() < 1e-6, "the other object is at {}", y0(&s, a));
+    click(&mut s);
+    assert_eq!(s.doc().unwrap().selection.key, None, "a click on the key object drops the key");
+}
+
+/// Dragging one of several selected objects moves them all and makes no key object.
+#[test]
+fn dragging_the_selection_makes_no_key_object() {
+    use designcraft_tools::{PointerEvent, PointerKind};
+    let mut s = session();
+    let a = s.execute("frame.create", &json!({"rect": [50.0, 100.0, 100.0, 150.0], "content": "unassigned"})).unwrap()["id"].as_u64().unwrap();
+    let b = s.execute("frame.create", &json!({"rect": [200.0, 100.0, 260.0, 150.0], "content": "unassigned"})).unwrap()["id"].as_u64().unwrap();
+    s.execute("selection.set", &json!({"ids": [a, b]})).unwrap();
+    s.set_tool("selection");
+    let v = ViewInfo::at_zoom(1.0);
+    s.pointer(&PointerEvent::new(PointerKind::Down, 230.0, 125.0), v).unwrap();
+    s.pointer(&PointerEvent::new(PointerKind::Drag, 250.0, 135.0), v).unwrap();
+    s.pointer(&PointerEvent::new(PointerKind::Up, 250.0, 135.0), v).unwrap();
+    assert_eq!(s.doc().unwrap().selection.key, None);
+}
+
+/// Shift-clicking the key object out of the selection drops the key.
+#[test]
+fn deselecting_the_key_object_drops_the_key() {
+    let mut s = session();
+    let a = s.execute("frame.create", &json!({"rect": [50.0, 100.0, 100.0, 150.0]})).unwrap()["id"].as_u64().unwrap();
+    let b = s.execute("frame.create", &json!({"rect": [200.0, 100.0, 260.0, 150.0]})).unwrap()["id"].as_u64().unwrap();
+    s.execute("selection.set", &json!({"ids": [a, b]})).unwrap();
+    s.execute("selection.key", &json!({"id": b})).unwrap();
+    assert_eq!(s.doc().unwrap().selection.key, Some(designcraft_doc::ItemId(b)));
+    s.execute("selection.toggle", &json!({"id": b})).unwrap();
+    assert_eq!(s.doc().unwrap().selection.key, None);
+}
+
+/// A key object needs another object to align to: with one object selected there is nothing to key
+/// against, and the stored key would turn that object into the target as soon as a second one joins.
+#[test]
+fn a_single_selected_object_cannot_be_the_key_object() {
+    let mut s = session();
+    let a = s.execute("frame.create", &json!({"rect": [50.0, 100.0, 100.0, 150.0]})).unwrap()["id"].as_u64().unwrap();
+    let b = s.execute("frame.create", &json!({"rect": [200.0, 120.0, 300.0, 170.0]})).unwrap()["id"].as_u64().unwrap();
+    s.execute("selection.set", &json!({"ids": [a]})).unwrap();
+    assert!(s.execute("selection.key", &json!({"id": a})).is_err());
+    assert_eq!(s.doc().unwrap().selection.key, None);
+    s.execute("selection.toggle", &json!({"id": b})).unwrap();
+    assert_eq!(s.doc().unwrap().selection.key, None, "the second object joining must not find a key already set");
+}
+
+/// With the selection down to the key object alone, Align no longer has a key object: a single
+/// object aligns to the page, as before.
+#[test]
+fn a_lone_key_object_aligns_to_the_page() {
+    let mut s = session();
+    let a = s.execute("frame.create", &json!({"rect": [50.0, 100.0, 100.0, 150.0]})).unwrap()["id"].as_u64().unwrap();
+    let b = s.execute("frame.create", &json!({"rect": [200.0, 120.0, 300.0, 170.0]})).unwrap()["id"].as_u64().unwrap();
+    s.execute("selection.set", &json!({"ids": [a, b]})).unwrap();
+    s.execute("selection.key", &json!({"id": b})).unwrap();
+    s.execute("selection.toggle", &json!({"id": a})).unwrap();
+    assert_eq!(s.doc().unwrap().selection.key, None);
+    s.execute("object.align", &json!({"edge": "left"})).unwrap();
+    let x0 = s.doc().unwrap().doc.item(designcraft_doc::ItemId(b)).unwrap().bounds().x0;
+    assert!(x0.abs() < 1e-6, "the object is at {x0}, not at the page's left edge");
+}
+
+/// Aligning one object alone (`ids`) while it is the key object of a larger selection also takes
+/// the page.
+#[test]
+fn aligning_the_key_object_alone_takes_the_page() {
+    let mut s = session();
+    let a = s.execute("frame.create", &json!({"rect": [50.0, 100.0, 100.0, 150.0]})).unwrap()["id"].as_u64().unwrap();
+    let b = s.execute("frame.create", &json!({"rect": [200.0, 120.0, 300.0, 170.0]})).unwrap()["id"].as_u64().unwrap();
+    s.execute("selection.set", &json!({"ids": [a, b]})).unwrap();
+    s.execute("selection.key", &json!({"id": b})).unwrap();
+    s.execute("object.align", &json!({"edge": "left", "ids": [b]})).unwrap();
+    let x0 = s.doc().unwrap().doc.item(designcraft_doc::ItemId(b)).unwrap().bounds().x0;
+    assert!(x0.abs() < 1e-6, "the object is at {x0}, not at the page's left edge");
+}

crates/tools/src/select.rs

@@ -1,6 +1,7 @@
 //! Selection tool (V) and Direct Selection tool (A).
 //!
-//! Click selects the frontmost item (Shift toggles), drag moves (Alt duplicates, Shift constrains),
+//! Click selects the frontmost item (Shift toggles; a click on one of several selected items makes
+//! it the key object for Align, a second click drops it), drag moves (Alt duplicates, Shift constrains),
 //! the 8 bounding-box handles resize (Shift keeps proportions, Alt from centre, Cmd scales content),
 //! empty-canvas drags make a marquee (it selects the ruler guides it crosses when it touches no
 //! object), double-clicking a text frame switches to the Type tool and
@@ -223,6 +224,9 @@ pub struct SelectionTool {
     guides: Vec<Overlay>,
     /// Selected item to toggle if this Shift press never becomes a drag.
     shift_release: Option<u64>,
+    /// One of several selected items to make (or stop being) the key object for Align if this
+    /// press never becomes a drag.
+    key_release: Option<u64>,
     /// The loaded text cursor.
     loaded: Option<Loaded>,
     /// Frame whose live corners are being edited (diamonds shown).
@@ -231,7 +235,7 @@ pub struct SelectionTool {
 
 impl SelectionTool {
     pub fn new(direct: bool) -> Self {
-        Self { direct, drag: Drag::None, hover_handle: None, guides: vec![], shift_release: None, loaded: None, live: None }
+        Self { direct, drag: Drag::None, hover_handle: None, guides: vec![], shift_release: None, key_release: None, loaded: None, live: None }
     }
 
     /// [`bracket_at`] for this tool. With Direct Selection an anchor under the pointer wins: the
@@ -735,6 +739,7 @@ impl Tool for SelectionTool {
             }
             PointerKind::Down => {
                 self.shift_release = None;
+                self.key_release = None;
                 // Type on a path: a bracket moves where the text starts or ends.
                 if !ev.mods.shift
                     && let Some((id, end)) = self.bracket_at(cx, p)
@@ -825,6 +830,10 @@ impl Tool for SelectionTool {
                             }
                         } else if !cx.selection.contains(id) || (self.direct && !cx.selection.content) {
                             out.push(Action::Exec("selection.set".into(), json!({"ids": [id.0], "content": self.direct})));
+                        } else if !self.direct && cx.selection.items.len() > 1 {
+                            // InDesign: a click on one of several selected objects makes it the
+                            // key object, on release so that a drag still moves them all.
+                            self.key_release = Some(id.0);
                         }
                         let content = self.direct || on_selected_content(cx, p);
                         self.drag = Drag::Pending { start: p, hit: true, content };
@@ -852,6 +861,7 @@ impl Tool for SelectionTool {
                         return vec![];
                     }
                     self.shift_release = None;
+                    self.key_release = None;
                     if hit && !cx.selection.items.is_empty() {
                         // Direct Selection on items without content moves (or duplicates) them whole.
                         let content = content && cx.selection.items.iter().any(|i| cx.doc.item(*i).is_some_and(holds_content));
@@ -1124,6 +1134,7 @@ impl Tool for SelectionTool {
             PointerKind::Up => {
                 self.guides.clear();
                 let release = self.shift_release.take();
+                let key = self.key_release.take();
                 let d = std::mem::replace(&mut self.drag, Drag::None);
                 match d {
                     Drag::Move { .. }
@@ -1135,9 +1146,10 @@ impl Tool for SelectionTool {
                     | Drag::Bracket { .. } => {
                         vec![Action::Commit]
                     }
-                    Drag::Pending { .. } => match release {
-                        Some(id) => vec![Action::Exec("selection.toggle".into(), json!({"id": id}))],
-                        None => vec![],
+                    Drag::Pending { .. } => match (release, key) {
+                        (Some(id), _) => vec![Action::Exec("selection.toggle".into(), json!({"id": id}))],
+                        (None, Some(id)) => vec![Action::Exec("selection.key".into(), json!({"id": id}))],
+                        (None, None) => vec![],
                     },
                     Drag::Marquee { start, cur } => {
                         let r = Rect::from_points(start, cur);

crates/ui-egui/src/canvas.rs

@@ -1251,6 +1251,9 @@ fn draw_selection(app: &DesignApp, painter: &egui::Painter, xf: &Xf, doc: &Docum
         if sel.items.len() == 1 && sel.text.is_none() {
             // InDesign draws a 1 pt bounding box around the selection.
             painter.rect_stroke(xf.rect(a.transform_rect_bbox(it.bounds())), 0.0, Stroke::new(1.0, color), StrokeKind::Middle);
+        } else if sel.key == Some(*id) {
+            // The key object for Align: a thicker bounding box, as in InDesign.
+            painter.rect_stroke(xf.rect(a.transform_rect_bbox(it.bounds())), 0.0, Stroke::new(3.0, color), StrokeKind::Middle);
         }
         let b = xf.rect(a.transform_rect_bbox(it.bounds()));
         union = Some(union.map_or(b, |u| u.union(b)));

crates/ui-egui/src/panels/properties.rs

@@ -982,7 +982,9 @@ fn align_row(app: &mut DesignApp, ui: &mut Ui) {
     let t = Tokens::get(ui.ctx());
     ui.horizontal(|ui| {
         ui.spacing_mut().item_spacing.x = 8.0;
-        let to = app.ui.align_to.clone();
+        // A key object switches Align To to Key Object; choosing another target drops the key.
+        let key = app.session.active().and_then(|st| st.selection.key.filter(|_| st.selection.items.len() > 1));
+        let to = if key.is_some() { "keyObject".to_string() } else { app.ui.align_to.clone() };
         let to_label = match to.as_str() {
             "selection" => "Selection",
             "keyObject" => "Key Object",
@@ -1001,7 +1003,10 @@ fn align_row(app: &mut DesignApp, ui: &mut Ui) {
                 ("Align to Page", "page"),
                 ("Align to Spread", "spread"),
             ] {
-                if ui.selectable_label(app.ui.align_to == id, crate::rtl::widget(ui, crate::i18n::tr(&app.ui.language, label))).clicked() {
+                if ui.selectable_label(to == id, crate::rtl::widget(ui, crate::i18n::tr(&app.ui.language, label))).clicked() {
+                    if let Some(k) = key.filter(|_| id != "keyObject") {
+                        let _ = app.run("selection.key", json!({"id": k.0}));
+                    }
                     app.ui.align_to = id.into();
                     ui.close();
                 }