mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-10-26 02:58:32 -06:00
* M3: an attachment's row opens on a double-click and has a right-click menu (#3466) In Acrobat, a right-click on an attachment offers Open and Save, and a double-click opens it. PdfCraft's Attachments panel had only the buttons at the end of each row. Each row now senses clicks beneath its buttons (`UiBuilder::sense`, which egui registers under the row's children, so the buttons keep their own clicks): - A double-click opens a PDF in a new tab, as the Open button does, and saves any other file, since PdfCraft opens only PDFs. - A right-click offers the row's own actions with the buttons' labels: Open in a new tab (PDFs), Save attachment… and Delete attachment (the document's own attachments, when it may be modified). The name and details labels are no longer selectable, so a click on them reaches the row. * M3: a right-click on an attachment row's button opens the row's menu too (#3466) CodeRabbit on #4513: the menu hung on the row's response only, so a right-click on its Save, Open or Delete button opened nothing. The menu now answers the union of the row and its buttons (one menu, the row's id). Regression test right-clicks a button (fails without the fix). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LymhVaGZCWwB9HNEbLzeMM --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Livio Oswald
co-authored by
Claude Opus 5.5
Claude Opus 5.5
parent
7dd44301db
commit
0c124edc3e
@@ -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<egui::Response> = 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();
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<u8> {
|
||||
let objs: Vec<Vec<u8>> = 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::<Vec<_>>(), ["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()
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
@@ -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"
|
||||
|
||||
Reference in New Issue
Block a user