Start work on rendering channel participants in collab panel

Max Brunsfeld and mikayla created

Co-authored-by: mikayla <mikayla@zed.dev>

Change summary

crates/client/src/channel_store.rs       | 56 ++++++++++++++
crates/collab/src/db.rs                  | 20 -----
crates/collab/src/rpc.rs                 | 15 ++-
crates/collab/src/tests.rs               |  3 
crates/collab/src/tests/channel_tests.rs | 94 ++++++++++++++++++++++++-
crates/collab_ui/src/face_pile.rs        | 34 ++++----
crates/collab_ui/src/panel.rs            | 28 +++++--
crates/theme/src/theme.rs                |  1 
styles/src/style_tree/collab_panel.ts    |  1 
9 files changed, 192 insertions(+), 60 deletions(-)

Detailed changes

crates/client/src/channel_store.rs 🔗

@@ -7,11 +7,12 @@ use rpc::{proto, TypedEnvelope};
 use std::sync::Arc;
 
 type ChannelId = u64;
+type UserId = u64;
 
 pub struct ChannelStore {
     channels: Vec<Arc<Channel>>,
     channel_invitations: Vec<Arc<Channel>>,
-    channel_participants: HashMap<ChannelId, Vec<ChannelId>>,
+    channel_participants: HashMap<ChannelId, Vec<Arc<User>>>,
     client: Arc<Client>,
     user_store: ModelHandle<UserStore>,
     _rpc_subscription: Subscription,
@@ -60,6 +61,12 @@ impl ChannelStore {
         self.channels.iter().find(|c| c.id == channel_id).cloned()
     }
 
+    pub fn channel_participants(&self, channel_id: ChannelId) -> &[Arc<User>] {
+        self.channel_participants
+            .get(&channel_id)
+            .map_or(&[], |v| v.as_slice())
+    }
+
     pub fn create_channel(
         &self,
         name: &str,
@@ -78,7 +85,7 @@ impl ChannelStore {
     pub fn invite_member(
         &self,
         channel_id: ChannelId,
-        user_id: u64,
+        user_id: UserId,
         admin: bool,
     ) -> impl Future<Output = Result<()>> {
         let client = self.client.clone();
@@ -162,6 +169,8 @@ impl ChannelStore {
             .retain(|channel| !payload.remove_channels.contains(&channel.id));
         self.channel_invitations
             .retain(|channel| !payload.remove_channel_invitations.contains(&channel.id));
+        self.channel_participants
+            .retain(|channel_id, _| !payload.remove_channels.contains(channel_id));
 
         for channel in payload.channel_invitations {
             if let Some(existing_channel) = self
@@ -215,6 +224,49 @@ impl ChannelStore {
                 );
             }
         }
+
+        let mut all_user_ids = Vec::new();
+        let channel_participants = payload.channel_participants;
+        for entry in &channel_participants {
+            for user_id in entry.participant_user_ids.iter() {
+                if let Err(ix) = all_user_ids.binary_search(user_id) {
+                    all_user_ids.insert(ix, *user_id);
+                }
+            }
+        }
+
+        // TODO: Race condition if an update channels messages comes in while resolving avatars
+        let users = self
+            .user_store
+            .update(cx, |user_store, cx| user_store.get_users(all_user_ids, cx));
+        cx.spawn(|this, mut cx| async move {
+            let users = users.await?;
+
+            this.update(&mut cx, |this, cx| {
+                for entry in &channel_participants {
+                    let mut participants: Vec<_> = entry
+                        .participant_user_ids
+                        .iter()
+                        .filter_map(|user_id| {
+                            users
+                                .binary_search_by_key(&user_id, |user| &user.id)
+                                .ok()
+                                .map(|ix| users[ix].clone())
+                        })
+                        .collect();
+
+                    participants.sort_by_key(|u| u.id);
+
+                    this.channel_participants
+                        .insert(entry.channel_id, participants);
+                }
+
+                cx.notify();
+            });
+            anyhow::Ok(())
+        })
+        .detach();
+
         cx.notify();
     }
 }

crates/collab/src/db.rs 🔗

@@ -2200,26 +2200,6 @@ impl Database {
         ))
     }
 
-    async fn get_channel_members_for_room(
-        &self,
-        room_id: RoomId,
-        tx: &DatabaseTransaction,
-    ) -> Result<Vec<UserId>> {
-        let db_room = room::Model {
-            id: room_id,
-            ..Default::default()
-        };
-
-        let channel_users =
-            if let Some(channel) = db_room.find_related(channel::Entity).one(tx).await? {
-                self.get_channel_members_internal(channel.id, tx).await?
-            } else {
-                Vec::new()
-            };
-
-        Ok(channel_users)
-    }
-
     // projects
 
     pub async fn project_count_excluding_admins(&self) -> Result<usize> {

crates/collab/src/rpc.rs 🔗

@@ -2412,6 +2412,15 @@ fn build_initial_channels_update(
         });
     }
 
+    for (channel_id, participants) in channel_participants {
+        update
+            .channel_participants
+            .push(proto::ChannelParticipants {
+                channel_id: channel_id.to_proto(),
+                participant_user_ids: participants.into_iter().map(|id| id.to_proto()).collect(),
+            });
+    }
+
     for channel in channel_invites {
         update.channel_invitations.push(proto::Channel {
             id: channel.id.to_proto(),
@@ -2504,12 +2513,6 @@ fn channel_updated(
         None,
         channel_members
             .iter()
-            .filter(|user_id| {
-                !room
-                    .participants
-                    .iter()
-                    .any(|p| p.user_id == user_id.to_proto())
-            })
             .flat_map(|user_id| pool.user_connection_ids(*user_id)),
         |peer_id| {
             peer.send(

crates/collab/src/tests.rs 🔗

@@ -103,6 +103,9 @@ impl TestServer {
 
     async fn create_client(&mut self, cx: &mut TestAppContext, name: &str) -> TestClient {
         cx.update(|cx| {
+            if cx.has_global::<SettingsStore>() {
+                panic!("Same cx used to create two test clients")
+            }
             cx.set_global(SettingsStore::test(cx));
         });
 

crates/collab/src/tests/channel_tests.rs 🔗

@@ -1,5 +1,5 @@
 use call::ActiveCall;
-use client::Channel;
+use client::{Channel, User};
 use gpui::{executor::Deterministic, TestAppContext};
 use std::sync::Arc;
 
@@ -26,6 +26,7 @@ async fn test_basic_channels(
         .await
         .unwrap();
 
+    deterministic.run_until_parked();
     client_a.channel_store().read_with(cx_a, |channels, _| {
         assert_eq!(
             channels.channels(),
@@ -105,6 +106,13 @@ async fn test_basic_channels(
         .read_with(cx_b, |channels, _| assert_eq!(channels.channels(), &[]));
 }
 
+fn assert_participants_eq(participants: &[Arc<User>], expected_partitipants: &[u64]) {
+    assert_eq!(
+        participants.iter().map(|p| p.id).collect::<Vec<_>>(),
+        expected_partitipants
+    );
+}
+
 #[gpui::test]
 async fn test_channel_room(
     deterministic: Arc<Deterministic>,
@@ -116,7 +124,7 @@ async fn test_channel_room(
     let mut server = TestServer::start(&deterministic).await;
     let client_a = server.create_client(cx_a, "user_a").await;
     let client_b = server.create_client(cx_b, "user_b").await;
-    let client_c = server.create_client(cx_b, "user_c").await;
+    let client_c = server.create_client(cx_c, "user_c").await;
 
     let zed_id = server
         .make_channel(
@@ -134,8 +142,21 @@ async fn test_channel_room(
         .await
         .unwrap();
 
-    // TODO Test that B and C sees A in the channel room
+    // Give everyone a chance to observe user A joining
+    deterministic.run_until_parked();
+
+    client_a.channel_store().read_with(cx_a, |channels, _| {
+        assert_participants_eq(
+            channels.channel_participants(zed_id),
+            &[client_a.user_id().unwrap()],
+        );
+    });
+
     client_b.channel_store().read_with(cx_b, |channels, _| {
+        assert_participants_eq(
+            channels.channel_participants(zed_id),
+            &[client_a.user_id().unwrap()],
+        );
         assert_eq!(
             channels.channels(),
             &[Arc::new(Channel {
@@ -147,15 +168,41 @@ async fn test_channel_room(
         )
     });
 
+    client_c.channel_store().read_with(cx_c, |channels, _| {
+        assert_participants_eq(
+            channels.channel_participants(zed_id),
+            &[client_a.user_id().unwrap()],
+        );
+    });
+
     active_call_b
         .update(cx_b, |active_call, cx| active_call.join_channel(zed_id, cx))
         .await
         .unwrap();
 
-    // TODO Test that C sees A and B in the channel room
-
     deterministic.run_until_parked();
 
+    client_a.channel_store().read_with(cx_a, |channels, _| {
+        assert_participants_eq(
+            channels.channel_participants(zed_id),
+            &[client_a.user_id().unwrap(), client_b.user_id().unwrap()],
+        );
+    });
+
+    client_b.channel_store().read_with(cx_b, |channels, _| {
+        assert_participants_eq(
+            channels.channel_participants(zed_id),
+            &[client_a.user_id().unwrap(), client_b.user_id().unwrap()],
+        );
+    });
+
+    client_c.channel_store().read_with(cx_c, |channels, _| {
+        assert_participants_eq(
+            channels.channel_participants(zed_id),
+            &[client_a.user_id().unwrap(), client_b.user_id().unwrap()],
+        );
+    });
+
     let room_a = active_call_a.read_with(cx_a, |call, _| call.room().unwrap().clone());
     room_a.read_with(cx_a, |room, _| assert!(room.is_connected()));
     assert_eq!(
@@ -183,14 +230,47 @@ async fn test_channel_room(
         .await
         .unwrap();
 
-    // TODO Make sure that C sees A leave
+    deterministic.run_until_parked();
+
+    client_a.channel_store().read_with(cx_a, |channels, _| {
+        assert_participants_eq(
+            channels.channel_participants(zed_id),
+            &[client_b.user_id().unwrap()],
+        );
+    });
+
+    client_b.channel_store().read_with(cx_b, |channels, _| {
+        assert_participants_eq(
+            channels.channel_participants(zed_id),
+            &[client_b.user_id().unwrap()],
+        );
+    });
+
+    client_c.channel_store().read_with(cx_c, |channels, _| {
+        assert_participants_eq(
+            channels.channel_participants(zed_id),
+            &[client_b.user_id().unwrap()],
+        );
+    });
 
     active_call_b
         .update(cx_b, |active_call, cx| active_call.hang_up(cx))
         .await
         .unwrap();
 
-    // TODO Make sure that C sees B leave
+    deterministic.run_until_parked();
+
+    client_a.channel_store().read_with(cx_a, |channels, _| {
+        assert_participants_eq(channels.channel_participants(zed_id), &[]);
+    });
+
+    client_b.channel_store().read_with(cx_b, |channels, _| {
+        assert_participants_eq(channels.channel_participants(zed_id), &[]);
+    });
+
+    client_c.channel_store().read_with(cx_c, |channels, _| {
+        assert_participants_eq(channels.channel_participants(zed_id), &[]);
+    });
 
     active_call_a
         .update(cx_a, |active_call, cx| active_call.join_channel(zed_id, cx))

crates/collab_ui/src/face_pile.rs 🔗

@@ -7,34 +7,34 @@ use gpui::{
     },
     json::ToJson,
     serde_json::{self, json},
-    AnyElement, Axis, Element, LayoutContext, SceneBuilder, ViewContext,
+    AnyElement, Axis, Element, LayoutContext, SceneBuilder, View, ViewContext,
 };
 
 use crate::CollabTitlebarItem;
 
-pub(crate) struct FacePile {
+pub(crate) struct FacePile<V: View> {
     overlap: f32,
-    faces: Vec<AnyElement<CollabTitlebarItem>>,
+    faces: Vec<AnyElement<V>>,
 }
 
-impl FacePile {
-    pub fn new(overlap: f32) -> FacePile {
-        FacePile {
+impl<V: View> FacePile<V> {
+    pub fn new(overlap: f32) -> Self {
+        Self {
             overlap,
             faces: Vec::new(),
         }
     }
 }
 
-impl Element<CollabTitlebarItem> for FacePile {
+impl<V: View> Element<V> for FacePile<V> {
     type LayoutState = ();
     type PaintState = ();
 
     fn layout(
         &mut self,
         constraint: gpui::SizeConstraint,
-        view: &mut CollabTitlebarItem,
-        cx: &mut LayoutContext<CollabTitlebarItem>,
+        view: &mut V,
+        cx: &mut LayoutContext<V>,
     ) -> (Vector2F, Self::LayoutState) {
         debug_assert!(constraint.max_along(Axis::Horizontal) == f32::INFINITY);
 
@@ -53,8 +53,8 @@ impl Element<CollabTitlebarItem> for FacePile {
         bounds: RectF,
         visible_bounds: RectF,
         _layout: &mut Self::LayoutState,
-        view: &mut CollabTitlebarItem,
-        cx: &mut ViewContext<CollabTitlebarItem>,
+        view: &mut V,
+        cx: &mut ViewContext<V>,
     ) -> Self::PaintState {
         let visible_bounds = bounds.intersection(visible_bounds).unwrap_or_default();
 
@@ -80,8 +80,8 @@ impl Element<CollabTitlebarItem> for FacePile {
         _: RectF,
         _: &Self::LayoutState,
         _: &Self::PaintState,
-        _: &CollabTitlebarItem,
-        _: &ViewContext<CollabTitlebarItem>,
+        _: &V,
+        _: &ViewContext<V>,
     ) -> Option<RectF> {
         None
     }
@@ -91,8 +91,8 @@ impl Element<CollabTitlebarItem> for FacePile {
         bounds: RectF,
         _: &Self::LayoutState,
         _: &Self::PaintState,
-        _: &CollabTitlebarItem,
-        _: &ViewContext<CollabTitlebarItem>,
+        _: &V,
+        _: &ViewContext<V>,
     ) -> serde_json::Value {
         json!({
             "type": "FacePile",
@@ -101,8 +101,8 @@ impl Element<CollabTitlebarItem> for FacePile {
     }
 }
 
-impl Extend<AnyElement<CollabTitlebarItem>> for FacePile {
-    fn extend<T: IntoIterator<Item = AnyElement<CollabTitlebarItem>>>(&mut self, children: T) {
+impl<V: View> Extend<AnyElement<V>> for FacePile<V> {
+    fn extend<T: IntoIterator<Item = AnyElement<V>>>(&mut self, children: T) {
         self.faces.extend(children);
     }
 }

crates/collab_ui/src/panel.rs 🔗

@@ -40,6 +40,8 @@ use workspace::{
     Workspace,
 };
 
+use crate::face_pile::FacePile;
+
 #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
 struct RemoveChannel {
     channel_id: u64,
@@ -253,7 +255,7 @@ impl CollabPanel {
                             )
                         }
                         ListEntry::Channel(channel) => {
-                            Self::render_channel(&*channel, &theme.collab_panel, is_selected, cx)
+                            this.render_channel(&*channel, &theme.collab_panel, is_selected, cx)
                         }
                         ListEntry::ChannelInvite(channel) => Self::render_channel_invite(
                             channel.clone(),
@@ -1265,20 +1267,16 @@ impl CollabPanel {
     }
 
     fn render_channel(
+        &self,
         channel: &Channel,
         theme: &theme::CollabPanel,
         is_selected: bool,
         cx: &mut ViewContext<Self>,
     ) -> AnyElement<Self> {
         let channel_id = channel.id;
-        MouseEventHandler::<Channel, Self>::new(channel.id as usize, cx, |state, _cx| {
+        MouseEventHandler::<Channel, Self>::new(channel.id as usize, cx, |state, cx| {
             Flex::row()
-                .with_child({
-                    Svg::new("icons/file_icons/hash.svg")
-                        // .with_style(theme.contact_avatar)
-                        .aligned()
-                        .left()
-                })
+                .with_child({ Svg::new("icons/file_icons/hash.svg").aligned().left() })
                 .with_child(
                     Label::new(channel.name.clone(), theme.contact_username.text.clone())
                         .contained()
@@ -1287,6 +1285,20 @@ impl CollabPanel {
                         .left()
                         .flex(1., true),
                 )
+                .with_child(
+                    FacePile::new(theme.face_overlap).with_children(
+                        self.channel_store
+                            .read(cx)
+                            .channel_participants(channel_id)
+                            .iter()
+                            .filter_map(|user| {
+                                Some(
+                                    Image::from_data(user.avatar.clone()?)
+                                        .with_style(theme.contact_avatar),
+                                )
+                            }),
+                    ),
+                )
                 .constrained()
                 .with_height(theme.row_height)
                 .contained()

crates/theme/src/theme.rs 🔗

@@ -241,6 +241,7 @@ pub struct CollabPanel {
     pub disabled_button: IconButton,
     pub section_icon_size: f32,
     pub calling_indicator: ContainedText,
+    pub face_overlap: f32,
 }
 
 #[derive(Deserialize, Default, JsonSchema)]