Merge pull request #1869 from TrevorBrown-dev/fix/place-crosshair

M6: the arrows show and move a placement crosshair with a shape or frame tool chosen, and Enter places there (#1839)

crates/engine/src/canvas/tools.rs

@@ -201,6 +201,7 @@ impl ToolId {
                 | ToolId::Star
                 | ToolId::Line
                 | ToolId::Arrow
+                | ToolId::BoardShape
         )
     }
 }
@@ -591,6 +592,13 @@ pub struct ToolState {
     pub picked: Option<figcraft_color::Color>,
     /// The widget layers whose hover styles show while the pointer is over them (a live preview).
     pub(crate) widget_hover: Vec<NodeId>,
+    /// The placement crosshair: where Enter puts the next shape or frame once the arrows have
+    /// shown it (in the middle of the view) and moved it (`tool.crosshair`), in page coordinates
+    /// of the document and page in `place_on` (elsewhere it isn't shown: `Session::crosshair`).
+    /// Placing, Escape and a change of tool take it away.
+    pub place_at: Option<Point>,
+    /// The document (uid) and page the crosshair was shown on.
+    pub(crate) place_on: Option<(u64, NodeId)>,
 }
 
 impl ToolState {

crates/engine/src/canvas/tools/draw.rs

@@ -212,6 +212,8 @@ impl Session {
                     crate::normalize(ds.doc_mut(), &before);
                 }
                 self.commit_interaction()?;
+                // A placement by the pointer takes the placement crosshair with it, as Enter does.
+                self.tool.place_at = None;
                 if !self.prefs.keep_tool {
                     self.set_tool(ToolId::Move);
                 }

crates/engine/src/canvas/tools/keys.rs

@@ -84,9 +84,14 @@ impl Session {
         if self.tool.busy() {
             return refuse("finish the drag first");
         }
+        // The crosshair the arrows moved goes with the placement, which lands on it when no point
+        // is given.
+        let crosshair = self.crosshair();
+        self.tool.place_at = None;
         let v = &self.ds()?.view;
-        let (c, zoom) = (at.unwrap_or(Point::new(v.cx, v.cy)), v.zoom);
-        let (w, h) = tool.click_size();
+        let (c, zoom) = (at.or(crosshair).unwrap_or(Point::new(v.cx, v.cy)), v.zoom);
+        // The board Shapes tool centres its own shape on the press; the others get half a click's size.
+        let (w, h) = if matches!(tool, ToolId::BoardShape) { (0.0, 0.0) } else { tool.click_size() };
         let ev = |kind| PointerEvent::at(kind, c.x - w / 2.0, c.y - h / 2.0, zoom);
         let r = self.pointer(&ev(PointerKind::Down)).and_then(|()| self.pointer(&ev(PointerKind::Up)));
         if r.is_err() {
@@ -111,6 +116,16 @@ impl Session {
             self.tool.overlay = Overlay::default();
             return self.cancel_interaction();
         }
+        // The placement crosshair goes first; the tool stays. One left on another page or
+        // document goes quietly.
+        let shown = self.crosshair().is_some();
+        self.tool.place_at = None;
+        if shown {
+            if let Ok(ds) = self.ds_mut() {
+                ds.revision += 1;
+            }
+            return Ok(());
+        }
         if self.end_origin_edit() {
             return Ok(());
         }
@@ -174,6 +189,14 @@ impl Session {
     /// Snapshot of tool state for the UI / inspect.
     pub fn tool_info(&self) -> serde_json::Value {
         let shapes: Vec<serde_json::Value> = self.shape_handles().into_iter().map(|(h, at)| json!({"handle": h, "at": at})).collect();
-        json!({"tool": self.tool.tool.id(), "busy": self.tool.busy(), "overlay": self.tool.overlay, "arcHandles": self.arc_handles(), "shapeHandles": shapes})
+        let crosshair = self.crosshair().map(|p| [p.x, p.y]);
+        json!({"tool": self.tool.tool.id(), "busy": self.tool.busy(), "overlay": self.tool.overlay, "arcHandles": self.arc_handles(), "shapeHandles": shapes, "crosshair": crosshair})
+    }
+
+    /// The placement crosshair, while the document and page it was shown on are the ones in
+    /// front (`ToolState::place_at`).
+    pub fn crosshair(&self) -> Option<Point> {
+        let here = self.ds().ok().map(|d| (d.uid, d.page));
+        self.tool.place_at.filter(|_| here.is_some() && here == self.tool.place_on)
     }
 }

crates/engine/src/canvas/tools/pointer.rs

@@ -76,6 +76,10 @@ impl Session {
         if !matches!(t, ToolId::Move | ToolId::Scale | ToolId::Hand) && self.ds().is_ok_and(|d| d.vector_edit.is_some()) {
             let _ = crate::vectoredit::end(self);
         }
+        // The placement crosshair goes with a change of tool, not with the same tool chosen again.
+        if self.tool.tool != t {
+            self.tool.place_at = None;
+        }
         self.tool.tool = t;
         self.tool.gesture = Gesture::None;
         self.tool.pick_only = false;

crates/engine/src/cmd/view.rs

@@ -529,6 +529,42 @@ pub fn specs() -> Vec<CommandSpec> {
                 tool(s, t)
             }
         ),
+        spec!(
+            "tool.crosshair",
+            "Move the placement crosshair",
+            "{dx?, dy? (arrow-key steps, e.g. 1 or -1), big? (the large nudge, like Shift)} — the arrows with a shape or frame tool chosen and nothing selected: the first shows the crosshair in the middle of the view, the next move it by the nudge amount; Enter (`tool.place`) puts the object there",
+            false,
+            |s, p| {
+                if !s.tool.tool.draws_shape() {
+                    return Err(EngineError::Disabled("tool.crosshair".into(), "choose a shape or frame tool first".into()));
+                }
+                let step = if boolean(p, "big").unwrap_or(false) { s.prefs.nudge_big } else { s.prefs.nudge_small };
+                let dir = |k| num(p, k).filter(|v| v.is_finite()).unwrap_or(0.0).clamp(-100.0, 100.0) * step;
+                let (dx, dy) = (dir("dx"), dir("dy"));
+                let (uid, page, centre, half) = {
+                    let ds = s.ds()?;
+                    let z = ds.view.zoom.max(1e-6);
+                    (ds.uid, ds.page, figcraft_geom::Point::new(ds.view.cx, ds.view.cy), (ds.view.vw / z / 2.0, ds.view.vh / z / 2.0))
+                };
+                // "It will appear in the middle of the viewport": the first arrow shows it there (a
+                // crosshair left on another page or document starts over).
+                let at = match s.crosshair() {
+                    Some(c) => figcraft_geom::Point::new(c.x + dx, c.y + dy),
+                    None => centre,
+                };
+                s.tool.place_at = Some(at);
+                s.tool.place_on = Some((uid, page));
+                if let Ok(ds) = s.ds_mut() {
+                    // "Move the crosshair around the canvas": nudged past an edge of the view, the
+                    // view pans by the overshoot to keep it in sight.
+                    let v = &mut ds.view;
+                    v.cx += (at.x - (v.cx + half.0)).max(0.0) + (at.x - (v.cx - half.0)).min(0.0);
+                    v.cy += (at.y - (v.cy + half.1)).max(0.0) + (at.y - (v.cy - half.1)).min(0.0);
+                    ds.revision += 1;
+                }
+                Ok(json!({"x": at.x, "y": at.y}))
+            }
+        ),
         spec!(
             "tool.place",
             "Place with the tool",

crates/engine/src/keymap.rs

@@ -354,6 +354,7 @@ pub static GESTURES: &[(Group, &str, &str, &str)] = &[
     (Tools, "Pan with nothing selected (Shift: farther)", "← ↑ → ↓", "← ↑ → ↓"),
     (Transform, "Rotate a degree (Shift: 15)", "⌥⌘ ← ↑ → ↓", "Ctrl+Alt+← ↑ → ↓"),
     (Tools, "Place a shape or frame (its tool chosen)", "↵", "Enter"),
+    (Tools, "Show and move the placement crosshair (its tool chosen, nothing selected)", "← ↑ → ↓", "← ↑ → ↓"),
     (Zoom, "Zoom tool: click to zoom in, drag an area", "Hold Z", "Hold Z"),
     (Zoom, "Zoom tool: zoom out", "Hold ⌥Z", "Hold Alt+Z"),
     (Zoom, "Zoom around the pointer", "⌘ scroll", "Ctrl+scroll"),

crates/engine/src/lib.rs

@@ -469,6 +469,8 @@ mod penpot_export_tests;
 #[cfg(test)]
 mod pixel_preview_tests;
 #[cfg(test)]
+mod place_crosshair_tests;
+#[cfg(test)]
 mod placed_image_size_tests;
 #[cfg(test)]
 mod playground_tests;

crates/engine/src/place_crosshair_tests.rs

@@ -0,0 +1,122 @@
+//! The placement crosshair: the arrows with a shape or frame tool chosen show it in the middle of
+//! the view and move it, and Enter places the object there ("Use the arrow keys to activate the
+//! crosshair cursor. It will appear in the middle of the viewport indicating where the new object
+//! will be placed." / "Use the arrow keys to move the crosshair around the canvas or board, then
+//! press Return/Enter to place the object.", Accessibility at Figma, #1839).
+
+use serde_json::json;
+
+use crate::Session;
+use crate::tools::{PointerEvent, PointerKind};
+
+#[test]
+fn the_arrows_show_and_move_the_crosshair_and_enter_places_there() {
+    let mut s = Session::new();
+    s.execute("view.set", &json!({"zoom": 1.0, "cx": 300, "cy": 200})).unwrap();
+    assert!(s.execute("tool.crosshair", &json!({"dx": 1})).is_err(), "a shape or frame tool first");
+    s.execute("tool.rectangle", &json!({})).unwrap();
+    // The first arrow shows the crosshair in the middle of the view, unmoved; the next move it by
+    // the nudge amounts.
+    let r = s.execute("tool.crosshair", &json!({"dx": 1, "dy": 0})).unwrap();
+    assert_eq!((r["x"].as_f64(), r["y"].as_f64()), (Some(300.0), Some(200.0)));
+    s.execute("tool.crosshair", &json!({"dx": 1, "dy": 0})).unwrap();
+    let r = s.execute("tool.crosshair", &json!({"dx": 0, "dy": -1, "big": true})).unwrap();
+    assert_eq!((r["x"].as_f64(), r["y"].as_f64()), (Some(301.0), Some(190.0)), "a nudge, then a big nudge");
+    assert_eq!(s.tool_info()["crosshair"], json!([301.0, 190.0]));
+    let v = &s.ds().unwrap().view;
+    assert_eq!((v.cx, v.cy), (300.0, 200.0), "the view hasn't panned");
+    // Enter: the rectangle is centred on the crosshair, which goes.
+    let placed = s.execute("tool.place", &json!({})).unwrap();
+    let id = crate::params::id_of(&placed["ids"][0]).unwrap();
+    let b = s.doc().unwrap().world_bounds(id).unwrap();
+    assert_eq!((b.center().x, b.center().y), (301.0, 190.0));
+    assert_eq!(s.tool.place_at, None);
+    assert!(s.tool_info()["crosshair"].is_null());
+    // Escape takes a shown crosshair away before anything else, the tool staying; a tool change
+    // takes it away too.
+    s.execute("select.set", &json!({"ids": []})).unwrap();
+    s.execute("tool.rectangle", &json!({})).unwrap();
+    s.execute("tool.crosshair", &json!({})).unwrap();
+    assert!(s.tool.place_at.is_some());
+    s.escape().unwrap();
+    assert_eq!((s.tool.place_at, s.tool.tool), (None, crate::ToolId::Rectangle));
+    s.execute("tool.crosshair", &json!({})).unwrap();
+    s.execute("tool.ellipse", &json!({})).unwrap();
+    assert_eq!(s.tool.place_at, None);
+}
+
+#[test]
+fn the_crosshair_keeps_its_page_and_goes_with_a_pointer_placement_but_not_the_same_tool_again() {
+    let mut s = Session::new();
+    s.execute("view.set", &json!({"zoom": 1.0, "cx": 300, "cy": 200})).unwrap();
+    s.execute("tool.rectangle", &json!({})).unwrap();
+    s.execute("tool.crosshair", &json!({"dx": 1})).unwrap();
+    s.execute("tool.crosshair", &json!({"dx": 1})).unwrap();
+    // The tool already chosen named again keeps the crosshair: `tool.place {tool}` lands on it.
+    let placed = s.execute("tool.place", &json!({"tool": "rectangle"})).unwrap();
+    let id = crate::params::id_of(&placed["ids"][0]).unwrap();
+    assert_eq!(s.doc().unwrap().world_bounds(id).unwrap().center().x, 301.0);
+    // A click places too (the tool kept by the preference) and takes the crosshair with it.
+    s.prefs.keep_tool = true;
+    s.execute("select.set", &json!({"ids": []})).unwrap();
+    s.execute("tool.rectangle", &json!({})).unwrap();
+    s.execute("tool.crosshair", &json!({})).unwrap();
+    s.pointer(&PointerEvent::at(PointerKind::Down, 600.0, 600.0, 1.0)).unwrap();
+    s.pointer(&PointerEvent::at(PointerKind::Up, 600.0, 600.0, 1.0)).unwrap();
+    assert_eq!((s.tool.tool, s.tool.place_at), (crate::ToolId::Rectangle, None), "placed by the pointer: the crosshair goes, the tool stays");
+    // Another page in front: the crosshair shown on the first isn't there, Escape has nothing to
+    // take, and the next arrow starts over in the middle of this page's view.
+    s.execute("select.set", &json!({"ids": []})).unwrap();
+    s.execute("tool.crosshair", &json!({"dx": 5})).unwrap();
+    s.execute("page.add", &json!({})).unwrap();
+    s.execute("page.select", &json!({"index": 1})).unwrap();
+    s.execute("tool.rectangle", &json!({})).unwrap();
+    assert_eq!(s.crosshair(), None);
+    assert!(s.tool_info()["crosshair"].is_null());
+    s.escape().unwrap();
+    assert_eq!(s.tool.tool, crate::ToolId::Move, "Escape went on to the tool");
+    s.execute("tool.rectangle", &json!({})).unwrap();
+    let r = s.execute("tool.crosshair", &json!({"dx": 1})).unwrap();
+    let v = &s.ds().unwrap().view;
+    assert_eq!((r["x"].as_f64(), r["y"].as_f64()), (Some(v.cx), Some(v.cy)), "shown afresh in the middle of this page's view");
+}
+
+#[test]
+fn the_view_pans_to_keep_the_crosshair_in_sight() {
+    // "Use the arrow keys to move the crosshair around the canvas or board": nudged past an edge
+    // of the view, the view follows it by the overshoot; inside it, the view stays put.
+    let mut s = Session::new();
+    s.execute("view.set", &json!({"zoom": 10.0, "cx": 300, "cy": 200})).unwrap();
+    s.execute("tool.rectangle", &json!({})).unwrap();
+    s.execute("tool.crosshair", &json!({})).unwrap();
+    for _ in 0..6 {
+        s.execute("tool.crosshair", &json!({"dx": 1, "big": true})).unwrap();
+    }
+    let v = s.ds().unwrap().view.clone();
+    assert_eq!((s.tool.place_at.map(|p| p.x), v.cx, v.cy), (Some(360.0), 300.0, 200.0), "at the edge of the 120 px wide view: no pan yet");
+    s.execute("tool.crosshair", &json!({"dx": 1, "big": true})).unwrap();
+    let v = s.ds().unwrap().view.clone();
+    assert_eq!((v.cx, v.cy), (310.0, 200.0), "ten past the edge: the view moved ten");
+    for _ in 0..5 {
+        s.execute("tool.crosshair", &json!({"dy": -1, "big": true})).unwrap();
+    }
+    let v = s.ds().unwrap().view.clone();
+    assert_eq!((v.cx, v.cy), (310.0, 190.0), "the view is 80 px tall: 40 up is its edge, 50 moves it ten");
+}
+
+#[test]
+fn the_board_shapes_tool_places_on_the_crosshair_too() {
+    // "...move the crosshair around the canvas or board, then press Return/Enter to place the
+    // object": the board Shapes tool takes the crosshair like the shape tools, its shape centred
+    // on it (the tool centres its own 160 px shape on a press).
+    let mut s = crate::board_tests::board();
+    s.execute("view.set", &json!({"zoom": 1.0, "cx": 300, "cy": 200})).unwrap();
+    s.execute("tool.set", &json!({"tool": "shape"})).unwrap();
+    s.execute("tool.crosshair", &json!({})).unwrap();
+    s.execute("tool.crosshair", &json!({"dx": 2})).unwrap();
+    let placed = s.execute("tool.place", &json!({})).unwrap();
+    let id = crate::params::id_of(&placed["ids"][0]).unwrap();
+    let b = s.doc().unwrap().world_bounds(id).unwrap();
+    assert_eq!((b.center().x, b.center().y, b.width(), b.height()), (302.0, 200.0, 160.0, 160.0));
+    assert_eq!(s.tool.place_at, None);
+}

crates/ui-egui/locales/en.toml

@@ -1089,6 +1089,7 @@ name = "English"
 "Pages: {pages}" = "Pages: {pages}"
 "Paint bucket" = "Paint bucket"
 "Pan the canvas" = "Pan the canvas"
+"Show and move the placement crosshair (its tool chosen, nothing selected)" = "Show and move the placement crosshair (its tool chosen, nothing selected)"
 "Pan with nothing selected (Shift: farther)" = "Pan with nothing selected (Shift: farther)"
 "Panels" = "Panels"
 "Paper" = "Paper"

crates/ui-egui/locales/es.toml

@@ -2369,6 +2369,7 @@ status = "draft — needs native review"
 "tool.arrow" = "Flecha"
 "tool.comment" = "Comentario"
 "tool.connector" = "Conector"
+"tool.crosshair" = "Mover la cruz de colocación"
 "tool.ellipse" = "Elipse"
 "tool.eraser" = "Borrador"
 "tool.frame" = "Marco"

crates/ui-egui/src/canvas/overlays.rs

@@ -145,6 +145,7 @@ pub(super) fn overlays(app: &FigApp, painter: &egui::Painter, t: &Tokens) -> Vec
     }
     // Pen path.
     pen_overlay(app, painter, zoom);
+    crosshair_overlay(app, painter);
     // Eraser brush (vector edit mode).
     if let Some([x, y, r]) = ov.brush {
         let c = app.to_screen(x, y);
@@ -416,6 +417,18 @@ fn attach_points(app: &FigApp, painter: &egui::Painter, points: &[([f64; 2], boo
     }
 }
 
+/// The placement crosshair the arrows showed and moved: where Enter puts the next shape or frame.
+fn crosshair_overlay(app: &FigApp, painter: &egui::Painter) {
+    let Some(p) = app.session.crosshair().filter(|_| app.session.tool.tool.draws_shape()) else { return };
+    let c = app.to_screen(p.x, p.y);
+    let col = accent(painter);
+    for d in [vec2(1.0, 0.0), vec2(0.0, 1.0)] {
+        painter.line_segment([c - d * 12.0, c - d * 3.0], stroke(col, 1.5));
+        painter.line_segment([c + d * 3.0, c + d * 12.0], stroke(col, 1.5));
+    }
+    painter.circle_stroke(c, 3.0, stroke(col, 1.5));
+}
+
 fn pen_overlay(app: &FigApp, painter: &egui::Painter, zoom: f64) {
     let ov = &app.session.tool.overlay;
     if !ov.pen.is_empty() {

crates/ui-egui/src/control.rs

@@ -1030,6 +1030,12 @@ fn ui_key(app: &mut FigApp, ctx: &Context, p: &Value) -> Result<Value, String> {
                 "ArrowUp" => (0, -1),
                 _ => (0, 1),
             };
+            // A shape or frame tool chosen and nothing selected: the arrows show and move the
+            // placement crosshair, as the window's key path does.
+            if app.session.selection().is_empty() && app.session.tool.tool.draws_shape() && !app.session.tool.dev_mode && !(m.cmd || m.ctrl || m.alt)
+            {
+                return app.session.execute("tool.crosshair", &json!({"dx": dx, "dy": dy, "big": m.shift})).map_err(|e| e.to_string());
+            }
             let (cmd, params) = if (m.cmd || m.ctrl) && m.alt {
                 // Rotate a degree, 15 with Shift: down and right clockwise, up and left back, about
                 // the selection's rotation origin; nothing while points are being edited.

crates/ui-egui/src/shortcut_keys_tests.rs

@@ -54,6 +54,28 @@ fn arrows_pan_the_view_when_nothing_is_selected() {
     assert_eq!(u.app.session.doc().unwrap().world_bounds(r).unwrap().x0, 1.0);
 }
 
+/// With a shape or frame tool chosen and nothing selected, the arrows show the placement crosshair
+/// in the middle of the view and move it (Shift: the large nudge) instead of panning, and Enter
+/// places the object there (Accessibility at Figma, #1839).
+#[test]
+fn arrows_show_and_move_the_placement_crosshair_with_a_shape_tool() {
+    let mut u = Ui::new(Session::new());
+    u.app.run("select.set", json!({"ids": []}));
+    u.app.run("view.set", json!({"zoom": 2.0, "cx": 100, "cy": 50}));
+    u.app.run("tool.rectangle", json!({}));
+    press(&mut u, Key::ArrowRight, Modifiers::NONE);
+    assert_eq!(view(&u), (100.0, 50.0, 2.0), "no pan: the crosshair shows instead");
+    assert_eq!(u.app.session.tool.place_at.map(|p| (p.x, p.y)), Some((100.0, 50.0)), "in the middle of the view");
+    press(&mut u, Key::ArrowDown, Modifiers::SHIFT);
+    assert_eq!(u.app.session.tool.place_at.map(|p| (p.x, p.y)), Some((100.0, 60.0)), "moved by the large nudge");
+    press(&mut u, Key::Enter, Modifiers::NONE);
+    let sel = u.app.session.selection();
+    let b = u.app.session.doc().unwrap().world_bounds(sel[0]).unwrap();
+    assert_eq!((b.center().x, b.center().y), (100.0, 60.0), "placed at the crosshair");
+    assert!(u.app.session.tool.place_at.is_none(), "which goes with the placement");
+    assert_eq!(view(&u), (100.0, 50.0, 2.0));
+}
+
 #[test]
 fn keyboard_box_selection_key_and_cmd_arrows() {
     let mut s = Session::new();

crates/ui-egui/src/shortcuts.rs

@@ -480,7 +480,11 @@ fn key_event(app: &mut FigApp, key: Key, physical: Option<Key>, m: Modifiers, re
         _ => (0.0, 0.0),
     };
     if (dx, dy) != (0.0, 0.0) {
-        if app.session.selection().is_empty() {
+        if app.session.selection().is_empty() && app.session.tool.tool.draws_shape() && !app.session.tool.dev_mode && !m.command && !m.alt {
+            // A shape or frame tool chosen: the arrows show the placement crosshair in the middle
+            // of the view and move it; Enter places there (Accessibility at Figma).
+            app.run("tool.crosshair", json!({"dx": dx, "dy": dy, "big": m.shift}));
+        } else if app.session.selection().is_empty() {
             app.run("view.pan", json!({"dx": dx, "dy": dy, "big": m.shift}));
         } else if crate::menus::dev_mode_refuses(app, "transform.nudge") {
             // Dev Mode: said so, nothing moves.