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 f8532d9cc..b8e149eb3 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 { @@ -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 { + 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, } } @@ -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 { 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]}), diff --git a/crates/ui-egui/src/dialogs/tests.rs b/crates/ui-egui/src/dialogs/tests.rs index b253b1bc9..b77774570 100644 --- a/crates/ui-egui/src/dialogs/tests.rs +++ b/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:?}"); +}