From eaed3409f83fa483e58ada700f3a4cf6a2a47e1d Mon Sep 17 00:00:00 2001 From: Adrian Date: Wed, 21 Oct 2026 15:37:35 -0400 Subject: [PATCH] M14: the Preferences OK button stays in reach below the scrolling settings (#3557) The settings scroll when the window is short, but OK was the last thing inside the scrolled content, so in a short window it was off screen until you scrolled to it, and the dialog was cut off partway through the theme cards. OK is now a row below the scroll area, outside it. The scroll area gets the height the window leaves (at least 60 pt, so OK stays on screen in the smallest window, 520 pt, at the largest interface size, 300%) and a scrollbar of its own instead of the floating one that lay over the right edge of the theme cards. A scroll area starts from the room its modal offers, and a modal offers more each frame for the first few, so with OK outside the scroll the dialog opened small and stretched by about 45 pt a frame for ten frames, and clicks in the first frames missed. The scroll area is asked for its full height from the start (min_scrolled_height, which egui applies only when the settings overflow it), so it opens at full size and does not stretch in a tall window. - test: OK and the title are on screen at window heights from 173 to 1100; OK closes the dialog and does not move while the settings scroll, and a far-down setting scrolls into view between the title and OK; the dialog does not grow after it opens; a very tall window has no blank gap above OK. Both the original layout and the stretching variants fail them. - parity: misc.preferences-dialog evidence and what is still missing. --- ROADMAP.md | 2 + crates/ui-egui/src/dialogs.rs | 15 +++- crates/ui-egui/src/js_ui.rs | 17 +++- crates/ui-egui/tests/preferences_layout.rs | 98 ++++++++++++++++++++++ parity/acrobat-features.toml | 4 +- 5 files changed, 131 insertions(+), 5 deletions(-) create mode 100644 crates/ui-egui/tests/preferences_layout.rs diff --git a/ROADMAP.md b/ROADMAP.md index b1283e775..281f22050 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -591,3 +591,5 @@ Newest first: the date, what moved, and the new overall estimate. - **2026-10-13 (M7):** Edit a PDF ▸ Add content: Header and footer, Watermark and Background open an Add / Update / Remove menu (Update and Remove enabled only when the document has that mark), as Acrobat's Watermark control does. The catalogue gains `ToolItem::actions`, so any tool can list more commands (#1192). - **2026-10-18 (M8, #1350):** Published the history-preserving integration; simplified the inherited-form resource guard for Rust 1.96 Clippy. Formatting, 64 redaction tests, scoped Clippy, layering, assets and the scoped wasm check pass locally. CI remains pending; estimates unchanged. + +- **2026-10-21 (M14, #3557):** Preferences: the OK button stays below the scrolling settings instead of being the last thing in them, so it is in reach in a short window. diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 981db214b..82754376c 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -964,8 +964,19 @@ fn dialogs(app: &mut PdfCraftApp, ctx: &egui::Context) { } Dialog::Preferences => { // The settings outgrow a short window: scroll them rather than run off the screen. - let max = (ui.ctx().content_rect().height() - 120.0).max(200.0); - close = egui::ScrollArea::vertical().max_height(max).show(ui, |ui| crate::js_ui::preferences_body(ui, app, &t)).inner; + // The OK row stays below the scroll, in reach however far the settings are scrolled. + let max = (ui.ctx().content_rect().height() - 120.0 - crate::js_ui::PREFERENCES_BUTTON_ROW).max(crate::js_ui::PREFERENCES_MIN_SCROLL); + // A scroll area starts from the room the dialog offers it, and a modal offers more + // each frame for the first few, so with OK outside the scroll the dialog would grow + // over those frames. Asking for the full height as the least it may scroll in + // (which only applies when the settings overflow it) opens the dialog at full size. + // A scrollbar of its own: the floating one lies over the right edge of the theme + // cards and other full-width rows (#3557). + ui.scope(|ui| { + ui.spacing_mut().scroll.floating = false; + egui::ScrollArea::vertical().max_height(max).min_scrolled_height(max).show(ui, |ui| crate::js_ui::preferences_body(ui, app, &t)); + }); + close = crate::js_ui::preferences_buttons(ui); return; } Dialog::RecognizeText => { diff --git a/crates/ui-egui/src/js_ui.rs b/crates/ui-egui/src/js_ui.rs index df461fc6e..00b129b9b 100644 --- a/crates/ui-egui/src/js_ui.rs +++ b/crates/ui-egui/src/js_ui.rs @@ -261,7 +261,18 @@ fn reading_mode_label(mode: &str) -> &'static str { } /// Preferences: interface language and size, appearance, identity and JavaScript. Returns `true` to close. -pub(crate) fn preferences_body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) -> bool { +/// The height the Preferences dialog keeps below its scrolling settings for the OK row: the +/// button, and the space above it. +pub(crate) const PREFERENCES_BUTTON_ROW: f32 = 52.0; + +/// The least height the scrolling settings are given. The dialog around them takes about 80 more, +/// so this still fits the smallest window (520 pt) at the largest interface size (300%, which +/// leaves 173 pt): a taller floor would push OK off the screen again. +pub(crate) const PREFERENCES_MIN_SCROLL: f32 = 60.0; + +/// The settings, to be drawn in a scroll area; [`preferences_buttons`] goes below it, outside the +/// scroll, so OK is always in reach. +pub(crate) fn preferences_body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) { ui.label(egui::RichText::new(tl!("Preferences")).font(theme::semibold(18.0))); ui.horizontal(|ui| { ui.label(tl!("Interface language")); @@ -490,6 +501,10 @@ pub(crate) fn preferences_body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tok .color(t.text_muted), ); }); +} + +/// The Preferences dialog's OK row. Returns `true` to close. +pub(crate) fn preferences_buttons(ui: &mut egui::Ui) -> bool { ui.add_space(10.0); buttons(ui, tl!("OK"), &[]).is_some() } diff --git a/crates/ui-egui/tests/preferences_layout.rs b/crates/ui-egui/tests/preferences_layout.rs new file mode 100644 index 000000000..3b8d41ec0 --- /dev/null +++ b/crates/ui-egui/tests/preferences_layout.rs @@ -0,0 +1,98 @@ +//! The Preferences dialog keeps OK in reach in a short window (#3557): the settings scroll, the +//! OK row does not. + +use egui::Rect; +use egui_kittest::Harness; +use egui_kittest::kittest::Queryable; +use pdfcraft_ui_egui::PdfCraftApp; + +fn harness(height: f32) -> Harness<'static, PdfCraftApp> { + let mut h = Harness::builder().with_size(egui::vec2(1000.0, height)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.set_option("dialog", "preferences").unwrap(); + app + }); + h.run_steps(6); + h +} + +fn on_screen(rect: Rect, height: f32) -> bool { + rect.min.x >= 0.0 && rect.min.y >= 0.0 && rect.max.x <= 1000.0 && rect.max.y <= height +} + +#[test] +fn ok_is_on_screen_without_scrolling_at_any_window_height() { + // From a short laptop window to a tall monitor, the settings never fit the short ones. 173 is + // the smallest window (520 pt) at the largest interface size (300%), 208 is 250%. + for height in [173.0, 208.0, 260.0, 420.0, 480.0, 560.0, 700.0, 900.0, 1100.0] { + let h = harness(height); + let ok = h.get_by_label("OK").rect(); + assert!(on_screen(ok, height), "{height}: OK at {ok:?} is off the {height} pt window"); + let title = h.get_by_label("Preferences").rect(); + assert!(on_screen(title, height), "{height}: the title at {title:?} is off the window"); + // The OK row sits below the scrolled settings, never over them: the settings' last visible + // row ends above it. The title is above everything. + assert!(title.max.y < ok.min.y, "{height}: title {title:?} above OK {ok:?}"); + } +} + +#[test] +fn ok_closes_the_dialog_in_a_short_window_and_the_settings_still_scroll() { + let mut h = harness(480.0); + // A setting far down the list starts out below the visible settings; scrolling brings it in + // between the title and the OK row, and OK did not move. + let ok_before = h.get_by_label("OK").rect(); + let title = h.get_by_label("Preferences").rect(); + let far = h.get_by_label_contains("Reading colours").rect(); + assert!(far.min.y >= ok_before.min.y, "Reading colours starts out of sight: {far:?} vs OK {ok_before:?}"); + h.get_by_label_contains("Reading colours").scroll_to_me(); + h.run_steps(4); + let far = h.get_by_label_contains("Reading colours").rect(); + assert!(far.min.y >= title.max.y && far.max.y <= ok_before.min.y, "Reading colours scrolled in between the title and OK: {far:?}"); + assert_eq!(h.get_by_label("OK").rect(), ok_before, "OK stays put while the settings scroll"); + assert!(h.state().dialog.is_some()); + h.get_by_label("OK").click(); + h.run_steps(3); + assert!(h.state().dialog.is_none(), "OK closes Preferences"); +} + +#[test] +fn the_dialog_opens_at_its_full_size_and_does_not_grow_afterwards() { + // With OK outside the scroll, the scroll area started from what the modal offered, which + // grew some 45 pt a frame for ten frames: the dialog opened small and stretched, and clicks + // in its first frames landed on controls that were still moving. + for height in [260.0, 560.0, 900.0] { + let mut h = harness(height); + let settled = (h.get_by_label("Preferences").rect(), h.get_by_label("OK").rect()); + h.run_steps(8); + let later = (h.get_by_label("Preferences").rect(), h.get_by_label("OK").rect()); + assert_eq!(settled, later, "{height}: the dialog moved after it had been shown for six frames"); + } + // And it is in place within a few frames of opening, for the first click: a frame or two of + // settling is fine, ten frames of stretching is not. + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app + }); + h.run_steps(4); + h.state_mut().dialog = Some(pdfcraft_ui_egui::Dialog::Preferences); + h.run_steps(3); + let early = h.get_by_label("Right").rect(); + h.run_steps(8); + let late = h.get_by_label("Right").rect(); + assert!((early.min.y - late.min.y).abs() <= 2.0 && (early.min.x - late.min.x).abs() <= 2.0, "{early:?} then {late:?}"); +} + +#[test] +fn a_very_tall_window_does_not_leave_a_blank_gap_above_ok() { + // The settings are some 1 400 pt tall; in a taller window the scroll area is not stretched + // past them. + let mut h = harness(2600.0); + h.run_steps(6); + let ok = h.get_by_label("OK").rect(); + let last = h.get_by_label_contains("Enable Acrobat JavaScript").rect(); + assert!(on_screen(ok, 2600.0) && on_screen(last, 2600.0), "{last:?} {ok:?}"); + assert!(ok.min.y - last.max.y < 120.0, "a gap of {} pt between the last setting and OK", ok.min.y - last.max.y); +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 7aca7d5c8..5ccebf6e0 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -9792,8 +9792,8 @@ milestone = "M14" status = "partial" commands = ["app.preferences"] tools = ["js_enabled"] -evidence = ["crates/ui-egui/tests/javascript.rs::document_scripts_and_preferences", "crates/ui-egui/tests/ui.rs::preferences_selects_default_workspace_for_next_open", "crates/ui-egui/tests/control.rs::control_default_workspace_and_session_override", "apps/pdfcraft/src/settings.rs::xdg_preferences_use_config_instead_of_data", "apps/pdfcraft/src/settings.rs::migration_preserves_preferences_and_other_data"] -notes = "Preferences dialog with Interface language, Documents and view (persistent default workspace with session override), Identity, Fill & Sign (flatten on save, off by default) and JavaScript; other categories still live in their tools, and it isn't generated from a schema yet. XDG desktops store app.ron under XDG_CONFIG_HOME/pdfcraft or ~/.config/pdfcraft; existing data-folder preferences are copied without replacing configured preferences, with fallback on migration errors. Logs and recovery retain data locations. Portable/non-XDG locations are unchanged." +evidence = ["crates/ui-egui/tests/javascript.rs::document_scripts_and_preferences", "crates/ui-egui/tests/ui.rs::preferences_selects_default_workspace_for_next_open", "crates/ui-egui/tests/control.rs::control_default_workspace_and_session_override", "apps/pdfcraft/src/settings.rs::xdg_preferences_use_config_instead_of_data", "apps/pdfcraft/src/settings.rs::migration_preserves_preferences_and_other_data", "crates/ui-egui/tests/preferences_layout.rs::ok_is_on_screen_without_scrolling_at_any_window_height", "crates/ui-egui/tests/preferences_layout.rs::ok_closes_the_dialog_in_a_short_window_and_the_settings_still_scroll"] +notes = "Preferences dialog with Interface language, Documents and view (persistent default workspace with session override), Identity, Fill & Sign (flatten on save, off by default) and JavaScript; other categories still live in their tools, and it isn't generated from a schema yet. XDG desktops store app.ron under XDG_CONFIG_HOME/pdfcraft or ~/.config/pdfcraft; existing data-folder preferences are copied without replacing configured preferences, with fallback on migration errors. Logs and recovery retain data locations. Portable/non-XDG locations are unchanged. The settings scroll when the window is short (down to a 60 pt scroll area, so OK stays on screen in the smallest window at the largest interface size) and the OK row stays below them, in reach (#3557); the dialog still has no Cancel, and a window shorter than the dialog's title, one row and OK would still clip it." [[feature]] id = "misc.prefs-general"