Merge pull request #4583 from storytold/fix/eyedropper-alt-click-applies-sample

Fix #4382: the Eyedropper's Alt-click applies what it picked up when nothing is selected

crates/engine/src/cmd/appearance.rs

@@ -19,6 +19,8 @@
 //! shape that a Convert to Shape or Outline Object effect on them gives them, below or above the
 //! image as its Contents row says; new ones go right below that row.
 
+use std::sync::Arc;
+
 use serde_json::{Value, json};
 use vectorcraft_color::{BlendMode, Paint};
 use vectorcraft_doc::appearance::SharedStack;
@@ -123,7 +125,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Eyedropper",
             [],
             None,
-            "{source: id, ids?, pickUp?, apply? (trees as eyedropper.setOptions, merged over the Eyedropper Options for this call), reverse?: bool, append?: bool} copy the attributes both picked up and applied (by default the whole appearance stack, opacity and blend mode, and from type to type the character and paragraph attributes) from `source` to ids (default: highlighted characters while editing type, otherwise the object selection; highlighted characters reject enabled non-neutral object transparency), the fill, stroke and weight also to the paint defaults; `reverse` (Alt-click) copies from the first selected object (or ids) onto `source` instead; `append` (Shift+Alt-click) adds the source's fills, strokes and effects on top of each target's stack; placed gradients land at the same place relative to each target's bounds (defaults: fitted to new art) → {ids}",
+            "{source: id, ids?, pickUp?, apply? (trees as eyedropper.setOptions, merged over the Eyedropper Options for this call), reverse?: bool, append?: bool} copy the attributes both picked up and applied (by default the whole appearance stack, opacity and blend mode, and from type to type the character and paragraph attributes) from `source` to ids (default: highlighted characters while editing type, otherwise the object selection; highlighted characters reject enabled non-neutral object transparency), the fill, stroke and weight also to the paint defaults; `reverse` (Alt-click) copies from the first selected object (or ids) onto `source` instead, or with nothing selected what the Eyedropper picked up last (a click without Alt); `append` (Shift+Alt-click) adds the source's fills, strokes and effects on top of each target's stack; placed gradients land at the same place relative to each target's bounds (defaults: fitted to new art) → {ids}",
             has_doc,
             copy_from
         ),
@@ -844,15 +846,27 @@ fn copy_from(s: &mut Session, p: &Value) -> Result<Value> {
         Some(v) => outermost(&s.doc()?.doc, v),
         None => selected_roots(s)?,
     };
-    // Reversed, the first selected object's attributes go onto the clicked object.
-    let (src_id, mut targets) = if reverse {
-        let first = leaf_targets(s, &selected)?.into_iter().next().ok_or_else(|| bad(C, "select the object to copy from"))?;
-        (first, leaf_targets(s, &[clicked])?)
+    // Reversed, the first selected object's attributes go onto the clicked object; with nothing
+    // selected, what the Eyedropper picked up last does (#4382).
+    let (source, mut targets) = if reverse {
+        let source = match leaf_targets(s, &selected)?.into_iter().next() {
+            Some(first) => Some(first),
+            None if s.eyedropper_sample.is_some() => None,
+            None => return Err(bad(C, "select the object to copy from")),
+        };
+        (source, leaf_targets(s, &[clicked])?)
     } else {
-        (clicked, leaf_targets(s, &selected)?)
+        (Some(clicked), leaf_targets(s, &selected)?)
     };
-    targets.retain(|id| *id != src_id);
-    let picked = Picked::of(s.doc()?.doc.node(src_id).ok_or(EngineError::NoNode(src_id))?);
+    targets.retain(|id| Some(*id) != source);
+    let node = match source {
+        Some(id) => Arc::new(s.doc()?.doc.node(id).ok_or(EngineError::NoNode(id))?.clone()),
+        None => s.eyedropper_sample.clone().ok_or_else(|| bad(C, "select the object to copy from"))?,
+    };
+    let picked = Picked::of(&node);
+    if !reverse {
+        s.eyedropper_sample = Some(node);
+    }
     let range = range.filter(|(id, _, _)| targets.contains(id));
     // Character styles cannot represent object transparency. Refuse before even changing
     // paint defaults; applying it to the object would also alter the unselected characters.

crates/engine/src/lib.rs

@@ -90,6 +90,9 @@ pub struct Session {
     /// The Appearance panel's active fill/stroke row (`appearance.setActiveItem`); not saved. Read
     /// it through [`Session::appearance_item`], which drops it once the selection changes.
     pub(crate) active_appearance_item: Option<cmd::appearance::ActiveItem>,
+    /// What the Eyedropper picked up last (`appearance.copyFrom`): an Alt-click with nothing
+    /// selected puts it on the object clicked (#4382). Not saved.
+    pub(crate) eyedropper_sample: Option<Arc<vectorcraft_doc::Node>>,
     /// The last solid colour applied (the toolbar's Color button; `,` applies it again).
     pub last_solid: Color,
     /// The last gradient applied (the toolbar's Gradient button; `.` applies it again).

crates/engine/src/session/documents.rs

@@ -44,6 +44,7 @@ impl Session {
             menu: Default::default(),
             gradient_stop: None,
             active_appearance_item: None,
+            eyedropper_sample: None,
             last_solid: Color::WHITE,
             last_gradient: GradientPaint::new(Default::default()),
             recent_colors: vec![],

crates/engine/src/tests/eyedropper.rs

@@ -123,6 +123,22 @@ fn alt_click_applies_the_selection_to_the_clicked_object() {
     assert!(s.execute("appearance.copyFrom", &json!({"source": src.0, "reverse": true})).is_err(), "nothing to copy from");
 }
 
+/// #4382: with nothing selected, a click picks up an object's attributes and an Alt-click puts
+/// them on the object clicked, as the reference app's Eyedropper does.
+#[test]
+fn alt_click_with_nothing_selected_applies_what_was_picked_up() {
+    let mut s = session();
+    let (src, dst) = source_and_target(&mut s);
+    s.execute("select.none", &json!({})).unwrap();
+    click(&mut s, 325.0, 35.0, Mods::default());
+    // The source changes afterwards: what was picked up stays as it was.
+    s.execute("paint.setFill", &json!({"ids": [src.0], "color": "#00ff00"})).unwrap();
+    click(&mut s, 35.0, 35.0, Mods { alt: true, ..Default::default() });
+    let n = node(&s, dst);
+    assert_eq!((n.appearance.fill_paint().color().unwrap().to_hex(), n.appearance.stroke_width(), n.opacity), ("#ff0000".into(), 5.0, 0.5));
+    assert!(s.doc().unwrap().selection.objects.is_empty(), "nothing gets selected");
+}
+
 #[test]
 fn shift_alt_click_appends_the_appearance() {
     let mut s = session();

docs/mcp.md

@@ -2183,7 +2183,8 @@ whatever the options are then.
 ```
 
 The Eyedropper: `appearance.copyFrom {source}` copies what the Eyedropper Options pick up and apply from `source` to the
-selection (`ids`); `reverse: true` copies the selection's attributes onto `source` (Alt-click) and `append: true` adds
+selection (`ids`); `reverse: true` copies the selection's attributes onto `source` (Alt-click), or with nothing selected
+what the last copy without `reverse` picked up (a copy made then, kept for the session), and `append: true` adds
 the source's fills and strokes on top of each target's stack (Shift+Alt-click). The options live in the preferences:
 `eyedropper.setOptions {sampleSize?, pickUp?, apply?}` (or `prefs.get`/`prefs.set` with key `eyedropper`) reads and
 sets them; a tree names flags under `appearance` (`transparency`, `fill` `{color, transparency, overprint}`, `stroke`