ui: fix two-page continuous scrolling and add keyboard focus indicators

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.

crates/ui-egui/src/dialogs/preferences.rs

@@ -56,8 +56,18 @@ pub fn segmented_pill_choice_a11y<T: PartialEq + Copy>(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<T: PartialEq + Copy>(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")));

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();