Fix bug in channel rendering

Mikayla and Max created

Fix drag and drop stale state bug revealed by the channel panel

co-authored-by: Max <max@zed.dev>

Change summary

crates/channel/src/channel_store.rs       | 20 -----
crates/collab_ui/src/collab_panel.rs      | 84 +++++++++++-------------
crates/drag_and_drop/src/drag_and_drop.rs | 12 ++
3 files changed, 51 insertions(+), 65 deletions(-)

Detailed changes

crates/channel/src/channel_store.rs 🔗

@@ -3,10 +3,7 @@ mod channel_index;
 use crate::{channel_buffer::ChannelBuffer, channel_chat::ChannelChat};
 use anyhow::{anyhow, Result};
 use client::{Client, Subscription, User, UserId, UserStore};
-use collections::{
-    hash_map::{self, DefaultHasher},
-    HashMap, HashSet,
-};
+use collections::{hash_map, HashMap, HashSet};
 use futures::{channel::mpsc, future::Shared, Future, FutureExt, StreamExt};
 use gpui::{AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, Task, WeakModelHandle};
 use rpc::{
@@ -14,14 +11,7 @@ use rpc::{
     TypedEnvelope,
 };
 use serde_derive::{Deserialize, Serialize};
-use std::{
-    borrow::Cow,
-    hash::{Hash, Hasher},
-    mem,
-    ops::Deref,
-    sync::Arc,
-    time::Duration,
-};
+use std::{borrow::Cow, hash::Hash, mem, ops::Deref, sync::Arc, time::Duration};
 use util::ResultExt;
 
 use self::channel_index::ChannelIndex;
@@ -910,12 +900,6 @@ impl ChannelPath {
     pub fn channel_id(&self) -> ChannelId {
         self.0[self.0.len() - 1]
     }
-
-    pub fn unique_id(&self) -> u64 {
-        let mut hasher = DefaultHasher::new();
-        self.0.deref().hash(&mut hasher);
-        hasher.finish()
-    }
 }
 
 impl From<ChannelPath> for Cow<'static, ChannelPath> {

crates/collab_ui/src/collab_panel.rs 🔗

@@ -1787,7 +1787,7 @@ impl CollabPanel {
             is_dragged_over = true;
         }
 
-        MouseEventHandler::new::<Channel, _>(path.unique_id() as usize, cx, |state, cx| {
+        MouseEventHandler::new::<Channel, _>(ix, cx, |state, cx| {
             let row_hovered = state.hovered();
 
             let mut select_state = |interactive: &Interactive<ContainerStyle>| {
@@ -1822,47 +1822,43 @@ impl CollabPanel {
                         .flex(1., true),
                 )
                 .with_child(
-                    MouseEventHandler::new::<ChannelCall, _>(
-                        channel.id as usize,
-                        cx,
-                        move |_, cx| {
-                            let participants =
-                                self.channel_store.read(cx).channel_participants(channel_id);
-                            if !participants.is_empty() {
-                                let extra_count = participants.len().saturating_sub(FACEPILE_LIMIT);
-
-                                FacePile::new(theme.face_overlap)
-                                    .with_children(
-                                        participants
-                                            .iter()
-                                            .filter_map(|user| {
-                                                Some(
-                                                    Image::from_data(user.avatar.clone()?)
-                                                        .with_style(theme.channel_avatar),
-                                                )
-                                            })
-                                            .take(FACEPILE_LIMIT),
+                    MouseEventHandler::new::<ChannelCall, _>(ix, cx, move |_, cx| {
+                        let participants =
+                            self.channel_store.read(cx).channel_participants(channel_id);
+                        if !participants.is_empty() {
+                            let extra_count = participants.len().saturating_sub(FACEPILE_LIMIT);
+
+                            FacePile::new(theme.face_overlap)
+                                .with_children(
+                                    participants
+                                        .iter()
+                                        .filter_map(|user| {
+                                            Some(
+                                                Image::from_data(user.avatar.clone()?)
+                                                    .with_style(theme.channel_avatar),
+                                            )
+                                        })
+                                        .take(FACEPILE_LIMIT),
+                                )
+                                .with_children((extra_count > 0).then(|| {
+                                    Label::new(
+                                        format!("+{}", extra_count),
+                                        theme.extra_participant_label.text.clone(),
                                     )
-                                    .with_children((extra_count > 0).then(|| {
-                                        Label::new(
-                                            format!("+{}", extra_count),
-                                            theme.extra_participant_label.text.clone(),
-                                        )
-                                        .contained()
-                                        .with_style(theme.extra_participant_label.container)
-                                    }))
-                                    .into_any()
-                            } else if row_hovered {
-                                Svg::new("icons/speaker-loud.svg")
-                                    .with_color(theme.channel_hash.color)
-                                    .constrained()
-                                    .with_width(theme.channel_hash.width)
-                                    .into_any()
-                            } else {
-                                Empty::new().into_any()
-                            }
-                        },
-                    )
+                                    .contained()
+                                    .with_style(theme.extra_participant_label.container)
+                                }))
+                                .into_any()
+                        } else if row_hovered {
+                            Svg::new("icons/speaker-loud.svg")
+                                .with_color(theme.channel_hash.color)
+                                .constrained()
+                                .with_width(theme.channel_hash.width)
+                                .into_any()
+                        } else {
+                            Empty::new().into_any()
+                        }
+                    })
                     .on_click(MouseButton::Left, move |_, this, cx| {
                         this.join_channel_call(channel_id, cx);
                     }),
@@ -1875,7 +1871,7 @@ impl CollabPanel {
                         location: path.clone(),
                     }),
                 )
-                .with_id(path.unique_id() as usize)
+                .with_id(ix)
                 .with_style(theme.disclosure.clone())
                 .element()
                 .constrained()
@@ -1955,11 +1951,11 @@ impl CollabPanel {
         })
         .as_draggable(
             (channel.clone(), path.parent_id()),
-            move |e, (channel, _), cx: &mut ViewContext<Workspace>| {
+            move |modifiers, (channel, _), cx: &mut ViewContext<Workspace>| {
                 let theme = &theme::current(cx).collab_panel;
 
                 Flex::<Workspace>::row()
-                    .with_children(e.alt.then(|| {
+                    .with_children(modifiers.alt.then(|| {
                         Svg::new("icons/plus.svg")
                             .with_color(theme.channel_hash.color)
                             .constrained()

crates/drag_and_drop/src/drag_and_drop.rs 🔗

@@ -364,9 +364,15 @@ impl<V: 'static> Draggable<V> for MouseEventHandler<V> {
             DragAndDrop::<D>::drag_started(e, cx);
         })
         .on_drag(MouseButton::Left, move |e, _, cx| {
-            let payload = payload.clone();
-            let render = render.clone();
-            DragAndDrop::<D>::dragging(e, payload, cx, render)
+            if e.end {
+                cx.update_global::<DragAndDrop<D>, _, _>(|drag_and_drop, cx| {
+                    drag_and_drop.finish_dragging(cx)
+                })
+            } else {
+                let payload = payload.clone();
+                let render = render.clone();
+                DragAndDrop::<D>::dragging(e, payload, cx, render)
+            }
         })
     }
 }