mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-11-01 05:58:12 -06:00
Alt+Left / Alt+Right go back and forward through the view history (#3632)
Previous view and Next view had only Cmd+[ / Cmd+] (Ctrl elsewhere), and Alt+Left turned the page, since the arrow keys ignore Alt. The document view now takes Alt+Left / Alt+Right (Option on macOS) for them, as Acrobat and browsers do, before the arrows turn the page. Following a link already records where it was followed from. With a picture or objects selected in Edit text & images, the arrow-key nudge ran first and also matched a press with Alt, so Alt+Left moved the selection. It now takes only arrows pressed alone or with Shift, judged by each key press's own modifiers rather than the frame's. Help > Keyboard shortcuts lists the pair under the existing Previous view / Next view labels.
This commit is contained in:
@@ -369,10 +369,14 @@ pub fn shortcuts(view: &mut DocView, ctx: &egui::Context, speed: crate::scroll_s
|
||||
view.rotate_view(false);
|
||||
}
|
||||
zoom_shortcuts(view, &pressed);
|
||||
if pressed(cmd(Key::OpenBracket)) {
|
||||
// Alt+← / Alt+→ (⌥ on macOS) too, as in Acrobat and browsers, back to where a link was
|
||||
// followed from (#3632). Consumed here, before ← / → turn the page. egui matches the pattern
|
||||
// logically, so an extra Shift is ignored and Shift+Alt+← goes back as well (harmless).
|
||||
let alt = |k| KeyboardShortcut::new(Modifiers::ALT, k);
|
||||
if pressed(cmd(Key::OpenBracket)) || pressed(alt(Key::ArrowLeft)) {
|
||||
view.view_history(false);
|
||||
}
|
||||
if pressed(cmd(Key::CloseBracket)) {
|
||||
if pressed(cmd(Key::CloseBracket)) || pressed(alt(Key::ArrowRight)) {
|
||||
view.view_history(true);
|
||||
}
|
||||
let key = |k| (arrows || !matches!(k, Key::ArrowLeft | Key::ArrowRight | Key::ArrowUp | Key::ArrowDown)) && ctx.input(|i| i.key_pressed(k));
|
||||
|
||||
@@ -1463,6 +1463,13 @@ fn dialog_shortcuts(ui: &mut egui::Ui) -> bool {
|
||||
let pair = |a, b| format!("{} / {}", key(a), key(b));
|
||||
// Key names, translated where a catalog has them (not scanned as UI literals).
|
||||
let named = |k: &str| tl!(k).to_string();
|
||||
// Alt+← / Alt+→ (`canvas::shortcuts`): not a registered command, which has no Alt.
|
||||
let alt_arrows = if crate::commands::mac_shortcuts(ui.ctx()) {
|
||||
"⌥← / ⌥→".to_owned()
|
||||
} else {
|
||||
let alt = crate::i18n::key_name("Alt");
|
||||
format!("{alt}+← / {alt}+→")
|
||||
};
|
||||
let scroll = crate::i18n::fmt(tl!("Zoom in / out (also pinch or {key}-scroll)"), &[("key", crate::commands::command_modifier_label(ui.ctx()))]);
|
||||
for (k, v) in [
|
||||
(pair(FIND_NEXT, FIND_PREV), tl!("Next / previous match").to_string()),
|
||||
@@ -1476,6 +1483,7 @@ fn dialog_shortcuts(ui: &mut egui::Ui) -> bool {
|
||||
(pair(ROTATE_CW, ROTATE_CCW), tl!("Rotate view").to_string()),
|
||||
(format!("{} / {}", crate::i18n::key_name("Home"), crate::i18n::key_name("End")), tl!("First / last page").to_string()),
|
||||
(format!("← / →, {}", pair(PAGE_PREV, PAGE_NEXT)), tl!("Previous / next page").to_string()),
|
||||
(alt_arrows, format!("{} / {}", tl!("Previous view"), tl!("Next view"))),
|
||||
(named("V"), tl!("Select (V)").to_string()),
|
||||
(named("H / Space (hold)"), tl!("Hand (H)").to_string()),
|
||||
(crate::i18n::key_name("Delete").to_string(), tl!("Delete selected pages (Organize)").to_string()),
|
||||
|
||||
@@ -351,22 +351,28 @@ pub(crate) fn nudge_keys(ctx: &egui::Context, view: &mut DocView, doc: &Document
|
||||
let objects = view.objects.page.filter(|_| !view.objects.selected.is_empty() && view.objects.gesture.is_none());
|
||||
let Some(page) = objects.or(image.map(|(page, _)| page)) else { return };
|
||||
let Some(xf) = view.page_xform(page) else { return };
|
||||
use egui::{Key, Modifiers};
|
||||
use egui::Key;
|
||||
let mut step = egui::Vec2::ZERO;
|
||||
ctx.input_mut(|i| {
|
||||
for (key, direction) in [
|
||||
(Key::ArrowLeft, egui::vec2(-1.0, 0.0)),
|
||||
(Key::ArrowRight, egui::vec2(1.0, 0.0)),
|
||||
(Key::ArrowUp, egui::vec2(0.0, -1.0)),
|
||||
(Key::ArrowDown, egui::vec2(0.0, 1.0)),
|
||||
] {
|
||||
// Shift first: a plain key pattern also matches a Shift press.
|
||||
if i.consume_key(Modifiers::SHIFT, key) {
|
||||
step += direction * 10.0;
|
||||
} else if i.consume_key(Modifiers::NONE, key) {
|
||||
step += direction;
|
||||
// Only an arrow pressed alone or with Shift, judged by each press's own modifiers (the
|
||||
// frame's can differ when a key is let go in the same frame). Alt+← / Alt+→ go back and
|
||||
// forward through the view history (#3632) and Ctrl/⌘+← / → turn the page, both in
|
||||
// `canvas::shortcuts`.
|
||||
i.events.retain(|e| {
|
||||
let egui::Event::Key { key, pressed: true, modifiers, .. } = e else { return true };
|
||||
if modifiers.alt || modifiers.ctrl || modifiers.command || modifiers.mac_cmd {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
let direction = match key {
|
||||
Key::ArrowLeft => egui::vec2(-1.0, 0.0),
|
||||
Key::ArrowRight => egui::vec2(1.0, 0.0),
|
||||
Key::ArrowUp => egui::vec2(0.0, -1.0),
|
||||
Key::ArrowDown => egui::vec2(0.0, 1.0),
|
||||
_ => return true,
|
||||
};
|
||||
step += direction * if modifiers.shift { 10.0 } else { 1.0 };
|
||||
false
|
||||
});
|
||||
});
|
||||
if step == egui::Vec2::ZERO {
|
||||
return;
|
||||
|
||||
@@ -4242,8 +4242,9 @@ fn press(h: &mut Harness<'static, PdfCraftApp>, modifiers: Modifiers, key: Key)
|
||||
h.run_steps(4);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn arrow_keys_nudge_a_selected_image_instead_of_turning_the_page() {
|
||||
/// A two-page document whose first page draws one picture (a Form XObject), open in Edit text
|
||||
/// & images with the picture selected.
|
||||
fn selected_figure() -> Harness<'static, PdfCraftApp> {
|
||||
use pdfcraft_cos::{Dict, Document, Object, SaveOptions, Stream, write_incremental};
|
||||
let mut doc = Document::open(std::sync::Arc::new(fixture(2))).unwrap();
|
||||
let page = pdfcraft_model::pages(&doc)[0].clone();
|
||||
@@ -4271,12 +4272,18 @@ fn arrow_keys_nudge_a_selected_image_instead_of_turning_the_page() {
|
||||
h.run_steps(4);
|
||||
assert!(h.state_mut().execute("edit.edit_text"));
|
||||
h.run_steps(2);
|
||||
let id = h.state().views[0].id;
|
||||
let rect = |h: &Harness<'static, PdfCraftApp>| h.state().session.get(id).unwrap().page_images(0)[0].rect;
|
||||
let before = rect(&h);
|
||||
let r = h.state().views[0].page_screen_rect(0).expect("on screen");
|
||||
click_at(&mut h, egui::pos2(r.left() + 100.0 / 200.0 * r.width(), r.top() + (300.0 - 240.0) / 300.0 * r.height()));
|
||||
assert!(h.state().views[0].image_selection.is_some(), "selected");
|
||||
h
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn arrow_keys_nudge_a_selected_image_instead_of_turning_the_page() {
|
||||
let mut h = selected_figure();
|
||||
let id = h.state().views[0].id;
|
||||
let rect = |h: &Harness<'static, PdfCraftApp>| h.state().session.get(id).unwrap().page_images(0)[0].rect;
|
||||
let before = rect(&h);
|
||||
// As in Acrobat: an arrow moves it a point, Shift and an arrow ten.
|
||||
press(&mut h, Modifiers::NONE, Key::ArrowRight);
|
||||
press(&mut h, Modifiers::SHIFT, Key::ArrowDown);
|
||||
@@ -4289,6 +4296,46 @@ fn arrow_keys_nudge_a_selected_image_instead_of_turning_the_page() {
|
||||
assert!(h.state().views[0].image_selection.is_some(), "it stays selected for the next nudge");
|
||||
}
|
||||
|
||||
/// #3632: with a picture selected, Alt+← / Alt+→ still go back and forward through the view
|
||||
/// history. They don't nudge it (the nudge patterns also match a press with Alt), so nothing moves
|
||||
/// and the document isn't edited.
|
||||
#[test]
|
||||
fn alt_arrows_navigate_instead_of_nudging_a_selected_image() {
|
||||
let mut h = selected_figure();
|
||||
let id = h.state().views[0].id;
|
||||
let rect = |h: &Harness<'static, PdfCraftApp>| h.state().session.get(id).unwrap().page_images(0)[0].rect;
|
||||
let before = rect(&h);
|
||||
// A followed link's jump to page 2 and back, as `go_to_dest` makes it.
|
||||
h.state_mut().views[0].go_to_page(1);
|
||||
h.run_steps(2);
|
||||
h.state_mut().views[0].go_to_page(0);
|
||||
h.run_steps(2);
|
||||
assert!(h.state().views[0].image_selection.is_some(), "still selected, so a nudge could take the keys");
|
||||
press(&mut h, Modifiers::ALT, Key::ArrowLeft);
|
||||
assert_eq!(h.state().views[0].current, 1, "Alt+← went back");
|
||||
press(&mut h, Modifiers::ALT, Key::ArrowRight);
|
||||
assert_eq!(h.state().views[0].current, 0, "Alt+→ went forward");
|
||||
// Each press is judged by its own modifiers, not the frame's: Alt let go in the same frame
|
||||
// as Alt+← leaves the frame with none, and the press still goes back.
|
||||
let arrow = |key, modifiers| egui::Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers };
|
||||
h.event(arrow(Key::ArrowLeft, Modifiers::ALT));
|
||||
h.run_steps(4);
|
||||
assert_eq!(h.state().views[0].current, 1, "an Alt+← press went back");
|
||||
assert_eq!(rect(&h), before, "the picture didn't move");
|
||||
let doc = h.state().session.get(id).unwrap();
|
||||
assert!(!doc.dirty && doc.can_undo().is_none(), "nothing was edited");
|
||||
// And a plain → pressed before Alt goes down in the same frame still nudges.
|
||||
h.state_mut().views[0].go_to_page(0);
|
||||
h.run_steps(2);
|
||||
h.event(arrow(Key::ArrowRight, Modifiers::NONE));
|
||||
h.event(egui::Event::ModifiersChanged(Modifiers::ALT));
|
||||
h.run_steps(4);
|
||||
h.event(egui::Event::ModifiersChanged(Modifiers::NONE));
|
||||
h.run_steps(2);
|
||||
let moved = rect(&h);
|
||||
assert!((moved[0] - before[0] - 1.0).abs() < 0.01, "a plain → nudged a point: {before:?} → {moved:?}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_open_paragraph_moves_by_its_frame_and_by_the_arrow_keys() {
|
||||
let mut h = harness(2, |_| {});
|
||||
|
||||
@@ -70,6 +70,33 @@ fn revert_after_confirming() {
|
||||
assert_eq!((d.info.pages.len(), d.dirty), (2, false));
|
||||
}
|
||||
|
||||
/// #3632: Alt+← / Alt+→ go back to where a link was followed from and forward again, as in
|
||||
/// Acrobat and browsers, while ← / → still turn the page.
|
||||
#[test]
|
||||
fn alt_arrows_go_back_and_forward_through_the_view_history() {
|
||||
use egui::{Key, Modifiers};
|
||||
let mut h = harness();
|
||||
h.state_mut().active = Some(0);
|
||||
h.run_steps(2);
|
||||
// A followed link jumps the same way (`go_to_dest` → `go_to_page`).
|
||||
h.state_mut().views[0].go_to_page(3);
|
||||
h.run_steps(2);
|
||||
h.key_press_modifiers(Modifiers::ALT, Key::ArrowLeft);
|
||||
h.run_steps(2);
|
||||
assert_eq!(h.state().views[0].current, 0, "back to where the jump started");
|
||||
h.key_press_modifiers(Modifiers::ALT, Key::ArrowRight);
|
||||
h.run_steps(2);
|
||||
assert_eq!(h.state().views[0].current, 3, "forward to the jump's target");
|
||||
h.key_press(Key::ArrowLeft);
|
||||
h.run_steps(2);
|
||||
assert_eq!(h.state().views[0].current, 2, "← still turns the page");
|
||||
h.ctx.set_os(egui::os::OperatingSystem::Nix);
|
||||
assert!(h.state_mut().execute("help.shortcuts"));
|
||||
h.run_steps(3);
|
||||
h.get_by_label("Alt+← / Alt+→");
|
||||
h.get_by_label("Previous view / Next view");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn view_history_select_all_and_find_options() {
|
||||
let mut h = harness();
|
||||
|
||||
@@ -1178,8 +1178,8 @@ milestone = "M3"
|
||||
status = "shipped"
|
||||
commands = ["view.previous_view", "view.next_view"]
|
||||
tools = []
|
||||
evidence = ["crates/ui-egui/tests/it/viewer.rs::view_history_select_all_and_find_options", "crates/ui-egui/tests/it/control.rs::navigation_history_commands_and_rail_are_per_tab_and_non_editing", "crates/ui-egui/tests/it/control.rs::routine_continuous_scrolling_does_not_add_navigation_history"]
|
||||
notes = "Per-tab page history: explicit search-result jumps, page navigation, bookmarks and links; no continuous-scroll history or restart persistence; existing page-step history is retained. Rail buttons, View ▸ Page navigation and Ctrl/⌘[ / Ctrl/⌘]. UI control ui.command exposes view.previous_view / view.next_view; standalone engine sessions have no viewport history. Restores page tops, not exact scroll/zoom positions."
|
||||
evidence = ["crates/ui-egui/tests/it/viewer.rs::view_history_select_all_and_find_options", "crates/ui-egui/tests/it/control.rs::navigation_history_commands_and_rail_are_per_tab_and_non_editing", "crates/ui-egui/tests/it/control.rs::routine_continuous_scrolling_does_not_add_navigation_history", "crates/ui-egui/tests/it/viewer.rs::alt_arrows_go_back_and_forward_through_the_view_history", "crates/ui-egui/tests/it/editing.rs::alt_arrows_navigate_instead_of_nudging_a_selected_image", "crates/ui-egui/tests/it/comments.rs::alt_arrows_navigate_instead_of_nudging_the_selected_comment"]
|
||||
notes = "Per-tab page history: explicit search-result jumps, page navigation, bookmarks and links; no continuous-scroll history or restart persistence; existing page-step history is retained. Rail buttons, View ▸ Page navigation, Ctrl/⌘[ / Ctrl/⌘] and Alt/⌥+← / →, also with a picture, object or comment selected (the View menu shows only the bracket shortcuts). UI control ui.command exposes view.previous_view / view.next_view; standalone engine sessions have no viewport history. Restores page tops, not exact scroll/zoom positions."
|
||||
|
||||
[[feature]]
|
||||
id = "view.follow-links"
|
||||
|
||||
Reference in New Issue
Block a user