fix(ui): clicking a theme card in Preferences picks that theme (#1727)

Preferences > Appearance drew a Light and a Dark theme card, each with a
radio row, but neither the card nor the radio responded to a click: the
radio's response was discarded and the frame never sensed clicks. Only
the Appearance mode combo box changed the theme.

The whole card and its radio rows now run view.theme.light /
view.theme.dark, the same commands as the combo box, so the combo box
shows the choice. A card click is a manual choice that OS theme changes
leave alone, and Use system setting afterwards follows the OS again.

Co-authored-by: umesh429 <3474974+umesh429@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

crates/ui-egui/src/appearance.rs

@@ -83,9 +83,9 @@ fn preview(ui: &mut egui::Ui, kind: ThemeKind, width: f32) {
 }
 
 /// One theme card: its title, Active when it is what shows now, a preview of the chosen theme
-/// and a radio row per theme.
-fn card(ui: &mut egui::Ui, t: &Tokens, title: &str, active: bool, themes: &[(ThemeKind, &str)]) {
-    egui::Frame::new()
+/// and a radio row per theme. Returns true when the card or one of its radio rows was clicked.
+fn card(ui: &mut egui::Ui, t: &Tokens, title: &str, active: bool, themes: &[(ThemeKind, &str)]) -> bool {
+    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)
@@ -102,11 +102,16 @@ fn card(ui: &mut egui::Ui, t: &Tokens, title: &str, active: bool, themes: &[(The
             let shown = themes.first().map_or(ThemeKind::Light, |(kind, _)| *kind);
             preview(ui, shown, ui.available_width());
             ui.add_space(4.0);
+            let mut clicked = false;
             for (kind, label) in themes {
                 // One theme per card today, so it is always the chosen one.
-                let _ = ui.add(widgets::radio(*kind == shown, tl!(label)));
+                clicked |= ui.add(widgets::radio(*kind == shown, tl!(label))).clicked();
             }
+            clicked
         });
+    // The whole card picks its theme, not just the radio row.
+    let whole = inner.response.interact(egui::Sense::click()).on_hover_cursor(egui::CursorIcon::PointingHand);
+    inner.inner || whole.clicked()
 }
 
 /// The Appearance section of Preferences.
@@ -127,8 +132,12 @@ pub fn preferences_section(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens)
     let light_active = app.theme == ThemeKind::Light;
     ui.columns(2, |cols| {
         let [light, dark] = cols else { return };
-        card(light, t, tl!("Light theme"), light_active, LIGHT_THEMES);
-        card(dark, t, tl!("Dark theme"), !light_active, DARK_THEMES);
+        if card(light, t, tl!("Light theme"), light_active, LIGHT_THEMES) {
+            app.execute(mode_command(ThemePreference::Light));
+        }
+        if card(dark, t, tl!("Dark theme"), !light_active, DARK_THEMES) {
+            app.execute(mode_command(ThemePreference::Dark));
+        }
     });
 }
 
@@ -179,4 +188,32 @@ mod tests {
         h.run();
         assert_eq!(h.state().theme_preference, ThemePreference::System);
     }
+
+    /// Clicking a theme card, or its radio row, picks that theme, not only the mode combo box.
+    #[test]
+    fn clicking_a_theme_card_picks_that_theme() {
+        let mut app = PdfCraftApp::new();
+        app.set_option("language", "en").unwrap();
+        let mut fonts = false;
+        let mut h = Harness::builder().with_size(egui::vec2(640.0, 400.0)).build_ui_state(
+            move |ui, app: &mut PdfCraftApp| {
+                if !fonts {
+                    ui.ctx().set_fonts(theme::font_definitions());
+                    fonts = true;
+                    return;
+                }
+                let t = Tokens::get(ui.ctx());
+                preferences_section(ui, app, &t);
+            },
+            app,
+        );
+        h.run();
+        h.get_by_label("Dark gray").click();
+        h.run();
+        assert_eq!((h.state().theme_preference, h.state().theme), (ThemePreference::Dark, ThemeKind::Dark));
+        // The card's title is not a widget of its own: the click lands on the card.
+        h.get_by_label("Light theme").click();
+        h.run();
+        assert_eq!((h.state().theme_preference, h.state().theme), (ThemePreference::Light, ThemeKind::Light));
+    }
 }

crates/ui-egui/tests/ui_group/theme.rs

@@ -204,3 +204,64 @@ fn native_appearance_remains_inherited_for_every_saved_choice() {
         assert_eq!(h.ctx.options(|o| o.theme_preference), egui::ThemePreference::System);
     }
 }
+
+/// Preferences ▸ Appearance: a theme card is a manual choice that the mode combo box shows and
+/// OS changes leave alone; Use system setting afterwards follows the OS again.
+#[test]
+fn preferences_theme_cards_and_use_system_setting_stay_in_sync() {
+    use egui::accesskit::Role;
+    fn mode_shown(h: &Harness<'static, PdfCraftApp>) -> String {
+        let combo =
+            h.get_by(|n| n.role() == Role::ComboBox && n.value().is_some_and(|v| ["Use system setting", "Light", "Dark"].contains(&v.as_str())));
+        combo.value().unwrap_or_default()
+    }
+    let mut h = harness();
+    h.get_by_label("Display theme: Light gray").click_secondary();
+    h.run_steps(4);
+    h.get_by_label("Appearance settings…").click();
+    h.run_steps(10);
+    assert_theme(&h, ThemePreference::Light, ThemeKind::Light);
+    assert_eq!(mode_shown(&h), "Light");
+
+    // The OS is dark: the Dark card's radio row picks Dark, and the combo box says so.
+    // The cards sit below the fold of the Preferences scroll area at this window size.
+    h.get_by_label("Dark gray").scroll_to_me();
+    h.run_steps(3);
+    h.get_by_label("Dark gray").click();
+    h.run_steps(3);
+    assert_theme(&h, ThemePreference::Dark, ThemeKind::Dark);
+    assert_eq!(mode_shown(&h), "Dark");
+    // The Light card itself, not its radio row, picks Light, and a dark OS doesn't undo it.
+    h.get_by_label("Light theme").click();
+    h.run_steps(3);
+    h.input_mut().system_theme = Some(egui::Theme::Light);
+    h.run_steps(2);
+    h.input_mut().system_theme = Some(egui::Theme::Dark);
+    h.run_steps(2);
+    assert_theme(&h, ThemePreference::Light, ThemeKind::Light);
+    assert_eq!(mode_shown(&h), "Light");
+
+    // Use system setting follows the OS both ways after a card choice.
+    h.get_by(|n| n.role() == Role::ComboBox && n.value().as_deref() == Some("Light")).scroll_to_me();
+    h.run_steps(3);
+    h.get_by(|n| n.role() == Role::ComboBox && n.value().as_deref() == Some("Light")).click();
+    h.run_steps(3);
+    // Motion has a "Use system setting" radio of its own; this is the combo box's item.
+    h.get_by_role_and_label(Role::Button, "Use system setting").click();
+    h.run_steps(3);
+    assert_theme(&h, ThemePreference::System, ThemeKind::Dark);
+    assert_eq!(mode_shown(&h), "Use system setting");
+    h.input_mut().system_theme = Some(egui::Theme::Light);
+    h.run_steps(3);
+    assert_theme(&h, ThemePreference::System, ThemeKind::Light);
+    // Only the Light card is marked Active while the OS is light.
+    assert_eq!(h.query_all_by_label("Active").count(), 1);
+
+    // A card click leaves system mode again.
+    h.get_by_label("Dark theme").scroll_to_me();
+    h.run_steps(3);
+    h.get_by_label("Dark theme").click();
+    h.run_steps(3);
+    assert_theme(&h, ThemePreference::Dark, ThemeKind::Dark);
+    assert_eq!(mode_shown(&h), "Dark");
+}