Merge PR #3906: M14: the Preferences OK button stays in reach below the scrolling settings (#3557)

ROADMAP.md

@@ -600,3 +600,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.

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 => {

crates/ui-egui/src/js_ui.rs

@@ -260,8 +260,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;
+
+/// Preferences: interface language and size, appearance, identity and JavaScript. 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 +500,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()
 }

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);
+}

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"