Drag and drop tabs working. all known bugs fixed

K Simmons created

Change summary

crates/drag_and_drop/src/drag_and_drop.rs       |  2 
crates/gpui/src/elements/container.rs           | 20 ++++
crates/gpui/src/elements/mouse_event_handler.rs | 22 +++-
crates/gpui/src/presenter.rs                    | 55 +++++++----
crates/gpui/src/scene/mouse_region.rs           |  8 +
crates/theme/src/theme.rs                       |  1 
crates/workspace/src/pane.rs                    | 81 ++++++++++++++----
styles/src/styleTree/tabBar.ts                  |  1 
8 files changed, 143 insertions(+), 47 deletions(-)

Detailed changes

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

@@ -24,6 +24,8 @@ pub struct ContainerStyle {
     pub padding: Padding,
     #[serde(rename = "background")]
     pub background_color: Option<Color>,
+    #[serde(rename = "overlay")]
+    pub overlay_color: Option<Color>,
     #[serde(default)]
     pub border: Border,
     #[serde(default)]
@@ -119,6 +121,11 @@ impl Container {
         self
     }
 
+    pub fn with_overlay_color(mut self, color: Color) -> Self {
+        self.style.overlay_color = Some(color);
+        self
+    }
+
     pub fn with_border(mut self, border: Border) -> Self {
         self.style.border = border;
         self
@@ -245,7 +252,7 @@ impl Element for Container {
             cx.scene.push_layer(None);
             cx.scene.push_quad(Quad {
                 bounds: quad_bounds,
-                background: Default::default(),
+                background: self.style.overlay_color,
                 border: self.style.border,
                 corner_radius: self.style.corner_radius,
             });
@@ -264,6 +271,17 @@ impl Element for Container {
                     self.style.border.top_width(),
                 );
             self.child.paint(child_origin, visible_bounds, cx);
+
+            if self.style.overlay_color.is_some() {
+                cx.scene.push_layer(None);
+                cx.scene.push_quad(Quad {
+                    bounds: quad_bounds,
+                    background: self.style.overlay_color,
+                    border: Default::default(),
+                    corner_radius: 0.,
+                });
+                cx.scene.pop_layer();
+            }
         }
     }
 

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

@@ -20,6 +20,7 @@ pub struct MouseEventHandler {
     discriminant: (TypeId, usize),
     cursor_style: Option<CursorStyle>,
     handlers: HandlerSet,
+    hoverable: bool,
     padding: Padding,
 }
 
@@ -35,6 +36,7 @@ impl MouseEventHandler {
             cursor_style: None,
             discriminant: (TypeId::of::<Tag>(), id),
             handlers: Default::default(),
+            hoverable: true,
             padding: Default::default(),
         }
     }
@@ -119,6 +121,11 @@ impl MouseEventHandler {
         self
     }
 
+    pub fn with_hoverable(mut self, is_hoverable: bool) -> Self {
+        self.hoverable = is_hoverable;
+        self
+    }
+
     pub fn with_padding(mut self, padding: Padding) -> Self {
         self.padding = padding;
         self
@@ -160,12 +167,15 @@ impl Element for MouseEventHandler {
             });
         }
 
-        cx.scene.push_mouse_region(MouseRegion::from_handlers(
-            cx.current_view_id(),
-            Some(self.discriminant),
-            hit_bounds,
-            self.handlers.clone(),
-        ));
+        cx.scene.push_mouse_region(
+            MouseRegion::from_handlers(
+                cx.current_view_id(),
+                Some(self.discriminant),
+                hit_bounds,
+                self.handlers.clone(),
+            )
+            .with_hoverable(self.hoverable),
+        );
 
         self.child.paint(bounds.origin(), visible_bounds, cx);
     }

crates/gpui/src/presenter.rs 🔗

@@ -245,17 +245,21 @@ impl Presenter {
                     // MDN says that browsers handle this by starting from 'the most
                     // specific ancestor element that contained both [positions]'
                     // So we need to store the overlapping regions on mouse down.
-                    self.clicked_regions = self
-                        .mouse_regions
-                        .iter()
-                        .filter_map(|(region, _)| {
-                            region
-                                .bounds
-                                .contains_point(e.position)
-                                .then(|| region.clone())
-                        })
-                        .collect();
-                    self.clicked_button = Some(e.button);
+
+                    // If there is already clicked_button stored, don't replace it.
+                    if self.clicked_button.is_none() {
+                        self.clicked_regions = self
+                            .mouse_regions
+                            .iter()
+                            .filter_map(|(region, _)| {
+                                region
+                                    .bounds
+                                    .contains_point(e.position)
+                                    .then(|| region.clone())
+                            })
+                            .collect();
+                        self.clicked_button = Some(e.button);
+                    }
 
                     events_to_send.push(MouseRegionEvent::Down(DownRegionEvent {
                         region: Default::default(),
@@ -337,13 +341,18 @@ impl Presenter {
 
                 // GPUI elements are arranged by depth but sibling elements can register overlapping
                 // mouse regions. As such, hover events are only fired on overlapping elements which
-                // are at the same depth as the deepest element which overlaps with the mouse.
+                // are at the same depth as the topmost element which overlaps with the mouse.
 
                 match &region_event {
                     MouseRegionEvent::Hover(_) => {
                         let mut top_most_depth = None;
                         let mouse_position = self.mouse_position.clone();
                         for (region, depth) in self.mouse_regions.iter().rev() {
+                            // Allow mouse regions to appear transparent to hovers
+                            if !region.hoverable {
+                                continue;
+                            }
+
                             let contains_mouse = region.bounds.contains_point(mouse_position);
 
                             if contains_mouse && top_most_depth.is_none() {
@@ -359,26 +368,30 @@ impl Presenter {
                                     //Ensure that hover entrance events aren't sent twice
                                     if self.hovered_region_ids.insert(region_id) {
                                         valid_regions.push(region.clone());
+                                        invalidated_views.insert(region.view_id);
                                     }
                                 } else {
                                     // Ensure that hover exit events aren't sent twice
                                     if self.hovered_region_ids.remove(&region_id) {
                                         valid_regions.push(region.clone());
+                                        invalidated_views.insert(region.view_id);
                                     }
                                 }
                             }
                         }
                     }
                     MouseRegionEvent::Click(e) => {
-                        // Clear presenter state
-                        let clicked_regions =
-                            std::mem::replace(&mut self.clicked_regions, Vec::new());
-                        self.clicked_button = None;
-
-                        // Find regions which still overlap with the mouse since the last MouseDown happened
-                        for clicked_region in clicked_regions.into_iter().rev() {
-                            if clicked_region.bounds.contains_point(e.position) {
-                                valid_regions.push(clicked_region);
+                        if e.button == self.clicked_button.unwrap() {
+                            // Clear clicked regions and clicked button
+                            let clicked_regions =
+                                std::mem::replace(&mut self.clicked_regions, Vec::new());
+                            self.clicked_button = None;
+
+                            // Find regions which still overlap with the mouse since the last MouseDown happened
+                            for clicked_region in clicked_regions.into_iter().rev() {
+                                if clicked_region.bounds.contains_point(e.position) {
+                                    valid_regions.push(clicked_region);
+                                }
                             }
                         }
                     }

crates/gpui/src/scene/mouse_region.rs 🔗

@@ -17,6 +17,7 @@ pub struct MouseRegion {
     pub discriminant: Option<(TypeId, usize)>,
     pub bounds: RectF,
     pub handlers: HandlerSet,
+    pub hoverable: bool,
 }
 
 impl MouseRegion {
@@ -35,6 +36,7 @@ impl MouseRegion {
             discriminant,
             bounds,
             handlers,
+            hoverable: true,
         }
     }
 
@@ -48,6 +50,7 @@ impl MouseRegion {
             discriminant,
             bounds,
             handlers: HandlerSet::capture_all(),
+            hoverable: true,
         }
     }
 
@@ -120,6 +123,11 @@ impl MouseRegion {
         self.handlers = self.handlers.on_move(handler);
         self
     }
+
+    pub fn with_hoverable(mut self, is_hoverable: bool) -> Self {
+        self.hoverable = is_hoverable;
+        self
+    }
 }
 
 #[derive(Copy, Clone, Eq, PartialEq, Hash, Debug)]

crates/theme/src/theme.rs 🔗

@@ -77,6 +77,7 @@ pub struct TabBar {
     pub inactive_pane: TabStyles,
     pub dragged_tab: Tab,
     pub height: f32,
+    pub drop_target_overlay_color: Color,
 }
 
 impl TabBar {

crates/workspace/src/pane.rs 🔗

@@ -7,6 +7,7 @@ use drag_and_drop::{DragAndDrop, Draggable};
 use futures::StreamExt;
 use gpui::{
     actions,
+    color::Color,
     elements::*,
     geometry::{
         rect::RectF,
@@ -22,6 +23,7 @@ use project::{Project, ProjectEntryId, ProjectPath};
 use serde::Deserialize;
 use settings::{Autosave, Settings};
 use std::{any::Any, cell::RefCell, cmp, mem, path::Path, rc::Rc};
+use theme::Theme;
 use util::ResultExt;
 
 #[derive(Clone, Deserialize, PartialEq)]
@@ -475,15 +477,14 @@ impl Pane {
             // If the item already exists, move it to the desired destination and activate it
             pane.update(cx, |pane, cx| {
                 if existing_item_index != destination_index {
+                    cx.reparent(&item);
                     let existing_item_is_active = existing_item_index == pane.active_item_index;
 
                     pane.items.remove(existing_item_index);
-                    if existing_item_index < destination_index {
-                        destination_index -= 1;
-                    }
                     if existing_item_index < pane.active_item_index {
                         pane.active_item_index -= 1;
                     }
+                    destination_index = destination_index.min(pane.items.len());
 
                     pane.items.insert(destination_index, item.clone());
 
@@ -985,8 +986,9 @@ impl Pane {
 
         enum Tabs {}
         enum Tab {}
+        enum Filler {}
         let pane = cx.handle();
-        MouseEventHandler::new::<Tabs, _, _>(0, cx, |mouse_state, cx| {
+        MouseEventHandler::new::<Tabs, _, _>(0, cx, |_, cx| {
             let autoscroll = if mem::take(&mut self.autoscroll) {
                 Some(self.active_item_index)
             } else {
@@ -1011,14 +1013,22 @@ impl Pane {
                         let item = item.clone();
                         let pane = pane.clone();
                         let detail = detail.clone();
-                        let hovered = mouse_state.hovered;
 
                         let theme = cx.global::<Settings>().theme.clone();
 
-                        move |_, cx| {
+                        move |mouse_state, cx| {
                             let tab_style =
                                 theme.workspace.tab_bar.tab_style(pane_active, tab_active);
-                            Self::render_tab(&item, pane, detail, hovered, tab_style, cx)
+                            let hovered = mouse_state.hovered;
+                            Self::render_tab(
+                                &item,
+                                pane,
+                                detail,
+                                hovered,
+                                Self::tab_overlay_color(hovered, theme.as_ref(), cx),
+                                tab_style,
+                                cx,
+                            )
                         }
                     })
                     .with_cursor_style(if pane_active && tab_active {
@@ -1059,6 +1069,7 @@ impl Pane {
                                     dragged_item.pane.clone(),
                                     detail,
                                     false,
+                                    None,
                                     &tab_style,
                                     cx,
                                 )
@@ -1069,14 +1080,25 @@ impl Pane {
                 })
             }
 
+            // Use the inactive tab style along with the current pane's active status to decide how to render
+            // the filler
             let filler_style = theme.workspace.tab_bar.tab_style(pane_active, false);
             row.add_child(
-                Empty::new()
-                    .contained()
-                    .with_style(filler_style.container)
-                    .with_border(filler_style.container.border)
-                    .flex(1., true)
-                    .named("filler"),
+                MouseEventHandler::new::<Filler, _, _>(0, cx, |mouse_state, cx| {
+                    let mut filler = Empty::new()
+                        .contained()
+                        .with_style(filler_style.container)
+                        .with_border(filler_style.container.border);
+
+                    if let Some(overlay) = Self::tab_overlay_color(mouse_state.hovered, &theme, cx)
+                    {
+                        filler = filler.with_overlay_color(overlay);
+                    }
+
+                    filler.boxed()
+                })
+                .flex(1., true)
+                .named("filler"),
             );
 
             row.boxed()
@@ -1128,12 +1150,13 @@ impl Pane {
         pane: WeakViewHandle<Pane>,
         detail: Option<usize>,
         hovered: bool,
+        overlay: Option<Color>,
         tab_style: &theme::Tab,
         cx: &mut RenderContext<V>,
     ) -> ElementBox {
         let title = item.tab_content(detail, &tab_style, cx);
 
-        Flex::row()
+        let mut tab = Flex::row()
             .with_child(
                 Align::new({
                     let diameter = 7.0;
@@ -1216,10 +1239,13 @@ impl Pane {
                 .boxed(),
             )
             .contained()
-            .with_style(tab_style.container)
-            .constrained()
-            .with_height(tab_style.height)
-            .boxed()
+            .with_style(tab_style.container);
+
+        if let Some(overlay) = overlay {
+            tab = tab.with_overlay_color(overlay);
+        }
+
+        tab.constrained().with_height(tab_style.height).boxed()
     }
 
     fn handle_dropped_item(pane: &WeakViewHandle<Pane>, index: usize, cx: &mut EventContext) {
@@ -1237,6 +1263,23 @@ impl Pane {
             cx.propogate_event();
         }
     }
+
+    fn tab_overlay_color(
+        hovered: bool,
+        theme: &Theme,
+        cx: &mut RenderContext<Self>,
+    ) -> Option<Color> {
+        if hovered
+            && cx
+                .global::<DragAndDrop<Workspace>>()
+                .currently_dragged::<DraggedItem>()
+                .is_some()
+        {
+            Some(theme.workspace.tab_bar.drop_target_overlay_color)
+        } else {
+            None
+        }
+    }
 }
 
 impl Entity for Pane {
@@ -1327,7 +1370,7 @@ impl View for Pane {
                             tab_row
                                 .constrained()
                                 .with_height(cx.global::<Settings>().theme.workspace.tab_bar.height)
-                                .boxed()
+                                .named("tab bar")
                         })
                         .with_child(ChildView::new(&self.toolbar).boxed())
                         .with_child(ChildView::new(active_item).flex(1., true).boxed())

styles/src/styleTree/tabBar.ts 🔗

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