From cdb2057aab9afb9ec787373fcf6cb327de6a7fce Mon Sep 17 00:00:00 2001 From: Zinadin Zidan Date: Fri, 30 Oct 2026 21:36:32 +0600 Subject: [PATCH 1/3] Rectangle dialog: width and height in the ruler units, not always picas (#1964) A click with a frame tool opened the Rectangle dialog in picas whatever Preferences > Units & Increments or the rulers' unit menus said. It now shows the width in the horizontal ruler unit and the height in the vertical one, and reads bare numbers typed in each field the same way. The properties panel and the control channel open it the same way. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/app/commands.rs | 1 + crates/ui-egui/src/control.rs | 3 +- crates/ui-egui/src/dialogs.rs | 34 ++++++++++++++----- crates/ui-egui/src/dialogs/tests.rs | 34 +++++++++++++++++++ .../ui-egui/src/panels/properties/object.rs | 2 +- 5 files changed, 63 insertions(+), 11 deletions(-) diff --git a/crates/ui-egui/src/app/commands.rs b/crates/ui-egui/src/app/commands.rs index 44e35ef15..bb75c102e 100644 --- a/crates/ui-egui/src/app/commands.rs +++ b/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, ¶ms), UiRequest::Pick { purpose, params } => { diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index e4c6fe723..97a99f925 100644 --- a/crates/ui-egui/src/control.rs +++ b/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) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index bb3077103..63619da5e 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -79,13 +79,6 @@ 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 } } /// Set a field and report a genuine Find domain/mode transition to the caller. @@ -106,9 +99,13 @@ impl Dialog { } } fn m(&self, k: &str) -> Option { + self.m_in(k, self.unit) + } + /// A measure where a bare number is in `unit`. + fn m_in(&self, k: &str, unit: Unit) -> Option { 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, } } @@ -934,6 +931,25 @@ 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 +} + /// Layout › Create Guides…: guides go on the page in view (or the parent page being edited), not /// on the first page, and the gutters are typed in the document's ruler unit. pub fn open_create_guides(app: &mut DesignApp) { @@ -3277,7 +3293,7 @@ fn confirm_part1(app: &mut DesignApp, d: Dialog) -> Result { 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); + let h = d.m_in("height", doc_vertical_unit(app)).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]}), diff --git a/crates/ui-egui/src/dialogs/tests.rs b/crates/ui-egui/src/dialogs/tests.rs index 84cbf3bea..0bd29e02d 100644 --- a/crates/ui-egui/src/dialogs/tests.rs +++ b/crates/ui-egui/src/dialogs/tests.rs @@ -3521,3 +3521,37 @@ fn character_style_options_set_the_digits() { assert_eq!(ctl(&mut app, "ui.dialog.confirm", json!({}))["ok"], true); assert_eq!(char_style(&app, "Emphasis").unwrap().chars.digits, Some(designcraft_doc::Digits::Farsi)); } + +/// 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))); +} diff --git a/crates/ui-egui/src/panels/properties/object.rs b/crates/ui-egui/src/panels/properties/object.rs index 3819894fe..6b142e0d4 100644 --- a/crates/ui-egui/src/panels/properties/object.rs +++ b/crates/ui-egui/src/panels/properties/object.rs @@ -327,7 +327,7 @@ pub(super) fn appearance_section(app: &mut DesignApp, ui: &mut Ui, i: &SelInfo) let b = block(ui, FIELD_H); icons::paint(ui.painter(), sub(b, 3.0, 1.0, 19.0, 19.0), "corner", t.accent); if place(ui, sub(b, 34.0, 0.0, 56.0, FIELD_H), |ui| link_label(ui, crate::i18n::tr(&app.ui.language, "Corner"), 56.0)).clicked() { - app.ui.dialog = Some(crate::dialogs::Dialog::new("frameSize", json!({}))); + app.ui.dialog = Some(crate::dialogs::frame_size_dialog(app, json!({}))); } let (shape, size) = i.corner; if let Some(v) = place(ui, sub(b, 96.0, 0.0, 69.0, FIELD_H), |ui| { From 2e8bda16d0ee86ef913b1af3a62d3b1c9e501428 Mon Sep 17 00:00:00 2001 From: Zinadin Zidan Date: Fri, 30 Oct 2026 23:08:46 +0600 Subject: [PATCH 2/3] Rectangle dialog: every ruler unit shows the size and reads it back (#1964) A test for all eleven ruler units (points to Ha): the width and height the dialog shows read back as the frame's size, so OK never resizes it. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/dialogs/tests.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/crates/ui-egui/src/dialogs/tests.rs b/crates/ui-egui/src/dialogs/tests.rs index 0bd29e02d..5b5516c77 100644 --- a/crates/ui-egui/src/dialogs/tests.rs +++ b/crates/ui-egui/src/dialogs/tests.rs @@ -3554,4 +3554,15 @@ fn frame_size_dialog_follows_the_ruler_units() { // 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)); + } + } } From 486dcd53a2d12d49f373503040123da55789c408 Mon Sep 17 00:00:00 2001 From: Zinadin Zidan Date: Fri, 30 Oct 2026 23:58:45 +0600 Subject: [PATCH 3/3] Rectangle dialog: the height keeps the unit it was shown in (#1964) The dialog kept its width unit from when it opened but read the height in the vertical ruler unit of the moment, so rulers switched while it was open (the control channel's document.preferences) turned a typed 50 mm into 50 in. It now keeps the vertical unit too. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/dialogs.rs | 10 ++++++-- crates/ui-egui/src/dialogs/tests.rs | 38 +++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 63619da5e..b976887cd 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/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, } impl Dialog { @@ -79,7 +83,7 @@ impl Dialog { if id == "ruby" { fields.entry("section".to_string()).or_insert(json!("cjk.rubyPlacement")); } - 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 { @@ -947,6 +951,7 @@ pub fn frame_size_dialog(app: &DesignApp, params: Value) -> Dialog { } } d.unit = doc_unit(app); + d.vertical_unit = Some(doc_vertical_unit(app)); d } @@ -3293,7 +3298,8 @@ fn confirm_part1(app: &mut DesignApp, d: Dialog) -> Result { 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_in("height", doc_vertical_unit(app)).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]}), diff --git a/crates/ui-egui/src/dialogs/tests.rs b/crates/ui-egui/src/dialogs/tests.rs index 5b5516c77..e195a8c23 100644 --- a/crates/ui-egui/src/dialogs/tests.rs +++ b/crates/ui-egui/src/dialogs/tests.rs @@ -3566,3 +3566,41 @@ fn frame_size_dialog_follows_the_ruler_units() { } } } + +/// #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:?}"); +}