diff --git a/crates/ui-egui/src/dialogs/appearance.rs b/crates/ui-egui/src/dialogs/appearance.rs index 49888c9b2d..2b1b35400f 100644 --- a/crates/ui-egui/src/dialogs/appearance.rs +++ b/crates/ui-egui/src/dialogs/appearance.rs @@ -82,8 +82,8 @@ fn preview(ui: &mut egui::Ui, kind: ThemeKind, width: f32) { } /// One theme card: its title, Active when it is what shows now, and a preview of the chosen theme. -/// Returns true when the card was clicked or activated via keyboard. -fn card(ui: &mut egui::Ui, t: &Tokens, title: &str, active: bool, themes: &[(ThemeKind, &str)], query: &str) -> bool { +/// Returns whether the card was clicked and the control's Response. +fn card(ui: &mut egui::Ui, t: &Tokens, title: &str, active: bool, themes: &[(ThemeKind, &str)], query: &str) -> (bool, egui::Response) { let mut card_clicked = false; let frame_stroke = if active { Stroke::new(1.5, t.accent) } else { Stroke::new(1.0, t.border) }; let scope_res = ui.scope_builder(egui::UiBuilder::new().id_salt(title).sense(egui::Sense::click()), |ui| { @@ -125,25 +125,26 @@ fn card(ui: &mut egui::Ui, t: &Tokens, title: &str, active: bool, themes: &[(The if resp.clicked() || key_select { card_clicked = true; } - card_clicked + (card_clicked, resp) } /// The Appearance section of Preferences. #[cfg_attr(not(test), expect(dead_code, reason = "called by unit tests"))] pub fn preferences_section(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) { - let _ = preferences_section_with_query(ui, app, t, ""); + let mut ctx = crate::dialogs::preferences::PrefSearchContext::new("", false); + preferences_section_with_context(ui, app, t, &mut ctx); } -/// The Appearance section of Preferences with live search highlighting. -pub fn preferences_section_with_query(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens, query: &str) -> usize { - let mut matches = 0; - let (app_label, c) = crate::dialogs::preferences::highlight_widget_text(tl_ctx!("theme", "Appearance"), query, theme::semibold(13.0), t.text); - matches += c; - ui.label(app_label); +/// The Appearance section of Preferences with live search highlighting and scrolling context. +pub(crate) fn preferences_section_with_context( + ui: &mut egui::Ui, + app: &mut PdfCraftApp, + t: &Tokens, + ctx: &mut crate::dialogs::preferences::PrefSearchContext, +) { + ctx.label(ui, tl_ctx!("theme", "Appearance"), theme::semibold(13.0), t.text); ui.horizontal(|ui| { - let (label_text, c) = crate::dialogs::preferences::highlight_widget_text(tl!("Appearance mode"), query, theme::regular(13.0), t.text); - matches += c; - let label = ui.label(label_text); + let label = ctx.label(ui, tl!("Appearance mode"), theme::regular(13.0), t.text); let combo = craft_ui::ComboBox::from_id_salt("appearance-mode").selected_text(tl!(mode_label(app.theme_preference))).show_ui(ui, |ui| { for preference in [ThemePreference::System, ThemePreference::Light, ThemePreference::Dark] { if ui.selectable_label(app.theme_preference == preference, tl!(mode_label(preference))).clicked() { @@ -157,23 +158,29 @@ pub fn preferences_section_with_query(ui: &mut egui::Ui, app: &mut PdfCraftApp, }); ui.add_space(4.0); let light_active = app.theme == ThemeKind::Light; - matches += crate::dialogs::preferences::find_match_ranges(tl!("Light theme"), query).len(); - matches += crate::dialogs::preferences::find_match_ranges(tl!("Dark theme"), query).len(); + let light_title = tl!("Light theme"); + let dark_title = tl!("Dark theme"); + let light_matches = crate::dialogs::preferences::find_match_ranges(light_title, ctx.query).len(); + let dark_matches = crate::dialogs::preferences::find_match_ranges(dark_title, ctx.query).len(); + ctx.total_matches += light_matches + dark_matches; + ui.columns(2, |cols| { let [light, dark] = cols else { return }; - if card(light, t, tl!("Light theme"), light_active, LIGHT_THEMES, query) { + let (light_clicked, light_resp) = card(light, t, light_title, light_active, LIGHT_THEMES, ctx.query); + if light_clicked { app.execute(mode_command(ThemePreference::Light)); } - if card(dark, t, tl!("Dark theme"), !light_active, DARK_THEMES, query) { + ctx.scroll_if_first_match(&light_resp, light_matches); + + let (dark_clicked, dark_resp) = card(dark, t, dark_title, !light_active, DARK_THEMES, ctx.query); + if dark_clicked { app.execute(mode_command(ThemePreference::Dark)); } + ctx.scroll_if_first_match(&dark_resp, dark_matches); }); ui.add_space(4.0); // Acrobat's "Highlight existing fields": shared with the toggle in the form bar. - let (fields_text, c) = crate::dialogs::preferences::highlight_widget_text(tl!("Highlight fields"), query, theme::regular(13.0), t.text); - matches += c; - ui.add(crate::widgets::checkbox(&mut app.view_defaults.highlight_fields, fields_text)); - matches + ctx.checkbox(ui, &mut app.view_defaults.highlight_fields, tl!("Highlight fields"), t); } #[cfg(test)] diff --git a/crates/ui-egui/src/dialogs/preferences.rs b/crates/ui-egui/src/dialogs/preferences.rs index baa7094b2d..42f9f17798 100644 --- a/crates/ui-egui/src/dialogs/preferences.rs +++ b/crates/ui-egui/src/dialogs/preferences.rs @@ -31,18 +31,20 @@ pub fn segmented_pill_choice(ui: &mut egui::Ui, t: &Tokens, /// A segmented pill selection bar with separate accessibility labels. #[expect(dead_code, reason = "convenience helpers for non-searched segmented pill controls")] pub fn segmented_pill_choice_a11y(ui: &mut egui::Ui, t: &Tokens, current: &mut T, options: &[(T, &str, &str)]) -> bool { - segmented_pill_choice_a11y_query(ui, t, current, options, "") + segmented_pill_choice_a11y_query(ui, t, current, options, "").0 } /// A segmented pill selection bar with separate accessibility labels and optional query highlighting. +/// Returns whether the selection changed and the Response of the first matching option, if any. pub fn segmented_pill_choice_a11y_query( ui: &mut egui::Ui, t: &Tokens, current: &mut T, options: &[(T, &str, &str)], query: &str, -) -> bool { +) -> (bool, Option) { let mut changed = false; + let mut first_match_resp: Option = None; egui::Frame::new() .fill(t.field) .stroke(Stroke::new(1.0, t.border)) @@ -60,6 +62,10 @@ pub fn segmented_pill_choice_a11y_query( let (rect, resp) = ui.allocate_exact_size(vec2(w + 18.0, 24.0), Sense::click()); resp.widget_info(|| egui::WidgetInfo::selected(egui::Role::RadioButton, true, is_active, a11y_label)); + if first_match_resp.is_none() && !find_match_ranges(label, query).is_empty() { + first_match_resp = Some(resp.clone()); + } + if is_active { ui.painter().rect(rect, CornerRadius::same(12), t.accent, Stroke::NONE, egui::StrokeKind::Inside); } else { @@ -87,11 +93,12 @@ pub fn segmented_pill_choice_a11y_query( } }); }); - changed + (changed, first_match_resp) } /// An independent toggle pill button with optional query highlighting. -pub fn toggle_pill_query(ui: &mut egui::Ui, t: &Tokens, active: &mut bool, label: &str, query: &str) -> bool { +/// Returns whether the active state changed and the control's Response. +pub fn toggle_pill_query(ui: &mut egui::Ui, t: &Tokens, active: &mut bool, label: &str, query: &str) -> (bool, egui::Response) { let mut changed = false; let is_active = *active; let font = theme::medium(12.0); @@ -120,7 +127,7 @@ pub fn toggle_pill_query(ui: &mut egui::Ui, t: &Tokens, active: &mut bool, label *active = !*active; changed = true; } - changed + (changed, resp) } fn layout_pill_label(ui: &egui::Ui, label: &str, query: &str, font: egui::FontId, normal_color: Color32) -> std::sync::Arc { @@ -282,14 +289,18 @@ impl<'a> PrefSearchContext<'a> { Self { query, total_matches: 0, first_match_scrolled: false, should_scroll } } - pub fn checkbox(&mut self, ui: &mut egui::Ui, checked: &mut bool, label: &str, t: &Tokens) -> egui::Response { - let (widget_text, count) = highlight_widget_text(label, self.query, theme::regular(13.0), t.text); - self.total_matches += count; - let resp = ui.add(widgets::checkbox(checked, widget_text)); + pub fn scroll_if_first_match(&mut self, resp: &egui::Response, count: usize) { if count > 0 && self.should_scroll && !self.first_match_scrolled { resp.scroll_to_me(Some(egui::Align::Center)); self.first_match_scrolled = true; } + } + + pub fn checkbox(&mut self, ui: &mut egui::Ui, checked: &mut bool, label: &str, t: &Tokens) -> egui::Response { + let (widget_text, count) = highlight_widget_text(label, self.query, theme::regular(13.0), t.text); + self.total_matches += count; + let resp = ui.add(widgets::checkbox(checked, widget_text)); + self.scroll_if_first_match(&resp, count); resp } @@ -297,10 +308,7 @@ impl<'a> PrefSearchContext<'a> { let (widget_text, count) = highlight_widget_text(text, self.query, font, color); self.total_matches += count; let resp = ui.label(widget_text); - if count > 0 && self.should_scroll && !self.first_match_scrolled { - resp.scroll_to_me(Some(egui::Align::Center)); - self.first_match_scrolled = true; - } + self.scroll_if_first_match(&resp, count); resp } @@ -317,24 +325,38 @@ impl<'a> PrefSearchContext<'a> { } pub fn pill_choice(&mut self, ui: &mut egui::Ui, t: &Tokens, current: &mut T, options: &[(T, &str)]) -> bool { + let mut matches = 0; for &(_, label) in options { - self.total_matches += find_match_ranges(label, self.query).len(); + matches += find_match_ranges(label, self.query).len(); } + self.total_matches += matches; let with_a11y: Vec<(T, &str, &str)> = options.iter().map(|&(v, l)| (v, l, l)).collect(); - segmented_pill_choice_a11y_query(ui, t, current, &with_a11y, self.query) + let (changed, match_resp) = segmented_pill_choice_a11y_query(ui, t, current, &with_a11y, self.query); + if let Some(resp) = match_resp { + self.scroll_if_first_match(&resp, matches); + } + changed } pub fn pill_choice_a11y(&mut self, ui: &mut egui::Ui, t: &Tokens, current: &mut T, options: &[(T, &str, &str)]) -> bool { + let mut matches = 0; for &(_, label, _) in options { - self.total_matches += find_match_ranges(label, self.query).len(); + matches += find_match_ranges(label, self.query).len(); } - segmented_pill_choice_a11y_query(ui, t, current, options, self.query) + self.total_matches += matches; + let (changed, match_resp) = segmented_pill_choice_a11y_query(ui, t, current, options, self.query); + if let Some(resp) = match_resp { + self.scroll_if_first_match(&resp, matches); + } + changed } pub fn toggle_pill(&mut self, ui: &mut egui::Ui, t: &Tokens, active: &mut bool, label: &str) -> bool { let matches = find_match_ranges(label, self.query).len(); self.total_matches += matches; - toggle_pill_query(ui, t, active, label, self.query) + let (changed, resp) = toggle_pill_query(ui, t, active, label, self.query); + self.scroll_if_first_match(&resp, matches); + changed } } @@ -647,7 +669,7 @@ fn card_appearance(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens, ctx: &m ui.ctx().request_repaint(); } ui.add_space(4.0); - ctx.total_matches += crate::appearance::preferences_section_with_query(ui, app, t, ctx.query); + crate::appearance::preferences_section_with_context(ui, app, t, ctx); ui.add_space(8.0); reading_colours_section(ui, app, t, ctx); } diff --git a/crates/ui-egui/tests/it/preferences_layout.rs b/crates/ui-egui/tests/it/preferences_layout.rs index cb5e2570dd..e0a5d4e1cb 100644 --- a/crates/ui-egui/tests/it/preferences_layout.rs +++ b/crates/ui-egui/tests/it/preferences_layout.rs @@ -210,3 +210,44 @@ fn preferences_dialog_scrollbar_spacing_and_controls() { // Search box right edge is aligned within the dialog assert!(search.max.x <= ok.max.x); } + +#[test] +fn search_scrolls_option_only_and_theme_card_matches_into_visible_viewport() { + let mut h = harness(480.0); + let title = h.get_by(|n| n.role() == egui::accesskit::Role::Label && n.value().as_deref() == Some("Preferences")).rect(); + let ok = h.get_by_label("OK").rect(); + + // "Two-page view" is an option in Card 3 (Documents and view). + // In a 480 pt window, it starts out below the visible scroll viewport. + let opt_before = h.get_by_label("Two-page view").rect(); + assert!(opt_before.min.y >= ok.min.y, "Two-page option starts out below the visible viewport: {opt_before:?} vs OK {ok:?}"); + + // Searching "Two-page" (which only occurs inside that segmented pill option) + // auto-scrolls the matching option into the visible viewport between title and OK. + h.get_by_role_and_label(egui::accesskit::Role::TextInput, "Search settings…").click(); + h.run_steps(1); + h.get_by_role_and_label(egui::accesskit::Role::TextInput, "Search settings…").type_text("Two-page"); + h.run_steps(4); + + let opt_after = h.get_by_label("Two-page view").rect(); + assert!( + opt_after.min.y >= title.max.y && opt_after.max.y <= ok.min.y, + "Two-page option scrolled into visible scroll viewport: {opt_after:?} between title {title:?} and OK {ok:?}" + ); + + // Clear search and test a theme-card-only match + h.get_by_label("Clear search").click(); + h.run_steps(4); + + // Search for "Dark theme" (which occurs only in the dark theme card in Card 2). + h.get_by_role_and_label(egui::accesskit::Role::TextInput, "Search settings…").click(); + h.run_steps(1); + h.get_by_role_and_label(egui::accesskit::Role::TextInput, "Search settings…").type_text("Dark theme"); + h.run_steps(4); + + let dark_card = h.get_by_label("Dark theme").rect(); + assert!( + dark_card.min.y >= title.max.y && dark_card.max.y <= ok.min.y, + "Dark theme card scrolled into visible viewport: {dark_card:?} between title {title:?} and OK {ok:?}" + ); +}