Collab panel touch ups (#2855)

Mikayla Maki created

This will also fix the bug that @JosephTLyons observed where accepting a
channel invite would not show sub channels.

Release Notes:

- Offline section is now collapsed by default
- Manage members now shows full list
- Dragging of docks now follows the mouse exactly, and double clicks
reset size. (https://github.com/zed-industries/community/issues/1816)

Change summary

crates/ai/src/assistant.rs                 |   6 
crates/client/src/channel_store.rs         |   2 
crates/collab/src/db.rs                    |   6 
crates/collab/src/tests/channel_tests.rs   | 102 ++++++++++++++++
crates/collab_ui/src/collab_panel.rs       |   6 
crates/gpui/src/app.rs                     |   8 +
crates/gpui/src/elements.rs                |  17 ++
crates/gpui/src/elements/component.rs      |   5 
crates/gpui/src/elements/resizable.rs      | 147 +++++++++++++++++++----
crates/project_panel/src/project_panel.rs  |   4 
crates/terminal_view/src/terminal_panel.rs |   6 
crates/workspace/src/dock.rs               |  18 +-
crates/workspace/src/workspace.rs          |   7 
styles/src/style_tree/collab_modals.ts     |   3 
14 files changed, 282 insertions(+), 55 deletions(-)

Detailed changes

crates/ai/src/assistant.rs 🔗

@@ -726,10 +726,10 @@ impl Panel for AssistantPanel {
         }
     }
 
-    fn set_size(&mut self, size: f32, cx: &mut ViewContext<Self>) {
+    fn set_size(&mut self, size: Option<f32>, cx: &mut ViewContext<Self>) {
         match self.position(cx) {
-            DockPosition::Left | DockPosition::Right => self.width = Some(size),
-            DockPosition::Bottom => self.height = Some(size),
+            DockPosition::Left | DockPosition::Right => self.width = size,
+            DockPosition::Bottom => self.height = size,
         }
         cx.notify();
     }

crates/client/src/channel_store.rs 🔗

@@ -441,10 +441,12 @@ impl ChannelStore {
 
             for channel in payload.channels {
                 if let Some(existing_channel) = self.channels_by_id.get_mut(&channel.id) {
+                    // FIXME: We may be missing a path for this existing channel in certain cases
                     let existing_channel = Arc::make_mut(existing_channel);
                     existing_channel.name = channel.name;
                     continue;
                 }
+
                 self.channels_by_id.insert(
                     channel.id,
                     Arc::new(Channel {

crates/collab/src/db.rs 🔗

@@ -3650,7 +3650,11 @@ impl Database {
         let ancestor_ids = self.get_channel_ancestors(id, tx).await?;
         let user_ids = channel_member::Entity::find()
             .distinct()
-            .filter(channel_member::Column::ChannelId.is_in(ancestor_ids.iter().copied()))
+            .filter(
+                channel_member::Column::ChannelId
+                    .is_in(ancestor_ids.iter().copied())
+                    .and(channel_member::Column::Accepted.eq(true)),
+            )
             .select_only()
             .column(channel_member::Column::UserId)
             .into_values::<_, QueryUserIds>()

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

@@ -770,6 +770,108 @@ async fn test_call_from_channel(
     });
 }
 
+#[gpui::test]
+async fn test_lost_channel_creation(
+    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;
+
+    server
+        .make_contacts(&mut [(&client_a, cx_a), (&client_b, cx_b)])
+        .await;
+
+    let channel_id = server.make_channel("x", (&client_a, cx_a), &mut []).await;
+
+    // Invite a member
+    client_a
+        .channel_store()
+        .update(cx_a, |channel_store, cx| {
+            channel_store.invite_member(channel_id, client_b.user_id().unwrap(), false, cx)
+        })
+        .await
+        .unwrap();
+
+    deterministic.run_until_parked();
+
+    // Sanity check
+    assert_channel_invitations(
+        client_b.channel_store(),
+        cx_b,
+        &[ExpectedChannel {
+            depth: 0,
+            id: channel_id,
+            name: "x".to_string(),
+            user_is_admin: false,
+        }],
+    );
+
+    let subchannel_id = client_a
+        .channel_store()
+        .update(cx_a, |channel_store, cx| {
+            channel_store.create_channel("subchannel", Some(channel_id), cx)
+        })
+        .await
+        .unwrap();
+
+    deterministic.run_until_parked();
+
+    // Make sure A sees their new channel
+    assert_channels(
+        client_a.channel_store(),
+        cx_a,
+        &[
+            ExpectedChannel {
+                depth: 0,
+                id: channel_id,
+                name: "x".to_string(),
+                user_is_admin: true,
+            },
+            ExpectedChannel {
+                depth: 1,
+                id: subchannel_id,
+                name: "subchannel".to_string(),
+                user_is_admin: true,
+            },
+        ],
+    );
+
+    // Accept the invite
+    client_b
+        .channel_store()
+        .update(cx_b, |channel_store, _| {
+            channel_store.respond_to_channel_invite(channel_id, true)
+        })
+        .await
+        .unwrap();
+
+    deterministic.run_until_parked();
+
+    // B should now see the channel
+    assert_channels(
+        client_b.channel_store(),
+        cx_b,
+        &[
+            ExpectedChannel {
+                depth: 0,
+                id: channel_id,
+                name: "x".to_string(),
+                user_is_admin: false,
+            },
+            ExpectedChannel {
+                depth: 1,
+                id: subchannel_id,
+                name: "subchannel".to_string(),
+                user_is_admin: false,
+            },
+        ],
+    );
+}
+
 #[derive(Debug, PartialEq)]
 struct ExpectedChannel {
     depth: usize,

crates/collab_ui/src/collab_panel.rs 🔗

@@ -397,7 +397,7 @@ impl CollabPanel {
                 project: workspace.project().clone(),
                 subscriptions: Vec::default(),
                 match_candidates: Vec::default(),
-                collapsed_sections: Vec::default(),
+                collapsed_sections: vec![Section::Offline],
                 workspace: workspace.weak_handle(),
                 client: workspace.app_state().client.clone(),
                 context_menu_on_selected: true,
@@ -2391,8 +2391,8 @@ impl Panel for CollabPanel {
             .unwrap_or_else(|| settings::get::<CollaborationPanelSettings>(cx).default_width)
     }
 
-    fn set_size(&mut self, size: f32, cx: &mut ViewContext<Self>) {
-        self.width = Some(size);
+    fn set_size(&mut self, size: Option<f32>, cx: &mut ViewContext<Self>) {
+        self.width = size;
         self.serialize(cx);
         cx.notify();
     }

crates/gpui/src/app.rs 🔗

@@ -577,6 +577,14 @@ impl AppContext {
         }
     }
 
+    pub fn optional_global<T: 'static>(&self) -> Option<&T> {
+        if let Some(global) = self.globals.get(&TypeId::of::<T>()) {
+            Some(global.downcast_ref().unwrap())
+        } else {
+            None
+        }
+    }
+
     pub fn upgrade(&self) -> App {
         App(self.weak_self.as_ref().unwrap().upgrade().unwrap())
     }

crates/gpui/src/elements.rs 🔗

@@ -186,16 +186,27 @@ pub trait Element<V: View>: 'static {
         Tooltip::new::<Tag>(id, text, action, style, self.into_any(), cx)
     }
 
-    fn resizable(
+    /// Uses the the given element to calculate resizes for the given tag
+    fn provide_resize_bounds<Tag: 'static>(self) -> BoundsProvider<V, Tag>
+    where
+        Self: 'static + Sized,
+    {
+        BoundsProvider::<_, Tag>::new(self.into_any())
+    }
+
+    /// Calls the given closure with the new size of the element whenever the
+    /// handle is dragged. This will be calculated in relation to the bounds
+    /// provided by the given tag
+    fn resizable<Tag: 'static>(
         self,
         side: HandleSide,
         size: f32,
-        on_resize: impl 'static + FnMut(&mut V, f32, &mut ViewContext<V>),
+        on_resize: impl 'static + FnMut(&mut V, Option<f32>, &mut ViewContext<V>),
     ) -> Resizable<V>
     where
         Self: 'static + Sized,
     {
-        Resizable::new(self.into_any(), side, size, on_resize)
+        Resizable::new::<Tag>(self.into_any(), side, size, on_resize)
     }
 
     fn mouse<Tag: 'static>(self, region_id: usize) -> MouseEventHandler<V>

crates/gpui/src/elements/component.rs 🔗

@@ -82,6 +82,9 @@ impl<V: View, C: Component<V> + 'static> Element<V> for ComponentAdapter<V, C> {
         view: &V,
         cx: &ViewContext<V>,
     ) -> serde_json::Value {
-        element.debug(view, cx)
+        serde_json::json!({
+            "type": "ComponentAdapter",
+            "child": element.debug(view, cx),
+        })
     }
 }

crates/gpui/src/elements/resizable.rs 🔗

@@ -1,14 +1,14 @@
 use std::{cell::RefCell, rc::Rc};
 
+use collections::HashMap;
 use pathfinder_geometry::vector::{vec2f, Vector2F};
 use serde_json::json;
 
 use crate::{
     geometry::rect::RectF,
     platform::{CursorStyle, MouseButton},
-    scene::MouseDrag,
-    AnyElement, Axis, Element, LayoutContext, MouseRegion, PaintContext, SceneBuilder,
-    SizeConstraint, View, ViewContext,
+    AnyElement, AppContext, Axis, Element, LayoutContext, MouseRegion, PaintContext, SceneBuilder,
+    SizeConstraint, TypeTag, View, ViewContext,
 };
 
 #[derive(Copy, Clone, Debug)]
@@ -27,15 +27,6 @@ impl HandleSide {
         }
     }
 
-    /// 'before' is in reference to the standard english document ordering of left-to-right
-    /// then top-to-bottom
-    fn before_content(self) -> bool {
-        match self {
-            HandleSide::Left | HandleSide::Top => true,
-            HandleSide::Right | HandleSide::Bottom => false,
-        }
-    }
-
     fn relevant_component(&self, vector: Vector2F) -> f32 {
         match self.axis() {
             Axis::Horizontal => vector.x(),
@@ -43,14 +34,6 @@ impl HandleSide {
         }
     }
 
-    fn compute_delta(&self, e: MouseDrag) -> f32 {
-        if self.before_content() {
-            self.relevant_component(e.prev_mouse_position) - self.relevant_component(e.position)
-        } else {
-            self.relevant_component(e.position) - self.relevant_component(e.prev_mouse_position)
-        }
-    }
-
     fn of_rect(&self, bounds: RectF, handle_size: f32) -> RectF {
         match self {
             HandleSide::Top => RectF::new(bounds.origin(), vec2f(bounds.width(), handle_size)),
@@ -69,21 +52,29 @@ impl HandleSide {
     }
 }
 
+fn get_bounds(tag: TypeTag, cx: &AppContext) -> Option<&(RectF, RectF)>
+where
+{
+    cx.optional_global::<ProviderMap>()
+        .and_then(|map| map.0.get(&tag))
+}
+
 pub struct Resizable<V: View> {
     child: AnyElement<V>,
+    tag: TypeTag,
     handle_side: HandleSide,
     handle_size: f32,
-    on_resize: Rc<RefCell<dyn FnMut(&mut V, f32, &mut ViewContext<V>)>>,
+    on_resize: Rc<RefCell<dyn FnMut(&mut V, Option<f32>, &mut ViewContext<V>)>>,
 }
 
 const DEFAULT_HANDLE_SIZE: f32 = 4.0;
 
 impl<V: View> Resizable<V> {
-    pub fn new(
+    pub fn new<Tag: 'static>(
         child: AnyElement<V>,
         handle_side: HandleSide,
         size: f32,
-        on_resize: impl 'static + FnMut(&mut V, f32, &mut ViewContext<V>),
+        on_resize: impl 'static + FnMut(&mut V, Option<f32>, &mut ViewContext<V>),
     ) -> Self {
         let child = match handle_side.axis() {
             Axis::Horizontal => child.constrained().with_max_width(size),
@@ -94,6 +85,7 @@ impl<V: View> Resizable<V> {
         Self {
             child,
             handle_side,
+            tag: TypeTag::new::<Tag>(),
             handle_size: DEFAULT_HANDLE_SIZE,
             on_resize: Rc::new(RefCell::new(on_resize)),
         }
@@ -139,6 +131,14 @@ impl<V: View> Element<V> for Resizable<V> {
                 handle_region,
             )
             .on_down(MouseButton::Left, |_, _: &mut V, _| {}) // This prevents the mouse down event from being propagated elsewhere
+            .on_click(MouseButton::Left, {
+                let on_resize = self.on_resize.clone();
+                move |click, v, cx| {
+                    if click.click_count == 2 {
+                        on_resize.borrow_mut()(v, None, cx);
+                    }
+                }
+            })
             .on_drag(MouseButton::Left, {
                 let bounds = bounds.clone();
                 let side = self.handle_side;
@@ -146,16 +146,30 @@ impl<V: View> Element<V> for Resizable<V> {
                 let min_size = side.relevant_component(constraint.min);
                 let max_size = side.relevant_component(constraint.max);
                 let on_resize = self.on_resize.clone();
+                let tag = self.tag;
                 move |event, view: &mut V, cx| {
                     if event.end {
                         return;
                     }
-                    let new_size = min_size
-                        .max(prev_size + side.compute_delta(event))
-                        .min(max_size)
-                        .round();
+
+                    let Some((bounds, _)) = get_bounds(tag, cx) else {
+                        return;
+                    };
+
+                    let new_size_raw = match side {
+                        // Handle on top side of element => Element is on bottom
+                        HandleSide::Top => bounds.height() + bounds.origin_y() - event.position.y(),
+                        // Handle on right side of element => Element is on left
+                        HandleSide::Right => event.position.x() - bounds.lower_left().x(),
+                        // Handle on left side of element => Element is on the right
+                        HandleSide::Left => bounds.width() + bounds.origin_x() - event.position.x(),
+                        // Handle on bottom side of element => Element is on the top
+                        HandleSide::Bottom => event.position.y() - bounds.lower_left().y(),
+                    };
+
+                    let new_size = min_size.max(new_size_raw).min(max_size).round();
                     if new_size != prev_size {
-                        on_resize.borrow_mut()(view, new_size, cx);
+                        on_resize.borrow_mut()(view, Some(new_size), cx);
                     }
                 }
             }),
@@ -201,3 +215,80 @@ impl<V: View> Element<V> for Resizable<V> {
         })
     }
 }
+
+#[derive(Debug, Default)]
+struct ProviderMap(HashMap<TypeTag, (RectF, RectF)>);
+
+pub struct BoundsProvider<V: View, P> {
+    child: AnyElement<V>,
+    phantom: std::marker::PhantomData<P>,
+}
+
+impl<V: View, P: 'static> BoundsProvider<V, P> {
+    pub fn new(child: AnyElement<V>) -> Self {
+        Self {
+            child,
+            phantom: std::marker::PhantomData,
+        }
+    }
+}
+
+impl<V: View, P: 'static> Element<V> for BoundsProvider<V, P> {
+    type LayoutState = ();
+
+    type PaintState = ();
+
+    fn layout(
+        &mut self,
+        constraint: crate::SizeConstraint,
+        view: &mut V,
+        cx: &mut crate::LayoutContext<V>,
+    ) -> (pathfinder_geometry::vector::Vector2F, Self::LayoutState) {
+        (self.child.layout(constraint, view, cx), ())
+    }
+
+    fn paint(
+        &mut self,
+        scene: &mut crate::SceneBuilder,
+        bounds: pathfinder_geometry::rect::RectF,
+        visible_bounds: pathfinder_geometry::rect::RectF,
+        _: &mut Self::LayoutState,
+        view: &mut V,
+        cx: &mut crate::PaintContext<V>,
+    ) -> Self::PaintState {
+        cx.update_default_global::<ProviderMap, _, _>(|map, _| {
+            map.0.insert(TypeTag::new::<P>(), (bounds, visible_bounds));
+        });
+
+        self.child
+            .paint(scene, bounds.origin(), visible_bounds, view, cx)
+    }
+
+    fn rect_for_text_range(
+        &self,
+        range_utf16: std::ops::Range<usize>,
+        _: pathfinder_geometry::rect::RectF,
+        _: pathfinder_geometry::rect::RectF,
+        _: &Self::LayoutState,
+        _: &Self::PaintState,
+        view: &V,
+        cx: &crate::ViewContext<V>,
+    ) -> Option<pathfinder_geometry::rect::RectF> {
+        self.child.rect_for_text_range(range_utf16, view, cx)
+    }
+
+    fn debug(
+        &self,
+        _: pathfinder_geometry::rect::RectF,
+        _: &Self::LayoutState,
+        _: &Self::PaintState,
+        view: &V,
+        cx: &crate::ViewContext<V>,
+    ) -> serde_json::Value {
+        serde_json::json!({
+            "type": "Provider",
+            "providing": format!("{:?}", TypeTag::new::<P>()),
+            "child": self.child.debug(view, cx),
+        })
+    }
+}

crates/project_panel/src/project_panel.rs 🔗

@@ -1651,8 +1651,8 @@ impl workspace::dock::Panel for ProjectPanel {
             .unwrap_or_else(|| settings::get::<ProjectPanelSettings>(cx).default_width)
     }
 
-    fn set_size(&mut self, size: f32, cx: &mut ViewContext<Self>) {
-        self.width = Some(size);
+    fn set_size(&mut self, size: Option<f32>, cx: &mut ViewContext<Self>) {
+        self.width = size;
         self.serialize(cx);
         cx.notify();
     }

crates/terminal_view/src/terminal_panel.rs 🔗

@@ -362,10 +362,10 @@ impl Panel for TerminalPanel {
         }
     }
 
-    fn set_size(&mut self, size: f32, cx: &mut ViewContext<Self>) {
+    fn set_size(&mut self, size: Option<f32>, cx: &mut ViewContext<Self>) {
         match self.position(cx) {
-            DockPosition::Left | DockPosition::Right => self.width = Some(size),
-            DockPosition::Bottom => self.height = Some(size),
+            DockPosition::Left | DockPosition::Right => self.width = size,
+            DockPosition::Bottom => self.height = size,
         }
         self.serialize(cx);
         cx.notify();

crates/workspace/src/dock.rs 🔗

@@ -1,4 +1,4 @@
-use crate::{StatusItemView, Workspace};
+use crate::{StatusItemView, Workspace, WorkspaceBounds};
 use context_menu::{ContextMenu, ContextMenuItem};
 use gpui::{
     elements::*, platform::CursorStyle, platform::MouseButton, Action, AnyViewHandle, AppContext,
@@ -13,7 +13,7 @@ pub trait Panel: View {
     fn position_is_valid(&self, position: DockPosition) -> bool;
     fn set_position(&mut self, position: DockPosition, cx: &mut ViewContext<Self>);
     fn size(&self, cx: &WindowContext) -> f32;
-    fn set_size(&mut self, size: f32, cx: &mut ViewContext<Self>);
+    fn set_size(&mut self, size: Option<f32>, cx: &mut ViewContext<Self>);
     fn icon_path(&self, cx: &WindowContext) -> Option<&'static str>;
     fn icon_tooltip(&self) -> (String, Option<Box<dyn Action>>);
     fn icon_label(&self, _: &WindowContext) -> Option<String> {
@@ -50,7 +50,7 @@ pub trait PanelHandle {
     fn set_zoomed(&self, zoomed: bool, cx: &mut WindowContext);
     fn set_active(&self, active: bool, cx: &mut WindowContext);
     fn size(&self, cx: &WindowContext) -> f32;
-    fn set_size(&self, size: f32, cx: &mut WindowContext);
+    fn set_size(&self, size: Option<f32>, cx: &mut WindowContext);
     fn icon_path(&self, cx: &WindowContext) -> Option<&'static str>;
     fn icon_tooltip(&self, cx: &WindowContext) -> (String, Option<Box<dyn Action>>);
     fn icon_label(&self, cx: &WindowContext) -> Option<String>;
@@ -82,7 +82,7 @@ where
         self.read(cx).size(cx)
     }
 
-    fn set_size(&self, size: f32, cx: &mut WindowContext) {
+    fn set_size(&self, size: Option<f32>, cx: &mut WindowContext) {
         self.update(cx, |this, cx| this.set_size(size, cx))
     }
 
@@ -373,7 +373,7 @@ impl Dock {
         }
     }
 
-    pub fn resize_active_panel(&mut self, size: f32, cx: &mut ViewContext<Self>) {
+    pub fn resize_active_panel(&mut self, size: Option<f32>, cx: &mut ViewContext<Self>) {
         if let Some(entry) = self.panel_entries.get_mut(self.active_panel_index) {
             entry.panel.set_size(size, cx);
             cx.notify();
@@ -386,7 +386,7 @@ impl Dock {
                 .into_any()
                 .contained()
                 .with_style(self.style(cx))
-                .resizable(
+                .resizable::<WorkspaceBounds>(
                     self.position.to_resize_handle_side(),
                     active_entry.panel.size(cx),
                     |_, _, _| {},
@@ -423,7 +423,7 @@ impl View for Dock {
             ChildView::new(active_entry.panel.as_any(), cx)
                 .contained()
                 .with_style(style)
-                .resizable(
+                .resizable::<WorkspaceBounds>(
                     self.position.to_resize_handle_side(),
                     active_entry.panel.size(cx),
                     |dock: &mut Self, size, cx| dock.resize_active_panel(size, cx),
@@ -700,8 +700,8 @@ pub mod test {
             self.size
         }
 
-        fn set_size(&mut self, size: f32, _: &mut ViewContext<Self>) {
-            self.size = size;
+        fn set_size(&mut self, size: Option<f32>, _: &mut ViewContext<Self>) {
+            self.size = size.unwrap_or(300.);
         }
 
         fn icon_path(&self, _: &WindowContext) -> Option<&'static str> {

crates/workspace/src/workspace.rs 🔗

@@ -553,6 +553,8 @@ struct FollowerState {
     items_by_leader_view_id: HashMap<ViewId, Box<dyn FollowableItemHandle>>,
 }
 
+enum WorkspaceBounds {}
+
 impl Workspace {
     pub fn new(
         workspace_id: WorkspaceId,
@@ -3776,6 +3778,7 @@ impl View for Workspace {
                                     }))
                                     .with_children(self.render_notifications(&theme.workspace, cx)),
                             ))
+                            .provide_resize_bounds::<WorkspaceBounds>()
                             .flex(1.0, true),
                     )
                     .with_child(ChildView::new(&self.status_bar, cx))
@@ -4859,7 +4862,9 @@ mod tests {
                 panel_1.size(cx)
             );
 
-            left_dock.update(cx, |left_dock, cx| left_dock.resize_active_panel(1337., cx));
+            left_dock.update(cx, |left_dock, cx| {
+                left_dock.resize_active_panel(Some(1337.), cx)
+            });
             assert_eq!(
                 workspace
                     .right_dock()

styles/src/style_tree/collab_modals.ts 🔗

@@ -76,7 +76,8 @@ export default function channel_modal(): any {
                 },
 
             },
-            max_height: 400,
+            // FIXME: due to a bug in the picker's size calculation, this must be 600
+            max_height: 600,
             max_width: 540,
             title: {
                 ...text(theme.middle, "sans", "on", { size: "lg" }),