mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-11-01 14:08:23 -06:00
fix(ui): review fixes for the page router: Escape on text drags, the open frame's sides, a stale owner
From a review of this branch: - Escape mid-drag with a text markup tool (Highlight, Underline, Strikeout, Squiggly, Replace Text) or Redact over text left the half-made selection on the page: the cancel took the Escape that would have cleared it. It is dropped now. The marks made from a selection at its end (after the page tools, which the release blocking did not reach) also wait for a cancelled press's button; egui ends its own drag on Escape without a drag stop, so they were not made in practice, but nothing but egui stopped them. - The open paragraph's frame took the press above and below the text only: its sides still gave edge grips (a closed paragraph's rewrap handles) on the Editor layer, over the frame. Edges now belong to closed paragraphs only. - A press in the frame right after a release kept the last press's owner until a page claimed it, so a drag from the margin beside a page could move the comment just clicked. A new press now drops the last owner before the pages give it theirs. - A release and a new press in one frame no longer leave the new press cancelled. - Added content is picked up with the tools that act on it (Select, Add text, Edit text), not with every tool while Edit a PDF is open. - The layer docs in page_cursor.rs say what lies on each layer now. New tests an_open_paragraphs_frame_takes_the_box_beside_its_text and a_drag_from_beside_the_page_right_after_a_click_leaves_the_comment_alone (the release and the next press in one frame) fail on the previous commit. escape_cancels_a_highlight_being_dragged_over_text (with a control: the drag highlights without Escape) passes there too, for the reason above, and stays as a guard. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
bcc71601d4
commit
4b3033c869
@@ -1444,7 +1444,8 @@ impl PageHits<'_> {
|
||||
};
|
||||
// Added text and pictures, over the page's own paragraphs and under the objects selected
|
||||
// together.
|
||||
let added = (editing_content && can_modify).then(|| crate::content_ui::hit(xf, info, page, added, view, p)).flatten();
|
||||
let picks_up = matches!(tool, QuickTool::Select | QuickTool::AddText | QuickTool::EditText);
|
||||
let added = (editing_content && can_modify && picks_up).then(|| crate::content_ui::hit(xf, info, page, added, view, p)).flatten();
|
||||
[link_hit(info, xf, page, p), comments::hit(pcx, view, p), pictures, paragraphs, added, objects, grips, fields]
|
||||
.into_iter()
|
||||
.flatten()
|
||||
@@ -1688,9 +1689,12 @@ fn page_overlays(ui: &mut egui::Ui, view: &mut DocView, frame: &mut PageFrame<'_
|
||||
}
|
||||
crate::image_pick::drop_stale(doc, view);
|
||||
crate::image_pick::paint(painter, &xf, i, info, view);
|
||||
comments::page_after_text(resp, &pcx, view);
|
||||
if tool == QuickTool::Redact && can_modify {
|
||||
crate::redact_ui::after_text(resp, i, info, view);
|
||||
// A cancelled press ends no selection: no markup or redaction is made from it.
|
||||
if !view.press_cancelled {
|
||||
comments::page_after_text(resp, &pcx, view);
|
||||
if tool == QuickTool::Redact && can_modify {
|
||||
crate::redact_ui::after_text(resp, i, info, view);
|
||||
}
|
||||
}
|
||||
if input.area_tool {
|
||||
crate::redact_ui::paint(painter, i, view, pointer);
|
||||
@@ -1892,11 +1896,19 @@ fn link_hit(info: &DocInfo, xf: &PageXform, page: usize, p: Pos2) -> Option<crat
|
||||
/// placed, drawn, moved, resized or opened, and the press is ignored until the button is up. The
|
||||
/// frame of the release still belongs to the cancelled press.
|
||||
fn cancel_on_escape(ctx: &egui::Context, view: &mut DocView, ptr: &PagePointer) {
|
||||
if view.press_cancelled && !ptr.any_down && ptr.edges.is_empty() {
|
||||
// Over once the button is up and its release seen, or when a new press comes (its release and
|
||||
// the next press can share a frame).
|
||||
let new_press = matches!(ptr.edges.last(), Some(crate::page_pointer::Edge::Press(_)));
|
||||
if view.press_cancelled && (new_press || !ptr.any_down && ptr.edges.is_empty()) {
|
||||
view.press_cancelled = false;
|
||||
}
|
||||
if !view.press_cancelled && ptr.origin.is_some() && ptr.primary_down && ctx.input_mut(|i| i.consume_key(egui::Modifiers::NONE, egui::Key::Escape))
|
||||
{
|
||||
// A text selection being dragged goes with its drag: the markup, redaction or Replace
|
||||
// Text that its end would make is not made.
|
||||
if view.selection_drag {
|
||||
view.clear_selection();
|
||||
}
|
||||
view.cancel_pointer_gestures();
|
||||
view.press_cancelled = true;
|
||||
}
|
||||
@@ -1983,6 +1995,11 @@ fn draw_pages(
|
||||
let ptr = PagePointer::read(ui, &resp, modal);
|
||||
cancel_on_escape(ui.ctx(), view, &ptr);
|
||||
view.router.settle(ptr.any_down, !ptr.edges.is_empty());
|
||||
// A new press is no longer the last one's, wherever it lands (the page it is on gives it
|
||||
// its owner; in the gap between pages it has none).
|
||||
if ptr.pressed().is_some() {
|
||||
view.router.cancel();
|
||||
}
|
||||
let pointer = ptr.pos;
|
||||
// A press on a page makes it the current one (as in Acrobat), so a page command with
|
||||
// no selection (Rotate, Delete) acts on the page the reader pointed at, not on a page
|
||||
|
||||
@@ -20,18 +20,19 @@ use crate::QuickTool;
|
||||
pub(crate) enum Layer {
|
||||
/// The page under the active tool: the crosshair it places or draws with, its I-beam or hand.
|
||||
ToolArea,
|
||||
/// The page's text, to select, and Edit text's paragraphs.
|
||||
/// The page's text, to select; Edit text's paragraphs, and its pictures beneath them.
|
||||
Text,
|
||||
/// Links.
|
||||
Link,
|
||||
/// Comments: notes, shapes and markup.
|
||||
/// Text markup (highlights, underlines…), which lies on the text: under a form field.
|
||||
Comment,
|
||||
/// Form fields.
|
||||
Field,
|
||||
/// What a drag moves whole: signatures and Fill & Sign's marks, added text and images,
|
||||
/// selected objects, pictures in Edit text.
|
||||
/// What a drag moves whole: notes, shapes, signatures and Fill & Sign's marks, added text and
|
||||
/// pictures, the objects selected together, Prepare's fields.
|
||||
Placed,
|
||||
/// The grips of the selected item: resize, rewrap and crop handles, paragraph edges.
|
||||
/// The grips of the selected item: resize, rewrap, rotate and crop handles, paragraph edges,
|
||||
/// the frame of the paragraph open for typing.
|
||||
Handle,
|
||||
/// An open editor: a paragraph being typed in, a picture being cropped.
|
||||
Editor,
|
||||
@@ -45,9 +46,10 @@ const LAYERS: [Layer; 9] =
|
||||
|
||||
impl Layer {
|
||||
/// Whether a press with `tool` acts on what lies on this layer, so that the layer's claims
|
||||
/// count. Only the Select tool follows links (the Link tool edits them) and picks up
|
||||
/// comments: every other tool draws, places or selects text over them. The other layers are
|
||||
/// claimed only by the modules of the tools that act on them, so they count for every tool.
|
||||
/// count. Only the Select tool follows links (the Link tool edits them) and selects text
|
||||
/// markup: every other tool draws, places or selects text over them. The other layers are
|
||||
/// claimed only by the modules of the tools that act on them (each module's hit test filters
|
||||
/// by tool), so they count for every tool.
|
||||
pub(crate) fn reached_by(self, tool: QuickTool) -> bool {
|
||||
match self {
|
||||
Layer::Link => matches!(tool, QuickTool::Select | QuickTool::Link),
|
||||
|
||||
@@ -289,7 +289,8 @@ pub(crate) fn hit(xf: &PageXform, info: &DocInfo, page: usize, lines: &[pdfcraft
|
||||
return None;
|
||||
}
|
||||
let boxes: Vec<Rect> = lines.iter().map(|l| paragraph_box(xf, info, page, l.rect)).collect();
|
||||
let (block, grip) = grab_at(&boxes, p, has_edges(info, page))?;
|
||||
// Edges rewrap closed paragraphs only: around the open one lies its frame (`object_ui`).
|
||||
let (block, grip) = grab_at(&boxes, p, has_edges(info, page) && active.is_none())?;
|
||||
let paragraph = |part| Owner::Paragraph { block, part };
|
||||
Some(match (active, grip) {
|
||||
(None, Grip::Left) => Hit::new(paragraph(ParagraphPart::Left), Layer::Handle, egui::CursorIcon::ResizeHorizontal),
|
||||
|
||||
@@ -4357,6 +4357,30 @@ fn an_open_paragraphs_frame_shows_move_wherever_a_press_takes_the_box() {
|
||||
assert!(h.state().views[0].line_editor.is_none(), "and a click there selects the box");
|
||||
}
|
||||
|
||||
/// The open paragraph's frame takes hold of the box on its sides too: there the edges of a closed
|
||||
/// paragraph would rewrap it, but around the open one lies its frame.
|
||||
#[test]
|
||||
fn an_open_paragraphs_frame_takes_the_box_beside_its_text() {
|
||||
let mut h = harness(2, |_| {});
|
||||
assert!(h.state_mut().execute("edit.edit_text"));
|
||||
h.run_steps(2);
|
||||
let id = h.state().views[0].id;
|
||||
let before = h.state().session.get(id).unwrap().text_blocks(0)[0].clone();
|
||||
let r = h.state().views[0].page_screen_rect(0).expect("on screen");
|
||||
let k = r.width() / 200.0;
|
||||
let screen = |x: f64, y: f64| egui::pos2(r.left() + x as f32 * k, r.top() + (300.0 - y as f32) * k);
|
||||
let middle = (before.rect[1] + before.rect[3]) / 2.0;
|
||||
click_at(&mut h, screen((before.rect[0] + before.rect[2]) / 2.0, middle));
|
||||
assert!(h.state().views[0].line_editor.is_some(), "a click on the text opens it for typing");
|
||||
// 6 px left of the text: 4 px past the frame drawn 2 px out.
|
||||
let beside = screen(before.rect[0], middle) - egui::vec2(6.0, 0.0);
|
||||
h.hover_at(beside);
|
||||
h.run_steps(3);
|
||||
assert_eq!(h.output().platform_output.cursor_icon, egui::CursorIcon::Move, "the frame offers to move the box there");
|
||||
click_at(&mut h, beside);
|
||||
assert!(h.state().views[0].line_editor.is_none(), "and a click there selects the box");
|
||||
}
|
||||
|
||||
/// Contributor-original text, with an off-origin CropBox. Keep the moving paragraph last in
|
||||
/// source order so an ordinary hit picks it again when two independent boxes fully overlap.
|
||||
fn paragraph_drag_fixture(content: &str, rotation: i64, unit: i64) -> Vec<u8> {
|
||||
|
||||
@@ -802,6 +802,57 @@ fn escape_cancels_a_signature_rectangle() {
|
||||
assert!(s.dialog.is_none(), "and no signing dialog opens: {:?}", s.dialog);
|
||||
}
|
||||
|
||||
/// A highlight dragged over the line of [`text_pdf`] at 150 %, from x 45 to x 150, with Escape
|
||||
/// before the release when `escape`; the comments then on the page.
|
||||
fn drag_a_highlight(escape: bool) -> usize {
|
||||
let mut h = open(text_pdf(), "150", false);
|
||||
h.state_mut().quick_tool = QuickTool::Comment(CommentTool::Highlight);
|
||||
h.run_steps(4);
|
||||
let (a, b) = (at(&h, 45.0, 303.0), at(&h, 150.0, 303.0));
|
||||
assert_eq!(wait_for_cursor(&mut h, a, |c| c == egui::CursorIcon::Text), egui::CursorIcon::Text, "the page's text has arrived");
|
||||
h.drag_at(a);
|
||||
h.run_steps(1);
|
||||
h.hover_at(b);
|
||||
h.run_steps(2);
|
||||
if escape {
|
||||
h.key_press(egui::Key::Escape);
|
||||
h.run_steps(1);
|
||||
}
|
||||
h.drop_at(b);
|
||||
h.run_steps(4);
|
||||
let s = h.state();
|
||||
s.session.get(s.views[0].id).unwrap().info.annotations.len()
|
||||
}
|
||||
|
||||
/// Escape mid-drag abandons a highlight being drawn over text: the release makes no highlight.
|
||||
#[test]
|
||||
fn escape_cancels_a_highlight_being_dragged_over_text() {
|
||||
assert_eq!(drag_a_highlight(false), 1, "control: the drag highlights the text");
|
||||
assert_eq!(drag_a_highlight(true), 0, "with Escape before the release, no highlight is made");
|
||||
}
|
||||
|
||||
/// A click on a comment, and in the frame of its release a press beside the page, then a drag
|
||||
/// from there: the drag is not the comment's, so it does not move it.
|
||||
#[test]
|
||||
fn a_drag_from_beside_the_page_right_after_a_click_leaves_the_comment_alone() {
|
||||
let mut h = open(comment_on_text_pdf(), "150", false);
|
||||
let on_comment = at(&h, 120.0, 315.0);
|
||||
let beside = page(&h).left_center() - egui::vec2(20.0, 0.0);
|
||||
let before = square(&h).expect("the rectangle");
|
||||
h.hover_at(on_comment);
|
||||
h.run_steps(1);
|
||||
h.drag_at(on_comment);
|
||||
h.run_steps(1);
|
||||
let button = |pos, pressed| egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, modifiers: egui::Modifiers::NONE };
|
||||
one_frame(&mut h, [button(on_comment, false), egui::Event::PointerMoved(beside), button(beside, true)]);
|
||||
h.run_steps(1);
|
||||
h.hover_at(beside + egui::vec2(0.0, 60.0));
|
||||
h.run_steps(2);
|
||||
h.drop_at(beside + egui::vec2(0.0, 60.0));
|
||||
h.run_steps(4);
|
||||
assert_eq!(square(&h), Some(before), "the comment stays where it was");
|
||||
}
|
||||
|
||||
/// Only the primary button draws a signature rectangle: a right-drag opens no signing dialog.
|
||||
#[test]
|
||||
fn a_right_drag_draws_no_signature_rectangle() {
|
||||
|
||||
Reference in New Issue
Block a user