From 4684e01e89d18c8ce25dd7971106554810c8908f Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sat, 12 Sep 2026 09:55:27 +0700 Subject: [PATCH] update --- crates/signed_core/src/inbox.rs | 194 +++++++++++++-- crates/signed_state/src/inbox.rs | 24 +- crates/signed_ui/src/user_avatar.rs | 13 +- crates/workspace/src/views/inbox.rs | 364 +++++++++++----------------- docs/inbox-plan.md | 121 +++++++-- 5 files changed, 443 insertions(+), 273 deletions(-) diff --git a/crates/signed_core/src/inbox.rs b/crates/signed_core/src/inbox.rs index 57aed3f..463ad48 100644 --- a/crates/signed_core/src/inbox.rs +++ b/crates/signed_core/src/inbox.rs @@ -4,7 +4,7 @@ use std::time::Duration; use nostr::prelude::*; use serde::{Deserialize, Serialize}; -use crate::{COVER_NOTE_KIND, RepoAddr}; +use crate::{COVER_NOTE_KIND, RepoAddr, activity_subject}; /// Window before `now` that an advanced cutoff retreats to. const ADVANCE_WINDOW: Duration = Duration::from_secs(3 * 24 * 60 * 60); @@ -12,33 +12,92 @@ const ADVANCE_WINDOW: Duration = Duration::from_secs(3 * 24 * 60 * 60); /// Window before `now` that a mark-all cutoff retreats to. const MARK_ALL_WINDOW: Duration = Duration::from_secs(10 * 24 * 60 * 60); -/// A thread of notification events sharing one root +/// A thread of notification and own-activity events sharing one root. #[derive(Debug, Clone)] pub struct InboxItem { - /// The root issue, patch or pull request the notifications belong to. + /// The root issue, patch or pull request the events belong to. pub root: EventId, + /// The root event itself, when it is known locally. + pub root_event: Option, /// Kind of the root event, when it is known locally. pub root_kind: Option, /// Repository the root belongs to, from the root's `a` tag. pub address: Option, - /// Events in the group, newest first. + /// Notification events directed at the user, newest first. pub events: Vec, + /// The user's own events in the thread, newest first. + pub own_events: Vec, /// Unread event ids, oldest first. pub unread_ids: Vec, - /// Whether every event in the group is archived. + /// Whether every notification event in the thread is archived. pub archived: bool, } impl InboxItem { - /// Timestamp of the newest event in the group. + /// Title of the thread, read from its root issue/patch/PR when known. + pub fn title(&self) -> String { + self.root_event + .as_ref() + .or_else(|| self.own_events.first()) + .or_else(|| self.events.first()) + .map(activity_subject) + .unwrap_or_else(|| "Untitled".to_string()) + } + + /// Kind shown for the thread. + pub fn kind(&self) -> Option { + self.root_kind.or_else(|| { + self.root_event + .as_ref() + .or_else(|| self.own_events.first()) + .or_else(|| self.events.first()) + .map(|event| event.kind) + }) + } + + /// Timestamp of the newest event in the thread. pub fn latest_activity(&self) -> Timestamp { - self.events - .first() + self.root_event + .as_ref() + .into_iter() + .chain(self.own_events.first()) + .chain(self.events.first()) .map(|event| event.created_at) + .max() .unwrap_or_default() } - /// Whether the group has an unread event still visible in the inbox. + /// Up to `limit` events of the thread, oldest first. + pub fn timeline(&self, limit: usize) -> Vec { + let mut seen: HashSet = HashSet::new(); + let mut events: Vec = Vec::new(); + + if let Some(root) = &self.root_event { + seen.insert(root.id); + events.push(root.clone()); + } + + let mut rest: Vec = self + .own_events + .iter() + .chain(self.events.iter()) + .filter(|event| seen.insert(event.id)) + .cloned() + .collect(); + + rest.sort_by(|a, b| { + b.created_at + .cmp(&a.created_at) + .then_with(|| b.id.to_hex().cmp(&a.id.to_hex())) + }); + rest.truncate(limit.saturating_sub(events.len())); + events.extend(rest); + + events.sort_by_key(|event| event.created_at); + events + } + + /// Whether the thread has an unread event still visible in the inbox. pub fn is_unread(&self) -> bool { !self.archived && !self.unread_ids.is_empty() } @@ -53,7 +112,9 @@ impl InboxItem { .map(|event| event.id) .collect(); - self.archived = self.events.iter().all(|event| state.is_archived(event)); + // A thread without notification events is never archived. + self.archived = + !self.events.is_empty() && self.events.iter().all(|event| state.is_archived(event)); } } @@ -97,10 +158,17 @@ where } } -/// Group notification events by root, newest activity first. -pub fn group(events: E, me: PublicKey, state: &InboxReadState, lookup: &L) -> Vec +/// Group notification events and the user's own events into one item per thread. +pub fn group( + events: E, + own: O, + me: PublicKey, + state: &InboxReadState, + lookup: &L, +) -> Vec where E: IntoIterator, + O: IntoIterator, L: Fn(EventId) -> Option, { let mut groups: HashMap> = HashMap::new(); @@ -114,14 +182,23 @@ where groups.entry(root).or_default().push(event); } - let mut items: Vec = groups + let mut own_groups: HashMap> = HashMap::new(); + for event in own { + let root = notification_root(&event, lookup).unwrap_or(event.id); + own_groups.entry(root).or_default().push(event); + } + + let mut roots: Vec = groups.keys().chain(own_groups.keys()).copied().collect(); + roots.sort(); + roots.dedup(); + + let mut items: Vec = roots .into_iter() - .map(|(root, mut events)| { - events.sort_by(|a, b| { - b.created_at - .cmp(&a.created_at) - .then_with(|| b.id.to_hex().cmp(&a.id.to_hex())) - }); + .map(|root| { + let mut events = groups.remove(&root).unwrap_or_default(); + let mut own_events = own_groups.remove(&root).unwrap_or_default(); + sort_newest_first(&mut events); + sort_newest_first(&mut own_events); let root_event = lookup(root); @@ -131,7 +208,9 @@ where address: root_event .as_ref() .and_then(|event| event.tags.coordinates().next()), + root_event, events, + own_events, unread_ids: Vec::new(), archived: false, }; @@ -149,6 +228,15 @@ where items } +/// Sort thread events newest first, ties broken by id. +fn sort_newest_first(events: &mut [Event]) { + events.sort_by(|a, b| { + b.created_at + .cmp(&a.created_at) + .then_with(|| b.id.to_hex().cmp(&a.id.to_hex())) + }); +} + /// Read and archive state of the inbox, a high-water-mark model. #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct InboxReadState { @@ -392,6 +480,15 @@ mod tests { signed(author, Kind::GitIssue, Vec::new(), at) } + fn titled_issue(author: &Keys, title: &str, at: u64) -> Event { + signed( + author, + Kind::GitIssue, + vec![Tag::parse(["subject", title]).expect("valid subject tag")], + at, + ) + } + #[test] fn issue_and_pull_request_are_their_own_root() { let events = [ @@ -496,6 +593,7 @@ mod tests { let events = [issue.clone(), comment.clone(), other_issue.clone(), mine]; let items = group( events, + Vec::new(), me.public_key(), &InboxReadState::default(), &lookup(&[]), @@ -518,6 +616,7 @@ mod tests { let events = [issue.clone(), older.clone(), newer.clone()]; let items = group( events, + Vec::new(), me.public_key(), &InboxReadState::default(), &lookup(&[]), @@ -532,6 +631,7 @@ mod tests { }; let items = group( [issue.clone(), older, newer], + Vec::new(), me.public_key(), &state, &lookup(&[]), @@ -557,6 +657,7 @@ mod tests { let events = [issue.clone(), comment]; let items = group( events.clone(), + Vec::new(), me.public_key(), &InboxReadState::default(), &lookup(&events), @@ -566,6 +667,59 @@ mod tests { assert_eq!(items[0].address, issue.tags.coordinates().next()); } + #[test] + fn group_merges_own_events_into_the_matching_thread() { + let me = keys(1); + let issue = titled_issue(&me, "Add retry logic", 100); + let mine = signed( + &me, + Kind::Comment, + vec![ + uppercase_e_tag(&issue), + Tag::parse(["K", "1621"]).expect("K tag"), + ], + 150, + ); + let reply = signed( + &keys(2), + Kind::Comment, + vec![ + uppercase_e_tag(&issue), + Tag::parse(["K", "1621"]).expect("K tag"), + ], + 200, + ); + + let context = [issue.clone(), mine.clone(), reply.clone()]; + let items = group( + [reply.clone()], + [issue.clone(), mine.clone()], + me.public_key(), + &InboxReadState::default(), + &lookup(&context), + ); + + assert_eq!(items.len(), 1); + assert_eq!(items[0].root, issue.id); + assert_eq!( + items[0].root_event.as_ref().map(|event| event.id), + Some(issue.id) + ); + assert_eq!(items[0].kind(), Some(Kind::GitIssue)); + assert_eq!(items[0].title(), "Add retry logic"); + assert_eq!(items[0].events, vec![reply.clone()]); + // The own events are kept apart from the notifications, newest first. + assert_eq!(items[0].own_events, vec![mine.clone(), issue.clone()]); + assert_eq!( + items[0] + .timeline(5) + .iter() + .map(|event| event.id) + .collect::>(), + vec![issue.id, mine.id, reply.id] + ); + } + #[test] fn mark_all_read_marks_known_recent_events() { let me = keys(1); @@ -663,9 +817,11 @@ mod tests { let second = issue(&keys(2), now.as_secs() - 1000); let mut item = InboxItem { root: first.id, + root_event: None, root_kind: None, address: None, events: vec![second.clone(), first.clone()], + own_events: Vec::new(), unread_ids: Vec::new(), archived: false, }; diff --git a/crates/signed_state/src/inbox.rs b/crates/signed_state/src/inbox.rs index 51ccf62..011dba1 100644 --- a/crates/signed_state/src/inbox.rs +++ b/crates/signed_state/src/inbox.rs @@ -128,23 +128,16 @@ impl Inbox { } } -/// Derive the inbox home screen's lists for `me` from the local database. -/// -/// Returns the notification groups, the user's own git activity and the number -/// of non-archived groups with an unread event. +/// Derive the inbox home screen's threads for `me` from the local database. pub async fn query_inbox( client: &Client, me: PublicKey, state: &InboxReadState, -) -> Result<(Vec, Vec, usize), Error> { +) -> Result<(Vec, usize), Error> { let deletion_events = client.database().query(filters::deletions()).await?; let deletions = Deletions::from_events(deletion_events); - let (notification_events, by_id) = fetch_notifications(client, me, &deletions).await?; - let notifications = inbox::group(notification_events, me, state, &|id| { - by_id.get(&id).cloned() - }); - let unread_count = notifications.iter().filter(|item| item.is_unread()).count(); + let (notification_events, mut by_id) = fetch_notifications(client, me, &deletions).await?; let mut activity = Vec::new(); for event in client @@ -155,16 +148,17 @@ pub async fn query_inbox( if deletions.is_deleted(&event) || !filters::is_git_activity(&event) { continue; } + by_id.entry(event.id).or_insert_with(|| event.clone()); activity.push(event); } - activity.sort_by(|a, b| { - b.created_at - .cmp(&a.created_at) - .then_with(|| b.id.to_hex().cmp(&a.id.to_hex())) + let items = inbox::group(notification_events, activity, me, state, &|id| { + by_id.get(&id).cloned() }); - Ok((notifications, activity, unread_count)) + let unread_count = items.iter().filter(|item| item.is_unread()).count(); + + Ok((items, unread_count)) } /// `d` tag identifying the inbox state event of `me`. diff --git a/crates/signed_ui/src/user_avatar.rs b/crates/signed_ui/src/user_avatar.rs index 6d986a9..c080a56 100644 --- a/crates/signed_ui/src/user_avatar.rs +++ b/crates/signed_ui/src/user_avatar.rs @@ -1,7 +1,7 @@ use gpui::prelude::*; use gpui::{App, SharedString, StyleRefinement, Window}; use gpui_component::avatar::Avatar; -use gpui_component::{ActiveTheme, Sizable, StyledExt}; +use gpui_component::{ActiveTheme, Sizable, Size, StyledExt}; /// A small user avatar from gpui-component [`Avatar`], rounded with the theme radius. /// It shows the user's picture or falls back to name initials. @@ -9,6 +9,7 @@ use gpui_component::{ActiveTheme, Sizable, StyledExt}; pub struct UserAvatar { name: SharedString, picture: Option, + size: Size, style: StyleRefinement, } @@ -19,6 +20,7 @@ impl UserAvatar { Self { name: name.into(), picture: None, + size: Size::Small, style: StyleRefinement::default(), } } @@ -30,6 +32,13 @@ impl UserAvatar { } } +impl Sizable for UserAvatar { + fn with_size(mut self, size: impl Into) -> Self { + self.size = size.into(); + self + } +} + impl Styled for UserAvatar { fn style(&mut self) -> &mut StyleRefinement { &mut self.style @@ -43,6 +52,6 @@ impl RenderOnce for UserAvatar { .when_some(self.picture, |this, url| this.src(url)) .rounded(cx.theme().radius) .refine_style(&self.style) - .small() + .with_size(self.size) } } diff --git a/crates/workspace/src/views/inbox.rs b/crates/workspace/src/views/inbox.rs index 4ce8b2b..109f661 100644 --- a/crates/workspace/src/views/inbox.rs +++ b/crates/workspace/src/views/inbox.rs @@ -12,10 +12,8 @@ use gpui::{ }; use gpui_component::button::{Button, ButtonVariants}; use gpui_component::{ActiveTheme, Icon, IconName, IconNamed, Sizable, StyledExt, h_flex, v_flex}; -use nostr::prelude::{Event, EventId, Kind, Timestamp}; -use signed_core::{ - COVER_NOTE_KIND, InboxItem, InboxReadState, RepoAddr, activity_subject, filters, -}; +use nostr::prelude::{Event, EventId, Kind, PublicKey, Timestamp}; +use signed_core::{COVER_NOTE_KIND, InboxItem, InboxReadState, RepoAddr, filters}; use signed_state::{ Backend, BackendEvent, ProfileStore, RefreshGate, RefreshRequest, RepoListStore, query_inbox, }; @@ -30,29 +28,21 @@ const REFRESH_DEBOUNCE: Duration = Duration::from_millis(300); /// Extra list rows measured above and below the visible area. const LIST_OVERDRAW: Pixels = px(400.); -/// A repository's slice of the inbox: its notification groups and own activity. +/// Maximum number of sub-activity lines shown under a thread row. +const MAX_SUB_ACTIVITIES: usize = 5; + +/// A repository's slice of the inbox: the threads that belong to it. struct InboxSection { /// Repository the section groups, `None` for items without one. address: Option, - /// Number of notification groups with an unread event. + /// Number of threads with an unread event. unread: usize, - /// Notification and activity rows, newest first. - entries: Vec, + /// Indices into the threads, newest activity first. + entries: Vec, /// Timestamp of the newest entry, used to order the sections. latest: Timestamp, } -/// One row inside a repository section, as an index into the inbox's own lists. -#[derive(Clone, Copy)] -enum InboxEntry { - /// Index into the notification groups. - Notification(usize), - /// Index into the user's own activity. - Activity(usize), -} - -/// One row of the flattened inbox: a repository header, one of its entries, or -/// the empty state of a repository without any. #[derive(Clone, Copy)] enum InboxRow { Repo(usize), @@ -63,15 +53,13 @@ enum InboxRow { pub struct InboxView { focus_handle: FocusHandle, dock_area: WeakEntity, - /// Notifications grouped by thread root, newest activity first. - notifications: Arc>, - /// The user's own recent git activity, newest first. - activity: Arc>, - /// The notification and activity lists grouped by repository, newest first. + /// One row per thread, merging notifications and own activity, newest first. + threads: Arc>, + /// The threads grouped by repository, newest first. sections: Arc>, /// The flattened repository headers and rows of the list. rows: Arc>, - /// Number of non-archived groups with an unread event. + /// Number of non-archived threads with an unread event. unread_count: usize, /// Copy of the global read state the current lists were derived with. state: InboxReadState, @@ -118,8 +106,7 @@ impl InboxView { Self { focus_handle: cx.focus_handle(), dock_area, - notifications: Arc::new(Vec::new()), - activity: Arc::new(Vec::new()), + threads: Arc::new(Vec::new()), sections: Arc::new(Vec::new()), rows: Arc::new(Vec::new()), unread_count: 0, @@ -144,17 +131,17 @@ impl InboxView { /// Re-derive from the global state when it is loaded or changes. pub fn sync_state(&mut self, cx: &mut Context) { - let inbox = Backend::global(cx).read(cx).inbox(); + let backend = Backend::global(cx); + let inbox = backend.read(cx).inbox(); + let (loaded, state) = { let inbox = inbox.read(cx); (inbox.is_loaded(), inbox.state().clone()) }; if !loaded { - let was_present = self.state_loaded - || !self.notifications.is_empty() - || !self.activity.is_empty() - || !self.sections.is_empty(); + let was_present = + self.state_loaded || !self.threads.is_empty() || !self.sections.is_empty(); self.clear(); if was_present { cx.notify(); @@ -240,7 +227,7 @@ impl InboxView { let work = cx.background_spawn(async move { query_inbox(&client, me, &state).await }); self.tasks.push(cx.spawn(async move |this, cx| { - let (notifications, activity, unread_count) = match work.await { + let (threads, unread_count) = match work.await { Ok(results) => results, Err(error) => { log::warn!("inbox refresh failed: {error}"); @@ -254,8 +241,7 @@ impl InboxView { return false; } - this.notifications = Arc::new(notifications); - this.activity = Arc::new(activity); + this.threads = Arc::new(threads); this.unread_count = unread_count; this.rebuild(cx); cx.notify(); @@ -273,22 +259,22 @@ impl InboxView { /// Recompute the unread and archived flags from the current state. fn regroup(&mut self, cx: &mut Context) { - let mut items = (*self.notifications).clone(); + let mut items = (*self.threads).clone(); for item in items.iter_mut() { item.apply_state(&self.state); } self.unread_count = items.iter().filter(|item| item.is_unread()).count(); - self.notifications = Arc::new(items); + self.threads = Arc::new(items); self.rebuild(cx); } - /// Regroup the current lists by repository and flatten them into rows. + /// Regroup the current threads by repository and flatten them into rows. fn rebuild(&mut self, cx: &mut Context) { let backend = Backend::global(cx); let repo_list = RepoListStore::global(cx); - let mut sections = group_sections(&self.notifications, &self.activity); + let mut sections = group_sections(&self.threads); if let Some(me) = backend.read(cx).current_user() { for announcement in repo_list.read(cx).announcements_of(&me) { @@ -317,8 +303,7 @@ impl InboxView { /// Forget everything derived for the current user. fn clear(&mut self) { - self.notifications = Arc::new(Vec::new()); - self.activity = Arc::new(Vec::new()); + self.threads = Arc::new(Vec::new()); self.sections = Arc::new(Vec::new()); self.rows = Arc::new(Vec::new()); self.unread_count = 0; @@ -328,9 +313,9 @@ impl InboxView { self.refresh = RefreshGate::default(); } - /// Every event in every group, archived groups included. + /// Every notification event in every thread, archived threads included. fn all_notification_events(&self) -> Vec { - self.notifications + self.threads .iter() .flat_map(|item| item.events.iter().cloned()) .collect() @@ -354,44 +339,36 @@ impl InboxView { return div().into_any_element(); }; - let Some(entry) = section.entries.get(entry_ix) else { + let Some(&thread_ix) = section.entries.get(entry_ix) else { return div().into_any_element(); }; - match *entry { - InboxEntry::Notification(item_ix) => { - let Some(item) = self.notifications.get(item_ix) else { - return div().into_any_element(); - }; + let Some(item) = self.threads.get(thread_ix) else { + return div().into_any_element(); + }; - let root = item.root; - let kind = item.root_kind; - let address = section.address.clone(); - let dock_area = self.dock_area.clone(); + let root = item.root; + let kind = item.root_kind; + let address = section.address.clone(); + let dock_area = self.dock_area.clone(); + let first = entry_ix == 0; + let last = entry_ix + 1 == section.entries.len(); - notification_row("inbox-row", ix, item, cx) - .on_click(move |_, window, cx| { - open_item(&dock_area, root, kind, address.clone(), window, cx); - }) - .into_any_element() - } - InboxEntry::Activity(event_ix) => { - let Some(event) = self.activity.get(event_ix) else { - return div().into_any_element(); - }; - activity_row(ix, event, cx) - } - } + thread("inbox-row", ix, item, first, last, cx) + .on_click(move |_, window, cx| { + open_item(&dock_area, root, kind, address.clone(), window, cx); + }) + .into_any_element() } } } } -/// Group the notification groups and own activity into one section per repository. -fn group_sections(notifications: &[InboxItem], activity: &[Event]) -> Vec { +/// Group the threads into one section per repository. +fn group_sections(threads: &[InboxItem]) -> Vec { let mut by_repo: HashMap, InboxSection> = HashMap::new(); - for (ix, item) in notifications.iter().enumerate() { + for (ix, item) in threads.iter().enumerate() { if item.archived { continue; } @@ -411,29 +388,16 @@ fn group_sections(notifications: &[InboxItem], activity: &[Event]) -> Vec = by_repo.into_values().collect(); for section in &mut sections { section.entries.sort_by(|a, b| { - entry_time(b, notifications, activity).cmp(&entry_time(a, notifications, activity)) + threads[*b] + .latest_activity() + .cmp(&threads[*a].latest_activity()) }); } @@ -441,20 +405,6 @@ fn group_sections(notifications: &[InboxItem], activity: &[Event]) -> Vec Timestamp { - match entry { - InboxEntry::Notification(ix) => notifications - .get(*ix) - .map(InboxItem::latest_activity) - .unwrap_or_default(), - InboxEntry::Activity(ix) => activity - .get(*ix) - .map(|event| event.created_at) - .unwrap_or_default(), - } -} - /// Flatten the sections into the list of repository headers and their rows. fn flatten_rows(sections: &[InboxSection]) -> Vec { let mut rows = Vec::new(); @@ -475,14 +425,6 @@ fn flatten_rows(sections: &[InboxSection]) -> Vec { rows } -/// Repository an activity event belongs to, from its `a` tag. -fn repo_address(event: &Event) -> Option { - event - .tags - .coordinates() - .find(|address| address.kind == Kind::GitRepoAnnouncement) -} - /// Display name of the repository at `addr`, from the announcement store. fn repo_name(addr: Option<&RepoAddr>, cx: &App) -> Option { let repo_list = RepoListStore::global(cx); @@ -526,15 +468,13 @@ fn empty_section_row(cx: &App) -> AnyElement { .w_full() .px_3() .text_xs() - .text_color(cx.theme().muted_foreground) - .bg(cx.theme().secondary) + .text_color(cx.theme().secondary_foreground) + .bg(cx.theme().secondary.alpha(0.6)) .rounded(cx.theme().radius) .child(SharedString::from("No activity yet.")) .into_any_element() } -/// Open the repository of a notification group, -/// and the issue or pull request detail when the group's root is one. fn open_item( dock_area: &WeakEntity, root: EventId, @@ -572,30 +512,40 @@ fn open_item( open_repo_item(dock_area, store, item, window, cx); } -fn notification_row(prefix: &'static str, ix: usize, item: &InboxItem, cx: &App) -> Stateful
{ - let Some(newest) = item.events.first() else { - return div().id((prefix, ix)); - }; - - let profile_store = ProfileStore::global(cx); - let profile = profile_store.read(cx).get(&newest.pubkey); - let author = profile.name(); - - let picture = profile.picture(); - let kind = item.root_kind.unwrap_or(newest.kind); - let subject = SharedString::from(activity_subject(newest)); - let age = relative_time(item.latest_activity()); +fn thread( + prefix: &'static str, + ix: usize, + item: &InboxItem, + first: bool, + last: bool, + cx: &App, +) -> Stateful
{ + let title = SharedString::from(item.title()); + let kind = item.kind().unwrap_or(Kind::Comment); let unread = item.is_unread(); + let backend = Backend::global(cx); + let me = backend.read(cx).current_user(); + + let mut timeline = v_flex().gap_2().w_full(); + + for event in item.timeline(MAX_SUB_ACTIVITIES) { + timeline = timeline.child(sub_activity_line(&event, me, cx)); + } + v_flex() .id((prefix, ix)) - .h_16() .w_full() .px_3() - .gap_1() - .justify_center() - .bg(cx.theme().secondary) - .hover(|this| this.bg(cx.theme().secondary_hover)) + .py_2() + .gap_2() + .bg(cx.theme().secondary.alpha(0.6)) + .when(first, |this| this.rounded_t(cx.theme().radius)) + .when(last, |this| this.rounded_b(cx.theme().radius)) + .when(!last, |this| { + this.border_b_1().border_color(cx.theme().border) + }) + .hover(|this| this.bg(cx.theme().secondary_hover.alpha(0.8))) .child( h_flex() .gap_2() @@ -613,7 +563,7 @@ fn notification_row(prefix: &'static str, ix: usize, item: &InboxItem, cx: &App) .min_w_0() .whitespace_nowrap() .text_ellipsis() - .child(subject), + .child(title), ) .child(div().flex_1()) .when(unread, |this| { @@ -626,72 +576,44 @@ fn notification_row(prefix: &'static str, ix: usize, item: &InboxItem, cx: &App) ) }), ) - .child( - h_flex() - .w_full() - .gap_2() - .child(div().w_6().flex_shrink_0()) - .child( - h_flex() - .flex_1() - .gap_1() - .items_center() - .text_xs() - .text_color(cx.theme().muted_foreground) - .child( - h_flex() - .gap_1() - .child(UserAvatar::new(author.clone()).picture(picture)) - .child(author.clone()), - ) - .child("opened") - .child(SharedString::from(kind_label(kind))) - .child(div().flex_1()) - .child(SharedString::from(age)), - ), - ) + .child(timeline) } -fn activity_row(ix: usize, event: &Event, cx: &App) -> AnyElement { - let kind = event.kind; - let subject = SharedString::from(activity_subject(event)); - let age = relative_time(event.created_at); +fn sub_activity_line(event: &Event, me: Option, cx: &App) -> AnyElement { + let profile_store = ProfileStore::global(cx).read(cx); + let profile = profile_store.get(&event.pubkey); - v_flex() - .id(("activity-row", ix)) - .h_16() + let name = if Some(event.pubkey) == me { + SharedString::from("You") + } else { + profile.name() + }; + + h_flex() .w_full() - .px_3() - .gap_1() - .justify_center() - .bg(cx.theme().secondary) - .hover(|this| this.bg(cx.theme().secondary_hover)) + .gap_2() + .items_center() + .child(div().w_6().flex_shrink_0()) .child( h_flex() - .gap_2() - .text_sm() - .whitespace_nowrap() - .text_ellipsis() + .flex_1() + .min_w_0() + .gap_1() + .items_center() + .text_xs() .child( - h_flex() - .size_6() - .flex_shrink_0() - .items_center() - .justify_center() - .child(kind_icon(kind)), + UserAvatar::new(name.clone()) + .picture(profile.picture()) + .xsmall(), ) - .child(subject), - ) - .child( - h_flex().gap_2().child(div().w_6().flex_shrink_0()).child( - h_flex() - .gap_1() - .text_xs() - .text_color(cx.theme().muted_foreground) - .child(SharedString::from(kind_label(kind))) - .child("·") - .child(SharedString::from(age)), - ), + .child(name) + .child(SharedString::from(activity_phrase(event.kind))) + .child(div().flex_1()) + .child( + div() + .text_color(cx.theme().muted_foreground) + .child(SharedString::from(relative_time(event.created_at))), + ), ) .into_any_element() } @@ -718,23 +640,23 @@ fn kind_icon(kind: Kind) -> Icon { .small() } -/// Short noun for a notification or activity kind. -fn kind_label(kind: Kind) -> &'static str { +/// Phrase describing an activity event, read as `[name] [phrase]`. +fn activity_phrase(kind: Kind) -> &'static str { if kind == COVER_NOTE_KIND { - return "note"; + return "added a note"; } match kind { - Kind::GitIssue => "issue", - Kind::GitPullRequest => "PR", - Kind::GitPullRequestUpdate => "PR update", - Kind::GitPatch => "patch", - Kind::Comment => "comment", - Kind::GitStatusOpen - | Kind::GitStatusApplied - | Kind::GitStatusClosed - | Kind::GitStatusDraft => "status", - _ => "activity", + Kind::GitIssue => "opened an issue", + Kind::GitPullRequest => "opened a PR", + Kind::GitPullRequestUpdate => "updated a PR", + Kind::GitPatch => "created a patch", + Kind::Comment => "commented", + Kind::GitStatusOpen => "opened a status", + Kind::GitStatusApplied => "applied a status", + Kind::GitStatusClosed => "closed a status", + Kind::GitStatusDraft => "drafted a status", + _ => "did something", } } @@ -818,20 +740,30 @@ impl Render for InboxView { })), ), ) - .child(div().relative().flex_1().min_h_0().px_4().pb_4().when_else( - rows.is_empty(), - |this| this.child(empty_state(IconName::Inbox, "You're all caught up.", cx)), - |this| { - this.child( - list( - self.list.clone(), - cx.processor(|this, ix, _window, cx| this.render_entry(ix, cx)), - ) - .size_full() - .min_h_0() - .into_any_element(), + .child( + div() + .relative() + .flex_1() + .min_h_0() + .px_4() + .when_else( + rows.is_empty(), + |this| { + this.child(empty_state(IconName::Inbox, "You're all caught up.", cx)) + }, + |this| { + this.child( + list( + self.list.clone(), + cx.processor(|this, ix, _window, cx| this.render_entry(ix, cx)), + ) + .size_full() + .min_h_0() + .into_any_element(), + ) + }, ) - }, - )) + .child(div().h_6().w_full().flex_shrink_0()), + ) } } diff --git a/docs/inbox-plan.md b/docs/inbox-plan.md index 5a7fae8..91d34ad 100644 --- a/docs/inbox-plan.md +++ b/docs/inbox-plan.md @@ -7,13 +7,14 @@ Ported from GitWorkshop's home screen, the `Dashboard` rendered at route `/` for > an account is active, and that home screen is the inbox. > **Status.** Phases 0-4 are implemented and green on `feat/inbox`, then the screen was redesigned to -> group notifications and activity **by repository** (see the repository-grouping note in §7). -> `cargo test -p signed_core` (68), `cargo test -p signed_state` (24), +> group **threads by repository** and to merge notifications with own activity into one row per thread +> (see the repository-grouping and thread-merge notes in §7). +> `cargo test -p signed_core` (69), `cargo test -p signed_state` (24), > `cargo test -p workspace` (7), `cargo test -p dock` (1), `cargo clippy -p workspace --all-targets` > clean, `cargo check --workspace --all-targets` succeeds. > Phase 5 is not started. This document reflects the implementation as it stands: the Phase 1 > refactors, the §4.3 split of the inbox into a thin global `Inbox` and a panel-owned derivation, the -> Phase 4 click-through, and the repository-grouped list. The Phase 3 bottom-dock sub-views were +> Phase 4 click-through, and the repository-grouped thread list. The Phase 3 bottom-dock sub-views were > removed before the redesign; their implementation notes in §7 are historical. ## 1. What the GitWorkshop home screen is @@ -72,20 +73,24 @@ Notes: `InboxView` is a center panel, opened by the sidebar's existing **Inbox** nav item. It is one bordered card holding a single virtual list. Every row is either a **repository header** or one of that -repository's **notifications / own activity**, newest first: +repository's **threads**, newest first. A thread merges the notifications directed at the user with +the user's own events in the same root, and shows the root's title plus up to five of its most recent +events: The sections are **all of the user's own repositories**, seeded from `RepoListStore`, plus any other -repository that has notifications or activity. Owned repositories with nothing to show render an +repository that has threads. Owned repositories with nothing to show render an empty state ("No activity yet.") under their header, and sort after the ones with activity (newest -announcement first). Items with no repository address fall into a single "Other repository" section. +announcement first). Threads with no repository address fall into a single "Other repository" section. ``` +-------------------------------------------------------------------------+ | Inbox (3 unread) [Mark all read] | |-------------------------------------------------------------------------| | [repo] you/repo-a (2) | -| [avatar] issue opened issue 2m | -| [avatar] commented on "..." comment 1h | -| [icon] "Add retry" patch 3d | +| [icon] Add retry logic (unread dot) | +| [avatar] You opened an issue 3d | +| [avatar] alice commented 2d | +| [icon] Fix flaky test | +| [avatar] You opened a PR 1h | |-------------------------------------------------------------------------| | [repo] you/repo-b | | No activity yet. | @@ -95,8 +100,8 @@ announcement first). Items with no repository address fall into a single "Other +-------------------------------------------------------------------------+ ``` -The sections are the repositories that actually have notifications or activity, ordered by their -newest row. A repository the user owns but that has no items is not shown. Items with no repository +The sections are the repositories that actually have threads, ordered by their +newest row. A repository the user owns but that has no items is not shown. Threads with no repository address fall into a single "Other repository" section. ## 4. Data layer @@ -155,24 +160,42 @@ gitworkshop's `isGitComment`. ```rust pub struct InboxItem { pub root: EventId, + /// The root event itself, when known locally; drives the row title. + pub root_event: Option, pub root_kind: Option, pub address: Option, - /// Events in the group, newest first. + /// Notification events directed at the user, newest first. pub events: Vec, + /// The user's own events in the same thread, newest first. + pub own_events: Vec, /// Unread event ids, oldest first. pub unread_ids: Vec, pub archived: bool, } +impl InboxItem { + /// Title of the thread root; falls back to the newest event it has. + pub fn title(&self) -> String; + /// Kind of the thread root; falls back to the newest event it has. + pub fn kind(&self) -> Option; + pub fn latest_activity(&self) -> Timestamp; + /// Up to `limit` most recent events of the thread, oldest first. + pub fn timeline(&self, limit: usize) -> Vec; + pub fn is_unread(&self) -> bool; + pub fn apply_state(&mut self, state: &InboxReadState); +} + /// The thread root of a notification event, or `None` if it isn't git-related. pub fn notification_root( event: &Event, lookup: &impl Fn(EventId) -> Option, ) -> Option; -/// Group notification events by root, newest activity first, self excluded. +/// Group the notifications directed at the user together with the user's own +/// events into one item per thread, newest activity first. pub fn group( events: impl IntoIterator, + own: impl IntoIterator, me: PublicKey, state: &InboxReadState, lookup: &impl Fn(EventId) -> Option, @@ -186,7 +209,10 @@ Root resolution, ported from `getNotificationRootId`: - NIP-22 comment (1111): uppercase `E` root pointer (SDK `nip22::extract_root`) - PR update (1619): uppercase `E` - statuses (1630-1633) / cover note (1624): NIP-10 root `e` -- self-authored events are excluded +- notification events authored by `me` are dropped; the user's own events are kept in + `own_events` instead, never in `events` +- `unread_ids` and `archived` are derived from `events` only, so the user's own activity is never + unread and a thread with only own events is never archived Read/archive state, the compact high-water-mark model: @@ -290,7 +316,7 @@ pub struct Backend { #[derive(Default)] pub struct Inbox { state: InboxReadState, - state_loaded: bool, + loaded: bool, } impl Inbox { @@ -302,6 +328,14 @@ impl Inbox { pub(crate) fn activate(&mut self, me: PublicKey, client: Client, cx); pub(crate) fn reset(&mut self, cx); } + +// inbox.rs (signed_state) +/// One item per thread, notifications and own activity merged. +pub async fn query_inbox( + client: &Client, + me: PublicKey, + state: &InboxReadState, +) -> Result<(Vec, usize), Error>; ``` **The panel owns the derivation.** `InboxView` itself holds the derived lists, the copy of the read @@ -314,8 +348,7 @@ badge only; there is no global count and no sidebar badge. pub struct InboxView { focus_handle: FocusHandle, dock_area: WeakEntity, - notifications: Arc>, - activity: Arc>, + threads: Arc>, // one row per thread, merged sections: Arc>, // grouped by repository rows: Arc>, // flattened list unread_count: usize, @@ -599,15 +632,15 @@ whose root is not an issue/PR/patch, or whose repository is not in `RepoListStor |---|---| | `crates/signed_core/Cargo.toml` | add `serde` | | `crates/signed_core/src/filters.rs` | `NOTIFICATION_KINDS`, `notification_comments`, `notifications`, `authored_activity`, `is_git_activity`, `deletions` | -| `crates/signed_core/src/inbox.rs` | **new**: `InboxItem`, `notification_root`, `group`, `InboxReadState`, tests | +| `crates/signed_core/src/inbox.rs` | **new**: `InboxItem` (root event, notifications, own events), `notification_root`, `group`, `InboxReadState`, tests | | `crates/signed_core/src/lib.rs` | `mod inbox;` and re-exports | | `crates/signed_state/Cargo.toml` | add `serde_json` | -| `crates/signed_state/src/inbox.rs` | thin global `Inbox` (NIP-78 read state, mark actions) and `query_inbox` (query, grouping, activity) | +| `crates/signed_state/src/inbox.rs` | thin global `Inbox` (NIP-78 read state, mark actions) and `query_inbox` (query, merge notifications + activity into threads) | | `crates/signed_state/src/backend.rs` | `inbox: Entity` field, construction, `inbox()` accessor, `sync_inbox`, `RepoListStore` import | | `crates/signed_state/src/refresh.rs` | doc comment lists `Inbox` among the `RefreshGate` users | | `crates/signed_state/src/lib.rs` | `mod inbox;`, re-export `Inbox` and `query_inbox`; re-export `RefreshGate` (no global install) | | `crates/dock/src/lib.rs` | `add_bottom_panel` helper (currently unused; left over from the removed sub-views) | -| `crates/workspace/src/views/inbox.rs` | `InboxView` home panel owning the derived lists, the repository grouping, and the notification click-through | +| `crates/workspace/src/views/inbox.rs` | `InboxView` home panel owning the threads, the repository grouping, and the thread click-through | | `crates/workspace/src/views/mod.rs` | `mod inbox; pub use inbox::InboxView;`; re-export `RepoItem`, `open_repo_item`, `open_repo_panel` | | `crates/workspace/src/views/sidebar/mod.rs` | `inbox` field, `open_inbox`, nav wiring | | `crates/workspace/src/views/repo_detail/mod.rs` | `RepoItem`, `open_repo_item`, `RepoDetailView::store()` | @@ -859,9 +892,55 @@ itself, so the global is thin again. `cargo check -p signed_core -p signed_state -p workspace --all-targets` succeeds, and `cargo test -p signed_core -p signed_state -p workspace -p dock` passes (68 / 24 / 7 / 1). +### Threads merged: notifications + activity (after the sidebar badge removal) + +Files: `crates/signed_core/src/inbox.rs`, `crates/signed_state/src/inbox.rs`, +`crates/workspace/src/views/inbox.rs`. + +Notifications and own activity were two separate row kinds that could describe the same thread. They +are now one item per thread: the notifications directed at the user and the user's own events in that +thread live in the same `InboxItem`. A row shows the thread root's title and up to five of the +thread's most recent events: + +``` +[icon] Add retry logic (unread dot) + [avatar] You opened an issue · 3d + [avatar] alice commented · 2d +``` + +- `InboxItem` gained `root_event: Option` and `own_events: Vec`. `events` keeps only the + notifications (others' events); `own_events` holds the user's own. `unread_ids`/`archived` are + derived from `events` alone, so own activity is never unread and a thread with only own events is + never archived (`apply_state` guards the empty case). +- New methods on `InboxItem`: `title()` (root event's subject, falling back to the newest event), + `kind()` (root kind, same fallback), and `timeline(limit)` (thread events deduplicated by id, + oldest first, always keeping the root event and filling the remaining slots with the most recent + others). +- `group` now takes both `events` (notifications) and `own` (the user's activity) and merges them on + the resolved root. Own events resolve through the same `notification_root`; an unresolved own event + becomes its own root. `query_inbox` returns `(Vec, usize)` - the separate activity list + is gone, and `by_id` is extended with the own events so a comment of ours resolves to its thread. +- The panel holds `threads: Arc>` instead of `notifications` + `activity`. The + `InboxEntry` enum, `entry_time`, `repo_address`, `related_activity`, `notification_row`, + `activity_row` and `kind_label` are gone. `thread_row` replaces both row kinds and is clickable like + the old notification row; `group_sections` now just buckets threads by `item.address`. +- `sub_activity_line` is unchanged and still renders `[avatar] [name] [phrase] · [ago]`, with `You` + for the signed-in user and `activity_phrase(kind)` for the verb. Rows are variable height + (`py_2`), which `gpui::list` auto-measures. +- Thread rows in a section are drawn as one stack: `render_entry` passes `first`/`last` within the + section (`entry_ix == 0` / `entry_ix + 1 == section.entries.len()`), and `thread_row` rounds the + outer edges (`rounded_t` on the first, `rounded_b` on the last, theme radius) and draws a + `border_b_1` divider on every row but the last. +- Trade-off: the row title is the thread root's, not the newest event's, so a comment thread no longer + previews the comment text. That is the point of the merge - the row identifies the thread. +- `cargo clippy -p signed_core -p signed_state -p workspace --all-targets` is clean, + `cargo check -p signed_core -p signed_state -p workspace --all-targets` succeeds, and + `cargo test -p signed_core -p signed_state -p workspace` passes (69 / 24 / 7). + ## 8. Validation -- `cargo test -p signed_core` (68 tests): root resolution, grouping, read-state cutoff, serde round-trip. +- `cargo test -p signed_core` (69 tests): root resolution, grouping, merging, read-state cutoff, serde + round-trip. - `cargo test -p signed_state` (24 tests): the `Inbox` / `query_inbox` paths that do not need GPUI (state round-trip, grouping helpers). - `cargo test -p workspace` (7 tests): repository-detail helpers.