From b9035271b0a8c773b8caede56ee435a574ff0997 Mon Sep 17 00:00:00 2001 From: RiceDee <345114586+RiceDee@users.noreply.github.com> Date: Sun, 25 Oct 2026 08:36:17 +1100 Subject: [PATCH] fix(ui): menu Undo takes back typing in the open text box first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Edit ▸ Undo with typing in a paragraph or added text box that the page doesn't have yet applied the typing (a new edit, which dropped the document's waiting Redo), closed the box and undid the typing all at once; the key takes it back a step at a time in the box. Menu Undo now goes to the box first, as Redo does, and is enabled while the box holds such typing. Undo and Redo share typing_step for the box's own history. --- crates/ui-egui/src/commands.rs | 4 +++- crates/ui-egui/src/editing.rs | 37 +++++++++++++++++++++++------- crates/ui-egui/tests/it/content.rs | 26 +++++++++++++++++++++ crates/ui-egui/tests/it/editing.rs | 29 +++++++++++++++++++++++ 4 files changed, 87 insertions(+), 9 deletions(-) diff --git a/crates/ui-egui/src/commands.rs b/crates/ui-egui/src/commands.rs index 25cb73beba..d1ef7a4505 100644 --- a/crates/ui-egui/src/commands.rs +++ b/crates/ui-egui/src/commands.rs @@ -84,8 +84,10 @@ impl PdfCraftApp { if self.combine_showing() && matches!(spec.needs, commands::Needs::Undo | commands::Needs::Redo) { return self.combine_can_undo(spec.needs == commands::Needs::Undo); } - // Redo also takes back what Undo took in a box open on the page, with no document step. + // Undo also takes back typing in a box open on the page, and Redo what Undo took there, + // with no document step. (commands::is_enabled(spec, &self.session, self.active_ids().map(|(_, id)| id)) + || spec.needs == commands::Needs::Undo && self.page_typing_unapplied() || spec.needs == commands::Needs::Redo && self.open_box_can_redo()) && (spec.id != "edit.copy" || self diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 207a3f4830..d495a52a46 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -214,11 +214,31 @@ impl PdfCraftApp { } if self.combine_showing() { self.combine_history_step(true); - } else { + } else if !self.undo_in_page_box() { self.history_step(true); } } + /// Undo from the menu or toolbar while a box on the page holds typing the page doesn't have + /// yet: the box takes the step, as it does for the key, one step of its typing at a time. + /// Applying the typing first was a new edit, which dropped the document's Redo, and then the + /// typing was taken back all at once. True when the box took the step. + 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 }; + if let Some(ed) = view.line_editor.as_mut().filter(|ed| ed.has_unsaved_text()) { + 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()) { + let Some(text) = typing_step(&ctx, crate::content_ui::input_id(), &d.text, true) else { return false }; + d.text = text; + return true; + } + false + } + pub fn redo(&mut self) { if self.combine_showing() { self.combine_history_step(false); @@ -235,12 +255,12 @@ impl PdfCraftApp { 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 }; if let Some(ed) = view.line_editor.as_mut() { - let Some(text) = redo_typing(&ctx, crate::edit_text_ui::input_id(), &ed.text) else { return ed.has_unsaved_text() }; + 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() { - let Some(text) = redo_typing(&ctx, crate::content_ui::input_id(), &d.text) else { return d.has_unapplied_typing() }; + 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; } @@ -1347,16 +1367,17 @@ fn has_typing_to_redo(ctx: &egui::Context, id: egui::Id, text: &str) -> bool { .is_some_and(|state| state.cursor.char_range().is_some_and(|range| state.undoer().has_redo(&(range, text.to_owned())))) } -/// The box's own Redo, as the key does it: the typing Undo took back, put back in the box's text, -/// with the box's history and cursor moved along. None when the box has nothing to redo. -fn redo_typing(ctx: &egui::Context, id: egui::Id, text: &str) -> Option { +/// The box's own Undo (or Redo), as the key does it: the box's text one step back (or forward) in +/// its typing, with the box's history and cursor moved along. None when there is no such step. +fn typing_step(ctx: &egui::Context, id: egui::Id, text: &str, undo: bool) -> Option { let mut state = egui::TextEdit::load_state(ctx, id)?; let mut undoer = state.undoer(); - let (range, redone) = undoer.redo(&(state.cursor.char_range()?, text.to_owned()))?.clone(); + let current = (state.cursor.char_range()?, text.to_owned()); + let (range, stepped) = if undo { undoer.undo(¤t) } else { undoer.redo(¤t) }?.clone(); state.set_undoer(undoer); state.cursor.set_char_range(Some(range)); egui::TextEdit::store_state(ctx, id, state); - Some(redone) + Some(stepped) } #[cfg(test)] diff --git a/crates/ui-egui/tests/it/content.rs b/crates/ui-egui/tests/it/content.rs index c2edfb8336..a17d0793ea 100644 --- a/crates/ui-egui/tests/it/content.rs +++ b/crates/ui-egui/tests/it/content.rs @@ -971,6 +971,32 @@ fn redo_from_the_menu_with_retyping_in_the_box_keeps_the_documents_redo() { assert_eq!(s.session.get(s.views[0].id).unwrap().can_redo(), Some("Edit content"), "the document's Redo still waits"); } +/// Undo from the menu with retyping the page doesn't have yet in an added text box: the box takes +/// it back and stays open, and the document's waiting Redo stays. +#[test] +fn undo_from_the_menu_takes_back_retyping_in_the_added_text_box() { + let mut h = harness(); + let rect = [40.0, 195.0, 240.0, 210.0]; + let text = pdfcraft_engine::AddedText { rect, text: "Reviewed".into(), ..Default::default() }; + assert!(h.state_mut().apply_edit(pdfcraft_engine::Edit::AddText { page: 0, text: text.clone() })); + let bold = pdfcraft_engine::AddedText { bold: true, ..text }; + assert!(h.state_mut().apply_edit(pdfcraft_engine::Edit::UpdateContent { page: 0, index: 0, content: pdfcraft_engine::AddedContent::Text(bold) })); + assert!(h.state_mut().execute("edit.undo")); + assert!(h.state_mut().execute("edit.text")); + h.run_steps(3); + let point = at(&h, 45.0, 195.0); + click(&mut h, point); + assert_eq!(h.state().views[0].content.draft.as_ref().and_then(|d| d.index), Some(0), "the editor opened on the text"); + h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::End); + h.event(egui::Event::Text(" twice".into())); + h.run_steps(2); + 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 it back in the box"); + let s = h.state(); + assert_eq!(s.session.get(s.views[0].id).unwrap().can_redo(), Some("Edit content"), "the document's Redo still waits"); +} + /// Redo from the menu after Undo in an added text box, with nothing for the document to redo: the /// menu item follows the box's own Redo too, and puts the typing back. #[test] diff --git a/crates/ui-egui/tests/it/editing.rs b/crates/ui-egui/tests/it/editing.rs index 8ce8073285..841633005e 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/crates/ui-egui/tests/it/editing.rs @@ -6383,6 +6383,35 @@ fn redo_from_the_menu_with_typing_in_the_box_keeps_the_documents_redo() { assert_eq!(s.session.get(s.views[0].id).unwrap().can_redo(), Some("Edit text"), "the document's Redo still waits"); } +/// Undo from the menu or toolbar with typing in the box the page doesn't have yet: the box takes +/// the typing back, as the key does, and stays open, and the document's waiting Redo stays. The +/// menu applied the typing first (a new edit, which dropped that Redo), closed the box and undid +/// the typing all at once. +#[test] +fn undo_from_the_menu_takes_back_typing_in_the_box() { + let mut h = harness(1, |_| {}); + assert!(h.state_mut().execute("edit.edit_text")); + h.run_steps(2); + open_paragraph(&mut h); + h.get_by_label("Bold").click(); + h.run_steps(4); + // Undo the bold (closing the editor), so the document has it to redo and nothing to undo. + h.key_press_modifiers(Modifiers::COMMAND, Key::Z); + h.run_steps(4); + 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); + assert!(h.state_mut().execute("edit.undo"), "Undo is enabled for the box's typing"); + h.run_steps(3); + let s = h.state(); + let ed = s.views[0].line_editor.as_ref().expect("the box stays open"); + assert_eq!(ed.text, original, "the typing is taken back in the box"); + let doc = s.session.get(s.views[0].id).unwrap(); + assert_eq!((doc.can_redo(), doc.can_undo()), (Some("Edit text"), None), "the document is untouched"); +} + /// Redo from the menu after Undo in the box took the typing back, with nothing for the document /// to redo: the menu item follows the box's own Redo too, and puts the typing back. It was /// disabled, as it followed only the document's Redo, and only the key could do it.