diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 2c2ee35d7..0bd62621b 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -470,6 +470,9 @@ pub struct DocView { clicks: u32, last_press_time: f64, last_press_pos: Option, + /// Full screen: a click's page turn, when it falls due (input time) and which way. It waits + /// out the double-click time, as the click may be the first of a double-click selecting a word. + present_turn: Option<(f64, bool)>, last_queue: Vec, /// Finished renders wait in the pool instead of reaching the view (tests): what the view /// shows before a raster arrives can be looked at however fast the renderer is. @@ -761,6 +764,7 @@ impl DocView { clicks: 0, last_press_time: 0.0, last_press_pos: None, + present_turn: None, last_queue: Vec::new(), hold_rasters: false, viewport_w: 800.0, @@ -4042,9 +4046,24 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { Some(comments::CanvasAction::CopyImageFile(p, i)) => image_action = Some(crate::edit_text_ui::ImageAction::CopyFile(p, i)), None => {} } - // Full screen: the click turned the page unless it did something else first. + // Full screen: the click turns the page unless it did something else first. It waits out the + // double-click time: turning on the first click's release sent a double-click's second click, + // meant to select a word, to the next page (review on #2070). A selection cancels it, and so + // does leaving full screen; a double-click elsewhere still turns once. + let now = ui.input(|i| i.time); if present_click && clicked_link.is_none() && view.selection.is_none() && view.clicks < 2 { - view.step_page(!present_back); + let delay = ui.ctx().options(|o| o.input_options.max_double_click_delay); + view.present_turn = Some((now + delay, !present_back)); + } + if let Some((due, forward)) = view.present_turn { + if view.selection.is_some() || view.presentation.is_none() { + view.present_turn = None; + } else if now >= due { + view.present_turn = None; + view.step_page(forward); + } else { + ui.ctx().request_repaint_after(std::time::Duration::from_secs_f64((due - now).max(0.0))); + } } match clicked_link { Some(LinkTarget::Page(p, dest)) => view.go_to_dest(p, dest, info), diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index cc7a486fc..d6c208083 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -521,6 +521,8 @@ pub struct PdfCraftApp { pub quick_tool: QuickTool, /// The tool to go back to when Space, held for a temporary Hand, is released. space_hand: Option, + /// The tool before full screen, given back on leaving it (see [`Self::apply_full_screen`]). + full_screen_tool: Option, /// Comment author, per-tool colours and widths, pin. pub comment_prefs: comments::CommentPrefs, /// Resolved colours, including the current OS theme when following the system. @@ -910,6 +912,7 @@ impl PdfCraftApp { right_panel_closed: false, quick_tool: QuickTool::Select, space_hand: None, + full_screen_tool: None, comment_prefs: Default::default(), theme: ThemeKind::Light, theme_preference: ThemePreference::Light, @@ -1597,6 +1600,14 @@ impl PdfCraftApp { /// time fitted to the screen while it is on (Acrobat's Full Screen Mode, #379, #1459), and /// gets its display back when it is off. pub fn apply_full_screen(&mut self, on: bool) { + // A click turns the page only with the Select tool, and full screen hides the tools: + // select it there, and give the tool back on leaving (review on #2070). + if on && !self.full_screen { + self.full_screen_tool = Some(self.quick_tool); + self.quick_tool = QuickTool::Select; + } else if !on && let Some(tool) = self.full_screen_tool.take() { + self.quick_tool = tool; + } self.full_screen = on; for view in &mut self.views { if on { diff --git a/crates/ui-egui/tests/ui_group/text_select.rs b/crates/ui-egui/tests/ui_group/text_select.rs index 9ed4e9d5d..cf484785d 100644 --- a/crates/ui-egui/tests/ui_group/text_select.rs +++ b/crates/ui-egui/tests/ui_group/text_select.rs @@ -544,3 +544,50 @@ fn coalesced_primary_clicks_select_line_then_spaced_click_clears() { assert_eq!(doc.saved_bytes(), saved); } } + +/// Review on #2070: in full screen a click goes forward a page, but a double-click selects the +/// word and stays: the click waits out the double-click time before turning. Turning on the first +/// click's release had sent the second click to the next page. Leaving full screen meanwhile +/// drops the pending turn. +#[test] +fn full_screen_double_click_selects_a_word_and_stays_on_the_page() { + // The same page twice. + let two_pages = String::from_utf8(two_lines()) + .unwrap() + .replace("/Kids [3 0 R] /Count 1", "/Kids [3 0 R 6 0 R] /Count 2") + .replace( + "trailer", + "6 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 300 200] /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> >> >> endobj\ntrailer", + ) + .into_bytes(); + let (mut h, _) = harness_with(two_pages, |_| {}); + h.state_mut().apply_full_screen(true); + crate::support::wait_until(&mut h, "the page in full screen", |app| app.views[0].glyph_screen_pos(0, 5).is_some() && !app.render_pending()); + let at = h.state().views[0].glyph_screen_pos(0, 5).expect("text layer"); + h.hover_at(at); + h.run_steps(1); + + // A double-click selects "quick" and stays on page 1, also once the double-click time is over. + click(&mut h, at); + click(&mut h, at); + assert_eq!(h.state().views[0].selected_text().as_deref(), Some("quick")); + h.run_steps(30); + assert_eq!(h.state().views[0].current, 0, "the double-click didn't turn the page"); + + // A single click turns it once the double-click time is over. + // Entering full screen again clears the selection. + h.state_mut().apply_full_screen(false); + h.state_mut().apply_full_screen(true); + h.run_steps(30); + click(&mut h, at); + h.run_steps(30); + assert_eq!(h.state().views[0].current, 1, "a click goes forward"); + + // A click right before leaving full screen doesn't turn the page after it. + h.state_mut().views[0].go_to_page(0); + h.run_steps(30); + click(&mut h, at); + h.state_mut().apply_full_screen(false); + h.run_steps(30); + assert_eq!(h.state().views[0].current, 0, "no page turn after leaving"); +} diff --git a/crates/ui-egui/tests/view.rs b/crates/ui-egui/tests/view.rs index 74ea5965b..df653bf6e 100644 --- a/crates/ui-egui/tests/view.rs +++ b/crates/ui-egui/tests/view.rs @@ -1759,3 +1759,26 @@ fn full_screen_keeps_two_page_view_as_spreads() { let v = &h.state().views[0]; assert!(v.continuous() && v.layout == PageLayout::TwoUp, "two-page continuous view comes back"); } + +/// Review on #2070: full screen hides the tools and a click turns the page only with Select, so +/// entering it from another tool selects Select, and leaving gives the tool back. +#[test] +fn full_screen_clicks_turn_pages_whatever_tool_was_on() { + use pdfcraft_ui_egui::QuickTool; + let mut h = harness(&[]); + h.run_steps(4); + h.state_mut().quick_tool = QuickTool::Hand; + h.key_press_modifiers(Modifiers::COMMAND, Key::L); + h.run_steps(6); + assert!(h.state().full_screen); + assert_eq!(h.state().quick_tool, QuickTool::Select, "Select in full screen"); + // Entering again (a second document asking for it) keeps the tool to give back. + h.state_mut().apply_full_screen(true); + let r = rect(&h, 0).expect("page 1 on screen"); + click_with(&mut h, r.center(), Modifiers::NONE); + assert_eq!(h.state().views[0].current, 1, "a click goes forward"); + h.key_press(Key::Escape); + h.run_steps(4); + assert!(!h.state().full_screen); + assert_eq!(h.state().quick_tool, QuickTool::Hand, "the tool from before is back"); +}