From 85cf039506ccc203bee931a20b22e440ad179021 Mon Sep 17 00:00:00 2001 From: nevaforget Date: Tue, 2 Jun 2026 14:31:50 +0200 Subject: [PATCH] refactor: power-confirm via PowerAction table (v0.6.14) Align the power-confirm flow to moonset's ActionDef pattern, in lockstep with moongreet: a PowerAction table + create_power_button factory replace the two hand-wired reboot/shutdown handlers and the loose-param show_power_confirm. Add an in-flight re-trigger guard (power_box.set_sensitive(false)) and clear a stale error_label when showing a new prompt. --- Cargo.lock | 2 +- Cargo.toml | 7 +- DECISIONS.md | 7 ++ src/lockscreen.rs | 158 +++++++++++++++++++++++++++++----------------- 4 files changed, 111 insertions(+), 63 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2b07668..01589f6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -575,7 +575,7 @@ dependencies = [ [[package]] name = "moonlock" -version = "0.6.13" +version = "0.6.14" dependencies = [ "gdk-pixbuf", "gdk4", diff --git a/Cargo.toml b/Cargo.toml index 3ab2c9d..e5e719d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "moonlock" -version = "0.6.13" +version = "0.6.14" edition = "2024" description = "A secure Wayland lockscreen with GTK4, PAM and fingerprint support" license = "MIT" @@ -28,6 +28,7 @@ tempfile = "3" glib-build-tools = "0.22" [profile.release] -lto = "fat" +lto = "thin" codegen-units = 1 -strip = true +strip = false +debug = true diff --git a/DECISIONS.md b/DECISIONS.md index 469d1e6..98d821d 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -2,6 +2,13 @@ Architectural and design decisions for Moonlock, in reverse chronological order. +## 2026-06-02 – Align power-confirm to moonset's ActionDef pattern (v0.6.14) + +- **Who**: ClaudeCode, Dom +- **Why**: A code review of moongreet's power-confirm (ported from this file) flagged the shared pattern as lower-altitude than moonset's: two near-identical reboot/shutdown handlers and a `show_power_confirm` taking loose `message`/`action_fn`/`error_message` params that can drift apart. moonset already solved this with an `ActionDef` table + button factory. Changed here in lockstep with moongreet to keep the three projects symmetric. +- **Tradeoffs**: A `PowerAction` struct + `power_actions()` table + `create_power_button` factory is slightly more machinery for two actions, but couples icon/prompt/error/action into one value (a mismatched prompt/action becomes unrepresentable) and makes a third action a one-line table entry. Did NOT touch `confirm_box: Rc>>` — moonset uses the same, it is the shared convention. +- **How**: Replaced the two hand-wired handlers with a loop over `power_actions()`; `show_power_confirm`/`execute_power_action` now take `PowerAction` (Copy) instead of three loose strings. Added an in-flight re-trigger guard via `power_box.set_sensitive(false)` (re-enabled on failure), matching moonset; also clear a stale `error_label` when showing a new prompt. + ## 2026-05-04 – Drop PAM account/session stack, remove `check_account`, drop `pam_acct_mgmt` from password path (v0.6.13) - **Who**: ClaudeCode, Dom diff --git a/src/lockscreen.rs b/src/lockscreen.rs index 8f8d672..214cc14 100644 --- a/src/lockscreen.rs +++ b/src/lockscreen.rs @@ -170,55 +170,17 @@ pub fn create_lockscreen_window( power_box.set_margin_end(16); power_box.set_margin_bottom(16); - let reboot_btn = gtk::Button::new(); - reboot_btn.set_icon_name("system-reboot-symbolic"); - reboot_btn.add_css_class("power-button"); - reboot_btn.set_tooltip_text(Some(strings.reboot_tooltip)); - reboot_btn.connect_clicked(clone!( - #[weak] - confirm_area, - #[strong] - confirm_box, - #[weak] - error_label, - move |_| { - show_power_confirm( - strings.reboot_confirm, - power::reboot, - strings.reboot_failed, - strings, - &confirm_area, - &confirm_box, - &error_label, - ); - } - )); - power_box.append(&reboot_btn); - - let shutdown_btn = gtk::Button::new(); - shutdown_btn.set_icon_name("system-shutdown-symbolic"); - shutdown_btn.add_css_class("power-button"); - shutdown_btn.set_tooltip_text(Some(strings.shutdown_tooltip)); - shutdown_btn.connect_clicked(clone!( - #[weak] - confirm_area, - #[strong] - confirm_box, - #[weak] - error_label, - move |_| { - show_power_confirm( - strings.shutdown_confirm, - power::shutdown, - strings.shutdown_failed, - strings, - &confirm_area, - &confirm_box, - &error_label, - ); - } - )); - power_box.append(&shutdown_btn); + for action in power_actions() { + let button = create_power_button( + action, + strings, + &power_box, + &confirm_area, + &confirm_box, + &error_label, + ); + power_box.append(&button); + } overlay.add_overlay(&power_box); @@ -688,23 +650,84 @@ fn set_default_avatar( image.set_icon_name(Some("avatar-default-symbolic")); } +/// Definition for a single power-action button (reboot, shutdown). +/// Couples icon, prompt, error text and action so a button cannot be wired +/// with a mismatched prompt/action pair. Mirrors moonset's `ActionDef`. +#[derive(Clone, Copy)] +struct PowerAction { + icon_name: &'static str, + tooltip_attr: fn(&Strings) -> &'static str, + confirm_attr: fn(&Strings) -> &'static str, + error_attr: fn(&Strings) -> &'static str, + action_fn: fn() -> Result<(), PowerError>, +} + +/// The power actions offered on the lockscreen. +fn power_actions() -> [PowerAction; 2] { + [ + PowerAction { + icon_name: "system-reboot-symbolic", + tooltip_attr: |s| s.reboot_tooltip, + confirm_attr: |s| s.reboot_confirm, + error_attr: |s| s.reboot_failed, + action_fn: power::reboot, + }, + PowerAction { + icon_name: "system-shutdown-symbolic", + tooltip_attr: |s| s.shutdown_tooltip, + confirm_attr: |s| s.shutdown_confirm, + error_attr: |s| s.shutdown_failed, + action_fn: power::shutdown, + }, + ] +} + +/// Build a power-action icon button wired to the confirmation flow. +fn create_power_button( + action: PowerAction, + strings: &'static Strings, + power_box: >k::Box, + confirm_area: >k::Box, + confirm_box: &Rc>>, + error_label: >k::Label, +) -> gtk::Button { + let button = gtk::Button::new(); + button.set_icon_name(action.icon_name); + button.add_css_class("power-button"); + button.set_tooltip_text(Some((action.tooltip_attr)(strings))); + button.connect_clicked(clone!( + #[weak] + power_box, + #[weak] + confirm_area, + #[strong] + confirm_box, + #[weak] + error_label, + move |_| { + show_power_confirm(action, strings, &power_box, &confirm_area, &confirm_box, &error_label); + } + )); + button +} + /// Show inline power confirmation. fn show_power_confirm( - message: &'static str, - action_fn: fn() -> Result<(), PowerError>, - error_message: &'static str, + action: PowerAction, strings: &'static Strings, + power_box: >k::Box, confirm_area: >k::Box, confirm_box: &Rc>>, error_label: >k::Label, ) { dismiss_power_confirm(confirm_area, confirm_box); + error_label.set_visible(false); let new_box = gtk::Box::new(gtk::Orientation::Vertical, 8); new_box.set_halign(gtk::Align::Center); new_box.set_margin_top(16); - let confirm_label = gtk::Label::new(Some(message)); + let confirm_label = gtk::Label::new(Some((action.confirm_attr)(strings))); confirm_label.add_css_class("confirm-label"); new_box.append(&confirm_label); @@ -714,6 +737,8 @@ fn show_power_confirm( let yes_btn = gtk::Button::with_label(strings.confirm_yes); yes_btn.add_css_class("confirm-yes"); yes_btn.connect_clicked(clone!( + #[weak] + power_box, #[weak] confirm_area, #[strong] @@ -721,8 +746,7 @@ fn show_power_confirm( #[weak] error_label, move |_| { - dismiss_power_confirm(&confirm_area, &confirm_box); - execute_power_action(action_fn, error_message, &error_label); + execute_power_action(action, strings, &power_box, &confirm_area, &confirm_box, &error_label); } )); button_row.append(&yes_btn); @@ -753,28 +777,44 @@ fn dismiss_power_confirm(confirm_area: >k::Box, confirm_box: &Rc Result<(), PowerError>, - error_message: &'static str, + action: PowerAction, + strings: &'static Strings, + power_box: >k::Box, + confirm_area: >k::Box, + confirm_box: &Rc>>, error_label: >k::Label, ) { + dismiss_power_confirm(confirm_area, confirm_box); + + let action_fn = action.action_fn; + let error_message = (action.error_attr)(strings); + + // Desensitize the power buttons so a double-click or keyboard repeat cannot + // fire the same action twice while it is in flight. + power_box.set_sensitive(false); + glib::spawn_future_local(clone!( + #[weak] + power_box, #[weak] error_label, async move { - let result = gio::spawn_blocking(move || action_fn()).await; + let result = gio::spawn_blocking(action_fn).await; match result { Ok(Ok(())) => {} Ok(Err(e)) => { log::error!("Power action failed: {e}"); error_label.set_text(error_message); error_label.set_visible(true); + power_box.set_sensitive(true); } Err(_) => { log::error!("Power action panicked"); error_label.set_text(error_message); error_label.set_visible(true); + power_box.set_sensitive(true); } } }