From 7e75ae31afc76d8f32bafb92f219e641dfde4e2b Mon Sep 17 00:00:00 2001 From: Mollusk Date: Sun, 14 Jun 2026 23:06:49 -0400 Subject: [PATCH] feat(notify): per-sound notification toggles (W6) Add a per-event enable checkbox next to each chime in Settings so a user can silence individual sounds (e.g. keep 'message'/peer-join but drop reconnect chimes) while the master 'Enable sound notifications' toggle stays as the global kill-switch. The gate lives in one place at the play() seam: a pure should_play(master, sound) AND that's unit-tested, fed by a per-sound AtomicBool array in notify keyed by a stable Sound::index/ALL. Flags persist as 8 sound_*_enabled bools in AppConfig (default true, so upgrades are silent-change-free) with sound_enabled/set_sound_enabled accessors centralizing the field mapping. The per-sound checkbox greys out (drops on_toggle) while the master is off. +3 notify unit tests (should_play truth table, index bijection, flag set/query independence) + extended config backward-compat test. 230 lib tests green, clippy clean. Co-Authored-By: Claude Opus 4.8 --- src/app/mod.rs | 23 ++++++++++-- src/config.rs | 68 ++++++++++++++++++++++++++++++++++++ src/notify.rs | 94 ++++++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 180 insertions(+), 5 deletions(-) diff --git a/src/app/mod.rs b/src/app/mod.rs index 5d6e03e..1370de2 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -141,6 +141,8 @@ pub enum AppMessage { ToggleNotifications(bool), ToggleEchoCancellation(bool), CustomSoundPathChanged(Sound, String), + /// Toggle the per-sound enable flag for a single chime (W6). + ToggleSoundEnabled(Sound, bool), ToggleMicTest(bool), /// Start/stop recording the call; the core confirms via Recording{Started,Stopped}. ToggleRecording, @@ -282,6 +284,9 @@ impl Default for AppState { config.controls_width = clamp_controls_width(config.controls_width, ww); config.chat_drawer_width = clamp_chat_drawer_width(config.chat_drawer_width, ww); notify::set_enabled(config.notifications_enabled); + for sound in Sound::ALL { + notify::set_sound_enabled(sound, config.sound_enabled(sound)); + } let _ = controller.send(CoreCommand::SetNoiseGateThreshold(config.noise_gate_threshold)); let _ = controller.send(CoreCommand::SetInputVolume(config.input_volume)); let _ = controller.send(CoreCommand::SetOutputVolume(config.output_volume)); @@ -724,6 +729,11 @@ fn update(state: &mut AppState, message: AppMessage) -> Task { Sound::ReconnectFailed => state.config.custom_sound_reconnect_failed = path_opt, } } + AppMessage::ToggleSoundEnabled(sound, enabled) => { + state.config.set_sound_enabled(sound, enabled); + state.config.save(); + notify::set_sound_enabled(sound, enabled); + } AppMessage::ToggleRecording => { // Optimistic intent; the core flips `recording` for real via the // Recording{Started,Stopped} events (so a failed start won't lie). @@ -1121,12 +1131,21 @@ fn view(state: &AppState) -> Element<'_, AppMessage> { Some(false) => text("✗ File not found").size(10).color(color_red), }; + // Per-sound enable checkbox (W6). Interactive only while the master + // toggle is on — without it every chime is muted anyway, so we drop + // `on_toggle` to render it greyed out and inert. + let mut enable = checkbox(state.config.sound_enabled(sound)).size(16); + if state.config.notifications_enabled { + enable = enable.on_toggle(move |on| AppMessage::ToggleSoundEnabled(sound, on)); + } + column![ row![ + enable, text(label).size(12).color(color_subtext), horizontal_space(), validation_widget, - ].align_y(iced::alignment::Vertical::Center), + ].spacing(6).align_y(iced::alignment::Vertical::Center), text_input("Default (embedded)...", path) .on_input(move |val| AppMessage::CustomSoundPathChanged(sound, val)) .style(t_style) @@ -1460,7 +1479,7 @@ fn view(state: &AppState) -> Element<'_, AppMessage> { .label("Enable sound notifications") .on_toggle(AppMessage::ToggleNotifications), vertical_space(6.0), - text("Custom chime files (WAV paths) — leave blank for the built-in sounds.").size(12).color(color_subtext), + text("Tick a sound to enable its chime; untick to silence just that one. Optional WAV path overrides the built-in sound (blank = built-in).").size(12).color(color_subtext), row![ path_field("Self Join", Sound::SelfJoin), path_field("Peer Join", Sound::PeerJoin), diff --git a/src/config.rs b/src/config.rs index 2d6a248..fb394cf 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,3 +1,4 @@ +use crate::notify::Sound; use crate::theme::AppTheme; use serde::{Deserialize, Serialize}; use std::fs; @@ -198,6 +199,26 @@ pub struct AppConfig { pub custom_sound_mic_toggle: Option, #[serde(default)] pub custom_sound_reconnect_failed: Option, + /// Per-sound enable flags (W6). The master `notifications_enabled` toggle + /// gates ALL chimes; these let the user silence individual events while the + /// master stays on. A chime plays only if the master AND its flag are true. + /// Default true so behaviour is unchanged until a user opts a sound out. + #[serde(default = "default_true")] + pub sound_self_join_enabled: bool, + #[serde(default = "default_true")] + pub sound_peer_join_enabled: bool, + #[serde(default = "default_true")] + pub sound_peer_leave_enabled: bool, + #[serde(default = "default_true")] + pub sound_reconnect_attempt_enabled: bool, + #[serde(default = "default_true")] + pub sound_reconnected_enabled: bool, + #[serde(default = "default_true")] + pub sound_self_leave_enabled: bool, + #[serde(default = "default_true")] + pub sound_mic_toggle_enabled: bool, + #[serde(default = "default_true")] + pub sound_reconnect_failed_enabled: bool, /// Optional override for the `pixelpass` binary location (screen share). /// Empty / unset = look it up on `$PATH`. Hand-editable; no Settings UI yet. #[serde(default)] @@ -246,6 +267,14 @@ impl Default for AppConfig { custom_sound_self_leave: None, custom_sound_mic_toggle: None, custom_sound_reconnect_failed: None, + sound_self_join_enabled: true, + sound_peer_join_enabled: true, + sound_peer_leave_enabled: true, + sound_reconnect_attempt_enabled: true, + sound_reconnected_enabled: true, + sound_self_leave_enabled: true, + sound_mic_toggle_enabled: true, + sound_reconnect_failed_enabled: true, pixelpass_path: None, window_width: default_window_width(), window_height: default_window_height(), @@ -256,6 +285,35 @@ impl Default for AppConfig { } impl AppConfig { + /// Whether the chime for `sound` is enabled (its own per-sound flag; does + /// NOT factor in the master `notifications_enabled` toggle — see `notify`). + pub fn sound_enabled(&self, sound: Sound) -> bool { + match sound { + Sound::SelfJoin => self.sound_self_join_enabled, + Sound::PeerJoin => self.sound_peer_join_enabled, + Sound::PeerLeave => self.sound_peer_leave_enabled, + Sound::ReconnectAttempt => self.sound_reconnect_attempt_enabled, + Sound::Reconnected => self.sound_reconnected_enabled, + Sound::SelfLeave => self.sound_self_leave_enabled, + Sound::MicToggle => self.sound_mic_toggle_enabled, + Sound::ReconnectFailed => self.sound_reconnect_failed_enabled, + } + } + + /// Set the per-sound enable flag for `sound`. + pub fn set_sound_enabled(&mut self, sound: Sound, enabled: bool) { + match sound { + Sound::SelfJoin => self.sound_self_join_enabled = enabled, + Sound::PeerJoin => self.sound_peer_join_enabled = enabled, + Sound::PeerLeave => self.sound_peer_leave_enabled = enabled, + Sound::ReconnectAttempt => self.sound_reconnect_attempt_enabled = enabled, + Sound::Reconnected => self.sound_reconnected_enabled = enabled, + Sound::SelfLeave => self.sound_self_leave_enabled = enabled, + Sound::MicToggle => self.sound_mic_toggle_enabled = enabled, + Sound::ReconnectFailed => self.sound_reconnect_failed_enabled = enabled, + } + } + fn config_path() -> Option { dirs::config_dir().map(|mut p| { p.push("peerspeak"); @@ -325,6 +383,16 @@ mod tests { assert!(deserialized.custom_sound_self_leave.is_none()); assert!(deserialized.custom_sound_mic_toggle.is_none()); assert!(deserialized.custom_sound_reconnect_failed.is_none()); + // Configs predating the per-sound flags (W6) enable every chime, so an + // upgrade is silent-change-free. + for sound in Sound::ALL { + assert!(deserialized.sound_enabled(sound), "{sound:?} should default on"); + } + // The accessor and mutator agree round-trip. + let mut cfg = AppConfig::default(); + cfg.set_sound_enabled(Sound::PeerJoin, false); + assert!(!cfg.sound_enabled(Sound::PeerJoin)); + assert!(cfg.sound_enabled(Sound::SelfJoin)); // Configs predating the remembered window size load the default size. assert_eq!(deserialized.window_width, 900.0); assert_eq!(deserialized.window_height, 760.0); diff --git a/src/notify.rs b/src/notify.rs index b1e1684..0e7baf9 100644 --- a/src/notify.rs +++ b/src/notify.rs @@ -17,16 +17,45 @@ use std::sync::{Mutex, OnceLock}; static ENABLED: AtomicBool = AtomicBool::new(true); -/// Enable or disable all notification chimes globally. +/// Per-sound enable flags (W6), indexed by `Sound::index`. The master `ENABLED` +/// toggle gates everything; these silence individual events while the master +/// stays on. Both must be true for a chime to play (see `should_play`). All +/// start true so behaviour is unchanged until a user opts a sound out. +static SOUND_ENABLED: [AtomicBool; Sound::COUNT] = { + // The repeat-init `const` is the standard way to build an atomic array; the + // interior-mutability lint is a false positive here (we want one fresh + // `AtomicBool` per slot, which is exactly what this does). + #[allow(clippy::declare_interior_mutable_const)] + const TRUE: AtomicBool = AtomicBool::new(true); + [TRUE; Sound::COUNT] +}; + +/// Enable or disable all notification chimes globally (the master toggle). pub fn set_enabled(enabled: bool) { ENABLED.store(enabled, Ordering::Relaxed); } -/// Check if notifications are enabled. +/// Check if notifications are enabled (the master toggle). pub fn is_enabled() -> bool { ENABLED.load(Ordering::Relaxed) } +/// Enable or disable the chime for a single event, independent of the master. +pub fn set_sound_enabled(sound: Sound, enabled: bool) { + SOUND_ENABLED[sound.index()].store(enabled, Ordering::Relaxed); +} + +/// Whether the chime for `sound` is enabled (its own flag only, not the master). +pub fn is_sound_enabled(sound: Sound) -> bool { + SOUND_ENABLED[sound.index()].load(Ordering::Relaxed) +} + +/// The single gate deciding whether a chime plays: the master toggle AND the +/// per-sound flag must both be on. Pure so the rule is unit-testable. +pub fn should_play(master_enabled: bool, sound_enabled: bool) -> bool { + master_enabled && sound_enabled +} + /// A notification event with a distinct chime. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -50,6 +79,35 @@ pub enum Sound { } impl Sound { + /// Every notification event, in a stable order (also the `index` order). + pub const ALL: [Sound; Sound::COUNT] = [ + Sound::SelfJoin, + Sound::PeerJoin, + Sound::PeerLeave, + Sound::ReconnectAttempt, + Sound::Reconnected, + Sound::SelfLeave, + Sound::MicToggle, + Sound::ReconnectFailed, + ]; + + /// Number of distinct notification events. + pub const COUNT: usize = 8; + + /// Stable 0-based index into the per-sound flag array. Must match `ALL`. + fn index(self) -> usize { + match self { + Sound::SelfJoin => 0, + Sound::PeerJoin => 1, + Sound::PeerLeave => 2, + Sound::ReconnectAttempt => 3, + Sound::Reconnected => 4, + Sound::SelfLeave => 5, + Sound::MicToggle => 6, + Sound::ReconnectFailed => 7, + } + } + /// Embedded WAV bytes for this sound. fn bytes(self) -> &'static [u8] { match self { @@ -106,7 +164,7 @@ pub fn validate_custom_path(path_str: &str) -> Option { /// Play a notification chime, fire-and-forget. Never blocks; never errors out. pub fn play(sound: Sound, custom_path: Option<&str>) { - if !is_enabled() { + if !should_play(is_enabled(), is_sound_enabled(sound)) { return; } let custom_path = custom_path.map(String::from); @@ -167,6 +225,36 @@ fn spawn_player(path: &Path) { mod tests { use super::*; + #[test] + fn test_should_play_truth_table() { + // Plays only when BOTH the master and the per-sound flag are on. + assert!(should_play(true, true)); + assert!(!should_play(true, false)); + assert!(!should_play(false, true)); + assert!(!should_play(false, false)); + } + + #[test] + fn test_sound_indices_unique_and_match_all() { + // `index()` must be a 0..COUNT bijection in `ALL` order, or the flag + // array would alias two sounds onto one slot. + for (expected, sound) in Sound::ALL.iter().enumerate() { + assert_eq!(sound.index(), expected, "index drift for {sound:?}"); + } + assert_eq!(Sound::ALL.len(), Sound::COUNT); + } + + #[test] + fn test_set_and_query_per_sound_flag() { + // Toggling one sound's flag is independent of the others. (Uses + // PeerLeave to avoid clashing with flags other tests might touch.) + set_sound_enabled(Sound::PeerLeave, false); + assert!(!is_sound_enabled(Sound::PeerLeave)); + assert!(is_sound_enabled(Sound::PeerJoin)); + set_sound_enabled(Sound::PeerLeave, true); + assert!(is_sound_enabled(Sound::PeerLeave)); + } + #[test] fn test_expand_tilde() { if let Some(home) = dirs::home_dir() {