Merge #1200: The colour picker's eyedropper applies the colour to the paint the picker edits (Reported in storytold/vectorcraft#3342) (@marcoarnulfo)

# Conflicts:
#	crates/engine/src/tools.rs

crates/engine/src/cmd/view.rs

@@ -537,7 +537,14 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!("tool.text", "Text", "{}", false, |s, _| tool(s, ToolId::Text)),
         spec!("tool.comment", "Comment", "{}", false, |s, _| tool(s, ToolId::Comment)),
         spec!("tool.shapeBuilder", "Shape builder", "{}", false, |s, _| tool(s, ToolId::ShapeBuilder)),
-        spec!("edit.pickColor", "Pick color", "{}", false, |s, _| tool(s, ToolId::Eyedropper)),
+        // `apply: false` is for the UI colour picker: the eyedropper only samples (into
+        // `ToolState::picked`) and the picker writes the colour to the paint it edits.
+        spec!("edit.pickColor", "Pick color", "{apply?}", false, |s, p| {
+            let r = tool(s, ToolId::Eyedropper)?;
+            s.tool.pick_only = !boolean(p, "apply").unwrap_or(true);
+            s.tool.picked = None;
+            Ok(r)
+        }),
     ]
 }
 

crates/engine/src/fx_behaviour_tests.rs

@@ -53,6 +53,29 @@ fn the_eyedropper_picks_the_colour_seen_and_fills_the_selection() {
     assert_eq!(s.doc().unwrap().node(b).unwrap().fills[0].solid_color().map(|c| c.to_hex()), Some("00FF00".into()));
 }
 
+#[test]
+fn the_eyedropper_of_a_colour_picker_samples_without_filling() {
+    // Opened from a colour picker (`apply: false`), the eyedropper hands the colour to the picker,
+    // which writes it to its own paint (a stroke, an effect...): the fill stays as it was.
+    let mut s = Session::new();
+    s.execute("page.setBackground", &json!({"color": "#FFFFFF"})).unwrap();
+    rect(&mut s, None, (0.0, 0.0, 100.0, 100.0), "#FF0000");
+    let b = rect(&mut s, None, (200.0, 0.0, 50.0, 50.0), "#00FF00");
+    s.execute("select.set", &json!({"ids": [b]})).unwrap();
+    let undo = s.undo_depth().0;
+    s.execute("edit.pickColor", &json!({"apply": false})).unwrap();
+    s.pointer(&PointerEvent::at(PointerKind::Down, 50.0, 50.0, 1.0)).unwrap();
+    assert_eq!(s.doc().unwrap().node(b).unwrap().fills[0].solid_color().map(|c| c.to_hex()), Some("00FF00".into()));
+    assert_eq!(s.undo_depth().0, undo, "nothing to undo");
+    assert_eq!(s.tool.picked.take().map(|c| c.to_hex()), Some("FF0000".into()));
+    assert_eq!(s.tool.tool, ToolId::Move, "picking ends the eyedropper");
+    // The I key's eyedropper fills the selection again.
+    s.execute("edit.pickColor", &json!({})).unwrap();
+    s.pointer(&PointerEvent::at(PointerKind::Down, 50.0, 50.0, 1.0)).unwrap();
+    assert_eq!(s.doc().unwrap().node(b).unwrap().fills[0].solid_color().map(|c| c.to_hex()), Some("FF0000".into()));
+    assert_eq!(s.tool.picked, None);
+}
+
 #[test]
 fn blend_modes_and_effects_through_commands() {
     let mut s = Session::new();

crates/engine/src/tools.rs

@@ -512,6 +512,11 @@ pub struct ToolState {
     pub pencil_brush: Option<figcraft_doc::Brush>,
     /// The board tools' colours and choices.
     pub board: crate::board::BoardTool,
+    /// The eyedropper samples for a colour picker (`edit.pickColor` with `apply: false`): the
+    /// colour goes to `picked`, for the picker to write to the paint it edits, not to the fill.
+    pub pick_only: bool,
+    /// The colour the eyedropper sampled for a colour picker, until the picker takes it.
+    pub picked: Option<figcraft_color::Color>,
 }
 
 /// Pen pressure along drawn samples as width profile points `[t, scale]`: `t` by length, at most
@@ -973,6 +978,7 @@ impl Session {
         }
         self.tool.tool = t;
         self.tool.gesture = Gesture::None;
+        self.tool.pick_only = false;
         // The tool's cursor shows before the pointer moves.
         self.tool.overlay =
             Overlay { builder_remove: self.tool.builder_remove && t == ToolId::ShapeBuilder, cursor: t.cursor(), ..Overlay::default() };
@@ -2134,7 +2140,9 @@ impl Session {
             255,
         );
         let sel = ds.selection.clone();
-        if !sel.is_empty() {
+        if self.tool.pick_only {
+            self.tool.picked = Some(c);
+        } else if !sel.is_empty() {
             self.execute("node.set", &json!({"ids": sel, "props": {"fill": c.to_css_hex()}}))?;
         }
         self.recent_colors.retain(|x| *x != c);

crates/ui-egui/src/colorpicker.rs

@@ -436,7 +436,14 @@ fn alpha_slider(ui: &mut Ui, rect: Rect, color: Color) -> Option<(f32, bool)> {
 }
 
 pub fn show(app: &mut FigApp, ctx: &egui::Context) {
-    let Some(target) = app.ui.picker.clone() else { return };
+    let Some(target) = app.ui.picker.clone() else {
+        // A colour sampled for a picker that has closed has no paint to go to.
+        app.session.tool.picked = None;
+        if app.session.tool.pick_only {
+            app.run("tool.move", json!({}));
+        }
+        return;
+    };
     // A layer's picker belongs to the selection it was opened for: once that changes, close it
     // rather than keep editing layers that are no longer selected.
     let stale = match &target {
@@ -470,6 +477,10 @@ pub fn show(app: &mut FigApp, ctx: &egui::Context) {
         .open(&mut open)
         .show(ctx, |ui| {
             ui.spacing_mut().item_spacing = vec2(8.0, 8.0);
+            // The eyedropper's colour goes to this picker's paint (a stroke, an effect, a gradient stop...).
+            if let Some(c) = app.session.tool.picked.take() {
+                set_color(app, &target, &mut paint, c, stop_id(), ui);
+            }
             // Header.
             let (hr, _) = ui.allocate_exact_size(vec2(240.0, 40.0), Sense::hover());
             ui.painter().text(pos2(hr.min.x + 16.0, hr.center().y), Align2::LEFT_CENTER, tr("Custom"), semibold(11.0), t.text);
@@ -772,8 +783,7 @@ pub fn show(app: &mut FigApp, ctx: &egui::Context) {
                 )
                 .clicked()
                 {
-                    app.ui.picker = None;
-                    app.run("edit.pickColor", json!({}));
+                    app.run("edit.pickColor", json!({"apply": false}));
                     return;
                 }
                 let (c2, d2) = hue_slider(ui, hr, &mut hsb);

crates/ui-egui/src/colorpicker_eyedropper_tests.rs

@@ -0,0 +1,90 @@
+//! The eyedropper of a colour picker, driven through egui headless: the colour it samples goes to
+//! the paint the picker edits (a stroke, an effect), not to the layer's fill (vectorcraft#3342).
+
+use egui::accesskit::Role;
+use egui::{Event, Pos2, pos2};
+use figcraft_doc::NodeId;
+use figcraft_engine::{Session, ToolId};
+use serde_json::json;
+
+use crate::colorpicker::PickerTarget;
+use crate::layers_drag_tests::Ui;
+
+fn rect(s: &mut Session, x: f64, props: serde_json::Value) -> NodeId {
+    let v = s.execute("shape.rectangle", &json!({"x": x, "y": 0, "width": 60, "height": 60, "props": props})).unwrap();
+    NodeId(v["id"].as_u64().unwrap())
+}
+
+/// The middle of the picker's eyedropper button.
+fn eyedropper_button(u: &mut Ui) -> Pos2 {
+    u.ctx.enable_accesskit();
+    u.frame(vec![]);
+    let out = u.frame_output(vec![]);
+    let nodes = out.platform_output.accesskit_update.expect("an AccessKit update").nodes;
+    let n = nodes.iter().map(|(_, n)| n).find(|n| n.role() == Role::Button && n.label() == Some("Pick color (I)")).expect("the eyedropper button");
+    let b = n.bounds().expect("bounds");
+    pos2(((b.x0 + b.x1) / 2.0) as f32, ((b.y0 + b.y1) / 2.0) as f32)
+}
+
+fn click(u: &mut Ui, p: Pos2) {
+    u.button(p, true);
+    u.button(p, false);
+    u.frame(vec![Event::PointerMoved(p)]);
+    u.frame(vec![]);
+}
+
+#[test]
+fn the_pickers_eyedropper_applies_the_colour_to_the_paint_it_edits() {
+    for effect in [false, true] {
+        // A red layer to sample, and a green one with a blue stroke (and a shadow) to edit.
+        let mut s = Session::new();
+        s.execute("page.setBackground", &json!({"color": "#FFFFFF"})).unwrap();
+        let red = rect(&mut s, 0.0, json!({"fill": "#FF0000"}));
+        let b = rect(&mut s, 200.0, json!({"fill": "#00FF00", "stroke": "#0000FF", "strokeWeight": 2}));
+        s.execute("select.set", &json!({"ids": [b]})).unwrap();
+        let target = if effect {
+            s.execute("effect.add", &json!({"ids": [b], "type": "dropShadow"})).unwrap();
+            PickerTarget::Effect { ids: vec![b], index: 0, secondary: false }
+        } else {
+            PickerTarget::Paint { fills: false, index: 0, ids: vec![b] }
+        };
+        let mut u = Ui::new(s);
+        u.app.ui.picker = Some(target.clone());
+        let at = eyedropper_button(&mut u);
+        click(&mut u, at);
+        assert_eq!(u.app.session.tool.tool, ToolId::Eyedropper, "the button picks up the eyedropper");
+        let undo = u.app.session.undo_depth().0;
+        let on_red = u.app.to_screen(30.0, 30.0);
+        click(&mut u, on_red);
+        let doc = u.app.session.doc().unwrap();
+        let n = doc.node(b).unwrap();
+        let hex = |p: Option<figcraft_color::Color>| p.map(|c| c.to_hex()).unwrap_or_default();
+        let picked = if effect { hex(n.effects[0].color(false)) } else { hex(n.strokes[0].solid_color()) };
+        assert_eq!(picked, "FF0000", "the picker's paint takes the colour (effect: {effect})");
+        assert_eq!(hex(n.fills[0].solid_color()), "00FF00", "the fill is left alone (effect: {effect})");
+        assert!(doc.node(red).is_some());
+        assert_eq!(u.app.session.tool.tool, ToolId::Move, "picking ends the eyedropper");
+        assert_eq!(u.app.ui.picker, Some(target), "the picker stays open on its paint");
+        assert_eq!(u.app.session.undo_depth().0, undo + 1, "one undo step");
+    }
+}
+
+#[test]
+fn closing_the_picker_drops_its_eyedropper() {
+    // The colour sampled after the picker closed has no paint to go to: it must not wait for the
+    // next picker, and the eyedropper gives way to the Move tool.
+    let mut s = Session::new();
+    let b = rect(&mut s, 200.0, json!({"fill": "#00FF00", "stroke": "#0000FF"}));
+    s.execute("select.set", &json!({"ids": [b]})).unwrap();
+    let mut u = Ui::new(s);
+    u.app.ui.picker = Some(PickerTarget::Paint { fills: false, index: 0, ids: vec![b] });
+    let at = eyedropper_button(&mut u);
+    click(&mut u, at);
+    assert_eq!(u.app.session.tool.tool, ToolId::Eyedropper);
+    u.app.ui.picker = None;
+    u.app.session.tool.picked = figcraft_color::Color::from_hex("#FF0000");
+    u.frame(vec![]);
+    u.frame(vec![]);
+    assert_eq!(u.app.session.tool.tool, ToolId::Move);
+    assert_eq!(u.app.session.tool.picked, None, "a colour sampled for a closed picker is dropped");
+}

crates/ui-egui/src/lib.rs

@@ -90,6 +90,8 @@ mod canvas_fixes_tests;
 #[cfg(test)]
 mod canvas_image_rect_tests;
 #[cfg(test)]
+mod colorpicker_eyedropper_tests;
+#[cfg(test)]
 mod colorpicker_selection_tests;
 #[cfg(test)]
 mod comments_tests;