Collab UI improvements (#6805)

Conrad Irwin created

- Allow people to leave channels
- Make contacts use right click menu for call
- Add a hover menu to see who's in a channel

Release Notes:

- Fixed clicking on contacts to *not* call them immediately
- Added a hover menu on channels to see who is there
- Added a "Leave Channel" right click option

Change summary

crates/collab/src/db/queries/channels.rs   |   7 
crates/collab/src/tests/channel_tests.rs   |  22 ++
crates/collab/src/tests/following_tests.rs |   2 
crates/collab/src/tests/test_server.rs     |   4 
crates/collab_ui/src/collab_panel.rs       | 209 ++++++++++++++++++++---
crates/ui/src/components/tooltip.rs        |   2 
6 files changed, 213 insertions(+), 33 deletions(-)

Detailed changes

crates/collab/src/db/queries/channels.rs 🔗

@@ -473,8 +473,11 @@ impl Database {
     ) -> Result<RemoveChannelMemberResult> {
         self.transaction(|tx| async move {
             let channel = self.get_channel_internal(channel_id, &*tx).await?;
-            self.check_user_is_channel_admin(&channel, admin_id, &*tx)
-                .await?;
+
+            if member_id != admin_id {
+                self.check_user_is_channel_admin(&channel, admin_id, &*tx)
+                    .await?;
+            }
 
             let result = channel_member::Entity::delete_many()
                 .filter(

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

@@ -1306,6 +1306,28 @@ async fn test_invite_access(
     })
 }
 
+#[gpui::test]
+async fn test_leave_channel(cx_a: &mut TestAppContext, cx_b: &mut TestAppContext) {
+    let (_server, _client_a, client_b, channel_id) = TestServer::start2(cx_a, cx_b).await;
+
+    client_b
+        .channel_store()
+        .update(cx_b, |channel_store, cx| {
+            channel_store.remove_member(channel_id, client_b.user_id().unwrap(), cx)
+        })
+        .await
+        .unwrap();
+
+    cx_a.run_until_parked();
+
+    assert_eq!(
+        client_b
+            .channel_store()
+            .read_with(cx_b, |store, _| store.channels().count()),
+        0
+    );
+}
+
 #[gpui::test]
 async fn test_channel_moving(
     executor: BackgroundExecutor,

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

@@ -1575,7 +1575,7 @@ async fn test_following_across_workspaces(cx_a: &mut TestAppContext, cx_b: &mut
 
 #[gpui::test]
 async fn test_following_stops_on_unshare(cx_a: &mut TestAppContext, cx_b: &mut TestAppContext) {
-    let (client_a, client_b, channel_id) = TestServer::start2(cx_a, cx_b).await;
+    let (_, client_a, client_b, channel_id) = TestServer::start2(cx_a, cx_b).await;
 
     let (workspace_a, cx_a) = client_a.build_test_workspace(cx_a).await;
     client_a

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

@@ -118,7 +118,7 @@ impl TestServer {
     pub async fn start2(
         cx_a: &mut TestAppContext,
         cx_b: &mut TestAppContext,
-    ) -> (TestClient, TestClient, u64) {
+    ) -> (TestServer, TestClient, TestClient, u64) {
         let mut server = Self::start(cx_a.executor()).await;
         let client_a = server.create_client(cx_a, "user_a").await;
         let client_b = server.create_client(cx_b, "user_b").await;
@@ -127,7 +127,7 @@ impl TestServer {
             .await;
         cx_a.run_until_parked();
 
-        (client_a, client_b, channel_id)
+        (server, client_a, client_b, channel_id)
     }
 
     pub async fn start1<'a>(cx: &'a mut TestAppContext) -> TestClient {

crates/collab_ui/src/collab_panel.rs 🔗

@@ -15,10 +15,11 @@ use editor::{Editor, EditorElement, EditorStyle};
 use fuzzy::{match_strings, StringMatchCandidate};
 use gpui::{
     actions, canvas, div, fill, list, overlay, point, prelude::*, px, AnyElement, AppContext,
-    AsyncWindowContext, Bounds, ClipboardItem, DismissEvent, Div, EventEmitter, FocusHandle,
-    FocusableView, FontStyle, FontWeight, InteractiveElement, IntoElement, ListOffset, ListState,
-    Model, MouseDownEvent, ParentElement, Pixels, Point, PromptLevel, Render, SharedString, Styled,
-    Subscription, Task, TextStyle, View, ViewContext, VisualContext, WeakView, WhiteSpace,
+    AsyncWindowContext, Bounds, ClickEvent, ClipboardItem, DismissEvent, Div, EventEmitter,
+    FocusHandle, FocusableView, FontStyle, FontWeight, InteractiveElement, IntoElement, ListOffset,
+    ListState, Model, MouseDownEvent, ParentElement, Pixels, Point, PromptLevel, Render,
+    SharedString, Styled, Subscription, Task, TextStyle, View, ViewContext, VisualContext,
+    WeakView, WhiteSpace,
 };
 use menu::{Cancel, Confirm, SecondaryConfirm, SelectNext, SelectPrev};
 use project::{Fs, Project};
@@ -32,8 +33,8 @@ use smallvec::SmallVec;
 use std::{mem, sync::Arc};
 use theme::{ActiveTheme, ThemeSettings};
 use ui::{
-    prelude::*, Avatar, AvatarAvailabilityIndicator, Button, Color, ContextMenu, Icon, IconButton,
-    IconName, IconSize, Label, ListHeader, ListItem, Tooltip,
+    prelude::*, tooltip_container, Avatar, AvatarAvailabilityIndicator, Button, Color, ContextMenu,
+    Icon, IconButton, IconName, IconSize, Label, ListHeader, ListItem, Tooltip,
 };
 use util::{maybe, ResultExt, TryFutureExt};
 use workspace::{
@@ -366,9 +367,11 @@ impl CollabPanel {
             if !self.collapsed_sections.contains(&Section::ActiveCall) {
                 let room = room.read(cx);
 
-                if let Some(channel_id) = room.channel_id() {
-                    self.entries.push(ListEntry::ChannelNotes { channel_id });
-                    self.entries.push(ListEntry::ChannelChat { channel_id });
+                if query.is_empty() {
+                    if let Some(channel_id) = room.channel_id() {
+                        self.entries.push(ListEntry::ChannelNotes { channel_id });
+                        self.entries.push(ListEntry::ChannelChat { channel_id });
+                    }
                 }
 
                 // Populate the active user.
@@ -1122,7 +1125,9 @@ impl CollabPanel {
                     }),
                 );
 
+            let mut has_destructive_actions = false;
             if self.channel_store.read(cx).is_channel_admin(channel_id) {
+                has_destructive_actions = true;
                 context_menu = context_menu
                     .separator()
                     .entry(
@@ -1194,6 +1199,17 @@ impl CollabPanel {
                 );
             }
 
+            if self.channel_store.read(cx).is_root_channel(channel_id) {
+                if !has_destructive_actions {
+                    context_menu = context_menu.separator()
+                }
+                context_menu = context_menu.entry(
+                    "Leave Channel",
+                    None,
+                    cx.handler_for(&this, move |this, cx| this.leave_channel(channel_id, cx)),
+                );
+            }
+
             context_menu
         });
 
@@ -1213,6 +1229,60 @@ impl CollabPanel {
         cx.notify();
     }
 
+    fn deploy_contact_context_menu(
+        &mut self,
+        position: Point<Pixels>,
+        contact: Arc<Contact>,
+        cx: &mut ViewContext<Self>,
+    ) {
+        let this = cx.view().clone();
+        let in_room = ActiveCall::global(cx).read(cx).room().is_some();
+
+        let context_menu = ContextMenu::build(cx, |mut context_menu, _| {
+            let user_id = contact.user.id;
+
+            if contact.online && !contact.busy {
+                let label = if in_room {
+                    format!("Invite {} to join", contact.user.github_login)
+                } else {
+                    format!("Call {}", contact.user.github_login)
+                };
+                context_menu = context_menu.entry(label, None, {
+                    let this = this.clone();
+                    move |cx| {
+                        this.update(cx, |this, cx| {
+                            this.call(user_id, cx);
+                        });
+                    }
+                });
+            }
+
+            context_menu.entry("Remove Contact", None, {
+                let this = this.clone();
+                move |cx| {
+                    this.update(cx, |this, cx| {
+                        this.remove_contact(contact.user.id, &contact.user.github_login, cx);
+                    });
+                }
+            })
+        });
+
+        cx.focus_view(&context_menu);
+        let subscription =
+            cx.subscribe(&context_menu, |this, _, _: &DismissEvent, cx| {
+                if this.context_menu.as_ref().is_some_and(|context_menu| {
+                    context_menu.0.focus_handle(cx).contains_focused(cx)
+                }) {
+                    cx.focus_self();
+                }
+                this.context_menu.take();
+                cx.notify();
+            });
+        self.context_menu = Some((context_menu, position, subscription));
+
+        cx.notify();
+    }
+
     fn cancel(&mut self, _: &Cancel, cx: &mut ViewContext<Self>) {
         if self.take_editing_state(cx) {
             cx.focus_view(&self.filter_editor);
@@ -1613,9 +1683,6 @@ impl CollabPanel {
     }
 
     fn show_inline_context_menu(&mut self, _: &menu::ShowContextMenu, cx: &mut ViewContext<Self>) {
-        let Some(channel) = self.selected_channel() else {
-            return;
-        };
         let Some(bounds) = self
             .selection
             .and_then(|ix| self.list_state.bounds_for_item(ix))
@@ -1623,8 +1690,22 @@ impl CollabPanel {
             return;
         };
 
-        self.deploy_channel_context_menu(bounds.center(), channel.id, self.selection.unwrap(), cx);
-        cx.stop_propagation();
+        if let Some(channel) = self.selected_channel() {
+            self.deploy_channel_context_menu(
+                bounds.center(),
+                channel.id,
+                self.selection.unwrap(),
+                cx,
+            );
+            cx.stop_propagation();
+            return;
+        };
+
+        if let Some(contact) = self.selected_contact() {
+            self.deploy_contact_context_menu(bounds.center(), contact, cx);
+            cx.stop_propagation();
+            return;
+        };
     }
 
     fn selected_channel(&self) -> Option<&Arc<Channel>> {
@@ -1636,6 +1717,15 @@ impl CollabPanel {
             })
     }
 
+    fn selected_contact(&self) -> Option<Arc<Contact>> {
+        self.selection
+            .and_then(|ix| self.entries.get(ix))
+            .and_then(|entry| match entry {
+                ListEntry::Contact { contact, .. } => Some(contact.clone()),
+                _ => None,
+            })
+    }
+
     fn show_channel_modal(
         &mut self,
         channel_id: ChannelId,
@@ -1667,6 +1757,34 @@ impl CollabPanel {
         .detach();
     }
 
+    fn leave_channel(&self, channel_id: ChannelId, cx: &mut ViewContext<Self>) {
+        let Some(user_id) = self.user_store.read(cx).current_user().map(|u| u.id) else {
+            return;
+        };
+        let Some(channel) = self.channel_store.read(cx).channel_for_id(channel_id) else {
+            return;
+        };
+        let prompt_message = format!("Are you sure you want to leave \"#{}\"?", channel.name);
+        let answer = cx.prompt(
+            PromptLevel::Warning,
+            &prompt_message,
+            None,
+            &["Leave", "Cancel"],
+        );
+        cx.spawn(|this, mut cx| async move {
+            if answer.await? != 0 {
+                return Ok(());
+            }
+            this.update(&mut cx, |this, cx| {
+                this.channel_store.update(cx, |channel_store, cx| {
+                    channel_store.remove_member(channel_id, user_id, cx)
+                })
+            })?
+            .await
+        })
+        .detach_and_prompt_err("Failed to leave channel", cx, |_, _| None)
+    }
+
     fn remove_channel(&mut self, channel_id: ChannelId, cx: &mut ViewContext<Self>) {
         let channel_store = self.channel_store.clone();
         if let Some(channel) = channel_store.read(cx).channel_for_id(channel_id) {
@@ -2049,20 +2167,18 @@ impl CollabPanel {
 
     fn render_contact(
         &self,
-        contact: &Contact,
+        contact: &Arc<Contact>,
         calling: bool,
         is_selected: bool,
         cx: &mut ViewContext<Self>,
     ) -> impl IntoElement {
         let online = contact.online;
         let busy = contact.busy || calling;
-        let user_id = contact.user.id;
         let github_login = SharedString::from(contact.user.github_login.clone());
         let item = ListItem::new(github_login.clone())
             .indent_level(1)
             .indent_step_size(px(20.))
             .selected(is_selected)
-            .on_click(cx.listener(move |this, _, cx| this.call(user_id, cx)))
             .child(
                 h_flex()
                     .w_full()
@@ -2073,19 +2189,28 @@ impl CollabPanel {
                     })
                     .when(!calling, |el| {
                         el.child(
-                            IconButton::new("remove_contact", IconName::Close)
+                            IconButton::new("contact context menu", IconName::Ellipsis)
                                 .icon_color(Color::Muted)
                                 .visible_on_hover("")
-                                .tooltip(|cx| Tooltip::text("Remove Contact", cx))
                                 .on_click(cx.listener({
-                                    let github_login = github_login.clone();
-                                    move |this, _, cx| {
-                                        this.remove_contact(user_id, &github_login, cx);
+                                    let contact = contact.clone();
+                                    move |this, event: &ClickEvent, cx| {
+                                        this.deploy_contact_context_menu(
+                                            event.down.position,
+                                            contact.clone(),
+                                            cx,
+                                        );
                                     }
                                 })),
                         )
                     }),
             )
+            .on_secondary_mouse_down(cx.listener({
+                let contact = contact.clone();
+                move |this, event: &MouseDownEvent, cx| {
+                    this.deploy_contact_context_menu(event.position, contact.clone(), cx);
+                }
+            }))
             .start_slot(
                 // todo handle contacts with no avatar
                 Avatar::new(contact.user.avatar_uri.clone())
@@ -2097,10 +2222,7 @@ impl CollabPanel {
                     } else {
                         None
                     }),
-            )
-            .when(online && !busy, |el| {
-                el.on_click(cx.listener(move |this, _, cx| this.call(user_id, cx)))
-            });
+            );
 
         div()
             .id(github_login.clone())
@@ -2419,7 +2541,16 @@ impl CollabPanel {
                             ),
                     ),
             )
-            .tooltip(|cx| Tooltip::text("Join channel", cx))
+            .tooltip({
+                let channel_store = self.channel_store.clone();
+                move |cx| {
+                    cx.new_view(|_| JoinChannelTooltip {
+                        channel_store: channel_store.clone(),
+                        channel_id,
+                    })
+                    .into()
+                }
+            })
     }
 
     fn render_channel_editor(&self, depth: usize, _cx: &mut ViewContext<Self>) -> impl IntoElement {
@@ -2697,3 +2828,27 @@ impl Render for DraggedChannelView {
             .child(Label::new(self.channel.name.clone()))
     }
 }
+
+struct JoinChannelTooltip {
+    channel_store: Model<ChannelStore>,
+    channel_id: ChannelId,
+}
+
+impl Render for JoinChannelTooltip {
+    fn render(&mut self, cx: &mut ViewContext<Self>) -> impl IntoElement {
+        tooltip_container(cx, |div, cx| {
+            let participants = self
+                .channel_store
+                .read(cx)
+                .channel_participants(self.channel_id);
+
+            div.child(Label::new("Join Channel"))
+                .children(participants.iter().map(|participant| {
+                    h_flex()
+                        .gap_2()
+                        .child(Avatar::new(participant.avatar_uri.clone()))
+                        .child(Label::new(participant.github_login.clone()))
+                }))
+        })
+    }
+}

crates/ui/src/components/tooltip.rs 🔗

@@ -85,7 +85,7 @@ impl Render for Tooltip {
     }
 }
 
-fn tooltip_container<V>(
+pub fn tooltip_container<V>(
     cx: &mut ViewContext<V>,
     f: impl FnOnce(Div, &mut ViewContext<V>) -> Div,
 ) -> impl IntoElement {