diff --git a/crates/ui-egui/src/chrome.rs b/crates/ui-egui/src/chrome.rs index fb9e0a7c0d..491b316a35 100644 --- a/crates/ui-egui/src/chrome.rs +++ b/crates/ui-egui/src/chrome.rs @@ -638,148 +638,179 @@ pub fn right_rail(app: &mut PdfCraftApp, ui: &mut egui::Ui) { .exact_size(48.0) .frame(egui::Frame::NONE.fill(t.chrome).inner_margin(egui::Margin::symmetric(6, 8)).stroke(Stroke::new(1.0, t.divider))) .show(ui, |ui| { - ui.spacing_mut().item_spacing.y = 4.0; - let mut rail_button = |ui: &mut egui::Ui, panel: RightPanel, icon: &str, tip: &str, has: bool| { - let selected = app.right == Some(panel) && (app.mode == Mode::Read || app.docking.exposed(crate::docking::Panel::Inspector)); - let r = icons::button(ui, icon, 34.0, selected, tl!(tip)); - if has && !selected { - let c = r.rect.right_top() + vec2(-8.0, 8.0); - ui.painter().circle_filled(c, 3.0, t.accent); - } - if r.clicked() { - app.choose_right_panel(if selected { None } else { Some(panel) }); - } - }; - rail_button(ui, RightPanel::Comments, "message-square-text", "Comments", has_comments); - rail_button(ui, RightPanel::Bookmarks, "bookmark", "Bookmarks", has_outline); - rail_button(ui, RightPanel::Pages, "files", "Page thumbnails", false); - rail_button(ui, RightPanel::Fields, "text-cursor-input", "Form fields", has_fields); - rail_button(ui, RightPanel::Layers, "layers", "Layers", has_layers); - rail_button(ui, RightPanel::Attachments, "paperclip", "Attachments", has_files); - rail_button(ui, RightPanel::Signatures, "signature", "Signatures", has_signatures); - if has_check { - rail_button(ui, RightPanel::Accessibility, "accessibility", "Accessibility Checker", false); - } - rail_button(ui, RightPanel::Tags, "tag", "Tags", false); - - // Page navigation cluster at the bottom (as in Acrobat's rail). - let view = &mut app.views[index]; - let mut page_display = None; - ui.with_layout(Layout::bottom_up(Align::Center), |ui| { - ui.spacing_mut().item_spacing.y = 2.0; - let tip = crate::commands::key_tip(ui.ctx(), tl!("Zoom out ({key})"), crate::commands::ZOOM_OUT); - if icons::button(ui, "zoom-out", 32.0, false, &tip).clicked() { - view.zoom_step(false); - } - let tip = crate::commands::key_tip(ui.ctx(), tl!("Zoom in ({key})"), crate::commands::ZOOM_IN); - if icons::button(ui, "zoom-in", 32.0, false, &tip).clicked() { - view.zoom_step(true); - } - // Between rotate and zoom, as in Acrobat. Its command runs once `view` is free. - let tip = crate::i18n::fmt(tl!("Page display: {layout}"), &[("layout", tl!(view.layout.label()))]); - let resp = icons::button(ui, view.layout.icon(), 32.0, false, &tip); - page_display = egui::Popup::menu(&resp) - .show(|ui| { - ui.set_min_width(220.0); - rail_view_menu(ui, view) - }) - .and_then(|r| r.inner); - let tip = crate::commands::key_tip(ui.ctx(), tl!("Rotate view clockwise ({key})"), crate::commands::ROTATE_CW); - if icons::button(ui, "rotate-cw", 32.0, false, &tip).clicked() { - view.rotate_view(true); - } - // Not `columns-2` while fitting the page: that is the two-page view's icon. - let fit_icon = if view.fit == Fit::Width { "maximize" } else { "maximize-2" }; - if icons::button(ui, fit_icon, 32.0, false, tl!("Toggle fit page / fit width")).clicked() { - view.fit = if view.fit == Fit::Width { Fit::Page } else { Fit::Width }; - view.goto = Some((view.current, 0.0)); - } - let id = egui::Id::new("zoom-input"); - let draft = id.with("draft"); - let mut typed = ui.data(|d| d.get_temp::(draft)).unwrap_or_else(|| format!("{:.0}%", view.zoom * 100.0)); - let edit = egui::TextEdit::singleline(&mut typed) - .id(id) - .desired_width(40.0) - .horizontal_align(Align::Center) - .font(theme::medium(11.0)) - .text_color(t.text_muted) - .frame(egui::Frame::NONE) - .margin(vec2(2.0, 2.0)); - let r = ui.add(edit).on_hover_text(tl!("Zoom — type a percentage and press Enter")); - ui.ctx().accesskit_node_builder(id, |node| node.set_label(tl!("Zoom"))); - if r.gained_focus() { - let mut state = egui::TextEdit::load_state(ui.ctx(), id).unwrap_or_default(); - let all = egui::text::CCursorRange::two(egui::text::CCursor::new(0), egui::text::CCursor::new(typed.chars().count())); - state.cursor.set_char_range(Some(all)); - state.store(ui.ctx(), id); - } - if r.has_focus() { - ui.data_mut(|d| d.insert_temp(draft, typed.clone())); - } else { - ui.data_mut(|d| d.remove::(draft)); - if r.lost_focus() - && ui.input(|i| i.key_pressed(egui::Key::Enter)) - && let Some(zoom) = DocView::parse_zoom_percent(&typed) - { - view.set_zoom(zoom); + // The heights of the panel buttons and of the navigation cluster on the last frame. + let heights_id = ui.id().with("rail_heights"); + let measured = ui.data(|d| d.get_temp::<(f32, f32)>(heights_id)); + let (top, nav) = measured.unwrap_or_default(); + let height = ui.available_height(); + // Too short for both groups (a small window, or a large interface size), the rail + // scrolls instead of drawing the navigation over the panel buttons (#2891). A rail + // that fits keeps the plain layout, so neither it nor the canvas beside it moves. + let overflows = top + 4.0 + nav > height; + let mut body = |ui: &mut egui::Ui| { + // From the cursor: a panel's own rect is already as tall as the window. + let start = ui.cursor().top(); + ui.spacing_mut().item_spacing.y = 4.0; + let mut rail_button = |ui: &mut egui::Ui, panel: RightPanel, icon: &str, tip: &str, has: bool| { + let selected = app.right == Some(panel) && (app.mode == Mode::Read || app.docking.exposed(crate::docking::Panel::Inspector)); + let r = icons::button(ui, icon, 34.0, selected, tl!(tip)); + if has && !selected { + let c = r.rect.right_top() + vec2(-8.0, 8.0); + ui.painter().circle_filled(c, 3.0, t.accent); } - } - ui.add_space(6.0); - if icons::button(ui, "chevron-down", 30.0, false, tl!("Next page")).clicked() { - view.step_page(true); - } - if icons::button(ui, "chevron-up", 30.0, false, tl!("Previous page")).clicked() { - view.step_page(false); - } - // History is distinct from stepping to the adjacent page, and belongs to this tab. - for (forward, icon, label, shortcut, enabled) in [ - (true, "chevron-right", tl!("Next view"), crate::commands::NEXT_VIEW, !view.forward.is_empty()), - (false, "chevron-left", tl!("Previous view"), crate::commands::PREV_VIEW, !view.back.is_empty()), - ] { - let tip = format!("{label} ({})", crate::commands::shortcut_label(ui.ctx(), shortcut)); - if ui.add_enabled_ui(enabled, |ui| icons::button(ui, icon, 30.0, false, &tip)).inner.clicked() { - view.view_history(forward); + if r.clicked() { + app.choose_right_panel(if selected { None } else { Some(panel) }); } - } - ui.label(egui::RichText::new(page_count.to_string()).font(theme::regular(11.0)).color(t.text_muted)); - let page_input_id = view.page_input_id(); - // Consume replacement selection once, including when the editor has lost focus. - let reselect = std::mem::take(&mut view.page_input_reselect); - // Navigation updates the idle display, never the text being edited. - if !ui.memory(|m| m.has_focus(page_input_id)) { - view.page_input = view.current_page_label(labels.get(view.current).map(String::as_str)); - } else if reselect { - // The replacement changed the label while egui retained this editor's cursor. - let mut state = egui::TextEdit::load_state(ui.ctx(), page_input_id).unwrap_or_default(); - let end = egui::text::CCursor::new(view.page_input.chars().count()); - state.cursor.set_char_range(Some(egui::text::CCursorRange::two(egui::text::CCursor::new(0), end))); - state.store(ui.ctx(), page_input_id); - } - let edit = egui::TextEdit::singleline(&mut view.page_input) - .id(page_input_id) - .desired_width(34.0) - .horizontal_align(Align::Center) - .font(theme::medium(12.0)) - .margin(vec2(2.0, 4.0)); - let r = egui::Frame::NONE - .fill(t.field) - .stroke(Stroke::new(1.0, t.border)) - .corner_radius(CornerRadius::same(5)) - .show(ui, |ui| ui.add(edit)) - .inner - .on_hover_text(tl!("Current page — type a page number or label (such as iv) and press Enter")); - if r.lost_focus() { - if ui.input(|i| i.key_pressed(egui::Key::Enter)) { - // A page label first (logical page numbers, as Acrobat), then a number. - let typed = view.page_input.clone(); - view.go_to_typed(&typed, &labels); + }; + rail_button(ui, RightPanel::Comments, "message-square-text", "Comments", has_comments); + rail_button(ui, RightPanel::Bookmarks, "bookmark", "Bookmarks", has_outline); + rail_button(ui, RightPanel::Pages, "files", "Page thumbnails", false); + rail_button(ui, RightPanel::Fields, "text-cursor-input", "Form fields", has_fields); + rail_button(ui, RightPanel::Layers, "layers", "Layers", has_layers); + rail_button(ui, RightPanel::Attachments, "paperclip", "Attachments", has_files); + rail_button(ui, RightPanel::Signatures, "signature", "Signatures", has_signatures); + if has_check { + rail_button(ui, RightPanel::Accessibility, "accessibility", "Accessibility Checker", false); + } + rail_button(ui, RightPanel::Tags, "tag", "Tags", false); + + let top = ui.cursor().top() - start; + + // Page navigation cluster at the bottom (as in Acrobat's rail). + let view = &mut app.views[index]; + let mut page_display = None; + let cluster = |ui: &mut egui::Ui| { + ui.spacing_mut().item_spacing.y = 2.0; + let tip = crate::commands::key_tip(ui.ctx(), tl!("Zoom out ({key})"), crate::commands::ZOOM_OUT); + if icons::button(ui, "zoom-out", 32.0, false, &tip).clicked() { + view.zoom_step(false); + } + let tip = crate::commands::key_tip(ui.ctx(), tl!("Zoom in ({key})"), crate::commands::ZOOM_IN); + if icons::button(ui, "zoom-in", 32.0, false, &tip).clicked() { + view.zoom_step(true); + } + // Between rotate and zoom, as in Acrobat. Its command runs once `view` is free. + let tip = crate::i18n::fmt(tl!("Page display: {layout}"), &[("layout", tl!(view.layout.label()))]); + let resp = icons::button(ui, view.layout.icon(), 32.0, false, &tip); + page_display = egui::Popup::menu(&resp) + .show(|ui| { + ui.set_min_width(220.0); + rail_view_menu(ui, view) + }) + .and_then(|r| r.inner); + let tip = crate::commands::key_tip(ui.ctx(), tl!("Rotate view clockwise ({key})"), crate::commands::ROTATE_CW); + if icons::button(ui, "rotate-cw", 32.0, false, &tip).clicked() { + view.rotate_view(true); + } + // Not `columns-2` while fitting the page: that is the two-page view's icon. + let fit_icon = if view.fit == Fit::Width { "maximize" } else { "maximize-2" }; + if icons::button(ui, fit_icon, 32.0, false, tl!("Toggle fit page / fit width")).clicked() { + view.fit = if view.fit == Fit::Width { Fit::Page } else { Fit::Width }; + view.goto = Some((view.current, 0.0)); + } + let id = egui::Id::new("zoom-input"); + let draft = id.with("draft"); + let mut typed = ui.data(|d| d.get_temp::(draft)).unwrap_or_else(|| format!("{:.0}%", view.zoom * 100.0)); + let edit = egui::TextEdit::singleline(&mut typed) + .id(id) + .desired_width(40.0) + .horizontal_align(Align::Center) + .font(theme::medium(11.0)) + .text_color(t.text_muted) + .frame(egui::Frame::NONE) + .margin(vec2(2.0, 2.0)); + let r = ui.add(edit).on_hover_text(tl!("Zoom — type a percentage and press Enter")); + ui.ctx().accesskit_node_builder(id, |node| node.set_label(tl!("Zoom"))); + if r.gained_focus() { + let mut state = egui::TextEdit::load_state(ui.ctx(), id).unwrap_or_default(); + let all = egui::text::CCursorRange::two(egui::text::CCursor::new(0), egui::text::CCursor::new(typed.chars().count())); + state.cursor.set_char_range(Some(all)); + state.store(ui.ctx(), id); + } + if r.has_focus() { + ui.data_mut(|d| d.insert_temp(draft, typed.clone())); + } else { + ui.data_mut(|d| d.remove::(draft)); + if r.lost_focus() + && ui.input(|i| i.key_pressed(egui::Key::Enter)) + && let Some(zoom) = DocView::parse_zoom_percent(&typed) + { + view.set_zoom(zoom); + } + } + ui.add_space(6.0); + if icons::button(ui, "chevron-down", 30.0, false, tl!("Next page")).clicked() { + view.step_page(true); + } + if icons::button(ui, "chevron-up", 30.0, false, tl!("Previous page")).clicked() { + view.step_page(false); } - view.page_input = view.current_page_label(labels.get(view.current).map(String::as_str)); + // History is distinct from stepping to the adjacent page, and belongs to this tab. + for (forward, icon, label, shortcut, enabled) in [ + (true, "chevron-right", tl!("Next view"), crate::commands::NEXT_VIEW, !view.forward.is_empty()), + (false, "chevron-left", tl!("Previous view"), crate::commands::PREV_VIEW, !view.back.is_empty()), + ] { + let tip = format!("{label} ({})", crate::commands::shortcut_label(ui.ctx(), shortcut)); + if ui.add_enabled_ui(enabled, |ui| icons::button(ui, icon, 30.0, false, &tip)).inner.clicked() { + view.view_history(forward); + } + } + ui.label(egui::RichText::new(page_count.to_string()).font(theme::regular(11.0)).color(t.text_muted)); + let page_input_id = view.page_input_id(); + // Consume replacement selection once, including when the editor has lost focus. + let reselect = std::mem::take(&mut view.page_input_reselect); + // Navigation updates the idle display, never the text being edited. + if !ui.memory(|m| m.has_focus(page_input_id)) { + view.page_input = view.current_page_label(labels.get(view.current).map(String::as_str)); + } else if reselect { + // The replacement changed the label while egui retained this editor's cursor. + let mut state = egui::TextEdit::load_state(ui.ctx(), page_input_id).unwrap_or_default(); + let end = egui::text::CCursor::new(view.page_input.chars().count()); + state.cursor.set_char_range(Some(egui::text::CCursorRange::two(egui::text::CCursor::new(0), end))); + state.store(ui.ctx(), page_input_id); + } + let edit = egui::TextEdit::singleline(&mut view.page_input) + .id(page_input_id) + .desired_width(34.0) + .horizontal_align(Align::Center) + .font(theme::medium(12.0)) + .margin(vec2(2.0, 4.0)); + let r = egui::Frame::NONE + .fill(t.field) + .stroke(Stroke::new(1.0, t.border)) + .corner_radius(CornerRadius::same(5)) + .show(ui, |ui| ui.add(edit)) + .inner + .on_hover_text(tl!("Current page — type a page number or label (such as iv) and press Enter")); + if r.lost_focus() { + if ui.input(|i| i.key_pressed(egui::Key::Enter)) { + // A page label first (logical page numbers, as Acrobat), then a number. + let typed = view.page_input.clone(); + view.go_to_typed(&typed, &labels); + } + view.page_input = view.current_page_label(labels.get(view.current).map(String::as_str)); + } + }; + // Scrolling, the cluster goes below the buttons at its height from the last frame. + let nav = if overflows { + ui.allocate_ui_with_layout(vec2(ui.available_width(), nav), Layout::bottom_up(Align::Center), cluster).response.rect + } else { + ui.with_layout(Layout::bottom_up(Align::Center), cluster).response.rect + }; + if let Some(id) = page_display { + app.execute(id); } - }); - if let Some(id) = page_display { - app.execute(id); + (top, nav.height()) + }; + let heights = + if overflows { egui::ScrollArea::vertical().id_salt("rail").auto_shrink([false, false]).show(ui, body).inner } else { body(ui) }; + // This pass laid the groups out as if they fit, unmeasured or with the heights of a + // shorter rail (a button added since): when they don't, it is redone with the new + // heights rather than shown overlapping for a frame. Redone, it scrolls. + if !overflows && heights.0 + 4.0 + heights.1 > height { + ui.ctx().request_discard("right rail measured its groups"); } + ui.data_mut(|d| d.insert_temp(heights_id, heights)); }); } diff --git a/crates/ui-egui/tests/ui.rs b/crates/ui-egui/tests/ui.rs index b86f7699f0..e35c65f900 100644 --- a/crates/ui-egui/tests/ui.rs +++ b/crates/ui-egui/tests/ui.rs @@ -192,6 +192,82 @@ fn read_mode_rail_buttons_open_their_panels() { h.get_by_label("Alpha section"); } +/// The rail's panel buttons and its page navigation never overlap (#2891). A 1080 px tall +/// window is 720 pt at 150 % interface size and 540 pt at 200 %, both too short for the two +/// groups: the rail scrolls instead of drawing one over the other. +#[test] +fn a_short_rail_scrolls_instead_of_overlapping_its_controls() { + for scale in ["150%", "200%"] { + let mut h = Harness::builder().with_size(egui::vec2(1920.0, 1080.0)).build_eframe(move |_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.set_option("ui-scale", scale).unwrap(); + app.open_bytes("fixture.pdf", None, FIXTURE.to_vec()).expect("fixture opens"); + app + }); + h.run_steps(6); + assert_rail_controls_apart(&h, scale); + } +} + +/// The rail has not measured its two groups on the frame it first appears, and that frame is +/// the one shown: it must not overlap either, not only the frames after it. +#[test] +fn a_short_rail_does_not_overlap_on_its_first_frame() { + let mut h = Harness::builder().with_size(egui::vec2(1920.0, 1080.0)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.set_option("ui-scale", "200%").unwrap(); + app + }); + h.run_steps(4); + h.state_mut().open_bytes("fixture.pdf", None, FIXTURE.to_vec()).expect("fixture opens"); + h.step(); + assert_rail_controls_apart(&h, "200%"); +} + +/// A rail that fits can grow past its height between two frames: checking for accessibility adds +/// its button. At 785 pt the groups fit without it and not with it, and the frame where it appears +/// is laid out with the heights of the frame before: it must not overlap either. +#[test] +fn a_rail_that_grows_past_its_height_does_not_overlap_for_a_frame() { + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 785.0)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("fixture.pdf", None, FIXTURE.to_vec()).expect("fixture opens"); + app + }); + h.run_steps(6); + assert!(h.query_by_label("Accessibility Checker").is_none()); + assert_rail_controls_apart(&h, "100%"); + h.state_mut().run_accessibility_check(); + h.step(); + let tags = h.get_by_label("Tags").rect(); + assert!(h.query_all_by_label("Accessibility Checker").any(|n| (tags.left()..tags.right()).contains(&n.rect().center().x)), "the rail has grown"); + assert_rail_controls_apart(&h, "100%"); +} + +fn assert_rail_controls_apart(h: &Harness<'_, PdfCraftApp>, scale: &str) { + use egui::accesskit::Role; + use egui_kittest::kittest::by; + let tags = h.get_by_label("Tags").rect(); + let in_rail = |r: &egui::Rect| r.center().x > tags.left() - 4.0 && r.center().x < tags.right() + 4.0; + let rail: Vec<(String, egui::Rect)> = h + .query_all(by().predicate(|n| matches!(n.role(), Role::Button | Role::TextInput | Role::Label))) + .map(|n| (n.accesskit_node().label().unwrap_or_default(), n.rect())) + .filter(|(_, r)| in_rail(r)) + .collect(); + for label in ["Tags", "Signatures", "Zoom out", "Next page", "Previous view", "Page display"] { + assert!(rail.iter().any(|(l, _)| l.starts_with(label)), "{scale}: the rail has {label}: {rail:?}"); + } + for (i, (a, ra)) in rail.iter().enumerate() { + for (b, rb) in &rail[i + 1..] { + let o = ra.intersect(*rb); + assert!(o.width() <= 0.5 || o.height() <= 0.5, "{scale}: {a:?} {ra:?} overlaps {b:?} {rb:?}"); + } + } +} + /// A document that asks to open in Full Screen mode (`/PageMode /FullScreen`) is asked about /// first, as in Acrobat: "Not now" leaves the window as it is, "Full Screen" enters it. #[test]