diff --git a/crates/ui-egui/src/canvas/navigation.rs b/crates/ui-egui/src/canvas/navigation.rs index 46468a112d..731b2c2586 100644 --- a/crates/ui-egui/src/canvas/navigation.rs +++ b/crates/ui-egui/src/canvas/navigation.rs @@ -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)); diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 138b18e1c5..ebe5110cff 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -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()), diff --git a/crates/ui-egui/src/tools/object_ui.rs b/crates/ui-egui/src/tools/object_ui.rs index 96cc0adf25..114ccbd436 100644 --- a/crates/ui-egui/src/tools/object_ui.rs +++ b/crates/ui-egui/src/tools/object_ui.rs @@ -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; diff --git a/crates/ui-egui/tests/it/editing.rs b/crates/ui-egui/tests/it/editing.rs index 0d44f91067..6b5ab31bc9 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/crates/ui-egui/tests/it/editing.rs @@ -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, |_| {}); diff --git a/crates/ui-egui/tests/it/viewer.rs b/crates/ui-egui/tests/it/viewer.rs index f2a12b69aa..9b37970670 100644 --- a/crates/ui-egui/tests/it/viewer.rs +++ b/crates/ui-egui/tests/it/viewer.rs @@ -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(); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 51b4f55aea..897ed017bc 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -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"