Merge storytold/main into PR #1232

crates/ui-egui/src/app/commands.rs

@@ -83,6 +83,7 @@ impl DesignApp {
             let was_find = self.ui.dialog.as_ref().is_some_and(|d| d.id == "findChange");
             match r {
                 UiRequest::Dialog { id, params } if id == "colorTheme" => dialogs::open_color_theme(self, params),
+                UiRequest::Dialog { id, params } if id == "frameSize" => self.ui.dialog = Some(dialogs::frame_size_dialog(self, params)),
                 UiRequest::Dialog { id, params } => self.ui.dialog = Some(dialogs::Dialog::new(&id, params)),
                 UiRequest::View { params } => canvas::apply_view_request(self, &params),
                 UiRequest::Pick { purpose, params } => {

crates/ui-egui/src/control.rs

@@ -389,7 +389,8 @@ fn handle_request(app: &mut DesignApp, ctx: &egui::Context, req: &ControlRequest
             if id == "findChange" || app.ui.dialog.as_ref().is_some_and(|d| d.id == "findChange") {
                 app.session.reset_find_scope();
             }
-            let d = crate::dialogs::Dialog::new(id, p.get("fields").cloned().unwrap_or(Value::Null));
+            let fields = p.get("fields").cloned().unwrap_or(Value::Null);
+            let d = if id == "frameSize" { crate::dialogs::frame_size_dialog(app, fields) } else { crate::dialogs::Dialog::new(id, fields) };
             let v = serde_json::to_value(&d).unwrap_or_default();
             app.ui.dialog = Some(d);
             ok(v)

crates/ui-egui/src/dialogs.rs

@@ -22,6 +22,10 @@ pub struct Dialog {
     /// Set from the active document when the dialog is shown or confirmed (picas without one).
     #[serde(skip)]
     pub unit: Unit,
+    /// The vertical ruler unit, for a dialog with a height field (the Rectangle dialog): kept
+    /// from when it opened, so a bare `50` means what it did then. `None` falls back to `unit`.
+    #[serde(skip)]
+    pub vertical_unit: Option<Unit>,
 }
 
 impl Dialog {
@@ -81,14 +85,7 @@ impl Dialog {
         if id == "ruby" {
             fields.entry("section".to_string()).or_insert(json!("cjk.rubyPlacement"));
         }
-        if id == "frameSize" {
-            for k in ["width", "height"] {
-                if let Some(v) = fields.get(k).and_then(Value::as_f64) {
-                    fields.insert(k.into(), json!(format_measure(v, Unit::Picas)));
-                }
-            }
-        }
-        Dialog { id: id.into(), fields, unit: Unit::Picas }
+        Dialog { id: id.into(), fields, unit: Unit::Picas, vertical_unit: None }
     }
     /// Set a field and report a genuine Find domain/mode transition to the caller.
     pub(crate) fn set_field(&mut self, key: &str, value: Value) -> bool {
@@ -108,9 +105,13 @@ impl Dialog {
         }
     }
     fn m(&self, k: &str) -> Option<f64> {
+        self.m_in(k, self.unit)
+    }
+    /// A measure where a bare number is in `unit`.
+    fn m_in(&self, k: &str, unit: Unit) -> Option<f64> {
         match self.fields.get(k) {
             Some(Value::Number(n)) => n.as_f64(),
-            Some(Value::String(s)) => parse_measure(s, self.unit).ok(),
+            Some(Value::String(s)) => parse_measure(s, unit).ok(),
             _ => None,
         }
     }
@@ -1503,6 +1504,26 @@ fn doc_unit(app: &DesignApp) -> Unit {
     app.session.active().map(|s| s.doc.settings.horizontal_units).unwrap_or(Unit::Picas)
 }
 
+/// The document's vertical ruler unit: what a bare number means in a height field.
+fn doc_vertical_unit(app: &DesignApp) -> Unit {
+    app.session.active().map(|s| s.doc.settings.vertical_units).unwrap_or(Unit::Picas)
+}
+
+/// The Rectangle dialog a click with a frame tool opens: the width (points in `params`) shown in
+/// the horizontal ruler unit, the height in the vertical one, as Preferences › Units &
+/// Increments and the rulers' menus set them (#1964).
+pub fn frame_size_dialog(app: &DesignApp, params: Value) -> Dialog {
+    let mut d = Dialog::new("frameSize", params);
+    for (k, unit) in [("width", doc_unit(app)), ("height", doc_vertical_unit(app))] {
+        if let Some(v) = d.fields.get(k).and_then(Value::as_f64) {
+            d.fields.insert(k.into(), json!(format_measure(v, unit)));
+        }
+    }
+    d.unit = doc_unit(app);
+    d.vertical_unit = Some(doc_vertical_unit(app));
+    d
+}
+
 /// Layout › Create Guides…: guides go on every page of the spread in view (or of the parent
 /// spread being edited), as InDesign's do; "This page" narrows them to
 /// the page in view. The gutters are typed in the document's ruler unit.
@@ -4056,8 +4077,12 @@ fn format_settings(app: &mut DesignApp, ctx: &egui::Context, d: &mut Dialog) {
         d.fields.remove(FORMAT_EDIT);
         return;
     };
-    let mut fd =
-        Dialog { id: "changeFormat".into(), fields: d.fields.get(FORMAT_EDIT).and_then(Value::as_object).cloned().unwrap_or_default(), unit: d.unit };
+    let mut fd = Dialog {
+        id: "changeFormat".into(),
+        fields: d.fields.get(FORMAT_EDIT).and_then(Value::as_object).cloned().unwrap_or_default(),
+        unit: d.unit,
+        vertical_unit: None,
+    };
     let base = json!({});
     let cf = CharFields { base: &base, sparse: true };
     let find = fd.s("for") == "find";
@@ -4220,7 +4245,8 @@ fn confirm_part1(app: &mut DesignApp, d: Dialog) -> Result<Value, String> {
             let x = d.fields.get("x").and_then(Value::as_f64).unwrap_or(0.0);
             let y = d.fields.get("y").and_then(Value::as_f64).unwrap_or(0.0);
             let w = d.m("width").unwrap_or(72.0);
-            let h = d.m("height").unwrap_or(72.0);
+            // In the units the dialog showed, not ones changed while it was open.
+            let h = d.m_in("height", d.vertical_unit.unwrap_or(d.unit)).unwrap_or(72.0);
             app.run(
                 "frame.create",
                 json!({"spread": d.fields.get("spread").cloned().unwrap_or(json!(0)), "shape": d.s("shape"), "content": d.s("content"), "rect": [x, y, x + w, y + h]}),

crates/ui-egui/src/dialogs/tests.rs

@@ -4814,3 +4814,86 @@ fn convert_dialogs_use_their_separators() {
     // (The empty paragraph after a table that ends its story stays after the text.)
     assert_eq!(story.trim_end(), "Name:, Address:\nCity:, State:");
 }
+
+/// The Rectangle dialog a frame-tool click opens shows the width in the horizontal ruler unit and
+/// the height in the vertical one, not always picas, and reads bare numbers the same way (#1964).
+#[test]
+fn frame_size_dialog_follows_the_ruler_units() {
+    let mut app = DesignApp::new(designcraft_engine::Session::new(), crate::Services::default());
+    app.run("file.new", json!({})).unwrap();
+    app.run("document.preferences", json!({"horizontalUnits": "inches", "verticalUnits": "millimeters"})).unwrap();
+    // As the frame tool asks: a click at (36, 36), 72 pt by 72 pt.
+    app.session.ui_requests.push(designcraft_engine::UiRequest::Dialog {
+        id: "frameSize".into(),
+        params: json!({"spread": 0, "shape": "rectangle", "content": "graphic", "x": 36.0, "y": 36.0, "width": 72.0, "height": 72.0}),
+    });
+    app.after_engine();
+    let d = app.ui.dialog.as_ref().expect("the Rectangle dialog");
+    assert_eq!((d.id.as_str(), d.unit), ("frameSize", Unit::Inches));
+    assert_eq!(d.s("width"), format_measure(72.0, Unit::Inches));
+    assert_eq!(d.s("height"), format_measure(72.0, Unit::Millimeters));
+    assert!(!d.s("width").contains('p') && !d.s("height").contains('p'), "no picas: {} x {}", d.s("width"), d.s("height"));
+    // Bare numbers typed in each field are in that field's unit: 2 in by 50 mm.
+    let d = app.ui.dialog.as_mut().unwrap();
+    d.fields.insert("width".into(), json!("2"));
+    d.fields.insert("height".into(), json!("50"));
+    let before = app.session.doc().unwrap().doc.clone();
+    confirm(&mut app).unwrap();
+    let after = &app.session.doc().unwrap().doc;
+    let added: Vec<_> = after.spreads[0].items.iter().filter(|i| !before.spreads[0].items.iter().any(|b| b.id == i.id)).collect();
+    assert_eq!(added.len(), 1, "one frame");
+    let b = added[0].bounds();
+    assert!((b.width() - 144.0).abs() < 0.01 && (b.height() - 50.0 / 25.4 * 72.0).abs() < 0.01, "{b:?}");
+    // The properties panel's link opens it in the same units.
+    let d = frame_size_dialog(&app, json!({"width": 36.0, "height": 36.0}));
+    assert_eq!((d.s("width"), d.s("height")), (format_measure(36.0, Unit::Inches), format_measure(36.0, Unit::Millimeters)));
+    // Every ruler unit: the shown size reads back as the size, so OK leaves a 72 pt frame 72 pt.
+    let units = serde_json::to_value(Unit::ALL).unwrap();
+    for (unit, name) in Unit::ALL.iter().zip(units.as_array().unwrap()) {
+        app.run("document.preferences", json!({"horizontalUnits": name, "verticalUnits": name})).unwrap();
+        let d = frame_size_dialog(&app, json!({"width": 72.0, "height": 145.0}));
+        assert_eq!(d.unit, *unit);
+        for (k, pt) in [("width", 72.0), ("height", 145.0)] {
+            let back = d.m_in(k, *unit).unwrap_or(f64::NAN);
+            assert!((back - pt).abs() < 0.05, "{name}: {k} shows {:?}, reads back {back}", d.s(k));
+        }
+    }
+}
+
+/// #2055 review: the Rectangle dialog reads the height in the vertical unit it showed when it
+/// opened. Rulers switched while it is open (the control channel) leave a typed 50 at 50 mm.
+#[test]
+fn frame_size_dialog_keeps_its_units_while_open() {
+    let mut app = DesignApp::new(designcraft_engine::Session::new(), crate::Services::default());
+    app.run("file.new", json!({})).unwrap();
+    app.run("document.preferences", json!({"horizontalUnits": "millimeters", "verticalUnits": "millimeters"})).unwrap();
+    app.session.ui_requests.push(designcraft_engine::UiRequest::Dialog {
+        id: "frameSize".into(),
+        params: json!({"spread": 0, "shape": "rectangle", "content": "graphic", "x": 36.0, "y": 36.0, "width": 72.0, "height": 72.0}),
+    });
+    app.after_engine();
+    let d = app.ui.dialog.as_mut().expect("the Rectangle dialog");
+    d.fields.insert("width".into(), json!("50"));
+    d.fields.insert("height".into(), json!("50"));
+    app.run("document.preferences", json!({"horizontalUnits": "inches", "verticalUnits": "inches"})).unwrap();
+    let before = app.session.doc().unwrap().doc.clone();
+    confirm(&mut app).unwrap();
+    let after = &app.session.doc().unwrap().doc;
+    let added: Vec<_> = after.spreads[0].items.iter().filter(|i| !before.spreads[0].items.iter().any(|b| b.id == i.id)).collect();
+    assert_eq!(added.len(), 1, "one frame");
+    let b = added[0].bounds();
+    let mm50 = 50.0 / 25.4 * 72.0;
+    assert!((b.width() - mm50).abs() < 0.01 && (b.height() - mm50).abs() < 0.01, "50 mm by 50 mm (about 141.73 pt): {b:?}");
+    // A dialog made without the vertical unit (`Dialog::new`) reads the height in its own unit.
+    let mut d = Dialog::new("frameSize", json!({"width": "2", "height": "3"}));
+    d.unit = Unit::Inches;
+    assert_eq!(d.vertical_unit, None);
+    app.ui.dialog = Some(d);
+    let before = app.session.doc().unwrap().doc.clone();
+    confirm(&mut app).unwrap();
+    let after = &app.session.doc().unwrap().doc;
+    let added: Vec<_> = after.spreads[0].items.iter().filter(|i| !before.spreads[0].items.iter().any(|b| b.id == i.id)).collect();
+    assert_eq!(added.len(), 1, "one frame");
+    let b = added[0].bounds();
+    assert!((b.width() - 144.0).abs() < 0.01 && (b.height() - 216.0).abs() < 0.01, "{b:?}");
+}