From 2c0121aebb098ccaab6473c8d8eddcc84f81dd5d Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 12 Nov 2026 09:14:42 -0800 Subject: [PATCH] Text: Text Frame Options from the Type tool stay with their document The dialog stores the frames it took from the caret or selection with the document state they came from. OK and Preview are refused when another document became active after the dialog opened, or when Revert replaced the document, since the stored frame ids can name a different frame there. --- crates/ui-egui/src/dialogs.rs | 19 +++++++++-- crates/ui-egui/src/dialogs/tests.rs | 53 +++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index f2689c927..b6141a75f 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1102,6 +1102,9 @@ fn seed_text_frame_options(app: &DesignApp, d: &mut Dialog) { // text into another frame does not change which frames the options apply to. if let Some(ids) = typing_frame_ids(app) { d.fields.insert("_targets".into(), ids); + // The document state those frames are in: another document, or the document after + // Revert, can reuse their ids. + d.fields.insert("_targetsDoc".into(), json!(app.session.active().map(|doc| doc.identity_epoch))); } d.fields.insert("_shown".into(), shown); } @@ -1605,14 +1608,24 @@ fn text_frame_params(app: &DesignApp, d: &Dialog) -> Result, json!(d.m("paragraphSpacingLimit").filter(|v| *v >= 0.0).ok_or_else(|| bad("the paragraph spacing limit is not a measure"))?), ); } - // Typing in a frame: the options apply to the frame that showed the caret, or to every frame - // that held part of the selected text, when the dialog opened. - if let Some(ids) = d.fields.get("_targets").cloned().or_else(|| typing_frame_ids(app)) { + if let Some(ids) = text_frame_targets(app, d).map_err(bad)? { p.insert("ids".into(), ids); } Ok(p) } +/// Typing in a frame: the frame that showed the caret, or every frame that held part of the +/// selected text, when the dialog opened. Those ids are refused once another document is active, +/// or once Revert has replaced the document. +fn text_frame_targets(app: &DesignApp, d: &Dialog) -> Result, &'static str> { + let Some(ids) = d.fields.get("_targets").cloned() else { return Ok(typing_frame_ids(app)) }; + let active = app.session.active().map(|doc| doc.identity_epoch); + if d.fields.get("_targetsDoc").and_then(Value::as_u64) != active { + return Err("the document the dialog opened on is no longer the active one"); + } + Ok(Some(ids)) +} + /// File › Document Setup, filled from the document. /// The active document's ruler unit (picas without a document). fn doc_unit(app: &DesignApp) -> Unit { diff --git a/crates/ui-egui/src/dialogs/tests.rs b/crates/ui-egui/src/dialogs/tests.rs index 272165b08..1d2bcd4a1 100644 --- a/crates/ui-egui/src/dialogs/tests.rs +++ b/crates/ui-egui/src/dialogs/tests.rs @@ -1389,6 +1389,59 @@ fn text_frame_options_set_columns_baseline_and_wrap_as_indesign_does() { assert_eq!((opts(&app).columns_kind, opts(&app).column_width), (designcraft_doc::ColumnsKind::FixedNumber, 120.0)); } +/// The frames Text Frame Options took while typing belong to the document it opened on: when +/// another document becomes active before OK, OK changes nothing there. +#[test] +fn text_frame_options_from_the_type_tool_stay_with_their_document() { + let mut app = crate::DesignApp::new(designcraft_engine::Session::new(), crate::Services::default()); + let make = |app: &mut crate::DesignApp| { + app.run("file.new", json!({})).unwrap(); + let r = app.run("frame.create", json!({"rect": [36, 36, 300, 300], "content": "text", "text": "Hello"})).unwrap(); + let (id, story) = (r["id"].as_u64().unwrap(), r["story"].as_u64().unwrap()); + app.run("text.select", json!({"story": story, "anchor": 2})).unwrap(); + id + }; + let a = make(&mut app); + let mut d = Dialog::new("textFrameOptions", json!({})); + seed_text_frame_options(&app, &mut d); + assert_eq!(d.fields.get("_targets"), Some(&json!([a]))); + d.fields.insert("columns".into(), json!(3)); + // A second document, with a caret in a frame that has the same id, becomes active. + let b = make(&mut app); + assert_eq!(a, b); + app.ui.dialog = Some(d); + let err = confirm(&mut app).unwrap_err().to_string(); + assert!(err.contains("no longer the active one"), "{err}"); + let columns = app.session.active().unwrap().doc.item(designcraft_doc::ItemId(b)).and_then(|i| i.text_frame().map(|t| t.options.columns)); + assert_eq!(columns, Some(1), "the other document's frame is unchanged"); +} + +/// Revert replaces the document while Text Frame Options taken while typing stay open: OK is +/// refused, since the saved frame ids can name another frame in the reverted document. +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn text_frame_options_from_the_type_tool_are_refused_after_revert() { + let mut app = crate::DesignApp::new(designcraft_engine::Session::new(), crate::Services::default()); + app.run("file.new", json!({})).unwrap(); + let dir = tempfile::tempdir().unwrap(); + app.run("file.saveAs", json!({"path": dir.path().join("revert.designcraft")})).unwrap(); + let a = app.run("frame.create", json!({"rect": [36, 36, 300, 120], "content": "text", "text": "Unsaved"})).unwrap(); + app.run("text.select", json!({"story": a["story"], "anchor": 1})).unwrap(); + let mut d = Dialog::new("textFrameOptions", json!({})); + seed_text_frame_options(&app, &mut d); + d.fields.insert("columns".into(), json!(3)); + app.run("file.revert", json!({})).unwrap(); + let b = app.run("frame.create", json!({"rect": [36, 200, 300, 300], "content": "text", "text": "After"})).unwrap(); + assert_eq!(a["id"], b["id"], "the reverted document gives a new frame the same id"); + app.ui.dialog = Some(d); + let err = confirm(&mut app).unwrap_err().to_string(); + assert!(err.contains("no longer the active one"), "{err}"); + assert!(app.ui.dialog.is_some(), "the dialog stays open"); + let b = designcraft_doc::ItemId(b["id"].as_u64().unwrap()); + let columns = app.session.active().unwrap().doc.item(b).and_then(|i| i.text_frame().map(|t| t.options.columns)); + assert_eq!(columns, Some(1), "the new frame is unchanged"); +} + #[test] fn baseline_grid_increment_error_explains_the_positive_measure_rule() { let mut app = crate::DesignApp::new(designcraft_engine::Session::new(), crate::Services::default());