Say unread on the tray icon itself, not only in its tooltip

crates/daemon/src/tray/linux.rs

@@ -3,10 +3,20 @@
 use std::sync::Arc;
 
 use anyhow::{Context, Result};
-use ksni::{Handle, Icon, MenuItem, ToolTip, Tray as KsniTray, TrayMethods, menu::StandardItem};
+use ksni::{
+    Category, Handle, Icon, MenuItem, Status, ToolTip, Tray as KsniTray, TrayMethods,
+    menu::StandardItem,
+};
 
 use crate::state::{StateHub, TrayState};
 
+/// The themed name the icon takes while something is waiting to be read.
+///
+/// A status name from the icon-naming spec, like the two below it, so it
+/// follows the user's theme instead of shipping pixels. There is no count in
+/// it — StatusNotifierItem has no badge, and the number is the tooltip's job.
+const UNREAD_ICON: &str = "mail-unread";
+
 /// The icon's model. ksni renders from this; the daemon updates it.
 struct Item {
     state: TrayState,
@@ -20,7 +30,31 @@ struct Item {
     click: crate::window::Toggle,
 }
 
+impl Item {
+    /// The count the icon and the tooltip both speak from.
+    ///
+    /// Zero while the connection is down: what we last heard is then a number
+    /// nothing is refreshing, and an icon asking to be looked at over a stale
+    /// count is worse than one saying the connection is what is wrong. One
+    /// method rather than the test written twice, so the icon and the tooltip
+    /// cannot disagree about what is unread.
+    fn unread(&self) -> u32 {
+        if self.state.connected {
+            self.state.unread
+        } else {
+            0
+        }
+    }
+}
+
 impl KsniTray for Item {
+    /// What kind of thing the icon is. Hosts group by this and some only
+    /// honour an attention state for a communications item, which is exactly
+    /// the state below.
+    fn category(&self) -> Category {
+        Category::Communications
+    }
+
     fn id(&self) -> String {
         // Stable across restarts so the host can keep the icon's position.
         "oxidezap".into()
@@ -33,17 +67,38 @@ impl KsniTray for Item {
     /// Named from the icon theme rather than shipping pixels: a themed name
     /// follows the user's icon set and dark/light switch for free.
     fn icon_name(&self) -> String {
-        if self.state.connected {
-            "user-available".into()
-        } else {
-            "user-offline".into()
+        match (self.state.connected, self.unread()) {
+            (false, _) => "user-offline".into(),
+            (true, 0) => "user-available".into(),
+            // Said here as well as in `attention_icon_name` because a host
+            // may honour one and not the other: the spec's attention icon is
+            // what a host swaps to for the status below, and a host that
+            // ignores the status entirely still reads this one. Both name the
+            // same icon, so the two answers cannot show different things.
+            (true, _) => UNREAD_ICON.into(),
         }
     }
 
     fn icon_pixmap(&self) -> Vec<Icon> {
         Vec::new()
     }
 
+    /// Unread mail is what the status is for: a host is asked to emphasise
+    /// the icon, and one that hides passive items keeps this one visible.
+    fn status(&self) -> Status {
+        if self.unread() > 0 {
+            Status::NeedsAttention
+        } else {
+            Status::Active
+        }
+    }
+
+    /// The icon a host swaps to under [`Status::NeedsAttention`]. Read only
+    /// in that state, so it can be unconditional.
+    fn attention_icon_name(&self) -> String {
+        UNREAD_ICON.into()
+    }
+
     /// A click on the icon. The host sends this for a left click, and what
     /// it means depends on what is up: away if there is a window, up if
     /// there is not. The daemon has no window, so both are requests — see
@@ -64,14 +119,24 @@ impl KsniTray for Item {
     fn menu_about_to_show(&mut self) {}
 
     fn tool_tip(&self) -> ToolTip {
-        let description = match (self.state.connected, self.state.unread) {
+        let description = match (self.state.connected, self.unread()) {
             (false, _) => "Disconnected".to_string(),
             (true, 0) => "Connected".to_string(),
             (true, 1) => "1 unread message".to_string(),
             (true, n) => format!("{n} unread messages"),
         };
+        // The count rides in the title too: a host that renders only the
+        // first line of a tooltip is otherwise told nothing, and the number
+        // is the one thing the icon itself cannot say.
+        let title = match self.unread() {
+            0 => "oxidezap".to_string(),
+            n => format!("oxidezap ({n})"),
+        };
         ToolTip {
-            title: "oxidezap".into(),
+            // Named so a tooltip that draws an icon draws the one the tray
+            // is showing rather than a default.
+            icon_name: self.icon_name(),
+            title,
             description,
             ..Default::default()
         }
@@ -160,3 +225,69 @@ pub async fn start(hub: Arc<StateHub>) -> Result<Box<dyn super::Tray>> {
         .context("registering a StatusNotifierItem (is a tray host running?)")?;
     Ok(Box::new(LinuxTray { handle }))
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// An icon over a state, with the two things the tray speaks back
+    /// through left at their defaults — nothing below asks them anything.
+    fn item(connected: bool, unread: u32) -> Item {
+        Item {
+            state: TrayState { connected, unread },
+            hub: StateHub::new(),
+            click: crate::window::Toggle::default(),
+        }
+    }
+
+    /// The report this comes from: messages waiting, and an icon that looked
+    /// exactly like an idle one. The tooltip knew; nothing a glance reaches
+    /// did.
+    #[test]
+    fn unread_reaches_the_icon_itself() {
+        let idle = item(true, 0);
+        let waiting = item(true, 3);
+
+        assert_ne!(
+            waiting.icon_name(),
+            idle.icon_name(),
+            "an icon with something to read must not look like one without"
+        );
+        assert_eq!(waiting.icon_name(), UNREAD_ICON);
+        assert_eq!(waiting.status(), Status::NeedsAttention);
+        assert_eq!(
+            waiting.attention_icon_name(),
+            waiting.icon_name(),
+            "a host honouring the status and one ignoring it must show the same icon"
+        );
+        assert_eq!(idle.status(), Status::Active);
+    }
+
+    /// A count nothing is refreshing is not news. The connection is what the
+    /// icon has to say then, and the tooltip already agreed.
+    #[test]
+    fn a_disconnected_icon_says_the_connection_and_not_a_stale_count() {
+        let offline = item(false, 3);
+
+        assert_eq!(offline.icon_name(), "user-offline");
+        assert_eq!(offline.status(), Status::Active);
+        assert_eq!(offline.tool_tip().description, "Disconnected");
+        assert_eq!(offline.tool_tip().title, "oxidezap");
+    }
+
+    /// The number itself has nowhere on the icon to live: SNI carries no
+    /// badge, so the tooltip is where it is said — in the title as well as
+    /// the description, since a host may render only the first line.
+    #[test]
+    fn the_tooltip_carries_the_count() {
+        assert_eq!(item(true, 0).tool_tip().title, "oxidezap");
+        assert_eq!(item(true, 1).tool_tip().title, "oxidezap (1)");
+        assert_eq!(item(true, 1).tool_tip().description, "1 unread message");
+        assert_eq!(item(true, 4).tool_tip().description, "4 unread messages");
+        assert_eq!(
+            item(true, 4).tool_tip().icon_name,
+            item(true, 4).icon_name(),
+            "a tooltip that draws an icon draws the one being shown"
+        );
+    }
+}

docs/gotchas.md

@@ -203,6 +203,23 @@ Non-obvious behaviour, and the reasoning behind it. Read the entry before changi
   what it says — and is re-read when the host opens the menu, which ksni
   does only for a tray that implements `menu_about_to_show`; the tray
   otherwise hears only `TrayState`, which a window attaching does not move.
+- **Unread has to reach the icon, not only its tooltip.** StatusNotifierItem
+  carries no badge and no number, so a count can only be spoken as a *state*:
+  the icon takes a themed `mail-unread` while anything is waiting, and the
+  item's status goes to `NeedsAttention`, which is also what keeps it visible
+  in a host that hides passive items. Both are said, because a host may honour
+  one and not the other — the status swaps to `attention_icon_name`, and a
+  host ignoring the status entirely still reads `icon_name` — and both name
+  the same icon so the two answers cannot differ. The category is
+  `Communications` for the same reason: some hosts only emphasise an item
+  that claims to be one. The number itself stays in the tooltip, in the title
+  as well as the description, since a host may render only the first line.
+  The count is read as zero while the connection is down (`Item::unread`,
+  which the icon and the tooltip both go through, so they cannot disagree):
+  what we last heard is then a number nothing is refreshing, and the one
+  useful thing to say at a glance is that the connection is what is wrong.
+  It was reported as an icon that looked idle over a full chat list — the
+  tooltip knew, and nothing a glance reaches did.
 - **What a file is sent as is decided in the front end; what it looks like is
   worked out where the bytes land.** Two questions, and they are answered in
   two places because they have two different pieces of evidence. The *kind* —