image_viewer: Fix PDF annotation tooling feedback and blank-space drags
- Show the armed tool on the annotation palette via the button's selected state; the previous selected_label workaround never rendered because the selected state was never set - Switch the page cursor to a crosshair while the pen tool is armed - Let highlight and text drags start on blank space and form the selection when the drag reaches text instead of dying silently - Cover the palette click, highlight, pen, and blank-space drag flows with a hermetic test
This commit is contained in:
1 parent
de4affc638
commit
38490b4554
1 file changed
+283
-8
@@ -1163,6 +1163,24 @@ impl ImageView {
|
|||||||
}
|
}
|
||||||
|
|
||||||
self.pdf_drag_position = Some(cursor_pos);
|
self.pdf_drag_position = Some(cursor_pos);
|
||||||
|
if self.pdf_selection.is_none() {
|
||||||
|
let hit = {
|
||||||
|
let item = self.image_item.read(cx);
|
||||||
|
let Some(pdf_info) = &item.pdf_info else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
self.find_page_and_char_at(cursor_pos, pdf_info)
|
||||||
|
};
|
||||||
|
if let Some((page_index, char_index)) = hit {
|
||||||
|
self.pdf_selection = Some(PdfSelection {
|
||||||
|
start_page: page_index,
|
||||||
|
start_char: char_index,
|
||||||
|
end_page: page_index,
|
||||||
|
end_char: char_index,
|
||||||
|
text: String::new(),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}
|
||||||
self.update_pdf_drag_selection(cursor_pos, cx);
|
self.update_pdf_drag_selection(cursor_pos, cx);
|
||||||
self.check_pdf_autoscroll(cx);
|
self.check_pdf_autoscroll(cx);
|
||||||
}
|
}
|
||||||
@@ -1235,9 +1253,11 @@ impl ImageView {
|
|||||||
self.ensure_pdf_selection_text(cx);
|
self.ensure_pdf_selection_text(cx);
|
||||||
}
|
}
|
||||||
None => {
|
None => {
|
||||||
self.pdf_mouse_down = false;
|
// Blank-space press: stay armed so a drag that later reaches
|
||||||
self.pdf_selection = None;
|
// text can form a selection; a click without drag selects
|
||||||
self.pdf_drag_position = None;
|
// nothing.
|
||||||
|
self.pdf_mouse_down = true;
|
||||||
|
self.pdf_drag_position = Some(click_pos);
|
||||||
self.pdf_autoscroll_task = None;
|
self.pdf_autoscroll_task = None;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -3508,6 +3528,7 @@ impl Render for ImageView {
|
|||||||
.child(
|
.child(
|
||||||
div()
|
div()
|
||||||
.id(("pdf-page-canvas", page_idx))
|
.id(("pdf-page-canvas", page_idx))
|
||||||
|
.debug_selector(move || format!("pdf-page-canvas-{page_idx}"))
|
||||||
.relative()
|
.relative()
|
||||||
.w(page_w)
|
.w(page_w)
|
||||||
.h(page_h)
|
.h(page_h)
|
||||||
@@ -3515,7 +3536,12 @@ impl Render for ImageView {
|
|||||||
.border_1()
|
.border_1()
|
||||||
.border_color(border_color)
|
.border_color(border_color)
|
||||||
.bg(gpui::white())
|
.bg(gpui::white())
|
||||||
.cursor(gpui::CursorStyle::IBeam)
|
.cursor(match self.annotation_tool {
|
||||||
|
PdfAnnotationTool::Pen => {
|
||||||
|
gpui::CursorStyle::Crosshair
|
||||||
|
}
|
||||||
|
_ => gpui::CursorStyle::IBeam,
|
||||||
|
})
|
||||||
.on_mouse_down(
|
.on_mouse_down(
|
||||||
MouseButton::Left,
|
MouseButton::Left,
|
||||||
cx.listener(move |this, event: &MouseDownEvent, window, cx| {
|
cx.listener(move |this, event: &MouseDownEvent, window, cx| {
|
||||||
@@ -3844,11 +3870,9 @@ impl Render for ImageView {
|
|||||||
};
|
};
|
||||||
let tool_button = |tool: PdfAnnotationTool, cx: &mut Context<Self>| {
|
let tool_button = |tool: PdfAnnotationTool, cx: &mut Context<Self>| {
|
||||||
let is_active = self.annotation_tool == tool;
|
let is_active = self.annotation_tool == tool;
|
||||||
let selected_label: Option<SharedString> =
|
|
||||||
is_active.then(|| SharedString::from(label(tool)));
|
|
||||||
Button::new(("pdf-annotation-tool", tool as usize), label(tool))
|
Button::new(("pdf-annotation-tool", tool as usize), label(tool))
|
||||||
.label_size(LabelSize::XSmall)
|
.label_size(LabelSize::XSmall)
|
||||||
.selected_label::<SharedString>(selected_label)
|
.toggle_state(is_active)
|
||||||
.on_click(cx.listener(move |this, _, _, cx| {
|
.on_click(cx.listener(move |this, _, _, cx| {
|
||||||
this.set_annotation_tool(tool, cx);
|
this.set_annotation_tool(tool, cx);
|
||||||
}))
|
}))
|
||||||
@@ -3856,6 +3880,7 @@ impl Render for ImageView {
|
|||||||
|
|
||||||
h_flex()
|
h_flex()
|
||||||
.id("pdf-annotation-tools")
|
.id("pdf-annotation-tools")
|
||||||
|
.debug_selector(|| "pdf-annotation-tools".into())
|
||||||
.absolute()
|
.absolute()
|
||||||
.top_2()
|
.top_2()
|
||||||
.left_2()
|
.left_2()
|
||||||
@@ -4632,7 +4657,9 @@ pub fn init(cx: &mut App) {
|
|||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
use fs::{FakeFs, Fs as _};
|
use fs::{FakeFs, Fs as _};
|
||||||
use gpui::{TestAppContext, VisualTestContext};
|
use gpui::{
|
||||||
|
Modifiers, MouseDownEvent, MouseMoveEvent, MouseUpEvent, TestAppContext, VisualTestContext,
|
||||||
|
};
|
||||||
use settings::SettingsStore;
|
use settings::SettingsStore;
|
||||||
use util::rel_path::rel_path;
|
use util::rel_path::rel_path;
|
||||||
|
|
||||||
@@ -5384,6 +5411,254 @@ mod tests {
|
|||||||
assert_eq!(cache.current_memory_bytes(), 0);
|
assert_eq!(cache.current_memory_bytes(), 0);
|
||||||
assert!(cache.is_empty());
|
assert!(cache.is_empty());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Single-page PDF with one text line ("Hello annotation world") near the
|
||||||
|
/// top, at the same spot the annotation drag test targets.
|
||||||
|
fn build_annotation_fixture() -> Vec<u8> {
|
||||||
|
let contents = b"BT /F1 24 Tf 72 700 Td (Hello annotation world) Tj ET";
|
||||||
|
let bodies: Vec<Vec<u8>> = vec![
|
||||||
|
b"<< /Type /Catalog /Pages 2 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 612 792] /Resources << /Font << /F1 4 0 R >> >> /Contents 5 0 R >>".to_vec(),
|
||||||
|
b"<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>".to_vec(),
|
||||||
|
format!(
|
||||||
|
"<< /Length {} >>\nstream\n{}\nendstream",
|
||||||
|
contents.len(),
|
||||||
|
String::from_utf8_lossy(contents)
|
||||||
|
)
|
||||||
|
.into_bytes(),
|
||||||
|
];
|
||||||
|
|
||||||
|
let mut out = b"%PDF-1.7\n".to_vec();
|
||||||
|
let mut offsets = Vec::new();
|
||||||
|
for (index, body) in bodies.iter().enumerate() {
|
||||||
|
offsets.push(out.len());
|
||||||
|
out.extend(format!("{} 0 obj\n", index + 1).into_bytes());
|
||||||
|
out.extend_from_slice(body);
|
||||||
|
out.extend(b"\nendobj\n");
|
||||||
|
}
|
||||||
|
let xref_offset = out.len();
|
||||||
|
out.extend(format!("xref\n0 {}\n", bodies.len() + 1).into_bytes());
|
||||||
|
out.extend(b"0000000000 65535 f \n");
|
||||||
|
for offset in &offsets {
|
||||||
|
out.extend(format!("{offset:010} 00000 n \n").into_bytes());
|
||||||
|
}
|
||||||
|
out.extend(
|
||||||
|
format!(
|
||||||
|
"trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{}\n%%EOF\n",
|
||||||
|
bodies.len() + 1,
|
||||||
|
xref_offset
|
||||||
|
)
|
||||||
|
.into_bytes(),
|
||||||
|
);
|
||||||
|
out
|
||||||
|
}
|
||||||
|
|
||||||
|
#[gpui::test]
|
||||||
|
async fn test_annotation_tools_apply_via_mouse(cx: &mut TestAppContext) {
|
||||||
|
init_test(cx);
|
||||||
|
let pdf_bytes = build_annotation_fixture();
|
||||||
|
let fs = FakeFs::new(cx.executor());
|
||||||
|
fs.create_dir(Path::new("/root"))
|
||||||
|
.await
|
||||||
|
.expect("test root should be created");
|
||||||
|
fs.insert_file("/root/annot.pdf", pdf_bytes).await;
|
||||||
|
|
||||||
|
let project = Project::test(fs, [Path::new("/root")], cx).await;
|
||||||
|
let worktree_id = cx.update(|cx| {
|
||||||
|
project
|
||||||
|
.read(cx)
|
||||||
|
.worktrees(cx)
|
||||||
|
.next()
|
||||||
|
.expect("test project should contain a worktree")
|
||||||
|
.read(cx)
|
||||||
|
.id()
|
||||||
|
});
|
||||||
|
let image_item = project
|
||||||
|
.update(cx, |project, cx| {
|
||||||
|
project.open_image(
|
||||||
|
ProjectPath {
|
||||||
|
worktree_id,
|
||||||
|
path: rel_path("annot.pdf").into(),
|
||||||
|
},
|
||||||
|
cx,
|
||||||
|
)
|
||||||
|
})
|
||||||
|
.await
|
||||||
|
.expect("test pdf should open");
|
||||||
|
|
||||||
|
let (image_view, cx) = cx
|
||||||
|
.add_window_view(|window, cx| ImageView::new(image_item.clone(), project, window, cx));
|
||||||
|
cx.run_until_parked();
|
||||||
|
cx.update(|window, _| window.refresh());
|
||||||
|
cx.run_until_parked();
|
||||||
|
|
||||||
|
let original_len = cx
|
||||||
|
.read(|cx| {
|
||||||
|
image_item
|
||||||
|
.read(cx)
|
||||||
|
.pdf_info
|
||||||
|
.as_ref()
|
||||||
|
.map(|i| i.pdf_bytes.len())
|
||||||
|
})
|
||||||
|
.expect("pdf should be loaded");
|
||||||
|
|
||||||
|
cx.update(|window, cx| {
|
||||||
|
image_view.update(cx, |view, cx| {
|
||||||
|
view.toggle_annotation_tools(&ToggleAnnotationTools, window, cx)
|
||||||
|
});
|
||||||
|
});
|
||||||
|
cx.run_until_parked();
|
||||||
|
cx.update(|window, _| window.refresh());
|
||||||
|
cx.run_until_parked();
|
||||||
|
|
||||||
|
// Click the palette's Highlight button (second tool button) like a user would.
|
||||||
|
let palette = cx
|
||||||
|
.debug_bounds("pdf-annotation-tools")
|
||||||
|
.expect("the annotation palette should be laid out");
|
||||||
|
cx.simulate_event(MouseDownEvent {
|
||||||
|
position: point(palette.origin.x + px(70.), palette.center().y),
|
||||||
|
button: MouseButton::Left,
|
||||||
|
modifiers: Modifiers::default(),
|
||||||
|
click_count: 1,
|
||||||
|
first_mouse: false,
|
||||||
|
});
|
||||||
|
cx.run_until_parked();
|
||||||
|
cx.simulate_event(MouseUpEvent {
|
||||||
|
position: point(palette.origin.x + px(70.), palette.center().y),
|
||||||
|
button: MouseButton::Left,
|
||||||
|
modifiers: Modifiers::default(),
|
||||||
|
click_count: 1,
|
||||||
|
});
|
||||||
|
cx.run_until_parked();
|
||||||
|
let armed = cx.read(|cx| image_view.read(cx).annotation_tool);
|
||||||
|
assert_eq!(
|
||||||
|
armed,
|
||||||
|
PdfAnnotationTool::Highlight,
|
||||||
|
"clicking the palette button should arm the highlight tool"
|
||||||
|
);
|
||||||
|
|
||||||
|
let canvas = cx
|
||||||
|
.debug_bounds("pdf-page-canvas-0")
|
||||||
|
.expect("page canvas should be laid out");
|
||||||
|
let drag = |cx: &mut VisualTestContext, from: Point<Pixels>, to: Point<Pixels>| {
|
||||||
|
cx.simulate_event(MouseDownEvent {
|
||||||
|
position: from,
|
||||||
|
button: MouseButton::Left,
|
||||||
|
modifiers: Modifiers::default(),
|
||||||
|
click_count: 1,
|
||||||
|
first_mouse: false,
|
||||||
|
});
|
||||||
|
cx.run_until_parked();
|
||||||
|
cx.simulate_event(MouseMoveEvent {
|
||||||
|
position: from + point(px(2.), px(0.)),
|
||||||
|
pressed_button: Some(MouseButton::Left),
|
||||||
|
modifiers: Modifiers::default(),
|
||||||
|
});
|
||||||
|
cx.run_until_parked();
|
||||||
|
cx.simulate_event(MouseMoveEvent {
|
||||||
|
position: to,
|
||||||
|
pressed_button: Some(MouseButton::Left),
|
||||||
|
modifiers: Modifiers::default(),
|
||||||
|
});
|
||||||
|
cx.run_until_parked();
|
||||||
|
cx.simulate_event(MouseUpEvent {
|
||||||
|
position: to,
|
||||||
|
button: MouseButton::Left,
|
||||||
|
modifiers: Modifiers::default(),
|
||||||
|
click_count: 1,
|
||||||
|
});
|
||||||
|
cx.run_until_parked();
|
||||||
|
};
|
||||||
|
|
||||||
|
let text_start = point(
|
||||||
|
canvas.origin.x + canvas.size.width * 0.25,
|
||||||
|
canvas.origin.y + canvas.size.height * 0.10,
|
||||||
|
);
|
||||||
|
let text_end = point(
|
||||||
|
canvas.origin.x + canvas.size.width * 0.55,
|
||||||
|
canvas.origin.y + canvas.size.height * 0.10,
|
||||||
|
);
|
||||||
|
drag(cx, text_start, text_end);
|
||||||
|
|
||||||
|
let (highlight_len, selection) = cx.read(|cx| {
|
||||||
|
(
|
||||||
|
image_view
|
||||||
|
.read(cx)
|
||||||
|
.pdf_edited_bytes
|
||||||
|
.as_ref()
|
||||||
|
.map(|bytes| bytes.len()),
|
||||||
|
image_view.read(cx).pdf_selection.clone(),
|
||||||
|
)
|
||||||
|
});
|
||||||
|
assert!(
|
||||||
|
highlight_len.is_some_and(|len| len > original_len),
|
||||||
|
"highlighting over text should commit edited bytes, got {highlight_len:?} vs {original_len}"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
selection.is_none(),
|
||||||
|
"the selection should clear after highlighting"
|
||||||
|
);
|
||||||
|
|
||||||
|
cx.update(|_window, cx| {
|
||||||
|
image_view.update(cx, |view, cx| {
|
||||||
|
view.set_annotation_tool(PdfAnnotationTool::Pen, cx);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
cx.run_until_parked();
|
||||||
|
|
||||||
|
let stroke_start = point(
|
||||||
|
canvas.origin.x + canvas.size.width * 0.3,
|
||||||
|
canvas.origin.y + canvas.size.height * 0.4,
|
||||||
|
);
|
||||||
|
let stroke_end = point(
|
||||||
|
canvas.origin.x + canvas.size.width * 0.6,
|
||||||
|
canvas.origin.y + canvas.size.height * 0.5,
|
||||||
|
);
|
||||||
|
drag(cx, stroke_start, stroke_end);
|
||||||
|
|
||||||
|
let pen_len = cx.read(|cx| {
|
||||||
|
image_view
|
||||||
|
.read(cx)
|
||||||
|
.pdf_edited_bytes
|
||||||
|
.as_ref()
|
||||||
|
.map(|bytes| bytes.len())
|
||||||
|
});
|
||||||
|
assert!(
|
||||||
|
pen_len.is_some_and(|len| len > highlight_len.unwrap_or(0)),
|
||||||
|
"pen stroke should grow the edited document further, got {pen_len:?} vs {highlight_len:?}"
|
||||||
|
);
|
||||||
|
|
||||||
|
// A highlight drag that starts on blank space must still commit once it
|
||||||
|
// reaches the text line.
|
||||||
|
cx.update(|_window, cx| {
|
||||||
|
image_view.update(cx, |view, cx| {
|
||||||
|
view.set_annotation_tool(PdfAnnotationTool::Highlight, cx)
|
||||||
|
});
|
||||||
|
});
|
||||||
|
cx.run_until_parked();
|
||||||
|
let blank_start = point(
|
||||||
|
canvas.origin.x + canvas.size.width * 0.5,
|
||||||
|
canvas.origin.y + canvas.size.height * 0.5,
|
||||||
|
);
|
||||||
|
let text_end = point(
|
||||||
|
canvas.origin.x + canvas.size.width * 0.5,
|
||||||
|
canvas.origin.y + canvas.size.height * 0.10,
|
||||||
|
);
|
||||||
|
drag(cx, blank_start, text_end);
|
||||||
|
|
||||||
|
let blank_drag_len = cx.read(|cx| {
|
||||||
|
image_view
|
||||||
|
.read(cx)
|
||||||
|
.pdf_edited_bytes
|
||||||
|
.as_ref()
|
||||||
|
.map(|bytes| bytes.len())
|
||||||
|
});
|
||||||
|
assert!(
|
||||||
|
blank_drag_len.is_some_and(|len| len > pen_len.unwrap_or(0)),
|
||||||
|
"a drag starting on blank space should still highlight text, got {blank_drag_len:?} vs {pen_len:?}"
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
mod persistence {
|
mod persistence {
|
||||||
|
|||||||
Reference in new issue
Block a user