From 2842eeaa713889b2fb306e72990050549ee1a5dd Mon Sep 17 00:00:00 2001 From: RiceDee <345114586+RiceDee@users.noreply.github.com> Date: Fri, 23 Oct 2026 13:38:28 +1100 Subject: [PATCH 1/7] fix(ui): Ctrl+Z in an open paragraph undoes the document, not stale text Ctrl+Z is left to a focused text field, so in the paragraph editor it went to the box's own Undo history. That history still held the paragraph's text from before the last formatting change, so Ctrl+Z put it back into a box laid out for the page after the change, a layout the page never had, and the document kept the change. The box shared its id across paragraphs too, so it could put back another paragraph's text. The history of the paragraph editor and the added-text box now starts at what the page has. Undo and Redo keep the keys only while the box holds typing the page doesn't have; then they reach the document. An open box is applied (as clicking away applies it) and closed before an Undo or Redo step, so it never shows a state the document has left; text it can't apply keeps it open and stops the step. --- crates/ui-egui/src/commands.rs | 5 ++- crates/ui-egui/src/content_ui.rs | 23 +++++++++++-- crates/ui-egui/src/edit_text_ui.rs | 29 +++++++++++++++- crates/ui-egui/src/editing.rs | 54 +++++++++++++++++++++++++++++ crates/ui-egui/tests/it/editing.rs | 55 ++++++++++++++++++++++++++++++ 5 files changed, 161 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/commands.rs b/crates/ui-egui/src/commands.rs index 92bc0b360e..ee19c168eb 100644 --- a/crates/ui-egui/src/commands.rs +++ b/crates/ui-egui/src/commands.rs @@ -726,7 +726,10 @@ impl PdfCraftApp { if matches!(spec.id, "view.previous_view" | "view.next_view") && !self.command_enabled(spec) { continue; } - if typing && !spec.in_text { + // A box typed into on the page (a paragraph being edited, added text) keeps Undo and + // Redo only while it holds typing the page doesn't have; then they reach the document. + let page_box = matches!(spec.id, "edit.undo" | "edit.redo") && self.page_box_focused(ctx) && !self.page_typing_unapplied(); + if typing && !spec.in_text && !page_box { continue; } let Some(key) = Key::from_name(s.key) else { continue }; diff --git a/crates/ui-egui/src/content_ui.rs b/crates/ui-egui/src/content_ui.rs index 674210529e..892e94ed67 100644 --- a/crates/ui-egui/src/content_ui.rs +++ b/crates/ui-egui/src/content_ui.rs @@ -66,8 +66,18 @@ pub struct TextDraft { /// Display space. pub rect: [f64; 4], pub text: String, + /// The box's formatting; its `text` is what the page has (empty for new text). pub style: AddedText, pub focus: bool, + /// The text the box's own Undo history starts from ([`crate::edit_text_ui::forget_typing_before`]). + pub typing_from: Option, +} + +impl TextDraft { + /// 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 + } } #[derive(Clone, Copy, Debug, PartialEq)] @@ -394,7 +404,7 @@ pub(crate) fn page_input( let page_width = info.pages.get(page).map_or(f64::INFINITY, |p| p.width as f64); let rect = [at[0], at[1] - style.size * 1.2, (at[0] + 200.0).min(page_width), at[1]]; style.rect = rect; - cv.draft = Some(TextDraft { page, index: None, rect, text: String::new(), style, focus: true }); + cv.draft = Some(TextDraft { page, index: None, rect, text: String::new(), style, focus: true, typing_from: None }); consumed = true; if typed.is_some() { view.pending_edit = typed; @@ -432,7 +442,7 @@ pub(crate) fn page_input( if let Some(edit) = cv.finish_draft(added) { view.pending_edit = Some(edit); } - cv.draft = Some(TextDraft { page, index: Some(i), rect: t.rect, text: t.text.clone(), style: t.clone(), focus: true }); + cv.draft = Some(TextDraft { page, index: Some(i), rect: t.rect, text: t.text.clone(), style: t.clone(), focus: true, typing_from: None }); cv.selected = Some((page, i)); consumed = true; return consumed; @@ -560,6 +570,8 @@ pub(crate) fn editor(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, ad // The box is white in every theme, so the caret must not follow the theme (pale in dark mode) (#655). ui.visuals_mut().text_cursor.stroke.color = crate::edit_text_ui::caret_color_on(Color32::WHITE); let [cr, cg, cb] = t.style.color.map(|v| (v.clamp(0.0, 1.0) * 255.0) as u8); + let input = input_id(); + crate::edit_text_ui::forget_typing_before(ui.ctx(), input, &mut t.typing_from, &t.style.text); ui.horizontal_top(|ui| { ui.spacing_mut().item_spacing.x = 4.0; let resp = ui.add( @@ -570,7 +582,7 @@ pub(crate) fn editor(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, ad .background_color(if background_ready { Color32::TRANSPARENT } else { Color32::WHITE }) .text_color(Color32::from_rgb(cr, cg, cb)) .hint_text(tl!("Type text")) - .id_salt("added-text-edit"), + .id(input), ); if t.focus { resp.request_focus(); @@ -633,6 +645,11 @@ fn finish(cv: &mut ContentView, d: TextDraft, added: &[Added]) -> Option { } } +/// The added-text box's text field. +pub(crate) fn input_id() -> egui::Id { + egui::Id::new("added-text-input") +} + /// How to use the tools, under the Add content list. pub(crate) fn hint(ui: &mut egui::Ui, t: &Tokens) { ui.label( diff --git a/crates/ui-egui/src/edit_text_ui.rs b/crates/ui-egui/src/edit_text_ui.rs index 3a1ab04367..f2c5c55151 100644 --- a/crates/ui-egui/src/edit_text_ui.rs +++ b/crates/ui-egui/src/edit_text_ui.rs @@ -256,6 +256,29 @@ pub struct LineEditor { /// horizontal scale (%), and what they were. pub extras: Extras, extras0: Extras, + /// The text the box's own Undo history starts from; see [`forget_typing_before`]. + typing_from: Option, +} + +/// The paragraph editor's text box. +pub(crate) fn input_id() -> egui::Id { + egui::Id::new("edit-text-line-input") +} + +/// The page text box `id` keeps its own Undo history, across every paragraph or box it has shown +/// (they share the id). Once the page has `text` (the box opened on it, or formatting applied it), +/// that history is cleared: Undo in the box takes back only what was typed since, and then reaches +/// the document. Kept, it put back text from another paragraph, or the whole paragraph as it was +/// before a format change, into a box showing the page as it is now. +pub(crate) fn forget_typing_before(ctx: &egui::Context, id: egui::Id, from: &mut Option, text: &str) { + if from.as_deref() == Some(text) { + return; + } + if let Some(mut state) = egui::TextEdit::load_state(ctx, id) { + state.clear_undoer(); + egui::TextEdit::store_state(ctx, id, state); + } + *from = Some(text.to_owned()); } #[derive(Clone, Copy, Debug, PartialEq)] @@ -1199,6 +1222,7 @@ fn open_editor( family_chosen: false, extras: Extras::default(), extras0: Extras::default(), + typing_from: None, }); } @@ -1668,8 +1692,10 @@ pub(crate) fn overlay(ctx: &egui::Context, view: &mut DocView, info: &DocInfo) - let galley = editor_galley(ui, &snapshot, text.as_str(), scale, wrap_width); if snapshot.changes_with(text.as_str()) { galley } else { unseen(galley) } }; + let input = input_id(); + forget_typing_before(ui.ctx(), input, &mut ed.typing_from, &ed.original); let output = egui::TextEdit::multiline(&mut ed.text) - .id(egui::Id::new("edit-text-line-input")) + .id(input) .font(font.clone()) .margin(egui::Margin::ZERO) .layouter(&mut layouter) @@ -1801,6 +1827,7 @@ mod tests { family_chosen: false, extras: Extras::default(), extras0: Extras::default(), + typing_from: None, }; (app, ed) } diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 5b821b6381..c10094b208 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -229,6 +229,9 @@ impl PdfCraftApp { fn history_step(&mut self, undo: bool) { let Some((i, id)) = self.active_ids() else { return }; + if !self.close_text_editors(i, id) { + return; + } let result = if undo { self.session.undo(id) } else { self.session.redo(id) }; match result { Ok(label) => { @@ -247,6 +250,57 @@ impl PdfCraftApp { } } + /// Before Undo or Redo: a paragraph or added text box open on the page shows the document as + /// it is before the step, so it is applied as clicking away applies it, then closed. Left + /// open, it kept showing what the step took away, and wrote from it when it was next applied. + /// What was typed is then the step Undo takes back. False (nothing is undone, and the box + /// stays open) while it holds text the standard fonts can't draw, or text that couldn't be + /// applied: it is shown, never lost. + pub(crate) fn close_text_editors(&mut self, i: usize, id: pdfcraft_engine::DocId) -> bool { + let Some(view) = self.views.get_mut(i) else { return true }; + if let Some(c) = view.content.blocked() { + view.content.hold_blocked(); + self.notify(crate::content_ui::undrawable_message(c)); + return false; + } + if let Some(ed) = view.line_editor.take() + && let Some(edit) = crate::edit_text_ui::finished_edit(ed.clone()) + && !self.apply_edit(edit) + { + if let Some(view) = self.views.get_mut(i) { + view.line_editor = Some(ed); + } + return false; + } + let added = self.session.get(id).map(|d| d.added().to_vec()).unwrap_or_default(); + let Some(view) = self.views.get_mut(i) else { return true }; + let draft = view.content.draft.clone(); + if let Some(edit) = view.content.finish_draft(&added) + && !self.apply_edit(edit) + { + if let Some(view) = self.views.get_mut(i) { + view.content.draft = draft; + } + return false; + } + true + } + + /// Whether the keyboard is in a box typed into on the page: a paragraph being edited, or added + /// text. + pub(crate) fn page_box_focused(&self, ctx: &egui::Context) -> bool { + let focused = ctx.memory(|m| m.focused()); + focused == Some(crate::edit_text_ui::input_id()) || focused == Some(crate::content_ui::input_id()) + } + + /// Typing in a box on the page (a paragraph being edited, added text) that the page doesn't + /// have yet: Undo and Redo keys take it back in the box first. + pub(crate) fn page_typing_unapplied(&self) -> bool { + self.active.and_then(|i| self.views.get(i)).is_some_and(|v| { + v.line_editor.as_ref().is_some_and(|ed| ed.has_unsaved_text()) || v.content.draft.as_ref().is_some_and(|d| d.has_unapplied_typing()) + }) + } + /// Apply any edit a view queued this frame (organize toolbar, keys). /// Returns false when a field refused the typing queued for it (its editor is open again). pub(crate) fn process_pending_edits(&mut self) -> bool { diff --git a/crates/ui-egui/tests/it/editing.rs b/crates/ui-egui/tests/it/editing.rs index 71ba9ccdac..f9228c1f05 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/crates/ui-egui/tests/it/editing.rs @@ -5980,3 +5980,58 @@ fn mode_tabs_keep_the_hand_over_a_paragraph_scrolled_under_them() { click_at(&mut h, convert); assert_eq!(h.state().mode, pdfcraft_ui_egui::Mode::Convert, "the tab takes the click, not the paragraph's editor"); } + +/// The paragraph opened for typing, as a person opens it: a click on its text. +fn open_paragraph(h: &mut Harness<'static, PdfCraftApp>) { + let r = h.state().views[0].page_screen_rect(0).expect("on screen"); + let at = egui::pos2(r.left() + 40.0 / 200.0 * r.width(), r.top() + (300.0 - 158.0) / 300.0 * r.height()); + click_at(h, at); + assert!(h.state().views[0].line_editor.is_some(), "the paragraph opens for typing"); +} + +/// Ctrl+Z after a formatting change, with the paragraph still open: the change is undone and +/// the editor closes. The box's own Undo used to take the key, putting back the text it had +/// before the change into a box laid out for the page after it, and the document kept the change. +#[test] +fn undo_with_the_paragraph_open_undoes_the_formatting() { + 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); + { + let s = h.state(); + let doc = s.session.get(s.views[0].id).unwrap(); + assert_eq!(doc.can_undo(), Some("Edit text")); + assert_eq!(doc.text_blocks(0)[0].base_font, "Helvetica-Bold"); + assert!(s.views[0].line_editor.is_some(), "still editing"); + } + h.key_press_modifiers(Modifiers::COMMAND, Key::Z); + h.run_steps(4); + let s = h.state(); + let doc = s.session.get(s.views[0].id).unwrap(); + assert_eq!(doc.can_undo(), None, "the formatting is undone"); + assert_eq!(doc.text_blocks(0)[0].base_font, "Helvetica"); + assert!(s.views[0].line_editor.is_none(), "the editor closed with the step it showed"); +} + +/// Undo while typing takes back the typing in the box first; the document is left alone. +#[test] +fn undo_while_typing_takes_back_the_typing_first() { + 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); + assert_ne!(h.state().views[0].line_editor.as_ref().unwrap().text, original); + h.key_press_modifiers(Modifiers::COMMAND, Key::Z); + h.run_steps(3); + let s = h.state(); + let ed = s.views[0].line_editor.as_ref().expect("still typing"); + assert_eq!(ed.text, original, "the typing is taken back"); + assert_eq!(s.session.get(s.views[0].id).unwrap().can_undo(), None, "the document is untouched"); +} From cac63cf3dbd3449274fd8ad9689cf7778c8c5ef9 Mon Sep 17 00:00:00 2001 From: RiceDee <345114586+RiceDee@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:05:23 +1100 Subject: [PATCH 2/7] fix(ui): Redo after Undo in a page text box puts the typing back With typing undone in a paragraph or added text box, its text matches the page again, so Undo and Redo went to the document. Redo then closed the box and redid a document change (or nothing), instead of putting the typing back as the box's own Redo did before. And Shift+Cmd+Z is also Cmd+Z to the Undo shortcut, so even a Redo the box kept was taken as a document Undo. Redo stays with the box while its own history has typing to put back (the test egui makes when it handles the key), and Shift+Cmd+Z is never a document Undo. --- crates/ui-egui/src/commands.rs | 10 ++++++++-- crates/ui-egui/src/editing.rs | 21 ++++++++++++++++++++ crates/ui-egui/tests/it/content.rs | 30 +++++++++++++++++++++++++++++ crates/ui-egui/tests/it/editing.rs | 31 ++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/commands.rs b/crates/ui-egui/src/commands.rs index ee19c168eb..fb4d6f1b6f 100644 --- a/crates/ui-egui/src/commands.rs +++ b/crates/ui-egui/src/commands.rs @@ -727,8 +727,14 @@ impl PdfCraftApp { continue; } // A box typed into on the page (a paragraph being edited, added text) keeps Undo and - // Redo only while it holds typing the page doesn't have; then they reach the document. - let page_box = matches!(spec.id, "edit.undo" | "edit.redo") && self.page_box_focused(ctx) && !self.page_typing_unapplied(); + // Redo only while it holds typing the page doesn't have, or Redo has typing to put + // back; then they reach the document. ⌘⇧Z is also ⌘Z to the Undo shortcut, so a + // Redo the box keeps is never taken as Undo. + let page_box = matches!(spec.id, "edit.undo" | "edit.redo") + && self.page_box_focused(ctx) + && !self.page_typing_unapplied() + && !(spec.id == "edit.redo" && self.page_box_can_redo(ctx)) + && !(spec.id == "edit.undo" && ctx.input(|i| i.modifiers.shift)); if typing && !spec.in_text && !page_box { continue; } diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index c10094b208..39e0fd1adf 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -293,6 +293,27 @@ impl PdfCraftApp { focused == Some(crate::edit_text_ui::input_id()) || focused == Some(crate::content_ui::input_id()) } + /// Whether the focused box on the page has typing its own Redo would put back: after Undo in + /// the box restores what the page has, Redo still belongs to the box, not the document. The + /// same test egui makes when it handles the key. + pub(crate) fn page_box_can_redo(&self, ctx: &egui::Context) -> bool { + let Some(view) = self.active.and_then(|i| self.views.get(i)) else { return false }; + let focused = ctx.memory(|m| m.focused()); + let (id, text) = if focused == Some(crate::edit_text_ui::input_id()) + && let Some(ed) = &view.line_editor + { + (crate::edit_text_ui::input_id(), &ed.text) + } else if focused == Some(crate::content_ui::input_id()) + && let Some(d) = &view.content.draft + { + (crate::content_ui::input_id(), &d.text) + } else { + return false; + }; + egui::TextEdit::load_state(ctx, id) + .is_some_and(|state| state.cursor.char_range().is_some_and(|range| state.undoer().has_redo(&(range, text.clone())))) + } + /// Typing in a box on the page (a paragraph being edited, added text) that the page doesn't /// have yet: Undo and Redo keys take it back in the box first. pub(crate) fn page_typing_unapplied(&self) -> bool { diff --git a/crates/ui-egui/tests/it/content.rs b/crates/ui-egui/tests/it/content.rs index 515e3e04f1..fa0c01367a 100644 --- a/crates/ui-egui/tests/it/content.rs +++ b/crates/ui-egui/tests/it/content.rs @@ -861,3 +861,33 @@ fn a_resized_added_picture_shows_only_at_its_new_size() { let pixels = h.render().unwrap(); assert!(red(&pixels, kept) > 100 && red(&pixels, gone) == 0, "the page raster has the picture at its new size"); } + +/// The same in an added text box: Redo after Undo puts the typing back in the box, and the +/// document's own Redo stays waiting. +#[test] +fn redo_after_undoing_typing_in_added_text_puts_it_back() { + 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); + h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::Z); + h.run_steps(3); + assert_eq!(h.state().views[0].content.draft.as_ref().map(|d| d.text.as_str()), Some("Reviewed")); + h.key_press_modifiers(egui::Modifiers::COMMAND | egui::Modifiers::SHIFT, egui::Key::Z); + h.run_steps(3); + assert_eq!(h.state().views[0].content.draft.as_ref().map(|d| d.text.as_str()), Some("Reviewed twice"), "Redo puts the typing back"); + let s = h.state(); + let doc = s.session.get(s.views[0].id).unwrap(); + assert_eq!(doc.can_redo(), Some("Edit content"), "the document's Redo still waits"); +} diff --git a/crates/ui-egui/tests/it/editing.rs b/crates/ui-egui/tests/it/editing.rs index f9228c1f05..49e882893d 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/crates/ui-egui/tests/it/editing.rs @@ -6035,3 +6035,34 @@ fn undo_while_typing_takes_back_the_typing_first() { assert_eq!(ed.text, original, "the typing is taken back"); assert_eq!(s.session.get(s.views[0].id).unwrap().can_undo(), None, "the document is untouched"); } + +/// Redo after Undo in the box puts the typing back, in the box: it doesn't reach the document, +/// whose own Redo (a formatting change waiting here) must stay waiting. +#[test] +fn redo_after_undoing_typing_puts_the_typing_back() { + 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. + h.key_press_modifiers(Modifiers::COMMAND, Key::Z); + h.run_steps(4); + assert_eq!(h.state().session.get(h.state().views[0].id).unwrap().can_redo(), Some("Edit text")); + open_paragraph(&mut h); + h.key_press_modifiers(Modifiers::COMMAND, Key::End); + h.event(egui::Event::Text(" more".into())); + h.run_steps(2); + let typed = h.state().views[0].line_editor.as_ref().unwrap().text.clone(); + h.key_press_modifiers(Modifiers::COMMAND, Key::Z); + h.run_steps(3); + assert_ne!(h.state().views[0].line_editor.as_ref().unwrap().text, typed, "the typing is taken back"); + h.key_press_modifiers(Modifiers::COMMAND | Modifiers::SHIFT, Key::Z); + h.run_steps(3); + let s = h.state(); + let ed = s.views[0].line_editor.as_ref().expect("still typing"); + assert_eq!(ed.text, typed, "Redo puts the typing back"); + let doc = s.session.get(s.views[0].id).unwrap(); + assert_eq!((doc.can_redo(), doc.text_blocks(0)[0].base_font.as_str()), (Some("Edit text"), "Helvetica"), "the document's Redo still waits"); +} From 188341c66730ccf99082e03544688c371a316bb6 Mon Sep 17 00:00:00 2001 From: RiceDee <345114586+RiceDee@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:55:09 +1100 Subject: [PATCH 3/7] test(ui): the Redo test checks Undo restores the exact paragraph text --- crates/ui-egui/tests/it/editing.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/tests/it/editing.rs b/crates/ui-egui/tests/it/editing.rs index 49e882893d..4bb74f82cd 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/crates/ui-egui/tests/it/editing.rs @@ -6051,13 +6051,14 @@ fn redo_after_undoing_typing_puts_the_typing_back() { h.run_steps(4); assert_eq!(h.state().session.get(h.state().views[0].id).unwrap().can_redo(), Some("Edit text")); 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); let typed = h.state().views[0].line_editor.as_ref().unwrap().text.clone(); h.key_press_modifiers(Modifiers::COMMAND, Key::Z); h.run_steps(3); - assert_ne!(h.state().views[0].line_editor.as_ref().unwrap().text, typed, "the typing is taken back"); + assert_eq!(h.state().views[0].line_editor.as_ref().unwrap().text, original, "the typing is taken back"); h.key_press_modifiers(Modifiers::COMMAND | Modifiers::SHIFT, Key::Z); h.run_steps(3); let s = h.state(); From 6c8fd74ca1c3b1b8c14e02631000990e9c641ac3 Mon Sep 17 00:00:00 2001 From: RiceDee <345114586+RiceDee@users.noreply.github.com> Date: Fri, 23 Oct 2026 16:14:01 +1100 Subject: [PATCH 4/7] fix(ui): an unchanged added-text box closes without an edit Finishing a box trimmed its text before comparing it with the item, so an unchanged box whose text was saved with trailing spaces finished as an edit. Before an Undo or Redo step that edit was applied, clearing the document's Redo. A box matching its item is now no edit. --- crates/ui-egui/src/content_ui.rs | 4 ++++ crates/ui-egui/tests/it/content.rs | 26 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/crates/ui-egui/src/content_ui.rs b/crates/ui-egui/src/content_ui.rs index 892e94ed67..788ba49600 100644 --- a/crates/ui-egui/src/content_ui.rs +++ b/crates/ui-egui/src/content_ui.rs @@ -639,6 +639,10 @@ fn finish(cv: &mut ContentView, d: TextDraft, added: &[Added]) -> Option { Some(i) => { let (_, a) = on_page(added, d.page).into_iter().find(|(k, _)| *k == i)?; let AddedContent::Text(old) = &a.content else { return None }; + // Unchanged is no edit, even for text saved with trailing spaces the trim would drop. + if *old == (AddedText { text: d.text.clone(), rect: d.rect, ..d.style.clone() }) { + return None; + } let updated = AddedText { text, rect: d.rect, ..d.style }; (*old != updated).then_some(Edit::UpdateContent { page: d.page, index: i, content: AddedContent::Text(updated) }) } diff --git a/crates/ui-egui/tests/it/content.rs b/crates/ui-egui/tests/it/content.rs index fa0c01367a..c24501dab3 100644 --- a/crates/ui-egui/tests/it/content.rs +++ b/crates/ui-egui/tests/it/content.rs @@ -891,3 +891,29 @@ fn redo_after_undoing_typing_in_added_text_puts_it_back() { let doc = s.session.get(s.views[0].id).unwrap(); assert_eq!(doc.can_redo(), Some("Edit content"), "the document's Redo still waits"); } + +/// 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. +#[test] +fn redo_with_an_unchanged_box_open_redoes_the_document() { + 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 })); + let pdfcraft_engine::AddedContent::Text(saved) = added(&h)[0].content.clone() else { panic!() }; + assert_eq!(saved.text, "Reviewed ", "the trailing space is kept"); + let bold = pdfcraft_engine::AddedText { bold: true, ..saved }; + 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::Modifiers::SHIFT, egui::Key::Z); + h.run_steps(4); + let pdfcraft_engine::AddedContent::Text(now) = &added(&h)[0].content else { panic!() }; + assert!(now.bold, "the waiting Redo was redone"); + assert!(h.state().views[0].content.draft.is_none(), "the box closed with the step"); +} From f1992f5aa1bde2ae5f9e3c7824527f626cb4e5bd Mon Sep 17 00:00:00 2001 From: RiceDee <345114586+RiceDee@users.noreply.github.com> Date: Sat, 24 Oct 2026 03:24:58 +1100 Subject: [PATCH 5/7] fix(ui): typing in one tab's text box never reaches another tab's The paragraph box, and the added-text box, each keep one Undo history under one egui id, shared by every tab. A box cleared it when it opened on new text, but a box in another tab on the same text (two copies of a document) kept it: typing taken back with Undo in one tab came back with Redo in the other. The document that last used the history is now kept beside it, and the history is cleared whenever another document's box takes it over. Tests: the paragraph and added-text cases, each with two copies of a document open. --- crates/ui-egui/src/content_ui.rs | 3 ++- crates/ui-egui/src/edit_text_ui.rs | 19 +++++++++------ crates/ui-egui/tests/it/content.rs | 37 +++++++++++++++++++++++++++++ crates/ui-egui/tests/it/editing.rs | 38 ++++++++++++++++++++++++++++++ 4 files changed, 89 insertions(+), 8 deletions(-) diff --git a/crates/ui-egui/src/content_ui.rs b/crates/ui-egui/src/content_ui.rs index 788ba49600..66ff781bbd 100644 --- a/crates/ui-egui/src/content_ui.rs +++ b/crates/ui-egui/src/content_ui.rs @@ -518,6 +518,7 @@ pub(crate) fn after_refresh(view: &mut DocView, added: &[Added]) { /// committed (a click away, or Done), and whether Done finished adding text. Discard or Escape /// throws the draft away. pub(crate) fn editor(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, added: &[Added]) -> (Option, bool) { + let doc_id = view.id; let Some(page) = view.content.draft.as_ref().map(|d| d.page) else { return (None, false) }; let Some(xf) = view.page_xform(page) else { return (None, false) }; let zoom = xf.rect.width() / xf.pw.max(1.0); @@ -571,7 +572,7 @@ pub(crate) fn editor(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, ad ui.visuals_mut().text_cursor.stroke.color = crate::edit_text_ui::caret_color_on(Color32::WHITE); let [cr, cg, cb] = t.style.color.map(|v| (v.clamp(0.0, 1.0) * 255.0) as u8); let input = input_id(); - crate::edit_text_ui::forget_typing_before(ui.ctx(), input, &mut t.typing_from, &t.style.text); + crate::edit_text_ui::forget_typing_before(ui.ctx(), input, doc_id, &mut t.typing_from, &t.style.text); ui.horizontal_top(|ui| { ui.spacing_mut().item_spacing.x = 4.0; let resp = ui.add( diff --git a/crates/ui-egui/src/edit_text_ui.rs b/crates/ui-egui/src/edit_text_ui.rs index f2c5c55151..8e5bf8d368 100644 --- a/crates/ui-egui/src/edit_text_ui.rs +++ b/crates/ui-egui/src/edit_text_ui.rs @@ -266,18 +266,22 @@ pub(crate) fn input_id() -> egui::Id { } /// The page text box `id` keeps its own Undo history, across every paragraph or box it has shown -/// (they share the id). Once the page has `text` (the box opened on it, or formatting applied it), -/// that history is cleared: Undo in the box takes back only what was typed since, and then reaches -/// the document. Kept, it put back text from another paragraph, or the whole paragraph as it was -/// before a format change, into a box showing the page as it is now. -pub(crate) fn forget_typing_before(ctx: &egui::Context, id: egui::Id, from: &mut Option, text: &str) { - if from.as_deref() == Some(text) { +/// in every tab (they share the id). Once the page has `text` (the box opened on it, or formatting +/// applied it), that history is cleared: Undo in the box takes back only what was typed since, and +/// then reaches the document. Kept, it put back text from another paragraph, or the whole paragraph +/// as it was before a format change, into a box showing the page as it is now. `from` is the box's +/// 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. +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"); + if from.as_deref() == Some(text) && ctx.data(|d| d.get_temp::(owner)) == Some(doc) { return; } if let Some(mut state) = egui::TextEdit::load_state(ctx, id) { state.clear_undoer(); egui::TextEdit::store_state(ctx, id, state); } + ctx.data_mut(|d| d.insert_temp(owner, doc)); *from = Some(text.to_owned()); } @@ -1577,6 +1581,7 @@ fn editor_galley(ui: &egui::Ui, ed: &LineEditor, text: &str, scale: f32, width: /// The inline editor; returns the edit once the text is applied. pub(crate) fn overlay(ctx: &egui::Context, view: &mut DocView, info: &DocInfo) -> Option { + let view_id = view.id; // Clicks outside the document (the Format text panel) keep the paragraph open. let outside = ctx.input(|i| i.pointer.latest_pos()).is_some_and(|p| !view.viewport_rect().contains(p)); let page = view.line_editor.as_ref()?.page; @@ -1693,7 +1698,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, &mut ed.typing_from, &ed.original); + forget_typing_before(ui.ctx(), input, view_id, &mut ed.typing_from, &ed.original); let output = egui::TextEdit::multiline(&mut ed.text) .id(input) .font(font.clone()) diff --git a/crates/ui-egui/tests/it/content.rs b/crates/ui-egui/tests/it/content.rs index c24501dab3..41957e6f69 100644 --- a/crates/ui-egui/tests/it/content.rs +++ b/crates/ui-egui/tests/it/content.rs @@ -917,3 +917,40 @@ fn redo_with_an_unchanged_box_open_redoes_the_document() { assert!(now.bold, "the waiting Redo was redone"); assert!(h.state().views[0].content.draft.is_none(), "the box closed with the step"); } + +/// Two copies of a document, the same added text open in each: the boxes share one Undo history, +/// so typing taken back with Undo in one tab came back with Redo in the other. +#[test] +fn typing_in_one_tab_never_reaches_another_tabs_added_text() { + 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 text = pdfcraft_engine::AddedText { rect: [40.0, 195.0, 240.0, 210.0], text: "Reviewed".into(), ..Default::default() }; + 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"); + }; + for tab in [0, 1] { + h.state_mut().active = Some(tab); + assert!(h.state_mut().apply_edit(pdfcraft_engine::Edit::AddText { page: 0, text: text.clone() })); + } + open_in(&mut h, 1); + open_in(&mut h, 0); + h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::End); + h.event(egui::Event::Text(" secret".into())); + h.run_steps(2); + h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::Z); + h.run_steps(3); + assert_eq!(h.state().views[0].content.draft.as_ref().map(|d| d.text.as_str()), Some("Reviewed"), "taken back"); + h.state_mut().active = Some(1); + h.run_steps(3); + h.key_press_modifiers(egui::Modifiers::COMMAND | egui::Modifiers::SHIFT, egui::Key::Z); + h.run_steps(3); + assert_eq!(h.state().views[1].content.draft.as_ref().map(|d| d.text.as_str()), Some("Reviewed"), "the other tab's typing stays out"); +} diff --git a/crates/ui-egui/tests/it/editing.rs b/crates/ui-egui/tests/it/editing.rs index 4bb74f82cd..e87697403f 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/crates/ui-egui/tests/it/editing.rs @@ -6067,3 +6067,41 @@ fn redo_after_undoing_typing_puts_the_typing_back() { let doc = s.session.get(s.views[0].id).unwrap(); assert_eq!((doc.can_redo(), doc.text_blocks(0)[0].base_font.as_str()), (Some("Edit text"), "Helvetica"), "the document's Redo still waits"); } + +/// 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. +#[test] +fn typing_in_one_tab_never_reaches_another_tabs_paragraph() { + 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); + let original = h.state().views[1].line_editor.as_ref().unwrap().text.clone(); + open_in(&mut h, 0); + assert_eq!(h.state().views[0].line_editor.as_ref().unwrap().text, original, "the same paragraph"); + h.key_press_modifiers(Modifiers::COMMAND, Key::End); + h.event(egui::Event::Text(" secret".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, "taken back"); + // The copy, its paragraph still open: Redo there has nothing of its own to put back. + h.state_mut().active = Some(1); + h.run_steps(3); + h.key_press_modifiers(Modifiers::COMMAND | Modifiers::SHIFT, Key::Z); + h.run_steps(3); + let s = h.state(); + let text = s.views[1].line_editor.as_ref().map(|ed| ed.text.clone()); + assert!(!text.as_deref().unwrap_or_default().contains("secret"), "the other tab's typing came in: {text:?}"); + assert_eq!(s.session.get(s.views[1].id).unwrap().text_blocks(0)[0].text, s.session.get(s.views[0].id).unwrap().text_blocks(0)[0].text); +} From 632ba10ed5c8b41d9f53a0385514aeadeb4b4b31 Mon Sep 17 00:00:00 2001 From: RiceDee <345114586+RiceDee@users.noreply.github.com> Date: Sat, 24 Oct 2026 18:17:48 +1100 Subject: [PATCH 6/7] fix(ui): Redo from the menu with typing in a page box stays in the box Redo from the menu or toolbar closed the box first, which applied its typing: a new edit that dropped the document's waiting Redo, and the Redo itself then failed. The step was gone. Redo now goes to the box as the key does: what Undo took back there comes back, and typing the page doesn't have yet stays as it is, with the document's Redo waiting. A new text box the document refused was put back without the request to select the item once it exists, which stayed behind. It is put back with the box. --- crates/ui-egui/src/editing.rs | 78 ++++++++++++++++++++++++++++-- crates/ui-egui/tests/it/content.rs | 33 +++++++++++++ crates/ui-egui/tests/it/editing.rs | 40 +++++++++++++++ 3 files changed, 148 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 39e0fd1adf..99cd53eff4 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -222,11 +222,31 @@ impl PdfCraftApp { pub fn redo(&mut self) { if self.combine_showing() { self.combine_history_step(false); - } else { + } else if !self.redo_in_page_box() { self.history_step(false); } } + /// Redo from the menu or toolbar while a box on the page has typing of its own: the step is + /// the box's, as the key's is. What Undo took back in the box comes back; typing the page + /// doesn't have yet stays as it is, since applying it first is a new edit, which drops the + /// document's Redo. True when the box took the step. + 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 }; + 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() }; + 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() }; + d.text = text; + return true; + } + false + } + fn history_step(&mut self, undo: bool) { let Some((i, id)) = self.active_ids() else { return }; if !self.close_text_editors(i, id) { @@ -274,12 +294,13 @@ impl PdfCraftApp { } let added = self.session.get(id).map(|d| d.added().to_vec()).unwrap_or_default(); let Some(view) = self.views.get_mut(i) else { return true }; - let draft = view.content.draft.clone(); + // Finishing new text also asks for it to be selected once it exists: put that back too. + let (draft, select_added) = (view.content.draft.clone(), view.content.select_added); if let Some(edit) = view.content.finish_draft(&added) && !self.apply_edit(edit) { if let Some(view) = self.views.get_mut(i) { - view.content.draft = draft; + (view.content.draft, view.content.select_added) = (draft, select_added); } return false; } @@ -1217,6 +1238,18 @@ fn bookmark_at<'a>(items: &'a [pdfcraft_render::OutlineItem], path: &[usize]) -> if rest.is_empty() { Some(item) } else { bookmark_at(&item.children, rest) } } +/// 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 { + 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(); + state.set_undoer(undoer); + state.cursor.set_char_range(Some(range)); + egui::TextEdit::store_state(ctx, id, state); + Some(redone) +} + #[cfg(test)] mod tests { use super::{SaveTarget, is_mail_temp_copy, write_atomically, write_atomically_syncing, write_atomically_with}; @@ -1493,6 +1526,45 @@ mod tests { out } + /// `one_page_pdf`, opening without a password, whose permissions refuse changes. + fn no_changes_pdf() -> Vec { + let mut doc = pdfcraft_cos::Document::open(std::sync::Arc::new(one_page_pdf())).unwrap(); + let encryption = pdfcraft_cos::NewEncryption { + algorithm: pdfcraft_cos::Algorithm::Aes256, + user_password: "", + owner_password: "owner", + permissions: -1 ^ 8, + encrypt_metadata: true, + seed: [4; 32], + }; + doc.set_encryption(&encryption).unwrap(); + pdfcraft_cos::write_full(&doc, &Default::default()).unwrap() + } + + /// Undo with a new text box open on a document that refuses changes: the box is put back as + /// it was. Finishing it had asked for the new item to be selected, and that request stayed + /// behind once the add was refused. + #[test] + fn a_refused_text_box_is_put_back_whole() { + let mut app = crate::PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("a.pdf", None, no_changes_pdf()).unwrap(); + let style = crate::content_ui::default_style(); + app.views[0].content.draft = Some(crate::content_ui::TextDraft { + page: 0, + index: None, + rect: [20.0, 20.0, 120.0, 40.0], + text: "Typed".into(), + style, + focus: false, + typing_from: None, + }); + app.undo(); + let content = &app.views[0].content; + assert_eq!(content.draft.as_ref().map(|d| d.text.as_str()), Some("Typed"), "the refused box stays open"); + assert_eq!(content.select_added, None, "nothing was added to select"); + } + #[test] fn save_asks_where_to_keep_an_email_attachment_and_leaves_the_temporary_copy_alone() { let dir = staging_dir("mail-temp"); diff --git a/crates/ui-egui/tests/it/content.rs b/crates/ui-egui/tests/it/content.rs index 41957e6f69..81d6e00049 100644 --- a/crates/ui-egui/tests/it/content.rs +++ b/crates/ui-egui/tests/it/content.rs @@ -892,6 +892,39 @@ fn redo_after_undoing_typing_in_added_text_puts_it_back() { assert_eq!(doc.can_redo(), Some("Edit content"), "the document's Redo still waits"); } +/// Redo from the menu with retyping the page doesn't have yet in an added text box: the typing +/// stays in the box and the document's Redo waits (closing the box first wrote the typing, which +/// dropped that Redo). After Undo in the box, Redo from the menu puts the typing back there. +#[test] +fn redo_from_the_menu_with_retyping_in_the_box_keeps_the_documents_redo() { + 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.redo")); + h.run_steps(3); + assert_eq!(h.state().views[0].content.draft.as_ref().map(|d| d.text.as_str()), Some("Reviewed twice"), "the typing stays in the box"); + h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::Z); + 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"); + assert!(h.state_mut().execute("edit.redo")); + h.run_steps(3); + assert_eq!(h.state().views[0].content.draft.as_ref().map(|d| d.text.as_str()), Some("Reviewed twice"), "Redo from the menu puts it back"); + 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 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 e87697403f..dd63003653 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/crates/ui-egui/tests/it/editing.rs @@ -6068,6 +6068,46 @@ fn redo_after_undoing_typing_puts_the_typing_back() { assert_eq!((doc.can_redo(), doc.text_blocks(0)[0].base_font.as_str()), (Some("Edit text"), "Helvetica"), "the document's Redo still waits"); } +/// Redo from the menu or toolbar with typing in the box the page doesn't have yet. Closing the +/// box first applied the typing, a new edit that dropped the document's waiting Redo, and the +/// Redo itself then failed: the step was gone. The typing now stays in the box and the Redo +/// waits; once Undo in the box takes the typing back, Redo from the menu puts it back there. +#[test] +fn redo_from_the_menu_with_typing_in_the_box_keeps_the_documents_redo() { + 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. + 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); + let typed = h.state().views[0].line_editor.as_ref().unwrap().text.clone(); + assert!(h.state_mut().execute("edit.redo")); + 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, typed, "the typing stays in the box, unapplied"); + let doc = s.session.get(s.views[0].id).unwrap(); + assert_eq!((doc.can_redo(), doc.text_blocks(0)[0].base_font.as_str()), (Some("Edit text"), "Helvetica"), "the document's Redo still waits"); + } + 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, "Undo takes the typing back in the box"); + assert!(h.state_mut().execute("edit.redo")); + h.run_steps(3); + let s = h.state(); + assert_eq!(s.views[0].line_editor.as_ref().expect("still open").text, typed, "Redo from the menu puts the typing back"); + assert_eq!(s.session.get(s.views[0].id).unwrap().can_redo(), Some("Edit text"), "the document's Redo still waits"); +} + /// 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. From 4f9b17165d2827d6638e6d2ebe583a18a5c59b58 Mon Sep 17 00:00:00 2001 From: RiceDee <345114586+RiceDee@users.noreply.github.com> Date: Sun, 25 Oct 2026 01:23:16 +1100 Subject: [PATCH 7/7] fix(ui): menu Redo is enabled for the open text box's own Redo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Edit ▸ Redo and the toolbar button followed only the document's Redo. After Undo in a paragraph or added text box took the typing back, with no document step to redo, they stayed disabled and only the key could put the typing back. Redo is now also enabled when the box open on the page has a Redo of its own: the step redo_in_page_box takes. It doesn't ask for the box's focus, which the menu has taken by then. --- crates/ui-egui/src/commands.rs | 4 +++- crates/ui-egui/src/editing.rs | 22 ++++++++++++++++++++-- crates/ui-egui/tests/it/content.rs | 25 +++++++++++++++++++++++++ crates/ui-egui/tests/it/editing.rs | 23 +++++++++++++++++++++++ 4 files changed, 71 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/commands.rs b/crates/ui-egui/src/commands.rs index 2ac0f82022..25cb73beba 100644 --- a/crates/ui-egui/src/commands.rs +++ b/crates/ui-egui/src/commands.rs @@ -84,7 +84,9 @@ 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); } - commands::is_enabled(spec, &self.session, self.active_ids().map(|(_, id)| id)) + // Redo also takes back what Undo took in a box open on the page, with no document step. + (commands::is_enabled(spec, &self.session, self.active_ids().map(|(_, id)| id)) + || spec.needs == commands::Needs::Redo && self.open_box_can_redo()) && (spec.id != "edit.copy" || self .active diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 455f8db4cc..207a3f4830 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -331,8 +331,19 @@ impl PdfCraftApp { } else { return false; }; - egui::TextEdit::load_state(ctx, id) - .is_some_and(|state| state.cursor.char_range().is_some_and(|range| state.undoer().has_redo(&(range, text.clone())))) + has_typing_to_redo(ctx, id, text) + } + + /// Whether the box open on the page has a Redo of its own, the step + /// [`Self::redo_in_page_box`] takes for Redo from the menu or toolbar: focus has moved to the + /// menu by then, so unlike [`Self::page_box_can_redo`] this doesn't ask for it. + pub(crate) fn open_box_can_redo(&self) -> bool { + let Some(ctx) = &self.ctx else { return false }; + let Some(view) = self.active.and_then(|i| self.views.get(i)) else { return false }; + if let Some(ed) = &view.line_editor { + return has_typing_to_redo(ctx, crate::edit_text_ui::input_id(), &ed.text); + } + view.content.draft.as_ref().is_some_and(|d| has_typing_to_redo(ctx, crate::content_ui::input_id(), &d.text)) } /// Typing in a box on the page (a paragraph being edited, added text) that the page doesn't @@ -1329,6 +1340,13 @@ fn bookmark_at<'a>(items: &'a [pdfcraft_render::OutlineItem], path: &[usize]) -> if rest.is_empty() { Some(item) } else { bookmark_at(&item.children, rest) } } +/// Whether the page text box `id`, showing `text`, has typing its own Redo would put back: the +/// test egui makes when it handles the key. +fn has_typing_to_redo(ctx: &egui::Context, id: egui::Id, text: &str) -> bool { + egui::TextEdit::load_state(ctx, id) + .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 { diff --git a/crates/ui-egui/tests/it/content.rs b/crates/ui-egui/tests/it/content.rs index 8abf757570..c2edfb8336 100644 --- a/crates/ui-egui/tests/it/content.rs +++ b/crates/ui-egui/tests/it/content.rs @@ -971,6 +971,31 @@ 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"); } +/// 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] +fn redo_from_the_menu_is_enabled_for_the_added_text_boxs_own_typing() { + 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 })); + 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); + h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::Z); + 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"); + assert_eq!(h.state().session.get(h.state().views[0].id).unwrap().can_redo(), None, "the document has nothing to redo"); + assert!(h.state_mut().execute("edit.redo"), "Redo is enabled for the box's own step"); + h.run_steps(3); + assert_eq!(h.state().views[0].content.draft.as_ref().map(|d| d.text.as_str()), Some("Reviewed twice"), "Redo puts it back"); +} + /// 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 4b26d26d21..8ce8073285 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/crates/ui-egui/tests/it/editing.rs @@ -6383,6 +6383,29 @@ 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"); } +/// 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. +#[test] +fn redo_from_the_menu_is_enabled_for_the_boxs_own_typing() { + 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); + let typed = h.state().views[0].line_editor.as_ref().unwrap().text.clone(); + 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"); + assert_eq!(h.state().session.get(h.state().views[0].id).unwrap().can_redo(), None, "the document has nothing to redo"); + assert!(h.state_mut().execute("edit.redo"), "Redo is enabled for the box's own step"); + h.run_steps(3); + assert_eq!(h.state().views[0].line_editor.as_ref().expect("still open").text, typed, "Redo puts the typing back"); +} + /// 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.