Fix joining descendant channels, style channel invites

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

crates/client/src/channel_store.rs

@@ -104,7 +104,7 @@ impl ChannelStore {
         parent_id: Option<ChannelId>,
     ) -> impl Future<Output = Result<ChannelId>> {
         let client = self.client.clone();
-        let name = name.to_owned();
+        let name = name.trim_start_matches("#").to_owned();
         async move {
             Ok(client
                 .request(proto::CreateChannel { name, parent_id })

crates/collab/src/db.rs

@@ -1381,16 +1381,8 @@ impl Database {
     ) -> Result<RoomGuard<JoinRoom>> {
         self.room_transaction(room_id, |tx| async move {
             if let Some(channel_id) = channel_id {
-                channel_member::Entity::find()
+                self.check_user_is_channel_member(channel_id, user_id, &*tx)
-                    .filter(
+                    .await?;
-                        channel_member::Column::ChannelId
-                            .eq(channel_id)
-                            .and(channel_member::Column::UserId.eq(user_id))
-                            .and(channel_member::Column::Accepted.eq(true)),
-                    )
-                    .one(&*tx)
-                    .await?
-                    .ok_or_else(|| anyhow!("no such channel membership"))?;
 
                 room_participant::ActiveModel {
                     room_id: ActiveValue::set(room_id),
@@ -1738,7 +1730,6 @@ impl Database {
             }
 
             let (channel_id, room) = self.get_channel_room(room_id, &tx).await?;
-
             let channel_members = if let Some(channel_id) = channel_id {
                 self.get_channel_members_internal(channel_id, &tx).await?
             } else {
@@ -3595,6 +3586,25 @@ impl Database {
         Ok(user_ids)
     }
 
+    async fn check_user_is_channel_member(
+        &self,
+        channel_id: ChannelId,
+        user_id: UserId,
+        tx: &DatabaseTransaction,
+    ) -> Result<()> {
+        let channel_ids = self.get_channel_ancestors(channel_id, tx).await?;
+        channel_member::Entity::find()
+            .filter(
+                channel_member::Column::ChannelId
+                    .is_in(channel_ids)
+                    .and(channel_member::Column::UserId.eq(user_id)),
+            )
+            .one(&*tx)
+            .await?
+            .ok_or_else(|| anyhow!("user is not a channel member"))?;
+        Ok(())
+    }
+
     async fn check_user_is_channel_admin(
         &self,
         channel_id: ChannelId,
@@ -3611,7 +3621,7 @@ impl Database {
             )
             .one(&*tx)
             .await?
-            .ok_or_else(|| anyhow!("user is not allowed to remove this channel"))?;
+            .ok_or_else(|| anyhow!("user is not a channel admin"))?;
         Ok(())
     }
 

crates/collab/src/tests/channel_tests.rs

@@ -313,6 +313,38 @@ fn assert_members_eq(
     );
 }
 
+#[gpui::test]
+async fn test_joining_channel_ancestor_member(
+    deterministic: Arc<Deterministic>,
+    cx_a: &mut TestAppContext,
+    cx_b: &mut TestAppContext,
+) {
+    deterministic.forbid_parking();
+    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 parent_id = server
+        .make_channel("parent", (&client_a, cx_a), &mut [(&client_b, cx_b)])
+        .await;
+
+    let sub_id = client_a
+        .channel_store()
+        .update(cx_a, |channel_store, _| {
+            channel_store.create_channel("sub_channel", Some(parent_id))
+        })
+        .await
+        .unwrap();
+
+    let active_call_b = cx_b.read(ActiveCall::global);
+
+    assert!(active_call_b
+        .update(cx_b, |active_call, cx| active_call.join_channel(sub_id, cx))
+        .await
+        .is_ok());
+}
+
 #[gpui::test]
 async fn test_channel_room(
     deterministic: Arc<Deterministic>,

crates/collab_ui/src/collab_panel.rs

@@ -120,7 +120,8 @@ pub enum Event {
 enum Section {
     ActiveCall,
     Channels,
-    Requests,
+    ChannelInvites,
+    ContactRequests,
     Contacts,
     Online,
     Offline,
@@ -404,8 +405,10 @@ impl CollabPanel {
         let old_entries = mem::take(&mut self.entries);
 
         if let Some(room) = ActiveCall::global(cx).read(cx).room() {
+            self.entries.push(ListEntry::Header(Section::ActiveCall, 0));
+
+            if !self.collapsed_sections.contains(&Section::ActiveCall) {
                 let room = room.read(cx);
-            let mut participant_entries = Vec::new();
 
                 // Populate the active user.
                 if let Some(user) = user_store.current_user() {
@@ -425,13 +428,13 @@ impl CollabPanel {
                     ));
                     if !matches.is_empty() {
                         let user_id = user.id;
-                    participant_entries.push(ListEntry::CallParticipant {
+                        self.entries.push(ListEntry::CallParticipant {
                             user,
                             is_pending: false,
                         });
                         let mut projects = room.local_participant().projects.iter().peekable();
                         while let Some(project) = projects.next() {
-                        participant_entries.push(ListEntry::ParticipantProject {
+                            self.entries.push(ListEntry::ParticipantProject {
                                 project_id: project.id,
                                 worktree_root_names: project.worktree_root_names.clone(),
                                 host_user_id: user_id,
@@ -462,21 +465,22 @@ impl CollabPanel {
                 for mat in matches {
                     let user_id = mat.candidate_id as u64;
                     let participant = &room.remote_participants()[&user_id];
-                participant_entries.push(ListEntry::CallParticipant {
+                    self.entries.push(ListEntry::CallParticipant {
                         user: participant.user.clone(),
                         is_pending: false,
                     });
                     let mut projects = participant.projects.iter().peekable();
                     while let Some(project) = projects.next() {
-                    participant_entries.push(ListEntry::ParticipantProject {
+                        self.entries.push(ListEntry::ParticipantProject {
                             project_id: project.id,
                             worktree_root_names: project.worktree_root_names.clone(),
                             host_user_id: participant.user.id,
-                        is_last: projects.peek().is_none() && participant.video_tracks.is_empty(),
+                            is_last: projects.peek().is_none()
+                                && participant.video_tracks.is_empty(),
                         });
                     }
                     if !participant.video_tracks.is_empty() {
-                    participant_entries.push(ListEntry::ParticipantScreen {
+                        self.entries.push(ListEntry::ParticipantScreen {
                             peer_id: participant.peer_id,
                             is_last: true,
                         });
@@ -486,16 +490,13 @@ impl CollabPanel {
                 // Populate pending participants.
                 self.match_candidates.clear();
                 self.match_candidates
-                .extend(
+                    .extend(room.pending_participants().iter().enumerate().map(
-                    room.pending_participants()
+                        |(id, participant)| StringMatchCandidate {
-                        .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,
@@ -504,16 +505,11 @@ impl CollabPanel {
                     &Default::default(),
                     executor.clone(),
                 ));
-            participant_entries.extend(matches.iter().map(|mat| ListEntry::CallParticipant {
+                self.entries
+                    .extend(matches.iter().map(|mat| ListEntry::CallParticipant {
                         user: room.pending_participants()[mat.candidate_id].clone(),
                         is_pending: true,
                     }));
-
-            if !participant_entries.is_empty() {
-                self.entries.push(ListEntry::Header(Section::ActiveCall, 0));
-                if !self.collapsed_sections.contains(&Section::ActiveCall) {
-                    self.entries.extend(participant_entries);
-                }
             }
         }
 
@@ -559,8 +555,6 @@ impl CollabPanel {
             }
         }
 
-        self.entries.push(ListEntry::Header(Section::Contacts, 0));
-
         let mut request_entries = Vec::new();
         let channel_invites = channel_store.channel_invitations();
         if !channel_invites.is_empty() {
@@ -586,8 +580,19 @@ impl CollabPanel {
                     .iter()
                     .map(|mat| ListEntry::ChannelInvite(channel_invites[mat.candidate_id].clone())),
             );
+
+            if !request_entries.is_empty() {
+                self.entries
+                    .push(ListEntry::Header(Section::ChannelInvites, 1));
+                if !self.collapsed_sections.contains(&Section::ChannelInvites) {
+                    self.entries.append(&mut request_entries);
+                }
+            }
         }
 
+        self.entries.push(ListEntry::Header(Section::Contacts, 0));
+
+        request_entries.clear();
         let incoming = user_store.incoming_contact_requests();
         if !incoming.is_empty() {
             self.match_candidates.clear();
@@ -647,8 +652,9 @@ impl CollabPanel {
         }
 
         if !request_entries.is_empty() {
-            self.entries.push(ListEntry::Header(Section::Requests, 1));
+            self.entries
-            if !self.collapsed_sections.contains(&Section::Requests) {
+                .push(ListEntry::Header(Section::ContactRequests, 1));
+            if !self.collapsed_sections.contains(&Section::ContactRequests) {
                 self.entries.append(&mut request_entries);
             }
         }
@@ -1043,9 +1049,10 @@ impl CollabPanel {
         let tooltip_style = &theme.tooltip;
         let text = match section {
             Section::ActiveCall => "Current Call",
-            Section::Requests => "Requests",
+            Section::ContactRequests => "Requests",
             Section::Contacts => "Contacts",
             Section::Channels => "Channels",
+            Section::ChannelInvites => "Invites",
             Section::Online => "Online",
             Section::Offline => "Offline",
         };
@@ -1055,15 +1062,13 @@ impl CollabPanel {
             Section::ActiveCall => Some(
                 MouseEventHandler::<AddContact, Self>::new(0, cx, |_, _| {
                     render_icon_button(
-                        &theme.collab_panel.leave_call_button,
+                        theme.collab_panel.leave_call_button.in_state(is_selected),
                         "icons/radix/exit.svg",
                     )
                 })
                 .with_cursor_style(CursorStyle::PointingHand)
                 .on_click(MouseButton::Left, |_, _, cx| {
-                    ActiveCall::global(cx)
+                    Self::leave_call(cx);
-                        .update(cx, |call, cx| call.hang_up(cx))
-                        .detach_and_log_err(cx);
                 })
                 .with_tooltip::<AddContact>(
                     0,
@@ -1076,7 +1081,7 @@ impl CollabPanel {
             Section::Contacts => Some(
                 MouseEventHandler::<LeaveCallContactList, Self>::new(0, cx, |_, _| {
                     render_icon_button(
-                        &theme.collab_panel.add_contact_button,
+                        theme.collab_panel.add_contact_button.in_state(is_selected),
                         "icons/user_plus_16.svg",
                     )
                 })
@@ -1094,7 +1099,10 @@ impl CollabPanel {
             ),
             Section::Channels => Some(
                 MouseEventHandler::<AddChannel, Self>::new(0, cx, |_, _| {
-                    render_icon_button(&theme.collab_panel.add_contact_button, "icons/plus_16.svg")
+                    render_icon_button(
+                        theme.collab_panel.add_contact_button.in_state(is_selected),
+                        "icons/plus_16.svg",
+                    )
                 })
                 .with_cursor_style(CursorStyle::PointingHand)
                 .on_click(MouseButton::Left, |_, this, cx| this.new_root_channel(cx))
@@ -1284,10 +1292,10 @@ impl CollabPanel {
         MouseEventHandler::<Channel, Self>::new(channel.id as usize, cx, |state, cx| {
             Flex::row()
                 .with_child(
-                    Svg::new("icons/channels.svg")
+                    Svg::new("icons/channel_hash.svg")
-                        .with_color(theme.add_channel_button.color)
+                        .with_color(theme.channel_hash.color)
                         .constrained()
-                        .with_width(14.)
+                        .with_width(theme.channel_hash.width)
                         .aligned()
                         .left(),
                 )
@@ -1313,11 +1321,15 @@ impl CollabPanel {
                             }),
                     ),
                 )
+                .align_children_center()
                 .constrained()
                 .with_height(theme.row_height)
                 .contained()
                 .with_style(*theme.contact_row.in_state(is_selected).style_for(state))
-                .with_margin_left(20. * channel.depth as f32)
+                .with_padding_left(
+                    theme.contact_row.default_style().padding.left
+                        + theme.channel_indent * channel.depth as f32,
+                )
         })
         .on_click(MouseButton::Left, move |_, this, cx| {
             this.join_channel(channel_id, cx);
@@ -1345,7 +1357,14 @@ impl CollabPanel {
         let button_spacing = theme.contact_button_spacing;
 
         Flex::row()
-            .with_child(Svg::new("icons/file_icons/hash.svg").aligned().left())
+            .with_child(
+                Svg::new("icons/channel_hash.svg")
+                    .with_color(theme.channel_hash.color)
+                    .constrained()
+                    .with_width(theme.channel_hash.width)
+                    .aligned()
+                    .left(),
+            )
             .with_child(
                 Label::new(channel.name.clone(), theme.contact_username.text.clone())
                     .contained()
@@ -1403,6 +1422,9 @@ impl CollabPanel {
                     .in_state(is_selected)
                     .style_for(&mut Default::default()),
             )
+            .with_padding_left(
+                theme.contact_row.default_style().padding.left + theme.channel_indent,
+            )
             .into_any()
     }
 
@@ -1532,30 +1554,23 @@ impl CollabPanel {
     }
 
     fn cancel(&mut self, _: &Cancel, cx: &mut ViewContext<Self>) {
-        let mut did_clear = self.filter_editor.update(cx, |editor, cx| {
+        if self.take_editing_state(cx).is_some() {
+            cx.focus(&self.filter_editor);
+        } else {
+            self.filter_editor.update(cx, |editor, cx| {
                 if editor.buffer().read(cx).len(cx) > 0 {
                     editor.set_text("", cx);
-                true
-            } else {
-                false
                 }
             });
-
-        did_clear |= self.take_editing_state(cx).is_some();
-
-        if !did_clear {
-            cx.emit(Event::Dismissed);
         }
+
+        self.update_entries(cx);
     }
 
     fn select_next(&mut self, _: &SelectNext, cx: &mut ViewContext<Self>) {
-        let mut ix = self.selection.map_or(0, |ix| ix + 1);
+        let ix = self.selection.map_or(0, |ix| ix + 1);
-        while let Some(entry) = self.entries.get(ix) {
+        if ix < self.entries.len() {
-            if entry.is_selectable() {
             self.selection = Some(ix);
-                break;
-            }
-            ix += 1;
         }
 
         self.list_state.reset(self.entries.len());
@@ -1569,16 +1584,9 @@ impl CollabPanel {
     }
 
     fn select_prev(&mut self, _: &SelectPrev, cx: &mut ViewContext<Self>) {
-        if let Some(mut ix) = self.selection.take() {
+        let ix = self.selection.take().unwrap_or(0);
-            while ix > 0 {
+        if ix > 0 {
-                ix -= 1;
+            self.selection = Some(ix - 1);
-                if let Some(entry) = self.entries.get(ix) {
-                    if entry.is_selectable() {
-                        self.selection = Some(ix);
-                        break;
-                    }
-                }
-            }
         }
 
         self.list_state.reset(self.entries.len());
@@ -1595,9 +1603,17 @@ impl CollabPanel {
         if let Some(selection) = self.selection {
             if let Some(entry) = self.entries.get(selection) {
                 match entry {
-                    ListEntry::Header(section, _) => {
+                    ListEntry::Header(section, _) => match section {
+                        Section::ActiveCall => Self::leave_call(cx),
+                        Section::Channels => self.new_root_channel(cx),
+                        Section::Contacts => self.toggle_contact_finder(cx),
+                        Section::ContactRequests
+                        | Section::Online
+                        | Section::Offline
+                        | Section::ChannelInvites => {
                             self.toggle_expanded(*section, cx);
                         }
+                    },
                     ListEntry::Contact { contact, calling } => {
                         if contact.online && !contact.busy && !calling {
                             self.call(contact.user.id, Some(self.project.clone()), cx);
@@ -1626,6 +1642,9 @@ impl CollabPanel {
                             });
                         }
                     }
+                    ListEntry::Channel(channel) => {
+                        self.join_channel(channel.id, cx);
+                    }
                     _ => {}
                 }
             }
@@ -1651,6 +1670,12 @@ impl CollabPanel {
         self.update_entries(cx);
     }
 
+    fn leave_call(cx: &mut ViewContext<Self>) {
+        ActiveCall::global(cx)
+            .update(cx, |call, cx| call.hang_up(cx))
+            .detach_and_log_err(cx);
+    }
+
     fn toggle_contact_finder(&mut self, cx: &mut ViewContext<Self>) {
         if let Some(workspace) = self.workspace.upgrade(cx) {
             workspace.update(cx, |workspace, cx| {
@@ -1666,23 +1691,17 @@ impl CollabPanel {
     }
 
     fn new_root_channel(&mut self, cx: &mut ViewContext<Self>) {
-        if self.channel_editing_state.is_none() {
         self.channel_editing_state = Some(ChannelEditingState { parent_id: None });
         self.update_entries(cx);
-        }
-
         cx.focus(self.channel_name_editor.as_any());
         cx.notify();
     }
 
     fn new_subchannel(&mut self, action: &NewChannel, cx: &mut ViewContext<Self>) {
-        if self.channel_editing_state.is_none() {
         self.channel_editing_state = Some(ChannelEditingState {
             parent_id: Some(action.channel_id),
         });
         self.update_entries(cx);
-        }
-
         cx.focus(self.channel_name_editor.as_any());
         cx.notify();
     }
@@ -1825,6 +1844,13 @@ impl View for CollabPanel {
     fn focus_in(&mut self, _: gpui::AnyViewHandle, cx: &mut ViewContext<Self>) {
         if !self.has_focus {
             self.has_focus = true;
+            if !self.context_menu.is_focused(cx) {
+                if self.channel_editing_state.is_some() {
+                    cx.focus(&self.channel_name_editor);
+                } else {
+                    cx.focus(&self.filter_editor);
+                }
+            }
             cx.emit(Event::Focus);
         }
     }
@@ -1931,16 +1957,6 @@ impl Panel for CollabPanel {
     }
 }
 
-impl ListEntry {
-    fn is_selectable(&self) -> bool {
-        if let ListEntry::Header(_, 0) = self {
-            false
-        } else {
-            true
-        }
-    }
-}
-
 impl PartialEq for ListEntry {
     fn eq(&self, other: &Self) -> bool {
         match self {

crates/collab_ui/src/collab_panel/channel_modal.rs

@@ -487,7 +487,7 @@ impl ChannelModalDelegate {
         });
         cx.spawn(|picker, mut cx| async move {
             update.await?;
-            picker.update(&mut cx, |picker, cx| {
+            picker.update(&mut cx, |picker, _| {
                 let this = picker.delegate_mut();
                 if let Some(member) = this.members.iter_mut().find(|m| m.user.id == user_id) {
                     member.admin = admin;
@@ -503,7 +503,7 @@ impl ChannelModalDelegate {
         });
         cx.spawn(|picker, mut cx| async move {
             update.await?;
-            picker.update(&mut cx, |picker, cx| {
+            picker.update(&mut cx, |picker, _| {
                 let this = picker.delegate_mut();
                 if let Some(ix) = this.members.iter_mut().position(|m| m.user.id == user_id) {
                     this.members.remove(ix);

crates/theme/src/theme.rs

@@ -220,12 +220,13 @@ pub struct CopilotAuthAuthorized {
 pub struct CollabPanel {
     #[serde(flatten)]
     pub container: ContainerStyle,
+    pub channel_hash: Icon,
     pub channel_modal: ChannelModal,
     pub user_query_editor: FieldEditor,
     pub user_query_editor_height: f32,
-    pub leave_call_button: IconButton,
+    pub leave_call_button: Toggleable<IconButton>,
-    pub add_contact_button: IconButton,
+    pub add_contact_button: Toggleable<IconButton>,
-    pub add_channel_button: IconButton,
+    pub add_channel_button: Toggleable<IconButton>,
     pub header_row: ContainedText,
     pub subheader_row: Toggleable<Interactive<ContainedText>>,
     pub leave_call: Interactive<ContainedText>,
@@ -239,6 +240,7 @@ pub struct CollabPanel {
     pub contact_username: ContainedText,
     pub contact_button: Interactive<IconButton>,
     pub contact_button_spacing: f32,
+    pub channel_indent: f32,
     pub disabled_button: IconButton,
     pub section_icon_size: f32,
     pub calling_indicator: ContainedText,

styles/src/style_tree/collab_panel.ts

@@ -51,6 +51,20 @@ export default function contacts_panel(): any {
         },
     }
 
+    const headerButton = toggleable({
+        base: {
+            color: foreground(layer, "on"),
+            button_width: 28,
+            icon_width: 16,
+        },
+        state: {
+            active: {
+                background: background(layer, "active"),
+                corner_radius: 8,
+            }
+        }
+    })
+
     return {
         channel_modal: channel_modal(),
         background: background(layer),
@@ -77,23 +91,16 @@ export default function contacts_panel(): any {
                 right: side_padding,
             },
         },
+        channel_hash: {
+            color: foreground(layer, "on"),
+            width: 14,
+        },
         user_query_editor_height: 33,
-        add_contact_button: {
+        add_contact_button: headerButton,
-            color: foreground(layer, "on"),
+        add_channel_button: headerButton,
-            button_width: 28,
+        leave_call_button: headerButton,
-            icon_width: 16,
-        },
-        add_channel_button: {
-            color: foreground(layer, "on"),
-            button_width: 28,
-            icon_width: 16,
-        },
-        leave_call_button: {
-            color: foreground(layer, "on"),
-            button_width: 28,
-            icon_width: 16,
-        },
         row_height: 28,
+        channel_indent: 10,
         section_icon_size: 8,
         header_row: {
             ...text(layer, "mono", { size: "sm", weight: "bold" }),