Fix some drag and drop issues including the mouse cursor not being locked to pointer, tooltips being incorrect when a dragged tab is used, and some subscription leaks from panes

crates/drag_and_drop/src/drag_and_drop.rs

@@ -4,7 +4,7 @@ use gpui::{
     elements::{Container, MouseEventHandler},
     geometry::vector::Vector2F,
     scene::DragRegionEvent,
-    Element, ElementBox, EventContext, MouseButton, RenderContext, View, ViewContext,
+    CursorStyle, Element, ElementBox, EventContext, MouseButton, RenderContext, View, ViewContext,
     WeakViewHandle,
 };
 
@@ -33,7 +33,6 @@ pub struct DragAndDrop<V: View> {
 
 impl<V: View> DragAndDrop<V> {
     pub fn new(parent: WeakViewHandle<V>, cx: &mut ViewContext<V>) -> Self {
-        // TODO: Figure out if detaching here would result in a memory leak
         cx.observe_global::<Self, _>(|cx| {
             if let Some(parent) = cx.global::<Self>().parent.upgrade(cx) {
                 parent.update(cx, |_, cx| cx.notify())
@@ -110,6 +109,7 @@ impl<V: View> DragAndDrop<V> {
                         .left()
                         .boxed()
                 })
+                .with_cursor_style(CursorStyle::Arrow)
                 .on_up(MouseButton::Left, |_, cx| {
                     cx.defer(|cx| {
                         cx.update_global::<Self, _, _>(|this, _| this.currently_dragged.take());

crates/gpui/src/app.rs

@@ -5103,6 +5103,7 @@ impl<T: Entity> From<WeakModelHandle<T>> for AnyWeakModelHandle {
     }
 }
 
+#[derive(Debug)]
 pub struct WeakViewHandle<T> {
     window_id: usize,
     view_id: usize,

crates/gpui/src/presenter.rs

@@ -381,7 +381,12 @@ impl Presenter {
                         }
                     }
                     MouseRegionEvent::Click(e) => {
-                        if e.button == self.clicked_button.unwrap() {
+                        // Only raise click events if the released button is the same as the one stored
+                        if self
+                            .clicked_button
+                            .map(|clicked_button| clicked_button == e.button)
+                            .unwrap_or(false)
+                        {
                             // Clear clicked regions and clicked button
                             let clicked_regions =
                                 std::mem::replace(&mut self.clicked_regions, Vec::new());

crates/gpui/src/scene/mouse_region_event.rs

@@ -169,7 +169,7 @@ impl MouseRegionEvent {
         match self {
             MouseRegionEvent::Move(_) => true,
             MouseRegionEvent::Drag(_) => false,
-            MouseRegionEvent::Hover(_) => true,
+            MouseRegionEvent::Hover(_) => false,
             MouseRegionEvent::Down(_) => true,
             MouseRegionEvent::Up(_) => true,
             MouseRegionEvent::Click(_) => true,

crates/workspace/src/pane.rs

@@ -172,7 +172,7 @@ pub enum Event {
     Focused,
     ActivateItem { local: bool },
     Remove,
-    RemoveItem,
+    RemoveItem { item_id: usize },
     Split(SplitDirection),
     ChangeItemTitle,
 }
@@ -453,7 +453,7 @@ impl Pane {
         item
     }
 
-    pub fn add_item(
+    pub(crate) fn add_item(
         workspace: &mut Workspace,
         pane: &ViewHandle<Pane>,
         item: Box<dyn ItemHandle>,
@@ -475,6 +475,8 @@ impl Pane {
             )
         };
 
+        item.added_to_pane(workspace, pane.clone(), cx);
+
         // Does the item already exist?
         if let Some(existing_item_index) = pane.read(cx).items.iter().position(|existing_item| {
             let existing_item_entry_ids = existing_item.project_entry_ids(cx);
@@ -516,8 +518,6 @@ impl Pane {
                 pane.activate_item(insertion_index, activate_pane, focus_item, cx);
             });
         } else {
-            // If the item doesn't already exist, add it and activate it
-            item.added_to_pane(workspace, pane.clone(), cx);
             pane.update(cx, |pane, cx| {
                 cx.reparent(&item);
                 pane.items.insert(insertion_index, item);
@@ -762,7 +762,7 @@ impl Pane {
         }
 
         let item = self.items.remove(item_ix);
-        cx.emit(Event::RemoveItem);
+        cx.emit(Event::RemoveItem { item_id: item.id() });
         if self.items.is_empty() {
             item.deactivated(cx);
             self.update_toolbar(cx);
@@ -1069,7 +1069,7 @@ impl Pane {
                             let detail = detail.clone();
                             move |dragged_item, cx: &mut RenderContext<Workspace>| {
                                 let tab_style = &theme.workspace.tab_bar.dragged_tab;
-                                Pane::render_tab(
+                                Self::render_tab(
                                     &dragged_item.item,
                                     dragged_item.pane.clone(),
                                     detail,

crates/workspace/src/workspace.rs

@@ -535,97 +535,123 @@ impl<T: Item> ItemHandle for ViewHandle<T> {
             }
         }
 
-        let mut pending_autosave = None;
+        if workspace
-        let mut cancel_pending_autosave = oneshot::channel::<()>().0;
+            .panes_by_item
-        let pending_update = Rc::new(RefCell::new(None));
+            .insert(self.id(), pane.downgrade())
-        let pending_update_scheduled = Rc::new(AtomicBool::new(false));
+            .is_none()
-        let pane = pane.downgrade();
+        {
-        cx.subscribe(self, move |workspace, item, event, cx| {
+            let mut pending_autosave = None;
-            let pane = if let Some(pane) = pane.upgrade(cx) {
+            let mut cancel_pending_autosave = oneshot::channel::<()>().0;
-                pane
+            let pending_update = Rc::new(RefCell::new(None));
-            } else {
+            let pending_update_scheduled = Rc::new(AtomicBool::new(false));
-                log::error!("unexpected item event after pane was dropped");
-                return;
-            };
 
-            if let Some(item) = item.to_followable_item_handle(cx) {
+            let mut event_subscription =
-                let leader_id = workspace.leader_for_pane(&pane);
+                Some(cx.subscribe(self, move |workspace, item, event, cx| {
+                    let pane = if let Some(pane) = workspace
+                        .panes_by_item
+                        .get(&item.id())
+                        .and_then(|pane| pane.upgrade(cx))
+                    {
+                        pane
+                    } else {
+                        log::error!("unexpected item event after pane was dropped");
+                        return;
+                    };
 
-                if leader_id.is_some() && item.should_unfollow_on_event(event, cx) {
+                    if let Some(item) = item.to_followable_item_handle(cx) {
-                    workspace.unfollow(&pane, cx);
+                        let leader_id = workspace.leader_for_pane(&pane);
-                }
 
-                if item.add_event_to_update_proto(event, &mut *pending_update.borrow_mut(), cx)
+                        if leader_id.is_some() && item.should_unfollow_on_event(event, cx) {
-                    && !pending_update_scheduled.load(SeqCst)
+                            workspace.unfollow(&pane, cx);
-                {
-                    pending_update_scheduled.store(true, SeqCst);
-                    cx.after_window_update({
-                        let pending_update = pending_update.clone();
-                        let pending_update_scheduled = pending_update_scheduled.clone();
-                        move |this, cx| {
-                            pending_update_scheduled.store(false, SeqCst);
-                            this.update_followers(
-                                proto::update_followers::Variant::UpdateView(proto::UpdateView {
-                                    id: item.id() as u64,
-                                    variant: pending_update.borrow_mut().take(),
-                                    leader_id: leader_id.map(|id| id.0),
-                                }),
-                                cx,
-                            );
-                        }
-                    });
-                }
-            }
-
-            if T::should_close_item_on_event(event) {
-                Pane::close_item(workspace, pane, item.id(), cx).detach_and_log_err(cx);
-                return;
-            }
-
-            if T::should_update_tab_on_event(event) {
-                pane.update(cx, |_, cx| {
-                    cx.emit(pane::Event::ChangeItemTitle);
-                    cx.notify();
-                });
-            }
-
-            if T::is_edit_event(event) {
-                if let Autosave::AfterDelay { milliseconds } = cx.global::<Settings>().autosave {
-                    let prev_autosave = pending_autosave
-                        .take()
-                        .unwrap_or_else(|| Task::ready(Some(())));
-                    let (cancel_tx, mut cancel_rx) = oneshot::channel::<()>();
-                    let prev_cancel_tx = mem::replace(&mut cancel_pending_autosave, cancel_tx);
-                    let project = workspace.project.downgrade();
-                    let _ = prev_cancel_tx.send(());
-                    pending_autosave = Some(cx.spawn_weak(|_, mut cx| async move {
-                        let mut timer = cx
-                            .background()
-                            .timer(Duration::from_millis(milliseconds))
-                            .fuse();
-                        prev_autosave.await;
-                        futures::select_biased! {
-                            _ = cancel_rx => return None,
-                            _ = timer => {}
                         }
 
-                        let project = project.upgrade(&cx)?;
+                        if item.add_event_to_update_proto(
-                        cx.update(|cx| Pane::autosave_item(&item, project, cx))
+                            event,
-                            .await
+                            &mut *pending_update.borrow_mut(),
-                            .log_err();
+                            cx,
-                        None
+                        ) && !pending_update_scheduled.load(SeqCst)
-                    }));
+                        {
-                }
+                            pending_update_scheduled.store(true, SeqCst);
-            }
+                            cx.after_window_update({
-        })
+                                let pending_update = pending_update.clone();
-        .detach();
+                                let pending_update_scheduled = pending_update_scheduled.clone();
+                                move |this, cx| {
+                                    pending_update_scheduled.store(false, SeqCst);
+                                    this.update_followers(
+                                        proto::update_followers::Variant::UpdateView(
+                                            proto::UpdateView {
+                                                id: item.id() as u64,
+                                                variant: pending_update.borrow_mut().take(),
+                                                leader_id: leader_id.map(|id| id.0),
+                                            },
+                                        ),
+                                        cx,
+                                    );
+                                }
+                            });
+                        }
+                    }
 
-        cx.observe_focus(self, move |workspace, item, focused, cx| {
+                    if T::should_close_item_on_event(event) {
-            if !focused && cx.global::<Settings>().autosave == Autosave::OnFocusChange {
+                        Pane::close_item(workspace, pane, item.id(), cx).detach_and_log_err(cx);
-                Pane::autosave_item(&item, workspace.project.clone(), cx).detach_and_log_err(cx);
+                        return;
-            }
+                    }
-        })
+
-        .detach();
+                    if T::should_update_tab_on_event(event) {
+                        pane.update(cx, |_, cx| {
+                            cx.emit(pane::Event::ChangeItemTitle);
+                            cx.notify();
+                        });
+                    }
+
+                    if T::is_edit_event(event) {
+                        if let Autosave::AfterDelay { milliseconds } =
+                            cx.global::<Settings>().autosave
+                        {
+                            let prev_autosave = pending_autosave
+                                .take()
+                                .unwrap_or_else(|| Task::ready(Some(())));
+                            let (cancel_tx, mut cancel_rx) = oneshot::channel::<()>();
+                            let prev_cancel_tx =
+                                mem::replace(&mut cancel_pending_autosave, cancel_tx);
+                            let project = workspace.project.downgrade();
+                            let _ = prev_cancel_tx.send(());
+                            pending_autosave = Some(cx.spawn_weak(|_, mut cx| async move {
+                                let mut timer = cx
+                                    .background()
+                                    .timer(Duration::from_millis(milliseconds))
+                                    .fuse();
+                                prev_autosave.await;
+                                futures::select_biased! {
+                                    _ = cancel_rx => return None,
+                                    _ = timer => {}
+                                }
+
+                                let project = project.upgrade(&cx)?;
+                                cx.update(|cx| Pane::autosave_item(&item, project, cx))
+                                    .await
+                                    .log_err();
+                                None
+                            }));
+                        }
+                    }
+                }));
+
+            cx.observe_focus(self, move |workspace, item, focused, cx| {
+                if !focused && cx.global::<Settings>().autosave == Autosave::OnFocusChange {
+                    Pane::autosave_item(&item, workspace.project.clone(), cx)
+                        .detach_and_log_err(cx);
+                }
+            })
+            .detach();
+
+            let item_id = self.id();
+            cx.observe_release(self, move |workspace, _, _| {
+                workspace.panes_by_item.remove(&item_id);
+                event_subscription.take();
+            })
+            .detach();
+        }
     }
 
     fn deactivated(&self, cx: &mut MutableAppContext) {
@@ -799,6 +825,7 @@ pub struct Workspace {
     left_sidebar: ViewHandle<Sidebar>,
     right_sidebar: ViewHandle<Sidebar>,
     panes: Vec<ViewHandle<Pane>>,
+    panes_by_item: HashMap<usize, WeakViewHandle<Pane>>,
     active_pane: ViewHandle<Pane>,
     status_bar: ViewHandle<StatusBar>,
     notifications: Vec<(TypeId, usize, Box<dyn NotificationHandle>)>,
@@ -910,6 +937,7 @@ impl Workspace {
             weak_self,
             center: PaneGroup::new(pane.clone()),
             panes: vec![pane.clone()],
+            panes_by_item: Default::default(),
             active_pane: pane.clone(),
             status_bar,
             notifications: Default::default(),
@@ -1631,8 +1659,13 @@ impl Workspace {
                     }
                     self.update_window_edited(cx);
                 }
-                pane::Event::RemoveItem => {
+                pane::Event::RemoveItem { item_id } => {
                     self.update_window_edited(cx);
+                    if let hash_map::Entry::Occupied(entry) = self.panes_by_item.entry(*item_id) {
+                        if entry.get().id() == pane.id() {
+                            entry.remove();
+                        }
+                    }
                 }
             }
         } else {
@@ -1663,6 +1696,9 @@ impl Workspace {
             cx.focus(self.panes.last().unwrap().clone());
             self.unfollow(&pane, cx);
             self.last_leaders_by_pane.remove(&pane.downgrade());
+            for removed_item in pane.read(cx).items() {
+                self.panes_by_item.remove(&removed_item.id());
+            }
             cx.notify();
         } else {
             self.active_item_path_changed(cx);

styles/src/styleTree/tabBar.ts

@@ -72,7 +72,7 @@ export default function tabBar(theme: Theme) {
   return {
     height,
     background: backgroundColor(theme, 300),
-    dropTargetOverlayColor: withOpacity(theme.textColor.muted, 0.8),
+    dropTargetOverlayColor: withOpacity(theme.textColor.muted, 0.6),
     border: border(theme, "primary", {
       left: true,
       bottom: true,