Integration: a unit per field, no Percent, not remembered, as in Photoshop (#7922)

Photoshop's Create Rectangle switches only the right-clicked field
(Width 35.28 mm, Height still 100 px), lists the six length units
without Percent, and opens in pixels again after OK. The PR shared one
unit between both fields, offered Percent, and saved the unit on every
OK, so after one use with the rulers in px the dialog stayed in px even
after the rulers changed to cm, though no unit was ever picked.

Now each field has its own widthUnit / heightUnit (set only by the
menu, never remembered), and the menu is rulers::unit_menu_of with
LENGTH_UNITS. The new test right-clicks the real field and clicks the
menu item through egui input.

Co-authored-by: Korg1ch <57859048+Korg1ch@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/ui-egui/src/rulers.rs

@@ -64,19 +64,28 @@ fn unit_label(u: Unit) -> &'static str {
 /// Every length unit, in the Preferences › Units & Rulers order.
 const UNITS: [Unit; 7] = [Unit::Pixels, Unit::Inches, Unit::Centimeters, Unit::Millimeters, Unit::Points, Unit::Picas, Unit::Percent];
 
-/// The ruler right-click menu: every length unit with its (translated) label and whether it's the
-/// one in effect. Picking one changes the ruler unit exactly as the preference pane does.
-fn unit_menu_items(current: Unit) -> Vec<(String, &'static str, bool)> {
-    UNITS.iter().map(|&u| (tl!(unit_label(u)).to_string(), u.name(), u == current)).collect()
+/// The units Photoshop's Create Rectangle (and the other shape dialogs) offer on a right-click of
+/// Width or Height: the lengths, without Percent.
+pub(crate) const LENGTH_UNITS: [Unit; 6] = [Unit::Pixels, Unit::Inches, Unit::Centimeters, Unit::Millimeters, Unit::Points, Unit::Picas];
+
+/// The ruler right-click menu: every unit of `units` with its (translated) label and whether it's
+/// the one in effect. Picking one changes the ruler unit exactly as the preference pane does.
+fn unit_menu_items(current: Unit, units: &[Unit]) -> Vec<(String, &'static str, bool)> {
+    units.iter().map(|&u| (tl!(unit_label(u)).to_string(), u.name(), u == current)).collect()
 }
 
 /// The length units menu that right-clicking a ruler (or Free Transform's W and H fields) opens;
 /// the chosen unit's name is returned (`None` if the menu is dismissed without a pick).
 pub(crate) fn unit_menu(ui: &mut egui::Ui, current: Unit) -> Option<&'static str> {
+    unit_menu_of(ui, current, &UNITS)
+}
+
+/// [`unit_menu`] with only `units` in it.
+pub(crate) fn unit_menu_of(ui: &mut egui::Ui, current: Unit, units: &[Unit]) -> Option<&'static str> {
     crate::widgets::menu_scroll(ui, |ui| {
         ui.set_min_width(150.0);
         let mut chosen = None;
-        for (label, name, checked) in unit_menu_items(current) {
+        for (label, name, checked) in unit_menu_items(current, units) {
             let label = if checked { format!("✓ {label}") } else { label };
             if ui.button(label).clicked() {
                 ui.close();
@@ -969,7 +978,7 @@ mod tests {
 
     #[test]
     fn unit_menu_lists_every_unit_with_the_current_one_checked() {
-        let items = unit_menu_items(Unit::Inches);
+        let items = unit_menu_items(Unit::Inches, &UNITS);
         assert_eq!(items.len(), Unit::NAMES.len());
         // Every unit is offered, labelled, and in the preference's order.
         assert_eq!(items[0], ("Pixels".to_string(), "pixels", false));

crates/ui-egui/src/shape_dialog.rs

@@ -2,8 +2,8 @@
 //! opens Photoshop's creation dialog instead of drawing. OK runs the same `shape.create` command as
 //! a drag, with the options bar's fill and stroke, at the clicked point (or centred on it).
 //! Width and height are stored in document pixels (so automation can set them with
-//! `ui.dialog.set`) and shown in the ruler unit, or in one picked from the Width and Height
-//! fields' right-click menu (remembered with the other values).
+//! `ui.dialog.set`) and shown in the ruler unit, or each in one picked from its right-click menu
+//! (`widthUnit`, `heightUnit`; not remembered, as in Photoshop).
 
 use egui::{Align2, Sense, pos2, vec2};
 use serde_json::{Map, Value, json};
@@ -18,7 +18,7 @@ const MARK: &str = "__createShape";
 /// Largest side the dialog accepts, in pixels (Photoshop's document limit).
 const MAX_PX: f64 = 300_000.0;
 /// Remembered values (document pixels; sides and radii come from the options bar).
-const KEYS: [&str; 5] = ["width", "height", "fromCenter", "starRatio", "unit"];
+const KEYS: [&str; 4] = ["width", "height", "fromCenter", "starRatio"];
 
 pub fn owns(fields: &Map<String, Value>) -> bool {
     fields.contains_key(MARK)
@@ -43,7 +43,6 @@ fn valid(key: &str, v: &Value) -> bool {
     match key {
         "width" | "height" => v.as_f64().is_some_and(|x| x.is_finite() && (1.0..=MAX_PX).contains(&x)),
         "starRatio" => v.as_f64().is_some_and(|x| x.is_finite() && (1.0..=100.0).contains(&x)),
-        "unit" => v.as_str().is_some_and(|u| Unit::parse(u).is_some()),
         _ => v.is_boolean(),
     }
 }
@@ -72,8 +71,6 @@ pub fn open(app: &mut PhotocraftApp, tool: Tool, at: [f64; 2]) -> Option<u64> {
     f.insert("width".into(), json!(100.0));
     f.insert("height".into(), json!(100.0));
     f.insert("fromCenter".into(), json!(false));
-    // The unit Width and Height show (the ruler unit until one is picked with a right-click).
-    f.insert("unit".into(), json!(app.session.prefs().units_and_rulers.rulers.name()));
     match kind {
         "rectangle" => {
             f.insert("radii".into(), json!([r, r, r, r]));
@@ -166,16 +163,16 @@ fn label(ui: &mut egui::Ui, text: &str) {
 pub fn body(app: &PhotocraftApp, ui: &mut egui::Ui, f: &mut Map<String, Value>) {
     ui.spacing_mut().item_spacing.y = 6.0;
     let kind = f.get(MARK).and_then(Value::as_str).unwrap_or("").to_string();
-    // Width and height in the dialog's unit (percent of the document's side): the ruler unit, or
-    // the one picked from the fields' right-click menu, as in Photoshop (#7786).
+    // Width and height each in its own unit (percent of the document's side): the ruler unit, or
+    // the one picked from that field's right-click menu, as in Photoshop (#7786).
     let ur = &app.session.prefs().units_and_rulers;
-    let unit = f.get("unit").and_then(Value::as_str).and_then(Unit::parse).unwrap_or(ur.rulers);
-    let ppi = ur.point_size.per_inch();
+    let (rulers, ppi) = (ur.rulers, ur.point_size.per_inch());
     let (dpi, size) = app
         .session
         .active()
         .map_or((72.0, [1.0, 1.0]), |d| (f64::from(d.doc.resolution_dpi.max(1.0)), [f64::from(d.doc.size.width), f64::from(d.doc.size.height)]));
-    for (key, text, extent) in [("width", tl!("Width:"), size[0]), ("height", tl!("Height:"), size[1])] {
+    for (key, unit_key, text, extent) in [("width", "widthUnit", tl!("Width:"), size[0]), ("height", "heightUnit", tl!("Height:"), size[1])] {
+        let unit = f.get(unit_key).and_then(Value::as_str).and_then(Unit::parse).unwrap_or(rulers);
         ui.horizontal(|ui| {
             label(ui, text);
             let px = num(f, key).unwrap_or(100.0);
@@ -187,10 +184,10 @@ pub fn body(app: &PhotocraftApp, ui: &mut egui::Ui, f: &mut Map<String, Value>)
                 let px = unit.to_px(f64::from(v), dpi, extent, ppi).clamp(1.0, MAX_PX);
                 f.insert(key.into(), json!((px * 1000.0).round() / 1000.0));
             }
-            // Right-click: the length units, for both fields (the sizes stay in pixels).
+            // Right-click: the length units for this field (the sizes stay in pixels).
             resp.context_menu(|ui| {
-                if let Some(name) = crate::rulers::unit_menu(ui, unit) {
-                    f.insert("unit".into(), json!(name));
+                if let Some(name) = crate::rulers::unit_menu_of(ui, unit, &crate::rulers::LENGTH_UNITS) {
+                    f.insert(unit_key.into(), json!(name));
                 }
             });
         });

crates/ui-egui/src/shape_dialog_tests.rs

@@ -249,56 +249,84 @@ fn the_dialog_body_renders_for_every_tool_and_unit() {
     }
 }
 
-/// The texts a dialog body draws.
-fn body_texts(app: &PhotocraftApp, fields: &mut serde_json::Map<String, Value>) -> Vec<String> {
-    let ctx = egui::Context::default();
-    let mut out = ctx.run_ui(egui::RawInput::default(), |ui| body(app, ui, fields));
+/// One frame of a dialog body with `events`: the texts it draws, each with its screen rect.
+fn frame(ctx: &egui::Context, app: &PhotocraftApp, fields: &mut serde_json::Map<String, Value>, events: Vec<egui::Event>) -> Vec<(String, egui::Rect)> {
+    let input = egui::RawInput { events, ..Default::default() };
+    let mut out = ctx.run_ui(input, |ui| body(app, ui, fields));
     out.textures_delta.clear();
     let mut texts = Vec::new();
     for clip in &out.shapes {
         if let egui::Shape::Text(t) = &clip.shape {
-            texts.push(t.galley.text().to_string());
+            texts.push((t.galley.text().to_string(), t.galley.rect.translate(t.pos.to_vec2())));
         }
     }
     texts
 }
 
-/// #7786: Width and Height show in the unit picked from their right-click menu (the ruler unit
-/// until then), the sizes stay in pixels, and OK remembers the unit with the other values.
+/// A click of `button` at `pos`, as frames: move there, press, release.
+fn click(ctx: &egui::Context, app: &PhotocraftApp, f: &mut serde_json::Map<String, Value>, pos: egui::Pos2, button: egui::PointerButton) {
+    let press = |pressed| egui::Event::PointerButton { pos, button, pressed, modifiers: egui::Modifiers::NONE };
+    frame(ctx, app, f, vec![egui::Event::PointerMoved(pos)]);
+    frame(ctx, app, f, vec![press(true)]);
+    frame(ctx, app, f, vec![press(false)]);
+}
+
+/// Each of Width and Height: its value and unit, and the value's screen rect.
+fn sides(texts: &[(String, egui::Rect)]) -> Vec<(String, String, egui::Rect)> {
+    ["Width:", "Height:"]
+        .iter()
+        .filter_map(|l| texts.iter().position(|(t, _)| t == l))
+        .filter_map(|i| {
+            let (v, r) = texts.get(i + 1)?.clone();
+            Some((v, texts.get(i + 2)?.0.clone(), r))
+        })
+        .collect()
+}
+
+/// #7786: right-clicking Width or Height offers the length units (no Percent, as in Photoshop's
+/// Create Rectangle) and switches only that field; the sizes stay in pixels and the unit isn't
+/// remembered, so the next dialog follows the ruler unit again.
 #[test]
-fn width_and_height_show_in_the_picked_unit_and_remember_it() {
+fn right_click_picks_the_unit_of_that_field_only_and_it_is_not_remembered() {
     let mut app = app();
     press(&mut app, Tool::Rectangle, [10.0, 10.0], [10.0, 10.0]);
     let id = only_dialog(&app);
     let mut f = app.ui.dialog_mut(id).unwrap().fields.clone();
-    assert_eq!(f["unit"], json!("pixels"), "the ruler unit at first");
-    // Each of Width and Height: its label, its value, its unit.
-    let sides = |texts: &[String]| -> Vec<(String, String)> {
-        ["Width:", "Height:"]
-            .iter()
-            .filter_map(|l| texts.iter().position(|t| t == l))
-            .filter_map(|i| Some((texts.get(i + 1)?.clone(), texts.get(i + 2)?.clone())))
-            .collect()
-    };
+    let ctx = egui::Context::default();
+    let shown = |s: &[(String, String, egui::Rect)]| s.iter().map(|(v, u, _)| (v.clone(), u.clone())).collect::<Vec<_>>();
     let px = ("100".to_string(), "px".to_string());
-    assert_eq!(sides(&body_texts(&app, &mut f)), [px.clone(), px], "shown in pixels");
-    // What the menu's Millimeters sets: 100 px at the document's 72 ppi is 35.28 mm.
-    f.insert("unit".into(), json!("mm"));
-    let mm = ("35.28".to_string(), "mm".to_string());
-    assert_eq!(sides(&body_texts(&app, &mut f)), [mm.clone(), mm]);
+    let texts = frame(&ctx, &app, &mut f, vec![]);
+    let s = sides(&texts);
+    assert_eq!(shown(&s), [px.clone(), px.clone()], "shown in the ruler unit, pixels");
+    // Right-click Width: its menu lists the lengths and no Percent.
+    click(&ctx, &app, &mut f, s[0].2.center(), egui::PointerButton::Secondary);
+    let menu = frame(&ctx, &app, &mut f, vec![]);
+    let has = |label: &str| menu.iter().any(|(t, _)| t.ends_with(label));
+    assert!(["Pixels", "Inches", "Centimeters", "Millimeters", "Points", "Picas"].iter().all(|l| has(l)), "{menu:?}");
+    assert!(!has("Percent"), "Photoshop's menu here has no Percent");
+    let mm_item = menu.iter().find(|(t, _)| t == "Millimeters").unwrap().1.center();
+    click(&ctx, &app, &mut f, mm_item, egui::PointerButton::Primary);
+    // Only Width changes: 100 px at the document's 72 ppi is 35.28 mm; the size stays in pixels.
+    let s = sides(&frame(&ctx, &app, &mut f, vec![]));
+    assert_eq!(shown(&s), [("35.28".to_string(), "mm".to_string()), px.clone()]);
+    assert_eq!((f["widthUnit"].as_str(), f.get("heightUnit")), (Some("mm"), None));
     assert_eq!(f["width"].as_f64(), Some(100.0), "the size stays in pixels");
-    set(&mut app, id, json!({"unit": "mm", "width": 50}));
+    // Height has its own menu, with its own unit checked: Inches switches Height alone.
+    click(&ctx, &app, &mut f, s[1].2.center(), egui::PointerButton::Secondary);
+    let menu = frame(&ctx, &app, &mut f, vec![]);
+    assert!(menu.iter().any(|(t, _)| t == "✓ Pixels"), "{menu:?}");
+    let in_item = menu.iter().find(|(t, _)| t == "Inches").unwrap().1.center();
+    click(&ctx, &app, &mut f, in_item, egui::PointerButton::Primary);
+    let s = sides(&frame(&ctx, &app, &mut f, vec![]));
+    assert_eq!(shown(&s), [("35.28".to_string(), "mm".to_string()), ("1.389".to_string(), "in".to_string())]);
+    // OK, then the rulers change: the next dialog shows the new ruler unit, not the pick.
+    set(&mut app, id, json!({"widthUnit": "mm", "width": 50}));
     crate::dialogs::confirm(&mut app, id).unwrap();
-    // The next Create Rectangle opens in millimetres, with the remembered size.
+    app.run("prefs.set", json!({"path": "unitsAndRulers.rulers", "value": "cm"})).unwrap();
     press(&mut app, Tool::Rectangle, [40.0, 40.0], [40.0, 40.0]);
     let id = only_dialog(&app);
-    let f = &app.ui.dialog_mut(id).unwrap().fields;
-    assert_eq!((f["unit"].as_str(), f["width"].as_f64()), (Some("mm"), Some(50.0)));
-    // A remembered unit that isn't one falls back to the ruler unit.
-    app.ui.close_dialog(id);
-    app.session.prefs.edit(|p| {
-        p.dialogs.insert("shape.create.rectangle".into(), json!({"unit": "furlongs"}));
-    });
-    press(&mut app, Tool::Rectangle, [40.0, 40.0], [40.0, 40.0]);
-    assert_eq!(app.ui.dialogs[0].fields["unit"], json!("pixels"));
+    let mut f = app.ui.dialog_mut(id).unwrap().fields.clone();
+    assert_eq!((f.get("widthUnit"), f["width"].as_f64()), (None, Some(50.0)), "the size is remembered, the unit isn't");
+    let s = sides(&frame(&egui::Context::default(), &app, &mut f, vec![]));
+    assert_eq!(shown(&s), [("1.76".to_string(), "cm".to_string()), ("3.53".to_string(), "cm".to_string())]);
 }