From f9fb3f78b2a63714387f46dd603f5f1194e3f8fb Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 7 Oct 2022 17:01:48 +0200 Subject: [PATCH] WIP: Render active call in contacts popover Co-Authored-By: Nathan Sobo --- crates/call/src/room.rs | 85 ++++++++---- crates/collab/src/integration_tests.rs | 2 +- crates/collab/src/rpc/store.rs | 26 ++-- crates/collab_ui/src/contacts_popover.rs | 159 +++++++++++++++++------ crates/rpc/proto/zed.proto | 2 +- crates/rpc/src/peer.rs | 2 +- crates/theme/src/theme.rs | 1 + styles/src/styleTree/contactsPopover.ts | 3 + 8 files changed, 198 insertions(+), 82 deletions(-) diff --git a/crates/call/src/room.rs b/crates/call/src/room.rs index 41648e6b24..9cad1ff211 100644 --- a/crates/call/src/room.rs +++ b/crates/call/src/room.rs @@ -4,7 +4,7 @@ use crate::{ }; use anyhow::{anyhow, Result}; use client::{proto, Client, PeerId, TypedEnvelope, User, UserStore}; -use collections::{HashMap, HashSet}; +use collections::{BTreeMap, HashSet}; use futures::StreamExt; use gpui::{AsyncAppContext, Entity, ModelContext, ModelHandle, MutableAppContext, Task}; use project::Project; @@ -19,8 +19,9 @@ pub enum Event { pub struct Room { id: u64, status: RoomStatus, - remote_participants: HashMap, - pending_users: Vec>, + remote_participants: BTreeMap, + pending_participants: Vec>, + participant_user_ids: HashSet, pending_call_count: usize, leave_when_empty: bool, client: Arc, @@ -62,8 +63,9 @@ impl Room { Self { id, status: RoomStatus::Online, + participant_user_ids: Default::default(), remote_participants: Default::default(), - pending_users: Default::default(), + pending_participants: Default::default(), pending_call_count: 0, subscriptions: vec![client.add_message_handler(cx.handle(), Self::handle_room_updated)], leave_when_empty: false, @@ -131,7 +133,7 @@ impl Room { fn should_leave(&self) -> bool { self.leave_when_empty && self.pending_room_update.is_none() - && self.pending_users.is_empty() + && self.pending_participants.is_empty() && self.remote_participants.is_empty() && self.pending_call_count == 0 } @@ -144,6 +146,8 @@ impl Room { cx.notify(); self.status = RoomStatus::Offline; self.remote_participants.clear(); + self.pending_participants.clear(); + self.participant_user_ids.clear(); self.subscriptions.clear(); self.client.send(proto::LeaveRoom { id: self.id })?; Ok(()) @@ -157,12 +161,16 @@ impl Room { self.status } - pub fn remote_participants(&self) -> &HashMap { + pub fn remote_participants(&self) -> &BTreeMap { &self.remote_participants } - pub fn pending_users(&self) -> &[Arc] { - &self.pending_users + pub fn pending_participants(&self) -> &[Arc] { + &self.pending_participants + } + + pub fn contains_participant(&self, user_id: u64) -> bool { + self.participant_user_ids.contains(&user_id) } async fn handle_room_updated( @@ -187,27 +195,29 @@ impl Room { room.participants .retain(|participant| Some(participant.user_id) != self.client.user_id()); - let participant_user_ids = room + let remote_participant_user_ids = room .participants .iter() .map(|p| p.user_id) .collect::>(); - let (participants, pending_users) = self.user_store.update(cx, move |user_store, cx| { - ( - user_store.get_users(participant_user_ids, cx), - user_store.get_users(room.pending_user_ids, cx), - ) - }); + let (remote_participants, pending_participants) = + self.user_store.update(cx, move |user_store, cx| { + ( + user_store.get_users(remote_participant_user_ids, cx), + user_store.get_users(room.pending_participant_user_ids, cx), + ) + }); self.pending_room_update = Some(cx.spawn(|this, mut cx| async move { - let (participants, pending_users) = futures::join!(participants, pending_users); + let (remote_participants, pending_participants) = + futures::join!(remote_participants, pending_participants); this.update(&mut cx, |this, cx| { - if let Some(participants) = participants.log_err() { - let mut seen_participants = HashSet::default(); + this.participant_user_ids.clear(); + if let Some(participants) = remote_participants.log_err() { for (participant, user) in room.participants.into_iter().zip(participants) { let peer_id = PeerId(participant.peer_id); - seen_participants.insert(peer_id); + this.participant_user_ids.insert(participant.user_id); let existing_project_ids = this .remote_participants @@ -234,19 +244,18 @@ impl Room { ); } - for participant_peer_id in - this.remote_participants.keys().copied().collect::>() - { - if !seen_participants.contains(&participant_peer_id) { - this.remote_participants.remove(&participant_peer_id); - } - } + this.remote_participants.retain(|_, participant| { + this.participant_user_ids.contains(&participant.user.id) + }); cx.notify(); } - if let Some(pending_users) = pending_users.log_err() { - this.pending_users = pending_users; + if let Some(pending_participants) = pending_participants.log_err() { + this.pending_participants = pending_participants; + for participant in &this.pending_participants { + this.participant_user_ids.insert(participant.id); + } cx.notify(); } @@ -254,6 +263,8 @@ impl Room { if this.should_leave() { let _ = this.leave(cx); } + + this.check_invariants(); }); })); @@ -261,6 +272,24 @@ impl Room { Ok(()) } + fn check_invariants(&self) { + #[cfg(any(test, feature = "test-support"))] + { + for participant in self.remote_participants.values() { + assert!(self.participant_user_ids.contains(&participant.user.id)); + } + + for participant in &self.pending_participants { + assert!(self.participant_user_ids.contains(&participant.id)); + } + + assert_eq!( + self.participant_user_ids.len(), + self.remote_participants.len() + self.pending_participants.len() + ); + } + } + pub(crate) fn call( &mut self, recipient_user_id: u64, diff --git a/crates/collab/src/integration_tests.rs b/crates/collab/src/integration_tests.rs index 9cde2b2206..c94d766f82 100644 --- a/crates/collab/src/integration_tests.rs +++ b/crates/collab/src/integration_tests.rs @@ -6475,7 +6475,7 @@ fn room_participants(room: &ModelHandle, cx: &mut TestAppContext) -> RoomP .map(|(_, participant)| participant.user.github_login.clone()) .collect(), pending: room - .pending_users() + .pending_participants() .iter() .map(|user| user.github_login.clone()) .collect(), diff --git a/crates/collab/src/rpc/store.rs b/crates/collab/src/rpc/store.rs index be7f798685..5b23ac92d5 100644 --- a/crates/collab/src/rpc/store.rs +++ b/crates/collab/src/rpc/store.rs @@ -229,7 +229,7 @@ impl Store { .retain(|participant| participant.peer_id != connection_id.0); if prev_participant_count == room.participants.len() { if connected_user.connection_ids.is_empty() { - room.pending_user_ids + room.pending_participant_user_ids .retain(|pending_user_id| *pending_user_id != user_id.to_proto()); result.room_id = Some(room_id); connected_user.active_call = None; @@ -239,7 +239,7 @@ impl Store { connected_user.active_call = None; } - if room.participants.is_empty() && room.pending_user_ids.is_empty() { + if room.participants.is_empty() && room.pending_participant_user_ids.is_empty() { self.rooms.remove(&room_id); } } else { @@ -432,10 +432,11 @@ impl Store { .get_mut(&room_id) .ok_or_else(|| anyhow!("no such room"))?; anyhow::ensure!( - room.pending_user_ids.contains(&user_id.to_proto()), + room.pending_participant_user_ids + .contains(&user_id.to_proto()), anyhow!("no such room") ); - room.pending_user_ids + room.pending_participant_user_ids .retain(|pending| *pending != user_id.to_proto()); room.participants.push(proto::Participant { user_id: user_id.to_proto(), @@ -490,7 +491,7 @@ impl Store { .ok_or_else(|| anyhow!("no such room"))?; room.participants .retain(|participant| participant.peer_id != connection_id.0); - if room.participants.is_empty() && room.pending_user_ids.is_empty() { + if room.participants.is_empty() && room.pending_participant_user_ids.is_empty() { self.rooms.remove(&room_id); } @@ -537,12 +538,13 @@ impl Store { "no such room" ); anyhow::ensure!( - room.pending_user_ids + room.pending_participant_user_ids .iter() .all(|user_id| UserId::from_proto(*user_id) != recipient_user_id), "cannot call the same user more than once" ); - room.pending_user_ids.push(recipient_user_id.to_proto()); + room.pending_participant_user_ids + .push(recipient_user_id.to_proto()); if let Some(initial_project_id) = initial_project_id { let project = self @@ -589,7 +591,7 @@ impl Store { .rooms .get_mut(&room_id) .ok_or_else(|| anyhow!("no such room"))?; - room.pending_user_ids + room.pending_participant_user_ids .retain(|user_id| UserId::from_proto(*user_id) != to_user_id); Ok(room) } @@ -635,7 +637,7 @@ impl Store { .rooms .get_mut(&room_id) .ok_or_else(|| anyhow!("no such room"))?; - room.pending_user_ids + room.pending_participant_user_ids .retain(|user_id| UserId::from_proto(*user_id) != recipient_user_id); let recipient = self.connected_users.get_mut(&recipient_user_id).unwrap(); @@ -663,7 +665,7 @@ impl Store { .rooms .get_mut(&active_call.room_id) .ok_or_else(|| anyhow!("no such room"))?; - room.pending_user_ids + room.pending_participant_user_ids .retain(|user_id| UserId::from_proto(*user_id) != recipient_user_id); Ok((room, recipient_connection_ids)) } else { @@ -1115,7 +1117,7 @@ impl Store { } for (room_id, room) in &self.rooms { - for pending_user_id in &room.pending_user_ids { + for pending_user_id in &room.pending_participant_user_ids { assert!( self.connected_users .contains_key(&UserId::from_proto(*pending_user_id)), @@ -1140,7 +1142,7 @@ impl Store { } assert!( - !room.pending_user_ids.is_empty() || !room.participants.is_empty(), + !room.pending_participant_user_ids.is_empty() || !room.participants.is_empty(), "room can't be empty" ); } diff --git a/crates/collab_ui/src/contacts_popover.rs b/crates/collab_ui/src/contacts_popover.rs index 074e816163..090720c7a6 100644 --- a/crates/collab_ui/src/contacts_popover.rs +++ b/crates/collab_ui/src/contacts_popover.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use crate::contact_finder; use call::ActiveCall; -use client::{Contact, User, UserStore}; +use client::{Contact, PeerId, User, UserStore}; use editor::{Cancel, Editor}; use fuzzy::{match_strings, StringMatchCandidate}; use gpui::{ @@ -41,6 +41,7 @@ struct Call { #[derive(Clone, Copy, PartialEq, Eq, Debug, PartialOrd, Ord)] enum Section { + ActiveCall, Requests, Online, Offline, @@ -49,6 +50,7 @@ enum Section { #[derive(Clone)] enum ContactEntry { Header(Section), + CallParticipant { user: Arc, is_pending: bool }, IncomingRequest(Arc), OutgoingRequest(Arc), Contact(Arc), @@ -62,6 +64,11 @@ impl PartialEq for ContactEntry { return section_1 == section_2; } } + ContactEntry::CallParticipant { user: user_1, .. } => { + if let ContactEntry::CallParticipant { user: user_2, .. } = other { + return user_1.id == user_2.id; + } + } ContactEntry::IncomingRequest(user_1) => { if let ContactEntry::IncomingRequest(user_2) = other { return user_1.id == user_2.id; @@ -157,6 +164,9 @@ impl ContactsPopover { cx, ) } + ContactEntry::CallParticipant { user, is_pending } => { + Self::render_call_participant(user, *is_pending, &theme.contacts_popover) + } ContactEntry::IncomingRequest(user) => Self::render_contact_request( user.clone(), this.user_store.clone(), @@ -186,7 +196,7 @@ impl ContactsPopover { let active_call = ActiveCall::global(cx); let mut subscriptions = Vec::new(); subscriptions.push(cx.observe(&user_store, |this, _, cx| this.update_entries(cx))); - subscriptions.push(cx.observe(&active_call, |_, _, cx| cx.notify())); + subscriptions.push(cx.observe(&active_call, |this, _, cx| this.update_entries(cx))); let mut this = Self { list_state, @@ -291,6 +301,66 @@ impl ContactsPopover { let prev_selected_entry = self.selection.and_then(|ix| self.entries.get(ix).cloned()); self.entries.clear(); + if let Some(room) = ActiveCall::global(cx).read(cx).room() { + let room = room.read(cx); + + self.entries.push(ContactEntry::Header(Section::ActiveCall)); + if !self.collapsed_sections.contains(&Section::ActiveCall) { + // Populate remote participants. + self.match_candidates.clear(); + self.match_candidates + .extend( + room.remote_participants() + .iter() + .map(|(peer_id, participant)| StringMatchCandidate { + id: peer_id.0 as usize, + string: participant.user.github_login.clone(), + char_bag: participant.user.github_login.chars().collect(), + }), + ); + let matches = executor.block(match_strings( + &self.match_candidates, + &query, + true, + usize::MAX, + &Default::default(), + executor.clone(), + )); + self.entries.extend(matches.iter().map(|mat| { + ContactEntry::CallParticipant { + user: room.remote_participants()[&PeerId(mat.candidate_id as u32)] + .user + .clone(), + is_pending: false, + } + })); + + // Populate pending participants. + self.match_candidates.clear(); + self.match_candidates + .extend(room.pending_participants().iter().enumerate().map( + |(id, participant)| StringMatchCandidate { + id, + string: participant.github_login.clone(), + char_bag: participant.github_login.chars().collect(), + }, + )); + let matches = executor.block(match_strings( + &self.match_candidates, + &query, + true, + usize::MAX, + &Default::default(), + executor.clone(), + )); + self.entries + .extend(matches.iter().map(|mat| ContactEntry::CallParticipant { + user: room.pending_participants()[mat.candidate_id].clone(), + is_pending: true, + })); + } + } + let mut request_entries = Vec::new(); let incoming = user_store.incoming_contact_requests(); if !incoming.is_empty() { @@ -359,7 +429,6 @@ impl ContactsPopover { let contacts = user_store.contacts(); if !contacts.is_empty() { - // Always put the current user first. self.match_candidates.clear(); self.match_candidates .extend( @@ -382,9 +451,16 @@ impl ContactsPopover { executor.clone(), )); - let (online_contacts, offline_contacts) = matches + let (mut online_contacts, offline_contacts) = matches .iter() .partition::, _>(|mat| contacts[mat.candidate_id].online); + if let Some(room) = ActiveCall::global(cx).read(cx).room() { + let room = room.read(cx); + online_contacts.retain(|contact| { + let contact = &contacts[contact.candidate_id]; + !room.contains_participant(contact.user.id) + }); + } for (matches, section) in [ (online_contacts, Section::Online), @@ -416,41 +492,46 @@ impl ContactsPopover { cx.notify(); } - fn render_active_call(&self, cx: &mut RenderContext) -> Option { - let room = ActiveCall::global(cx).read(cx).room()?; - let theme = &cx.global::().theme.contacts_popover; - - Some( - Flex::column() - .with_children(room.read(cx).pending_users().iter().map(|user| { - Flex::row() - .with_children(user.avatar.clone().map(|avatar| { - Image::new(avatar) - .with_style(theme.contact_avatar) - .aligned() - .left() - .boxed() - })) - .with_child( - Label::new( - user.github_login.clone(), - theme.contact_username.text.clone(), - ) - .contained() - .with_style(theme.contact_username.container) - .aligned() - .left() - .flex(1., true) - .boxed(), - ) - .constrained() - .with_height(theme.row_height) - .contained() - .with_style(theme.contact_row.default) - .boxed() - })) + fn render_call_participant( + user: &User, + is_pending: bool, + theme: &theme::ContactsPopover, + ) -> ElementBox { + Flex::row() + .with_children(user.avatar.clone().map(|avatar| { + Image::new(avatar) + .with_style(theme.contact_avatar) + .aligned() + .left() + .boxed() + })) + .with_child( + Label::new( + user.github_login.clone(), + theme.contact_username.text.clone(), + ) + .contained() + .with_style(theme.contact_username.container) .boxed(), - ) + ) + .with_children(if is_pending { + Some( + Label::new( + "Calling...".to_string(), + theme.calling_indicator.text.clone(), + ) + .contained() + .with_style(theme.calling_indicator.container) + .aligned() + .flex_float() + .boxed(), + ) + } else { + None + }) + .constrained() + .with_height(theme.row_height) + .boxed() } fn render_header( @@ -464,6 +545,7 @@ impl ContactsPopover { let header_style = theme.header_row.style_for(Default::default(), is_selected); let text = match section { + Section::ActiveCall => "Call", Section::Requests => "Requests", Section::Online => "Online", Section::Offline => "Offline", @@ -751,7 +833,6 @@ impl View for ContactsPopover { .with_height(theme.contacts_popover.user_query_editor_height) .boxed(), ) - .with_children(self.render_active_call(cx)) .with_child(List::new(self.list_state.clone()).flex(1., false).boxed()) .with_children( self.user_store diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 5f62e3585e..67f3afb461 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -154,7 +154,7 @@ message LeaveRoom { message Room { repeated Participant participants = 1; - repeated uint64 pending_user_ids = 2; + repeated uint64 pending_participant_user_ids = 2; } message Participant { diff --git a/crates/rpc/src/peer.rs b/crates/rpc/src/peer.rs index 834acd0afa..5b1ed6c2af 100644 --- a/crates/rpc/src/peer.rs +++ b/crates/rpc/src/peer.rs @@ -33,7 +33,7 @@ impl fmt::Display for ConnectionId { } } -#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)] +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] pub struct PeerId(pub u32); impl fmt::Display for PeerId { diff --git a/crates/theme/src/theme.rs b/crates/theme/src/theme.rs index d70aaed189..d66f30fe95 100644 --- a/crates/theme/src/theme.rs +++ b/crates/theme/src/theme.rs @@ -105,6 +105,7 @@ pub struct ContactsPopover { pub private_button: Interactive, pub section_icon_size: f32, pub invite_row: Interactive, + pub calling_indicator: ContainedText, } #[derive(Clone, Deserialize, Default)] diff --git a/styles/src/styleTree/contactsPopover.ts b/styles/src/styleTree/contactsPopover.ts index 7dfdb404d6..3e39746bdf 100644 --- a/styles/src/styleTree/contactsPopover.ts +++ b/styles/src/styleTree/contactsPopover.ts @@ -171,5 +171,8 @@ export default function contactsPopover(theme: Theme) { text: text(theme, "sans", "active", { size: "sm" }), }, }, + callingIndicator: { + ...text(theme, "mono", "primary", { size: "xs" }), + } } }