From 175992b5a2045d00cdf8a20f812ea4aac522b099 Mon Sep 17 00:00:00 2001 From: asadosteaksnl Date: Fri, 13 Nov 2026 02:42:37 +0100 Subject: [PATCH] fix(ui): Recognize Text's settings scroll in a short window, its buttons stay in reach (#6599) (#6760) * fix(ui): Recognize Text's settings scroll in a short window, its buttons stay in reach (#6599) On a small screen or at a large display scale the dialog was taller than the window: it was centred with its title and its Cancel / Recognize text row off the screen, and nothing scrolled. The four settings groups (Pages, Settings, Skip, Output folder) now sit in a vertical scroll area capped to the window's height, as Preferences does (#3557). The title, the engine notice and the button row stay outside it, so they are always on screen; when the settings fit, nothing changes. The test opens the dialog in 400 to 700 pt windows and checks the title and both buttons are on screen, and that the last group scrolls in between the title and the buttons while they stay put. Without the fix it fails at 400 pt with the title at y = -267. * contributors: credit asadosteaksnl as Kas * fix(ui): Recognize Text's settings scroll in as little as 40 pt, so its buttons fit the smallest window (#6599) A 120 pt floor still pushed the dialog off a window shorter than about 340 pt: at 173 pt (the smallest window, 520 pt, at the largest interface size, 300%) the title sat at y = -32. The test now checks 173, 208 and 260 pt windows too, as the Preferences layout test does. * test(ui): the short-window Recognize Text test clicks Cancel and checks a 640 pt wide window (#6599) Cancel being on screen isn't proof it can be clicked: the test now clicks it at every height and checks the dialog closes. The window is 640 pt across, a 1280-pixel screen at 200%, and every control is checked to fit across as well as down. * test(ui): the Recognize Text title stays put while the settings scroll (#6599) * Revert "contributors: credit asadosteaksnl as Kas" This reverts commit 52470f66a75f6d90c44da4f7f33d496b7cdd7828. * docs(ui): document the Recognize Text dialog's body (#6599) * test(ui): the Recognize Text test also covers tall windows where nothing scrolls (#6599) The dialog is about 935 pt tall, so it scrolls up to a 900 pt window; at 1100 pt everything fits. Both now run, so the test also pins the case where the settings fit and the layout is unchanged. --------- Co-authored-by: asadosteaksnl <353856209+asadosteaksnl@users.noreply.github.com> --- crates/ui-egui/src/workflows/ocr_ui.rs | 238 ++++++++++++++----------- crates/ui-egui/tests/it/ocr.rs | 41 +++++ 2 files changed, 171 insertions(+), 108 deletions(-) diff --git a/crates/ui-egui/src/workflows/ocr_ui.rs b/crates/ui-egui/src/workflows/ocr_ui.rs index 91378d9172..799da09d7d 100644 --- a/crates/ui-egui/src/workflows/ocr_ui.rs +++ b/crates/ui-egui/src/workflows/ocr_ui.rs @@ -233,6 +233,16 @@ pub(crate) fn keep_asking_while_probing(ctx: &egui::Context) { } } +/// The dialog's height outside its scrolling settings: the title, a notice of up to two lines, +/// the Cancel / Recognize text row and the modal's own margins. +const OCR_DIALOG_RESERVE: f32 = 220.0; +/// The least the settings scroll in, however short the window: small enough that the title and the +/// buttons still fit the smallest window at the largest interface size (173 pt), as Preferences. +const OCR_MIN_SCROLL: f32 = 40.0; + +/// The Recognize Text dialog: the title, the settings groups (scrolling when the window is too +/// short for them), any engine notice, and the Cancel / Recognize text row. Returns +/// `(recognize, cancel)`, which of those buttons was clicked this frame. pub(crate) fn body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) -> (bool, bool) { let pages = app.active_ids().and_then(|(_, id)| app.session.get(id)).map_or(1, |d| d.info.pages.len().max(1)); let d = &mut app.ocr_draft; @@ -248,115 +258,127 @@ pub(crate) fn body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) -> (boo }); ui.add_space(8.0); }; - group(ui, "Pages", &mut |ui| { - widgets::radio_value(ui, &mut d.pages, OcrPages::All, tl!("All pages")); - widgets::radio_value(ui, &mut d.pages, OcrPages::Current, tl!("Current page")); - ui.horizontal(|ui| { - widgets::radio_value(ui, &mut d.pages, OcrPages::Range, tl!("From")); - let on = d.pages == OcrPages::Range; - ui.add_enabled(on, egui::DragValue::new(&mut d.from).range(1..=pages)); - ui.label(tl!("to")); - ui.add_enabled(on, egui::DragValue::new(&mut d.to).range(1..=pages)); + // The settings outgrow a short window or a large display scale (#6599): they scroll, and + // the notices and the Cancel / Recognize text row stay below the scroll, always in reach. + let max = (ui.ctx().content_rect().height() - OCR_DIALOG_RESERVE).max(OCR_MIN_SCROLL); + egui::ScrollArea::vertical() + .id_salt("ocr-dialog-scroll") + .max_height(max) + // Only applies when the settings overflow: the dialog then opens at its full height + // instead of growing frame by frame, as Preferences does. + .min_scrolled_height(max) + .auto_shrink([false, true]) + .show(ui, |ui| { + group(ui, "Pages", &mut |ui| { + widgets::radio_value(ui, &mut d.pages, OcrPages::All, tl!("All pages")); + widgets::radio_value(ui, &mut d.pages, OcrPages::Current, tl!("Current page")); + ui.horizontal(|ui| { + widgets::radio_value(ui, &mut d.pages, OcrPages::Range, tl!("From")); + let on = d.pages == OcrPages::Range; + ui.add_enabled(on, egui::DragValue::new(&mut d.from).range(1..=pages)); + ui.label(tl!("to")); + ui.add_enabled(on, egui::DragValue::new(&mut d.to).range(1..=pages)); + }); + }); + group(ui, "Settings", &mut |ui| { + egui::Grid::new("ocr-settings").num_columns(2).spacing([12.0, 8.0]).show(ui, |ui| { + ui.label(tl!("Document language")); + let name = LANGUAGES.iter().find(|l| l.0 == d.language).map_or("English", |l| l.1); + craft_ui::ComboBox::from_id_salt("ocr-language").selected_text(tl!(name)).width(220.0).show_ui(ui, |ui| { + for (code, name) in LANGUAGES { + ui.selectable_value(&mut d.language, (*code).to_string(), tl!(*name)); + } + }); + ui.end_row(); + ui.label(tl!("Engine")); + let engine_name = match d.engine { + EngineChoice::Auto => tl!("Automatic"), + EngineChoice::Ocrs => tl!("Built-in"), + EngineChoice::Tesseract => tl!("Tesseract"), + }; + craft_ui::ComboBox::from_id_salt("ocr-engine").selected_text(engine_name).width(220.0).show_ui(ui, |ui| { + let choices: [(EngineChoice, &str); 3] = + [(EngineChoice::Auto, "Automatic"), (EngineChoice::Ocrs, "Built-in"), (EngineChoice::Tesseract, "Tesseract")]; + for (choice, name) in choices { + ui.selectable_value(&mut d.engine, choice, tl!(name)); + } + }); + ui.end_row(); + ui.label(tl!("Combine engines")); + let strategy_name = match d.strategy { + MergeStrategy::PrimaryOnly => tl!("Primary only"), + MergeStrategy::ConfidenceWeighted => tl!("Confidence weighted"), + MergeStrategy::RoverVote => tl!("ROVER vote"), + }; + craft_ui::ComboBox::from_id_salt("ocr-strategy").selected_text(strategy_name).width(220.0).show_ui(ui, |ui| { + let choices: [(MergeStrategy, &str); 3] = [ + (MergeStrategy::PrimaryOnly, "Primary only"), + (MergeStrategy::ConfidenceWeighted, "Confidence weighted"), + (MergeStrategy::RoverVote, "ROVER vote"), + ]; + for (choice, name) in choices { + ui.selectable_value(&mut d.strategy, choice, tl!(name)); + } + }); + ui.end_row(); + ui.label(tl!("Output")); + let mode_name = match d.output_mode { + OutputMode::Searchable => tl!("Searchable Image (Exact)"), + OutputMode::EditableText => tl!("Editable text"), + }; + craft_ui::ComboBox::from_id_salt("ocr-output").selected_text(mode_name).width(220.0).show_ui(ui, |ui| { + let s = ui + .selectable_label(d.output_mode == OutputMode::Searchable, tl!("Searchable Image (Exact)")) + .on_hover_text(tl!("Adds invisible text over each word; the page image is not changed")); + if s.clicked() { + d.output_mode = OutputMode::Searchable; + } + let e = ui + .selectable_label(d.output_mode == OutputMode::EditableText, tl!("Editable text")) + .on_hover_text(tl!("Writes a new document of visible text; the scan is not changed")); + if e.clicked() { + d.output_mode = OutputMode::EditableText; + } + }); + ui.end_row(); + ui.label(tl!("Downsample to")); + craft_ui::ComboBox::from_id_salt("ocr-dpi").selected_text(format!("{} dpi", d.dpi)).width(220.0).show_ui(ui, |ui| { + for v in [600, 300, 150, 72] { + ui.selectable_value(&mut d.dpi, v, format!("{v} dpi")); + } + }); + ui.end_row(); + }); + ui.add_space(4.0); + ui.add(crate::widgets::checkbox(&mut d.text_pages, tl!("Also recognize pages that already have text"))) + .on_hover_text(tl!("Pictures on those pages are recognized too; words already in the page's text are not added again")); + ui.add(crate::widgets::checkbox(&mut d.auto_rotate, tl!("Straighten pages scanned sideways or upside down"))); + ui.add(crate::widgets::checkbox(&mut d.deskew, tl!("Straighten skewed text lines"))); + ui.add(crate::widgets::checkbox(&mut d.denoise, tl!("Remove scan speckle"))); + ui.add(crate::widgets::checkbox(&mut d.binarize, tl!("Black and white (binarize)"))); + }); + group(ui, "Skip", &mut |ui| { + ui.add(crate::widgets::checkbox(&mut d.skip_text_pages, tl!("Skip pages that already have text"))) + .on_hover_text(tl!("Pages with renderable text are left alone and reported as skipped")); + ui.add(crate::widgets::checkbox(&mut d.skip_text_files, tl!("Skip files that already have text"))) + .on_hover_text(tl!("In multiple files: a file with any text is its own skipped bucket, and nothing is written for it")); + ui.add(crate::widgets::checkbox(&mut d.force_ocr, tl!("Force OCR (override skip options)"))) + .on_hover_text(tl!("Reads pages and files even where the skip options would leave them alone")); + }); + group(ui, "Output folder", &mut |ui| { + ui.horizontal(|ui| match &d.output_folder { + Some(f) => { + ui.monospace(f).on_hover_text(tl!("The searchable copies are written here under the source names")); + if ui.small_button(tl!("Clear")).clicked() { + d.output_folder = None; + } + } + None => { + ui.label(tl!("Automatic")).on_hover_text(tl!("A folder is asked for when a multiple-file run starts")); + } + }); + }); }); - }); - group(ui, "Settings", &mut |ui| { - egui::Grid::new("ocr-settings").num_columns(2).spacing([12.0, 8.0]).show(ui, |ui| { - ui.label(tl!("Document language")); - let name = LANGUAGES.iter().find(|l| l.0 == d.language).map_or("English", |l| l.1); - craft_ui::ComboBox::from_id_salt("ocr-language").selected_text(tl!(name)).width(220.0).show_ui(ui, |ui| { - for (code, name) in LANGUAGES { - ui.selectable_value(&mut d.language, (*code).to_string(), tl!(*name)); - } - }); - ui.end_row(); - ui.label(tl!("Engine")); - let engine_name = match d.engine { - EngineChoice::Auto => tl!("Automatic"), - EngineChoice::Ocrs => tl!("Built-in"), - EngineChoice::Tesseract => tl!("Tesseract"), - }; - craft_ui::ComboBox::from_id_salt("ocr-engine").selected_text(engine_name).width(220.0).show_ui(ui, |ui| { - let choices: [(EngineChoice, &str); 3] = - [(EngineChoice::Auto, "Automatic"), (EngineChoice::Ocrs, "Built-in"), (EngineChoice::Tesseract, "Tesseract")]; - for (choice, name) in choices { - ui.selectable_value(&mut d.engine, choice, tl!(name)); - } - }); - ui.end_row(); - ui.label(tl!("Combine engines")); - let strategy_name = match d.strategy { - MergeStrategy::PrimaryOnly => tl!("Primary only"), - MergeStrategy::ConfidenceWeighted => tl!("Confidence weighted"), - MergeStrategy::RoverVote => tl!("ROVER vote"), - }; - craft_ui::ComboBox::from_id_salt("ocr-strategy").selected_text(strategy_name).width(220.0).show_ui(ui, |ui| { - let choices: [(MergeStrategy, &str); 3] = [ - (MergeStrategy::PrimaryOnly, "Primary only"), - (MergeStrategy::ConfidenceWeighted, "Confidence weighted"), - (MergeStrategy::RoverVote, "ROVER vote"), - ]; - for (choice, name) in choices { - ui.selectable_value(&mut d.strategy, choice, tl!(name)); - } - }); - ui.end_row(); - ui.label(tl!("Output")); - let mode_name = match d.output_mode { - OutputMode::Searchable => tl!("Searchable Image (Exact)"), - OutputMode::EditableText => tl!("Editable text"), - }; - craft_ui::ComboBox::from_id_salt("ocr-output").selected_text(mode_name).width(220.0).show_ui(ui, |ui| { - let s = ui - .selectable_label(d.output_mode == OutputMode::Searchable, tl!("Searchable Image (Exact)")) - .on_hover_text(tl!("Adds invisible text over each word; the page image is not changed")); - if s.clicked() { - d.output_mode = OutputMode::Searchable; - } - let e = ui - .selectable_label(d.output_mode == OutputMode::EditableText, tl!("Editable text")) - .on_hover_text(tl!("Writes a new document of visible text; the scan is not changed")); - if e.clicked() { - d.output_mode = OutputMode::EditableText; - } - }); - ui.end_row(); - ui.label(tl!("Downsample to")); - craft_ui::ComboBox::from_id_salt("ocr-dpi").selected_text(format!("{} dpi", d.dpi)).width(220.0).show_ui(ui, |ui| { - for v in [600, 300, 150, 72] { - ui.selectable_value(&mut d.dpi, v, format!("{v} dpi")); - } - }); - ui.end_row(); - }); - ui.add_space(4.0); - ui.add(crate::widgets::checkbox(&mut d.text_pages, tl!("Also recognize pages that already have text"))) - .on_hover_text(tl!("Pictures on those pages are recognized too; words already in the page's text are not added again")); - ui.add(crate::widgets::checkbox(&mut d.auto_rotate, tl!("Straighten pages scanned sideways or upside down"))); - ui.add(crate::widgets::checkbox(&mut d.deskew, tl!("Straighten skewed text lines"))); - ui.add(crate::widgets::checkbox(&mut d.denoise, tl!("Remove scan speckle"))); - ui.add(crate::widgets::checkbox(&mut d.binarize, tl!("Black and white (binarize)"))); - }); - group(ui, "Skip", &mut |ui| { - ui.add(crate::widgets::checkbox(&mut d.skip_text_pages, tl!("Skip pages that already have text"))) - .on_hover_text(tl!("Pages with renderable text are left alone and reported as skipped")); - ui.add(crate::widgets::checkbox(&mut d.skip_text_files, tl!("Skip files that already have text"))) - .on_hover_text(tl!("In multiple files: a file with any text is its own skipped bucket, and nothing is written for it")); - ui.add(crate::widgets::checkbox(&mut d.force_ocr, tl!("Force OCR (override skip options)"))) - .on_hover_text(tl!("Reads pages and files even where the skip options would leave them alone")); - }); - group(ui, "Output folder", &mut |ui| { - ui.horizontal(|ui| match &d.output_folder { - Some(f) => { - ui.monospace(f).on_hover_text(tl!("The searchable copies are written here under the source names")); - if ui.small_button(tl!("Clear")).clicked() { - d.output_folder = None; - } - } - None => { - ui.label(tl!("Automatic")).on_hover_text(tl!("A folder is asked for when a multiple-file run starts")); - } - }); - }); keep_asking_while_probing(ui.ctx()); // The web build has no model files: it downloads the models the first time they're needed. #[cfg(target_arch = "wasm32")] diff --git a/crates/ui-egui/tests/it/ocr.rs b/crates/ui-egui/tests/it/ocr.rs index ec6af6d7de..483b1719cf 100644 --- a/crates/ui-egui/tests/it/ocr.rs +++ b/crates/ui-egui/tests/it/ocr.rs @@ -465,3 +465,44 @@ fn ocr_verify_screen_focus_ring_and_empty_walk() { assert!(moved, "Ctrl+Tab moves the focus"); assert_eq!(h.state().ocr_verify.focus, start, "the ring returns after the four panes"); } + +/// #6599: in a short window (a small screen, or a large display scale) the dialog's settings +/// scroll, and Cancel and Recognize text stay on screen below them instead of running off it. +#[test] +fn recognize_text_keeps_its_buttons_in_a_short_window() { + use egui::accesskit::Role; + // 173 is the smallest window (520 pt) at the largest interface size (300%), 208 is 250%. + // 640 pt across is a 1280-pixel-wide screen at 200%. At 1100 pt everything fits (the dialog + // is about 935 pt tall) and nothing scrolls. + let width = 640.0; + for height in [173.0, 208.0, 260.0, 400.0, 480.0, 560.0, 700.0, 900.0, 1100.0] { + let mut h = Harness::builder().with_size(egui::vec2(width, height)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("scan.pdf", None, scan()).unwrap(); + app + }); + h.run_steps(4); + assert!(h.state_mut().execute("ocr.recognize")); + h.run_steps(6); + let on_screen = |r: egui::Rect| r.min.x >= 0.0 && r.max.x <= width && r.min.y >= 0.0 && r.max.y <= height; + let title = h.get_by(|n| n.role() == Role::Label && n.value().as_deref() == Some("Recognize Text")).rect(); + let run = h.get_by_role_and_label(Role::Button, "Recognize text").rect(); + let cancel = h.get_by_role_and_label(Role::Button, "Cancel").rect(); + assert!(on_screen(title), "{height}: the title at {title:?} is off the window"); + assert!(on_screen(run), "{height}: Recognize text at {run:?} is off the window"); + assert!(on_screen(cancel), "{height}: Cancel at {cancel:?} is off the window"); + // The last group scrolls in between the title and the buttons, which do not move. + h.get_by(|n| n.role() == Role::Label && n.value().as_deref() == Some("Output folder")).scroll_to_me(); + h.run_steps(4); + let last = h.get_by(|n| n.role() == Role::Label && n.value().as_deref() == Some("Output folder")).rect(); + assert!(last.min.y >= title.max.y && last.max.y <= run.min.y, "{height}: Output folder {last:?} between {title:?} and {run:?}"); + assert_eq!(h.get_by_role_and_label(Role::Button, "Recognize text").rect(), run, "{height}: the buttons stay put"); + let title_after = h.get_by(|n| n.role() == Role::Label && n.value().as_deref() == Some("Recognize Text")).rect(); + assert_eq!(title_after, title, "{height}: the title stays put"); + // On screen is not enough: a click reaches Cancel and closes the dialog. + h.get_by_role_and_label(Role::Button, "Cancel").click(); + h.run_steps(3); + assert!(h.state().dialog.is_none(), "{height}: Cancel closes the dialog"); + } +}