diff --git a/crates/ui-egui/src/panels.rs b/crates/ui-egui/src/panels.rs index 09e99ab7a..f043a1e99 100644 --- a/crates/ui-egui/src/panels.rs +++ b/crates/ui-egui/src/panels.rs @@ -913,42 +913,87 @@ pub fn right_panel(app: &mut PdfCraftApp, ui: &mut egui::Ui) { empty(ui, &t, "paperclip", "This document has no attachments."); } for (ai, a) in info.attachments.iter().enumerate() { - egui::Frame::NONE.inner_margin(egui::Margin::symmetric(4, 6)).show(ui, |ui| { - ui.horizontal(|ui| { - ui.add(icons::image("paperclip", 16.0, t.icon)); - ui.vertical(|ui| { - ui.set_width(ui.available_width() - 70.0); - ui.add(egui::Label::new(egui::RichText::new(&a.name).font(theme::medium(13.0))).truncate()); - let mut meta = a.size.map(human_size).unwrap_or_default(); - if let pdfcraft_render::AttachmentSource::Annotation { page, .. } = a.source { - let on_page = crate::i18n::fmt( - tl!("on page {label}"), - &[("label", info.pages.get(page).map(|p| p.label.as_str()).unwrap_or("?"))], + let is_pdf = a.name.to_lowercase().ends_with(".pdf"); + // Only an attachment of the document itself can be deleted here. + let deletable = match &a.source { + pdfcraft_render::AttachmentSource::Document { key } if attachable => Some(key), + _ => None, + }; + // The whole row answers a double-click and a right-click (#3466), below + // its buttons, which still take their own clicks. + let mut buttons: Vec = Vec::new(); + let row = ui.scope_builder(egui::UiBuilder::new().sense(Sense::click()), |ui| { + egui::Frame::NONE.inner_margin(egui::Margin::symmetric(4, 6)).show(ui, |ui| { + ui.horizontal(|ui| { + ui.add(icons::image("paperclip", 16.0, t.icon)); + ui.vertical(|ui| { + ui.set_width(ui.available_width() - 70.0); + // Not selectable, so a click on the name or the details reaches the row. + ui.add( + egui::Label::new(egui::RichText::new(&a.name).font(theme::medium(13.0))).truncate().selectable(false), ); - meta = format!("{meta} · {on_page}"); - } - if let Some(d) = &a.description { - meta = format!("{meta} · {d}"); - } - ui.add(egui::Label::new(egui::RichText::new(meta).color(t.text_faint).small()).truncate()); + let mut meta = a.size.map(human_size).unwrap_or_default(); + if let pdfcraft_render::AttachmentSource::Annotation { page, .. } = a.source { + let on_page = crate::i18n::fmt( + tl!("on page {label}"), + &[("label", info.pages.get(page).map(|p| p.label.as_str()).unwrap_or("?"))], + ); + meta = format!("{meta} · {on_page}"); + } + if let Some(d) = &a.description { + meta = format!("{meta} · {d}"); + } + ui.add( + egui::Label::new(egui::RichText::new(meta).color(t.text_faint).small()).truncate().selectable(false), + ); + }); + ui.with_layout(Layout::right_to_left(Align::Center), |ui| { + let save = icons::button(ui, "file-down", 28.0, false, tl!("Save attachment…")); + if save.clicked() { + attachment_action = Some((ai, false)); + } + buttons.push(save); + if is_pdf { + let open = icons::button(ui, "file-input", 28.0, false, tl!("Open in a new tab")); + if open.clicked() { + attachment_action = Some((ai, true)); + } + buttons.push(open); + } + if let Some(key) = deletable { + let delete = icons::button(ui, "trash-2", 28.0, false, tl!("Delete attachment")); + if delete.clicked() { + panel_edit = Some(pdfcraft_engine::Edit::DeleteAttachment { key: key.clone() }); + } + buttons.push(delete); + } + }); }); - ui.with_layout(Layout::right_to_left(Align::Center), |ui| { - if icons::button(ui, "file-down", 28.0, false, tl!("Save attachment…")).clicked() { - attachment_action = Some((ai, false)); - } - if a.name.to_lowercase().ends_with(".pdf") - && icons::button(ui, "file-input", 28.0, false, tl!("Open in a new tab")).clicked() - { - attachment_action = Some((ai, true)); - } - if let pdfcraft_render::AttachmentSource::Document { key } = &a.source - && attachable - && icons::button(ui, "trash-2", 28.0, false, tl!("Delete attachment")).clicked() - { - panel_edit = Some(pdfcraft_engine::Edit::DeleteAttachment { key: key.clone() }); - } - }); - }); + }) + }); + // A double-click opens a PDF in a new tab, as Acrobat opens an attachment; + // any other file is saved, since PdfCraft opens only PDFs. + if row.response.double_clicked() { + attachment_action = Some((ai, is_pdf)); + } + // A right-click on one of the row's buttons is a right-click on the row: + // the union keeps the row's id, so it is one menu either way. + let menu = buttons.into_iter().fold(row.response.clone(), |row, button| row | button); + menu.context_menu(|ui| { + if is_pdf && ui.button(tl!("Open in a new tab")).clicked() { + attachment_action = Some((ai, true)); + ui.close(); + } + if ui.button(tl!("Save attachment…")).clicked() { + attachment_action = Some((ai, false)); + ui.close(); + } + if let Some(key) = deletable + && ui.button(tl!("Delete attachment")).clicked() + { + panel_edit = Some(pdfcraft_engine::Edit::DeleteAttachment { key: key.clone() }); + ui.close(); + } }); } } diff --git a/crates/ui-egui/tests/it/attachment_row.rs b/crates/ui-egui/tests/it/attachment_row.rs new file mode 100644 index 000000000..1a66723bb --- /dev/null +++ b/crates/ui-egui/tests/it/attachment_row.rs @@ -0,0 +1,189 @@ +//! An attachment's row in the Attachments panel answers a double-click and a right-click, as +//! in Acrobat (#3466), while its buttons keep taking their own clicks. + +use egui_kittest::Harness; +use egui_kittest::kittest::Queryable; +use pdfcraft_ui_egui::PdfCraftApp; + +/// A one-page PDF with no embedded files, to attach to. +const INNER: &[u8] = b"%PDF-1.7 +1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj +2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj +3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] >> endobj +trailer << /Root 1 0 R >> +%%EOF"; + +/// A one-page PDF with two embedded files: `inner.pdf` (a PDF) and `notes.txt`. +fn attachments_pdf() -> Vec { + let objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R /Names << /EmbeddedFiles << /Names [(inner.pdf) 5 0 R (notes.txt) 7 0 R] >> >> >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Resources << >> >>".to_vec(), + [format!("<< /Type /EmbeddedFile /Length {} >>\nstream\n", INNER.len()).as_bytes(), INNER, b"\nendstream"].concat(), + b"<< /Type /Filespec /F (inner.pdf) /UF (inner.pdf) /EF << /F 4 0 R >> >>".to_vec(), + b"<< /Type /EmbeddedFile /Length 11 >>\nstream\nhello notes\nendstream".to_vec(), + b"<< /Type /Filespec /F (notes.txt) /UF (notes.txt) /EF << /F 6 0 R >> >>".to_vec(), + ]; + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n", i + 1).as_bytes()); + out.extend_from_slice(o); + out.extend_from_slice(b"\nendobj\n"); + } + let xref = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + out +} + +/// The app with [`attachments_pdf`] open and the Attachments panel showing, at 60 frames a +/// second so two clicks a frame apart make a double-click. +fn harness() -> Harness<'static, PdfCraftApp> { + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).with_step_dt(1.0 / 60.0).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("doc.pdf", None, attachments_pdf()).unwrap(); + app.set_option("panel", "attachments").unwrap(); + app + }); + h.run_steps(4); + h +} + +/// A folder of its own for one test's saved file, removed afterwards. +struct Dir(std::path::PathBuf); + +impl Dir { + fn new(test: &str) -> Self { + let path = std::env::temp_dir().join(format!("pdfcraft-attachment-row-{test}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&path); + std::fs::create_dir_all(&path).unwrap(); + Self(path) + } +} + +impl Drop for Dir { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } +} + +/// Two pointer clicks a frame apart on `label` (an accessibility click is never a double-click). +fn double_click(h: &mut Harness<'static, PdfCraftApp>, label: &str) { + let at = row_name(h, label); + h.hover_at(at); + h.run_steps(1); + for _ in 0..2 { + h.event(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: true, modifiers: Default::default() }); + h.event(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: false, modifiers: Default::default() }); + h.run_steps(1); + } + h.run_steps(3); +} + +fn right_click(h: &mut Harness<'static, PdfCraftApp>, label: &str) { + let at = row_name(h, label); + h.hover_at(at); + h.run_steps(1); + h.event(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Secondary, pressed: true, modifiers: Default::default() }); + h.event(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Secondary, pressed: false, modifiers: Default::default() }); + h.run_steps(2); +} + +#[test] +fn double_clicking_a_pdf_attachment_opens_it_in_a_new_tab() { + let mut h = harness(); + assert_eq!(h.state().views.len(), 1); + double_click(&mut h, "inner.pdf"); + assert_eq!(h.state().views.len(), 2, "the attached PDF opens in a tab of its own"); +} + +#[test] +fn double_clicking_another_attachment_saves_it() { + let dir = Dir::new("double"); + let target = dir.0.join("saved notes.txt"); + let mut h = harness(); + h.state_mut().pick_override = Some(vec![target.to_string_lossy().into_owned()]); + double_click(&mut h, "notes.txt"); + assert_eq!(std::fs::read(&target).unwrap(), b"hello notes", "PdfCraft opens only PDFs, so it saves the rest"); + assert_eq!(h.state().views.len(), 1, "nothing opens"); +} + +#[test] +fn the_right_click_menu_opens_saves_and_deletes() { + let dir = Dir::new("menu"); + let target = dir.0.join("from menu.pdf"); + let mut h = harness(); + let count = |h: &Harness<'static, PdfCraftApp>, label: &str| h.query_all_by_label(label).count(); + // The rows have Save and Delete buttons each, and the PDF's row an Open button. + assert_eq!((count(&h, "Save attachment…"), count(&h, "Open in a new tab"), count(&h, "Delete attachment")), (2, 1, 2)); + // A text file's menu has Save and Delete, no Open. + right_click(&mut h, "notes.txt"); + assert_eq!((count(&h, "Save attachment…"), count(&h, "Open in a new tab"), count(&h, "Delete attachment")), (3, 1, 3)); + h.key_press(egui::Key::Escape); + h.run_steps(3); + // The PDF's menu: Save writes the PDF, not the text file whose row comes last. + right_click(&mut h, "inner.pdf"); + assert_eq!(count(&h, "Open in a new tab"), 2, "the PDF's menu offers Open"); + h.state_mut().pick_override = Some(vec![target.to_string_lossy().into_owned()]); + h.get_all_by_label("Save attachment…").last().unwrap().click(); + h.run_steps(3); + assert_eq!(std::fs::read(&target).unwrap(), INNER, "Save from inner.pdf's menu"); + right_click(&mut h, "inner.pdf"); + h.get_all_by_label("Open in a new tab").last().unwrap().click(); + h.run_steps(3); + assert_eq!(h.state().views.len(), 2, "Open from the menu"); + h.state_mut().active = Some(0); + h.run_steps(3); + // Delete from inner.pdf's menu leaves notes.txt, whose row's button would delete itself. + right_click(&mut h, "inner.pdf"); + h.get_all_by_label("Delete attachment").last().unwrap().click(); + h.run_steps(3); + let s = h.state(); + let doc = s.session.get(s.views[0].id).unwrap(); + assert_eq!(doc.info.attachments.iter().map(|a| a.name.as_str()).collect::>(), ["notes.txt"], "Delete from the menu"); + assert!(doc.can_undo().is_some_and(|l| l.starts_with("Delete attachment")), "as one undoable edit"); +} + +#[test] +fn the_row_buttons_still_take_their_own_clicks() { + let dir = Dir::new("buttons"); + let target = dir.0.join("from button.txt"); + let mut h = harness(); + h.state_mut().pick_override = Some(vec![target.to_string_lossy().into_owned()]); + // The rows list inner.pdf first, then notes.txt: the second Save button is notes.txt's. + h.get_all_by_label("Save attachment…").nth(1).unwrap().click(); + h.run_steps(3); + assert_eq!(std::fs::read(&target).unwrap(), b"hello notes", "the Save button under the row's click area"); + h.get_by_label("Open in a new tab").click(); + h.run_steps(3); + assert_eq!(h.state().views.len(), 2, "the Open button"); +} + +/// CodeRabbit on #4513: a right-click on one of a row's buttons opens that row's menu too, as a +/// right-click anywhere on the row does. +#[test] +fn a_right_click_on_a_row_button_opens_the_row_menu() { + let mut h = harness(); + let count = |h: &Harness<'static, PdfCraftApp>, label: &str| h.query_all_by_label(label).count(); + // inner.pdf's row comes first: its Save button is the first one. + let at = h.get_all_by_label("Save attachment…").next().expect("inner.pdf's Save button").rect().center(); + h.hover_at(at); + h.run_steps(1); + h.event(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Secondary, pressed: true, modifiers: Default::default() }); + h.event(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Secondary, pressed: false, modifiers: Default::default() }); + h.run_steps(2); + assert_eq!(count(&h, "Open in a new tab"), 2, "inner.pdf's menu, with Open, is open"); + assert_eq!(count(&h, "Save attachment…"), 3); +} + +/// Where the attachment named `label` shows in the panel: the lowest text with that name, below +/// a tab of the same name once the attachment is open. +fn row_name(h: &Harness<'static, PdfCraftApp>, label: &str) -> egui::Pos2 { + h.get_all_by_label(label).map(|n| n.rect()).max_by(|a, b| a.top().total_cmp(&b.top())).expect("the attachment is listed").center() +} diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index ca96e8bba..12fc0cd92 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -21,6 +21,7 @@ mod accessibility; mod actions; mod animate; mod attach_picker_target; +mod attachment_row; mod bookmark_position; mod bookmarks; mod bulk_fields; diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index f11c7a308..c68808220 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -1322,8 +1322,8 @@ milestone = "M3" status = "shipped" commands = [] tools = ["doc_info"] -evidence = ["crates/render/src/inspect.rs::attachments_of_both_kinds_are_listed_and_extracted"] -notes = "" +evidence = ["crates/render/src/inspect.rs::attachments_of_both_kinds_are_listed_and_extracted", "crates/ui-egui/tests/it/attachment_row.rs::the_right_click_menu_opens_saves_and_deletes", "crates/ui-egui/tests/it/attachment_row.rs::double_clicking_a_pdf_attachment_opens_it_in_a_new_tab"] +notes = "Each row has Save, Open in a new tab (PDFs) and Delete (the document's own attachments) buttons, the same items on a right-click, and a double-click that opens a PDF in a new tab or saves any other file (#3466). Not yet: Acrobat's Search attachments, and opening a non-PDF attachment in another program (it is saved instead)." [[feature]] id = "view.layers-panel"