diff --git a/crates/ui-egui/src/content_ui.rs b/crates/ui-egui/src/content_ui.rs index 04d626088d..77edee6876 100644 --- a/crates/ui-egui/src/content_ui.rs +++ b/crates/ui-egui/src/content_ui.rs @@ -75,6 +75,12 @@ pub struct TextDraft { } impl TextDraft { + /// Take the shared text box for this box ([`crate::edit_text_ui::forget_typing_before`]): its + /// own history, or none once the page has its text. + pub(crate) fn claim_typing(&mut self, ctx: &egui::Context, doc: pdfcraft_engine::DocId) { + crate::edit_text_ui::forget_typing_before(ctx, input_id(), doc, &mut self.typing_from, &self.style.text); + } + /// Typing the page doesn't have yet: Undo takes it back in the box before touching the document. pub(crate) fn has_unapplied_typing(&self) -> bool { self.text != self.style.text @@ -604,7 +610,7 @@ pub(crate) fn editor(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, ad ui.fonts_mut(|f| f.layout_job(job)) }; let input = input_id(); - crate::edit_text_ui::forget_typing_before(ui.ctx(), input, doc_id, &mut t.typing_from, &t.style.text); + t.claim_typing(ui.ctx(), doc_id); ui.horizontal_top(|ui| { ui.spacing_mut().item_spacing.x = 4.0; let mut edit = egui::TextEdit::multiline(&mut t.text) diff --git a/crates/ui-egui/src/edit_text_ui.rs b/crates/ui-egui/src/edit_text_ui.rs index 4724cbb6e5..a0f73eaf43 100644 --- a/crates/ui-egui/src/edit_text_ui.rs +++ b/crates/ui-egui/src/edit_text_ui.rs @@ -277,7 +277,9 @@ pub(crate) fn input_id() -> egui::Id { /// own record of that; which document `doc` last used the history is kept beside it, since a box /// in another tab on the same text would otherwise take that document's typing with its Redo. /// When another document's box takes the text box, the box it leaves keeps its history (and -/// caret) aside, and gets it back on its return, so its typing can still be undone there. +/// caret) aside, and gets it back on its return, so its typing can still be undone there. Each +/// box claims the text box when it is drawn, and Undo and Redo from the menu claim it first too, +/// since they can run before the box is drawn again after a tab switch. pub(crate) fn forget_typing_before(ctx: &egui::Context, id: egui::Id, doc: pdfcraft_engine::DocId, from: &mut Option, text: &str) { let owner = id.with("typing-doc"); let last = ctx.data(|d| d.get_temp::(owner)); @@ -297,11 +299,9 @@ pub(crate) fn forget_typing_before(ctx: &egui::Context, id: egui::Id, doc: pdfcr } // Only the same box, still on the text it opened on, takes its history back. let back = ctx.data_mut(|d| d.remove_temp::(aside(doc))).filter(|_| from.as_deref() == Some(text)); - let state = back.unwrap_or_else(|| { - let mut state = egui::TextEdit::load_state(ctx, id).unwrap_or_default(); - state.clear_undoer(); - state - }); + // A box new to the text box starts with nothing of the last one's: no history, caret or + // selection. + let state = back.unwrap_or_default(); egui::TextEdit::store_state(ctx, id, state); ctx.data_mut(|d| d.insert_temp(owner, doc)); *from = Some(text.to_owned()); @@ -421,9 +421,18 @@ impl LineEditor { self.look0 = self.look.clone(); self.extras0 = self.extras; self.original = self.text.clone(); + // The page now has this text: the box's history restarts, even when the format left the + // text as it was, so a Redo from before can't put undone typing back on top of it. + self.typing_from = None; self.focus = true; } + /// Take the shared text box for this paragraph ([`forget_typing_before`]): its own history, + /// or none once the page has its text. + pub(crate) fn claim_typing(&mut self, ctx: &egui::Context, doc: pdfcraft_engine::DocId) { + forget_typing_before(ctx, input_id(), doc, &mut self.typing_from, &self.original); + } + /// Adopt the rewritten paragraph's current geometry before the next overlay frame. pub(crate) fn refresh_source(&mut self, block: &pdfcraft_engine::TextBlock, lines: Vec) { self.source_rect = block.rect.map(|v| v as f32); @@ -1724,7 +1733,7 @@ pub(crate) fn overlay(ctx: &egui::Context, view: &mut DocView, info: &DocInfo) - if snapshot.changes_with(text.as_str()) { galley } else { unseen(galley) } }; let input = input_id(); - forget_typing_before(ui.ctx(), input, view_id, &mut ed.typing_from, &ed.original); + ed.claim_typing(ui.ctx(), view_id); let output = egui::TextEdit::multiline(&mut ed.text) .id(input) .font(font.clone()) diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 408474eaa9..a7ee6b3d54 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -231,12 +231,15 @@ impl PdfCraftApp { fn undo_in_page_box(&mut self) -> bool { let Some(ctx) = self.ctx.clone() else { return false }; let Some(view) = self.active.and_then(|i| self.views.get_mut(i)) else { return false }; + let doc = view.id; if let Some(ed) = view.line_editor.as_mut().filter(|ed| ed.has_unsaved_text()) { + ed.claim_typing(&ctx, doc); let Some(text) = typing_step(&ctx, crate::edit_text_ui::input_id(), &ed.text, true) else { return false }; ed.text = text; return true; } if let Some(d) = view.content.draft.as_mut().filter(|d| d.has_unapplied_typing()) { + d.claim_typing(&ctx, doc); let Some(text) = typing_step(&ctx, crate::content_ui::input_id(), &d.text, true) else { return false }; d.text = text; return true; @@ -259,12 +262,15 @@ impl PdfCraftApp { fn redo_in_page_box(&mut self) -> bool { let Some(ctx) = self.ctx.clone() else { return false }; let Some(view) = self.active.and_then(|i| self.views.get_mut(i)) else { return false }; + let doc = view.id; if let Some(ed) = view.line_editor.as_mut() { + ed.claim_typing(&ctx, doc); let Some(text) = typing_step(&ctx, crate::edit_text_ui::input_id(), &ed.text, false) else { return ed.has_unsaved_text() }; ed.text = text; return true; } if let Some(d) = view.content.draft.as_mut() { + d.claim_typing(&ctx, doc); let Some(text) = typing_step(&ctx, crate::content_ui::input_id(), &d.text, false) else { return d.has_unapplied_typing() }; d.text = text; return true; diff --git a/crates/ui-egui/tests/it/content.rs b/crates/ui-egui/tests/it/content.rs index 43771ed724..387cc1aed1 100644 --- a/crates/ui-egui/tests/it/content.rs +++ b/crates/ui-egui/tests/it/content.rs @@ -1058,6 +1058,42 @@ fn undo_after_a_round_trip_through_another_tab_takes_back_added_text_typing() { assert_eq!(draft(&h).as_deref(), Some("Reviewed"), "Undo takes the typing back in A's box"); } +/// Edit ▸ Undo chosen in the same frame as the switch back to tab A's added text box, before it +/// is drawn again: the shared text box still holds tab B's history, from a box on other text, so +/// A's box claims its own back before stepping. +#[test] +fn undo_in_the_frame_of_the_switch_back_takes_back_added_text_typing() { + let mut h = harness(); + h.state_mut().open_bytes("copy.pdf", None, include_bytes!("../data/form.pdf").to_vec()).unwrap(); + h.run_steps(4); + let rect = [40.0, 195.0, 240.0, 210.0]; + for (tab, text) in [(0, "Reviewed"), (1, "Approved")] { + h.state_mut().active = Some(tab); + let text = pdfcraft_engine::AddedText { rect, text: text.into(), ..Default::default() }; + assert!(h.state_mut().apply_edit(pdfcraft_engine::Edit::AddText { page: 0, text })); + } + let open_in = |h: &mut Harness<'static, PdfCraftApp>, tab: usize| { + h.state_mut().active = Some(tab); + h.run_steps(2); + assert!(h.state_mut().execute("edit.text")); + h.run_steps(3); + let r = h.state().views[tab].page_screen_rect(0).expect("on screen"); + let k = r.width() / 300.0; + click(h, r.min + egui::vec2(45.0 * k, 195.0 * k)); + assert_eq!(h.state().views[tab].content.draft.as_ref().and_then(|d| d.index), Some(0), "the editor opened on the text"); + }; + open_in(&mut h, 0); + h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::End); + h.event(egui::Event::Text(" twice".into())); + h.run_steps(2); + open_in(&mut h, 1); + // Back to A and Undo at once: no frame draws A's box in between. + h.state_mut().active = Some(0); + assert!(h.state_mut().execute("edit.undo")); + h.run_steps(3); + assert_eq!(h.state().views[0].content.draft.as_ref().map(|d| d.text.as_str()), Some("Reviewed"), "Undo takes the typing back in A's box"); +} + /// Redo with an added text box open and unchanged reaches the document's waiting Redo, even when /// the box's text ends in a space: closing the unchanged box writes nothing, so it can't clear /// that Redo first. diff --git a/crates/ui-egui/tests/it/editing.rs b/crates/ui-egui/tests/it/editing.rs index c1acc43fe7..da2a703f55 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/crates/ui-egui/tests/it/editing.rs @@ -6688,6 +6688,116 @@ fn undo_after_a_round_trip_through_another_tab_takes_back_the_typing() { assert_eq!(s.session.get(s.views[0].id).unwrap().can_redo(), Some("Edit text"), "the document's Redo still waits"); } +/// Typing taken back with Ctrl+Z, then Bold applied with the paragraph still open: Ctrl+Shift+Z +/// must not bring the typing back on top of the bold, as the box's history is cleared once the +/// page has its text. The format left the text as it was, so the box kept its history and its +/// Redo put "Page 1 more" back (FredApps's review of abc378c0). +#[test] +fn redo_after_formatting_never_brings_back_typing_undone_before_it() { + let mut h = harness(1, |_| {}); + assert!(h.state_mut().execute("edit.edit_text")); + h.run_steps(2); + open_paragraph(&mut h); + let original = h.state().views[0].line_editor.as_ref().unwrap().text.clone(); + h.key_press_modifiers(Modifiers::COMMAND, Key::End); + h.event(egui::Event::Text(" more".into())); + h.run_steps(2); + h.key_press_modifiers(Modifiers::COMMAND, Key::Z); + h.run_steps(3); + assert_eq!(h.state().views[0].line_editor.as_ref().unwrap().text, original, "the typing is taken back"); + h.get_by_label("Bold").click(); + h.run_steps(4); + assert_eq!(h.state().session.get(h.state().views[0].id).unwrap().can_undo(), Some("Edit text"), "the bold is applied"); + h.key_press_modifiers(Modifiers::COMMAND | Modifiers::SHIFT, Key::Z); + h.run_steps(3); + let text = h.state().views[0].line_editor.as_ref().map(|ed| ed.text.clone()); + assert_eq!(text.as_deref(), Some(original.as_str()), "the typing undone before the bold stays undone"); + // Nor through what the menu's Redo runs. + h.state_mut().redo(); + h.run_steps(3); + let s = h.state(); + let text = s.views[0].line_editor.as_ref().map(|ed| ed.text.clone()); + assert!(!text.as_deref().unwrap_or_default().contains("more"), "the menu's Redo brings it back: {text:?}"); + assert_eq!(s.session.get(s.views[0].id).unwrap().text_blocks(0)[0].base_font, "Helvetica-Bold", "the bold stays"); +} + +/// Edit ▸ Undo chosen in the same frame as the switch back to tab A, before A's box is drawn +/// again: the shared text box still holds tab B's history then. Stepping through it put B's +/// typing into A's box; the box now claims its own history back first (CodeRabbit on abc378c0). +#[test] +fn undo_in_the_frame_of_the_switch_back_takes_back_the_typing() { + let mut h = harness(1, |app| { + app.open_bytes("copy.pdf", None, fixture(1)).expect("the copy opens"); + }); + let open_in = |h: &mut Harness<'static, PdfCraftApp>, tab: usize| { + h.state_mut().active = Some(tab); + h.run_steps(2); + if h.state().views[tab].line_editor.is_none() { + assert!(h.state_mut().execute("edit.edit_text")); + h.run_steps(2); + } + let r = h.state().views[tab].page_screen_rect(0).expect("on screen"); + click_at(h, egui::pos2(r.left() + 40.0 / 200.0 * r.width(), r.top() + (300.0 - 158.0) / 300.0 * r.height())); + assert!(h.state().views[tab].line_editor.is_some(), "the paragraph opens for typing"); + }; + open_in(&mut h, 0); + let original = h.state().views[0].line_editor.as_ref().unwrap().text.clone(); + h.key_press_modifiers(Modifiers::COMMAND, Key::End); + h.event(egui::Event::Text(" more".into())); + h.run_steps(2); + // In B, a paragraph whose text differs from A's ("Page 1 B", applied with Bold), so B's + // history can't pass for A's. + open_in(&mut h, 1); + h.key_press_modifiers(Modifiers::COMMAND, Key::End); + h.event(egui::Event::Text(" B".into())); + h.run_steps(2); + h.get_by_label("Bold").click(); + h.run_steps(4); + assert_eq!(h.state().views[1].line_editor.as_ref().map(|ed| ed.text.as_str()), Some("Page 1 B"), "B's paragraph is open on its own text"); + // Back to A and Undo at once: no frame draws A's box in between. + h.state_mut().active = Some(0); + assert!(h.state_mut().execute("edit.undo"), "Undo is enabled"); + h.run_steps(3); + let ed = h.state().views[0].line_editor.clone().expect("the box stays open: Undo takes back the typing"); + assert_eq!(ed.text, original, "the typing is taken back in A's box"); +} + +/// Redo from the menu chosen in the same frame as a switch to tab B, whose paragraph is open on +/// the same text as A's: the shared text box still holds A's history then, with A's undone +/// typing to redo, so B's box claims its own history back before stepping. +#[test] +fn redo_in_the_frame_of_a_tab_switch_never_brings_another_tabs_typing() { + let mut h = harness(1, |app| { + app.open_bytes("copy.pdf", None, fixture(1)).expect("the copy opens"); + }); + let open_in = |h: &mut Harness<'static, PdfCraftApp>, tab: usize| { + h.state_mut().active = Some(tab); + h.run_steps(2); + assert!(h.state_mut().execute("edit.edit_text")); + h.run_steps(2); + let r = h.state().views[tab].page_screen_rect(0).expect("on screen"); + click_at(h, egui::pos2(r.left() + 40.0 / 200.0 * r.width(), r.top() + (300.0 - 158.0) / 300.0 * r.height())); + assert!(h.state().views[tab].line_editor.is_some(), "the paragraph opens for typing"); + }; + open_in(&mut h, 1); + open_in(&mut h, 0); + let original = h.state().views[0].line_editor.as_ref().unwrap().text.clone(); + h.key_press_modifiers(Modifiers::COMMAND, Key::End); + h.event(egui::Event::Text(" secret".into())); + h.run_steps(2); + assert!(h.state_mut().execute("edit.undo")); + h.run_steps(3); + assert_eq!(h.state().views[0].line_editor.as_ref().unwrap().text, original, "taken back in A"); + // To B and Redo at once: no frame draws B's box in between. + h.state_mut().active = Some(1); + h.state_mut().redo(); + h.run_steps(3); + let s = h.state(); + let b = s.views[1].line_editor.as_ref().map(|ed| ed.text.clone()); + assert!(!b.as_deref().unwrap_or_default().contains("secret"), "A's typing came back in B: {b:?}"); + assert!(!s.session.get(s.views[1].id).unwrap().text_blocks(0)[0].text.contains("secret"), "A's typing reached B's page"); +} + /// Two copies of a document, the same paragraph open in each. The paragraph boxes share one text /// box, and so one Undo history: a box opened on the same text as another tab's didn't clear it. /// Typing in one tab, taken back with Undo, then came back with Redo in the other tab.