diff --git a/crates/ui-egui/src/edit_text_ui.rs b/crates/ui-egui/src/edit_text_ui.rs index 0b0cc0bc1e..2502c2927f 100644 --- a/crates/ui-egui/src/edit_text_ui.rs +++ b/crates/ui-egui/src/edit_text_ui.rs @@ -757,11 +757,9 @@ impl LineEditor { let spans = block.edited_spans(&self.text); let matching_lines: Vec<&pdfcraft_engine::TextLine> = match &self.selection { - Some(range) if range.start < range.end => spans - .iter() - .filter(|(span, _)| span.start < range.end && span.end > range.start) - .filter_map(|(_, idx)| lines.get(*idx)) - .collect(), + Some(range) if range.start < range.end => { + spans.iter().filter(|(span, _)| span.start < range.end && span.end > range.start).filter_map(|(_, idx)| lines.get(*idx)).collect() + } _ => spans.iter().filter_map(|(_, idx)| lines.get(*idx)).collect(), }; @@ -793,11 +791,7 @@ impl LineEditor { if ["times", "serif", "roman", "courier", "helvetica", "arial"].iter().any(|s| lower.contains(s)) { return look.family.label().to_string(); } - if name.is_empty() { - tl!("Keep source fonts").to_string() - } else { - name.to_string() - } + if name.is_empty() { tl!("Keep source fonts").to_string() } else { name.to_string() } }; let first_font = line_font(matching_lines[0]); diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 876278d6ff..c2fc6f5b10 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -187,40 +187,7 @@ impl PdfCraftApp { } if let (Some((target, ambiguous_before)), Edit::EditPageImage { page, .. }) = (target_arrange_image, &edit) { let page_images = doc.page_images(*page); - let exact_matches: Vec = page_images - .iter() - .enumerate() - .filter(|(_, img)| img.name == target.name && img.object == target.object && img.rect == target.rect) - .map(|(idx, _)| idx) - .collect(); - let new_idx = if !ambiguous_before && exact_matches.len() == 1 { - Some(exact_matches[0]) - } else if !ambiguous_before && exact_matches.is_empty() { - let name_obj_matches: Vec = page_images - .iter() - .enumerate() - .filter(|(_, img)| img.name == target.name && img.object == target.object) - .map(|(idx, _)| idx) - .collect(); - if name_obj_matches.len() == 1 { - Some(name_obj_matches[0]) - } else if name_obj_matches.is_empty() { - let name_matches: Vec = - page_images.iter().enumerate().filter(|(_, img)| img.name == target.name).map(|(idx, _)| idx).collect(); - if name_matches.len() == 1 { Some(name_matches[0]) } else { None } - } else { - None - } - } else { - None - }; - if let Some(s) = view.image_selection.as_mut().filter(|s| s.page == *page) { - if let Some(idx) = new_idx { - s.index = idx; - } else { - view.image_selection = None; - } - } + re_resolve_image_selection(view, *page, &target, ambiguous_before, &page_images); } if std::mem::take(&mut view.comments.tool_done) && !self.comment_prefs.pinned { self.quick_tool = crate::QuickTool::Select; @@ -284,13 +251,33 @@ impl PdfCraftApp { fn history_step(&mut self, undo: bool) { let Some((i, id)) = self.active_ids() else { return }; + let target_image = self.views.get(i).and_then(|v| v.image_selection.as_ref()).and_then(|s| { + let doc = self.session.get(id)?; + let images = doc.page_images(s.page); + let target = images.get(s.index).cloned()?; + let ambiguous = images.iter().filter(|img| img.name == target.name && img.object == target.object && img.rect == target.rect).count() > 1; + Some((s.page, target, ambiguous)) + }); + let had_selection = self.views.get(i).and_then(|v| v.image_selection.as_ref()).is_some(); + let result = if undo { self.session.undo(id) } else { self.session.redo(id) }; match result { Ok(label) => { - if let Some(doc) = self.session.get(id) { - self.views[i].document_changed(&doc.info); + if let Some(doc) = self.session.get(id) + && let Some(view) = self.views.get_mut(i) + { + view.document_changed(&doc.info); + view.picked_image = None; + if let Some((page, target, ambiguous_before)) = target_image { + let page_images = doc.page_images(page); + re_resolve_image_selection(view, page, &target, ambiguous_before, &page_images); + } else if had_selection { + view.image_selection = None; + } + } + if let Some(view) = self.views.get_mut(i) { + view.comments.selected = None; } - self.views[i].comments.selected = None; let label = crate::i18n::edit_label(&label); if undo { self.notify_fmt("Undid {label}", &[("label", &label)]); @@ -1019,6 +1006,44 @@ impl PdfCraftApp { } } +fn re_resolve_image_selection( + view: &mut crate::DocView, + page: usize, + target: &pdfcraft_engine::PageImage, + ambiguous_before: bool, + page_images: &[pdfcraft_engine::PageImage], +) { + let exact_matches: Vec = page_images + .iter() + .enumerate() + .filter(|(_, img)| img.name == target.name && img.object == target.object && img.rect == target.rect) + .map(|(idx, _)| idx) + .collect(); + let new_idx = if !ambiguous_before && exact_matches.len() == 1 { + exact_matches.first().copied() + } else if !ambiguous_before && exact_matches.is_empty() { + let name_obj_matches: Vec = + page_images.iter().enumerate().filter(|(_, img)| img.name == target.name && img.object == target.object).map(|(idx, _)| idx).collect(); + if name_obj_matches.len() == 1 { + name_obj_matches.first().copied() + } else if name_obj_matches.is_empty() { + let name_matches: Vec = page_images.iter().enumerate().filter(|(_, img)| img.name == target.name).map(|(idx, _)| idx).collect(); + if name_matches.len() == 1 { name_matches.first().copied() } else { None } + } else { + None + } + } else { + None + }; + if let Some(s) = view.image_selection.as_mut().filter(|s| s.page == page) { + if let Some(idx) = new_idx { + s.index = idx; + } else { + view.image_selection = None; + } + } +} + /// The single page a content edit changes (`None` for other edits). Like comment edits, /// these leave every other page's bytes, geometry and text alone, so their rasters stay valid. fn content_page(edit: &Edit) -> Option { diff --git a/crates/ui-egui/src/format_text_ui.rs b/crates/ui-egui/src/format_text_ui.rs index cc137bafa2..2c8347cd7d 100644 --- a/crates/ui-egui/src/format_text_ui.rs +++ b/crates/ui-egui/src/format_text_ui.rs @@ -208,12 +208,16 @@ pub(crate) fn panel(ui: &mut egui::Ui, t: &Tokens, look: &mut AddedText, extras: labelled_icon(ui, t, "arrow-up-down", tl!("Line spacing")); let own = tl!("Auto").to_string(); let shown = if e.line_spacing == 0.0 { own.clone() } else { number(e.line_spacing) }; - let spacing_resp = egui::ComboBox::from_id_salt("format-line-spacing").selected_text(shown).width(half).show_ui(ui, |ui| { - ui.selectable_value(&mut e.line_spacing, 0.0, own.as_str()); - for v in LINE_SPACINGS { - ui.selectable_value(&mut e.line_spacing, v, number(v)); - } - }).response; + let spacing_resp = egui::ComboBox::from_id_salt("format-line-spacing") + .selected_text(shown) + .width(half) + .show_ui(ui, |ui| { + ui.selectable_value(&mut e.line_spacing, 0.0, own.as_str()); + for v in LINE_SPACINGS { + ui.selectable_value(&mut e.line_spacing, v, number(v)); + } + }) + .response; spacing_resp.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::ComboBox, ui.is_enabled(), tl!("Line spacing"))); ui.add_space(8.0); ui.add_enabled_ui(false, |ui| { diff --git a/crates/ui-egui/tests/it/control.rs b/crates/ui-egui/tests/it/control.rs index 745a024ff4..c7519a5d4d 100644 --- a/crates/ui-egui/tests/it/control.rs +++ b/crates/ui-egui/tests/it/control.rs @@ -1931,4 +1931,3 @@ fn accessible_labels_on_format_text_and_pin_dialog() { let char_spin = ok(&mut h, &c, "ui.inspect", json!({"query": "Character spacing", "role": "SpinButton"})); assert!(char_spin["count"].as_u64().unwrap() >= 1, "character spacing spin button has accessible label: {char_spin}"); } - diff --git a/crates/ui-egui/tests/it/editing.rs b/crates/ui-egui/tests/it/editing.rs index 4cedcf33b3..ec378add37 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/crates/ui-egui/tests/it/editing.rs @@ -6898,6 +6898,99 @@ fn arrange_unique_image_preserves_selection() { assert_eq!(sel.index, 1, "selection index tracks the unique image's new position"); } +#[test] +fn undo_arrange_re_resolves_image_selection_or_clears() { + use pdfcraft_cos::{Dict, Document, Object, SaveOptions, Stream, write_incremental}; + let mut doc = Document::open(std::sync::Arc::new(fixture(1))).unwrap(); + let page = pdfcraft_model::pages(&doc)[0].clone(); + let mut form = Dict::new(); + form.set(b"Subtype".to_vec(), Object::name("Form")); + form.set(b"BBox".to_vec(), Object::Array([0, 0, 200, 300].map(Object::Int).to_vec())); + let artwork1 = doc.add(Object::Stream(Stream::flate(form.clone(), b"0.2 0.5 0.9 rg 20 20 160 260 re f"))); + let artwork2 = doc.add(Object::Stream(Stream::flate(form, b"0.9 0.2 0.5 rg 40 40 100 100 re f"))); + let contents = doc.add(Object::Stream(Stream::flate(Dict::new(), b"q 1 0 0 1 0 0 cm /FigA Do Q q 1 0 0 1 0 0 cm /FigB Do Q"))); + doc.update_dict(page.obj, |d| { + d.set(b"Contents".to_vec(), Object::Ref(contents)); + let mut xo = Dict::new(); + xo.set(b"FigA".to_vec(), Object::Ref(artwork1)); + xo.set(b"FigB".to_vec(), Object::Ref(artwork2)); + let mut resources = Dict::new(); + resources.set(b"XObject".to_vec(), Object::Dict(xo)); + d.set(b"Resources".to_vec(), Object::Dict(resources)); + }) + .unwrap(); + let bytes = write_incremental(&doc, &SaveOptions::default()).unwrap(); + + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(move |_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("undo_arrange.pdf", None, bytes.clone()).expect("opens"); + app + }); + h.run_steps(4); + assert!(h.state_mut().execute("edit.edit_text")); + h.run_steps(2); + + let doc_id = h.state().views[0].id; + let images = h.state().session.get(doc_id).unwrap().page_images(0); + assert_eq!(images.len(), 2); + assert_eq!(images[0].name, "FigA"); + assert_eq!(images[1].name, "FigB"); + + // Select FigA (back image, index 0) + h.state_mut().views[0].image_selection = Some(pdfcraft_ui_egui::edit_text_ui::ImageSelection { + page: 0, + index: 0, + drag: None, + crop: None, + crop_mode: false, + crop_rect: None, + turning: false, + settled: None, + }); + h.run_steps(2); + + // Bring to front + let applied = h.state_mut().apply_edit(pdfcraft_engine::Edit::EditPageImage { + page: 0, + index: 0, + change: pdfcraft_engine::ImageEdit::Arrange(pdfcraft_engine::ArrangeOrder::BringToFront), + }); + assert!(applied); + h.run_steps(2); + + // Selection now tracks FigA at index 1 + let sel = h.state().views[0].image_selection.as_ref().expect("selected after arrange"); + assert_eq!(sel.index, 1); + + // Undo + assert!(h.state_mut().execute("edit.undo")); + h.run_steps(2); + + let doc_after = h.state().session.get(doc_id).unwrap(); + let images_after = doc_after.page_images(0); + assert_eq!(images_after[0].name, "FigA"); + assert_eq!(images_after[1].name, "FigB"); + + // After undoing Bring to front, selection must either be FigA (index 0) or None, never FigB (index 1) + let sel_after = h.state().views[0].image_selection.as_ref().expect("selection re-resolved to the same image"); + assert_eq!(sel_after.index, 0, "selection should point to FigA at index 0, not FigB at index 1"); + assert_eq!(images_after[sel_after.index].name, "FigA"); + + // Redo + assert!(h.state_mut().execute("edit.redo")); + h.run_steps(2); + + let doc_redo = h.state().session.get(doc_id).unwrap(); + let images_redo = doc_redo.page_images(0); + assert_eq!(images_redo[0].name, "FigB"); + assert_eq!(images_redo[1].name, "FigA"); + + let sel_redo = h.state().views[0].image_selection.as_ref().expect("selection re-resolved after redo"); + assert_eq!(sel_redo.index, 1, "selection should point to FigA at index 1"); + assert_eq!(images_redo[sel_redo.index].name, "FigA"); +} + /// #3061: a paragraph running past its page's bottom edge (an HTML converter relying on the /// page clip) opens in an editor that draws on its page only. It covered the next page. #[test] @@ -7350,4 +7443,3 @@ fn format_panel_shows_selection_size_and_updates_after_font_applied() { let font_node = h.get_by_role_and_label(Role::ComboBox, "Font"); assert_eq!(font_node.value().as_deref(), Some("Times"), "font combo box updates to Times"); } -