From ec533af1bb0c66e560d16ed64a5ae6f5bc7d6459 Mon Sep 17 00:00:00 2001 From: Combiler01 Date: Mon, 2 Nov 2026 10:43:27 +0530 Subject: [PATCH] ui: fix two-page continuous scrolling and add keyboard focus indicators --- crates/ui-egui/src/dialogs/appearance.rs | 59 ++++++++++------- crates/ui-egui/src/dialogs/preferences.rs | 65 +++++++------------ crates/ui-egui/tests/it/preferences_layout.rs | 7 +- 3 files changed, 63 insertions(+), 68 deletions(-) diff --git a/crates/ui-egui/src/dialogs/appearance.rs b/crates/ui-egui/src/dialogs/appearance.rs index 135337aaba..2b0ac0651c 100644 --- a/crates/ui-egui/src/dialogs/appearance.rs +++ b/crates/ui-egui/src/dialogs/appearance.rs @@ -1,7 +1,7 @@ //! Preferences › Appearance: the appearance mode (follow the system, light or dark) above a light //! and a dark theme card, each with a small picture of PdfCraft in that theme. -use egui::{Rect, RichText, pos2, vec2}; +use egui::{CornerRadius, Rect, RichText, Stroke, pos2, vec2}; use crate::PdfCraftApp; use crate::theme::{self, ThemeKind, ThemePreference, Tokens}; @@ -82,30 +82,41 @@ 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. +/// 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)]) -> bool { - let mut title_clicked = false; - let inner = egui::Frame::new() - .fill(t.card) - .stroke(egui::Stroke::new(1.0, if active { t.accent } else { t.border })) - .corner_radius(6.0) - .inner_margin(egui::Margin::same(8)) - .show(ui, |ui| { - ui.set_width(ui.available_width()); - ui.horizontal(|ui| { - let resp = ui.label(RichText::new(title).font(theme::semibold(12.5)).color(t.text)); - title_clicked = resp.interact(egui::Sense::click()).clicked(); - if active { - ui.label(RichText::new(tl!("Active")).color(t.accent_text).small()); - } - }); - ui.add_space(4.0); - let shown = themes.first().map_or(ThemeKind::Light, |(kind, _)| *kind); - preview(ui, shown, ui.available_width()); - }); - // The whole card picks its theme. - let whole = inner.response.interact(egui::Sense::click()).on_hover_cursor(egui::CursorIcon::PointingHand); - title_clicked || whole.clicked() + let card_w = ui.available_width(); + let (rect, resp) = ui.allocate_exact_size(vec2(card_w, 156.0), egui::Sense::click()); + resp.widget_info(|| egui::WidgetInfo::selected(egui::Role::RadioButton, true, active, title)); + + let press = theme::Press::track(ui, &resp); + let fill = press.fill(t.card, t.hover, t.pressed); + let stroke = if resp.has_focus() { + Stroke::new(2.0, t.accent_text) + } else if active { + Stroke::new(1.5, t.accent) + } else { + Stroke::new(1.0, t.border) + }; + ui.painter().rect(rect, CornerRadius::same(6), fill, stroke, egui::StrokeKind::Inside); + + let mut child = ui.new_child(egui::UiBuilder::new().max_rect(rect.shrink(8.0)).layout(egui::Layout::top_down(egui::Align::Min))); + child.horizontal(|ui| { + let font = theme::semibold(12.5); + let galley = crate::bidi::layout_ui(ui, title.to_owned(), font, t.text); + let (title_rect, _) = ui.allocate_exact_size(galley.size(), egui::Sense::hover()); + ui.painter().galley(title_rect.min, galley, t.text); + if active { + ui.label(RichText::new(tl!("Active")).color(t.accent_text).small()); + } + }); + child.add_space(4.0); + let preview_w = child.available_width(); + let shown = themes.first().map_or(ThemeKind::Light, |(kind, _)| *kind); + preview(&mut child, shown, preview_w); + + let resp = theme::hand(resp); + let key_select = resp.has_focus() && ui.input(|i| i.key_pressed(egui::Key::Space) || i.key_pressed(egui::Key::Enter)); + resp.clicked() || key_select } /// The Appearance section of Preferences. diff --git a/crates/ui-egui/src/dialogs/preferences.rs b/crates/ui-egui/src/dialogs/preferences.rs index 4147417277..d3bf0ed719 100644 --- a/crates/ui-egui/src/dialogs/preferences.rs +++ b/crates/ui-egui/src/dialogs/preferences.rs @@ -56,8 +56,18 @@ pub fn segmented_pill_choice_a11y(ui: &mut egui::Ui, t: &To } ui.painter().galley(rect.center() - label_galley.size() / 2.0, label_galley, text_color); + if resp.has_focus() { + ui.painter().rect_stroke( + rect.expand(1.0), + CornerRadius::same(13), + Stroke::new(2.0, t.accent_text), + egui::StrokeKind::Outside, + ); + } + let resp = theme::hand(resp); - if resp.clicked() && *current != value { + let key_select = resp.has_focus() && ui.input(|i| i.key_pressed(egui::Key::Space) || i.key_pressed(egui::Key::Enter)); + if (resp.clicked() || key_select) && *current != value { *current = value; changed = true; } @@ -68,7 +78,6 @@ pub fn segmented_pill_choice_a11y(ui: &mut egui::Ui, t: &To } /// An independent toggle pill button (e.g. for standalone toggles). -#[allow(dead_code)] pub fn toggle_pill(ui: &mut egui::Ui, t: &Tokens, active: &mut bool, label: &str) -> bool { let mut changed = false; let is_active = *active; @@ -88,8 +97,13 @@ pub fn toggle_pill(ui: &mut egui::Ui, t: &Tokens, active: &mut bool, label: &str ui.painter().rect(rect, CornerRadius::same(13), fill, stroke, egui::StrokeKind::Inside); ui.painter().galley(rect.center() - label_galley.size() / 2.0, label_galley, text_color); + if resp.has_focus() { + ui.painter().rect_stroke(rect.expand(1.0), CornerRadius::same(14), Stroke::new(2.0, t.accent_text), egui::StrokeKind::Outside); + } + let resp = theme::hand(resp); - if resp.clicked() { + let key_select = resp.has_focus() && ui.input(|i| i.key_pressed(egui::Key::Space) || i.key_pressed(egui::Key::Enter)); + if resp.clicked() || key_select { *active = !*active; changed = true; } @@ -451,13 +465,6 @@ fn card_appearance(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) { reading_colours_section(ui, app, t); } -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -enum DisplayModeChoice { - Single, - TwoUp, - Continuous, -} - /// Content for Card 3: Documents and View fn card_documents_and_view(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) { ui.horizontal(|ui| { @@ -485,37 +492,13 @@ fn card_documents_and_view(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) let defaults = &mut app.view_defaults; ui.horizontal(|ui| { ui.label(tl!("Default page display")); - let mut display_choice = if defaults.continuous { - DisplayModeChoice::Continuous - } else if defaults.layout == PageLayout::TwoUp { - DisplayModeChoice::TwoUp - } else { - DisplayModeChoice::Single - }; - if segmented_pill_choice( - ui, - t, - &mut display_choice, - &[ - (DisplayModeChoice::Single, tl!("Single page")), - (DisplayModeChoice::TwoUp, tl!("Two-page view")), - (DisplayModeChoice::Continuous, tl!("Continuous scrolling")), - ], - ) { - match display_choice { - DisplayModeChoice::Single => { - defaults.layout = PageLayout::Single; - defaults.continuous = false; - } - DisplayModeChoice::TwoUp => { - defaults.layout = PageLayout::TwoUp; - defaults.continuous = false; - } - DisplayModeChoice::Continuous => { - defaults.layout = PageLayout::Single; - defaults.continuous = true; - } - } + let mut layout_choice = defaults.layout; + if segmented_pill_choice(ui, t, &mut layout_choice, &[(PageLayout::Single, tl!("Single page")), (PageLayout::TwoUp, tl!("Two-page view"))]) { + defaults.layout = layout_choice; + } + let mut continuous = defaults.continuous; + if toggle_pill(ui, t, &mut continuous, tl!("Continuous scrolling")) { + defaults.continuous = continuous; } }); ui.add(widgets::checkbox(&mut defaults.show_page_gaps, tl!("Show gaps between pages"))); diff --git a/crates/ui-egui/tests/it/preferences_layout.rs b/crates/ui-egui/tests/it/preferences_layout.rs index a20d786965..6c693ddff0 100644 --- a/crates/ui-egui/tests/it/preferences_layout.rs +++ b/crates/ui-egui/tests/it/preferences_layout.rs @@ -120,17 +120,18 @@ fn segmented_pill_controls_switch_modes_and_layouts() { h.run_steps(2); assert_eq!(h.state().view_defaults.layout, pdfcraft_ui_egui::canvas::PageLayout::TwoUp); - // Switch continuous scrolling pill + // Switch continuous scrolling pill (can be combined with Two-page view) let continuous_before = h.state().view_defaults.continuous; h.get_by_label("Continuous scrolling").click(); h.run_steps(2); assert_eq!(h.state().view_defaults.continuous, !continuous_before); + assert_eq!(h.state().view_defaults.layout, pdfcraft_ui_egui::canvas::PageLayout::TwoUp, "two-page continuous scrolling is supported"); - // Switch to single page mode + // Switch to single page mode (continuous setting remains independent) h.get_by_label("Single page").click(); h.run_steps(2); assert_eq!(h.state().view_defaults.layout, pdfcraft_ui_egui::canvas::PageLayout::Single); - assert!(!h.state().view_defaults.continuous); + assert_eq!(h.state().view_defaults.continuous, !continuous_before); // Switch tools panel side via segmented pill h.get_by_label("Left").scroll_to_me();