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());