Merge pull request #1396 from zed-industries/better-empty-pane

Keith Simmons created

Fix Pane Focus Issues

Change summary

crates/context_menu/src/context_menu.rs         |   4 
crates/editor/src/editor.rs                     |   6 
crates/editor/src/link_go_to_definition.rs      |  11 +
crates/editor/src/mouse_context_menu.rs         |   7 +
crates/gpui/src/elements/mouse_event_handler.rs |   4 
crates/picker/src/picker.rs                     |   2 
crates/project_panel/src/project_panel.rs       |   4 
crates/search/src/project_search.rs             |  23 +++-
crates/theme/src/theme.rs                       |   6 
crates/workspace/src/drag_and_drop.rs           |   0 
crates/workspace/src/pane.rs                    | 100 +++++++++++++-----
crates/workspace/src/sidebar.rs                 |   2 
crates/workspace/src/workspace.rs               |  31 +++--
styles/src/styleTree/workspace.ts               |  29 +++-
14 files changed, 156 insertions(+), 73 deletions(-)

Detailed changes

crates/context_menu/src/context_menu.rs 🔗

@@ -355,7 +355,7 @@ impl ContextMenu {
                 .with_style(style.container)
                 .boxed()
         })
-        .on_mouse_down_out(MouseButton::Left, |_, cx| cx.dispatch_action(Cancel))
-        .on_mouse_down_out(MouseButton::Right, |_, cx| cx.dispatch_action(Cancel))
+        .on_down_out(MouseButton::Left, |_, cx| cx.dispatch_action(Cancel))
+        .on_down_out(MouseButton::Right, |_, cx| cx.dispatch_action(Cancel))
     }
 }

crates/editor/src/editor.rs 🔗

@@ -707,7 +707,7 @@ impl CompletionsMenu {
                             },
                         )
                         .with_cursor_style(CursorStyle::PointingHand)
-                        .on_mouse_down(MouseButton::Left, move |_, cx| {
+                        .on_down(MouseButton::Left, move |_, cx| {
                             cx.dispatch_action(ConfirmCompletion {
                                 item_ix: Some(item_ix),
                             });
@@ -840,7 +840,7 @@ impl CodeActionsMenu {
                                 .boxed()
                         })
                         .with_cursor_style(CursorStyle::PointingHand)
-                        .on_mouse_down(MouseButton::Left, move |_, cx| {
+                        .on_down(MouseButton::Left, move |_, cx| {
                             cx.dispatch_action(ConfirmCodeAction {
                                 item_ix: Some(item_ix),
                             });
@@ -2674,7 +2674,7 @@ impl Editor {
                 })
                 .with_cursor_style(CursorStyle::PointingHand)
                 .with_padding(Padding::uniform(3.))
-                .on_mouse_down(MouseButton::Left, |_, cx| {
+                .on_down(MouseButton::Left, |_, cx| {
                     cx.dispatch_action(ToggleCodeActions {
                         deployed_from_indicator: true,
                     });
@@ -7,7 +7,9 @@ use settings::Settings;
 use util::TryFutureExt;
 use workspace::Workspace;
 
-use crate::{Anchor, DisplayPoint, Editor, EditorSnapshot, GoToDefinition, Select, SelectPhase};
+use crate::{
+    Anchor, DisplayPoint, Editor, EditorSnapshot, Event, GoToDefinition, Select, SelectPhase,
+};
 
 #[derive(Clone, PartialEq)]
 pub struct UpdateGoToDefinitionLink {
@@ -276,6 +278,13 @@ pub fn go_to_fetched_definition(
     });
 
     if !definitions.is_empty() {
+        editor_handle.update(cx, |editor, cx| {
+            if !editor.focused {
+                cx.focus_self();
+                cx.emit(Event::Activate);
+            }
+        });
+
         Editor::navigate_to_definitions(workspace, editor_handle, definitions, cx);
     } else {
         editor_handle.update(cx, |editor, cx| {

crates/editor/src/mouse_context_menu.rs 🔗

@@ -2,7 +2,7 @@ use context_menu::ContextMenuItem;
 use gpui::{geometry::vector::Vector2F, impl_internal_actions, MutableAppContext, ViewContext};
 
 use crate::{
-    DisplayPoint, Editor, EditorMode, FindAllReferences, GoToDefinition, Rename, SelectMode,
+    DisplayPoint, Editor, EditorMode, Event, FindAllReferences, GoToDefinition, Rename, SelectMode,
     ToggleCodeActions,
 };
 
@@ -23,6 +23,11 @@ pub fn deploy_context_menu(
     &DeployMouseContextMenu { position, point }: &DeployMouseContextMenu,
     cx: &mut ViewContext<Editor>,
 ) {
+    if !editor.focused {
+        cx.focus_self();
+        cx.emit(Event::Activate);
+    }
+
     // Don't show context menu for inline editors
     if editor.mode() != EditorMode::Full {
         return;

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

@@ -43,7 +43,7 @@ impl MouseEventHandler {
         self
     }
 
-    pub fn on_mouse_down(
+    pub fn on_down(
         mut self,
         button: MouseButton,
         handler: impl Fn(MouseButtonEvent, &mut EventContext) + 'static,
@@ -61,7 +61,7 @@ impl MouseEventHandler {
         self
     }
 
-    pub fn on_mouse_down_out(
+    pub fn on_down_out(
         mut self,
         button: MouseButton,
         handler: impl Fn(MouseButtonEvent, &mut EventContext) + 'static,

crates/picker/src/picker.rs 🔗

@@ -90,7 +90,7 @@ impl<D: PickerDelegate> View for Picker<D> {
                                         .read(cx)
                                         .render_match(ix, state, ix == selected_ix, cx)
                                 })
-                                .on_mouse_down(MouseButton::Left, move |_, cx| {
+                                .on_down(MouseButton::Left, move |_, cx| {
                                     cx.dispatch_action(SelectIndex(ix))
                                 })
                                 .with_cursor_style(CursorStyle::PointingHand)

crates/project_panel/src/project_panel.rs 🔗

@@ -1082,7 +1082,7 @@ impl ProjectPanel {
                 }
             },
         )
-        .on_mouse_down(
+        .on_down(
             MouseButton::Right,
             move |MouseButtonEvent { position, .. }, cx| {
                 cx.dispatch_action(DeployContextMenu { entry_id, position })
@@ -1134,7 +1134,7 @@ impl View for ProjectPanel {
                     .expanded()
                     .boxed()
                 })
-                .on_mouse_down(
+                .on_down(
                     MouseButton::Right,
                     move |MouseButtonEvent { position, .. }, cx| {
                         // When deploying the context menu anywhere below the last project entry,

crates/search/src/project_search.rs 🔗

@@ -147,6 +147,7 @@ impl ProjectSearch {
 
 pub enum ViewEvent {
     UpdateTab,
+    Activate,
     EditorEvent(editor::Event),
 }
 
@@ -162,7 +163,9 @@ impl View for ProjectSearchView {
     fn render(&mut self, cx: &mut RenderContext<Self>) -> ElementBox {
         let model = &self.model.read(cx);
         if model.match_ranges.is_empty() {
-            let theme = &cx.global::<Settings>().theme;
+            enum Status {}
+
+            let theme = cx.global::<Settings>().theme.clone();
             let text = if self.query_editor.read(cx).text(cx).is_empty() {
                 ""
             } else if model.pending_search.is_some() {
@@ -170,12 +173,18 @@ impl View for ProjectSearchView {
             } else {
                 "No results"
             };
-            Label::new(text.to_string(), theme.search.results_status.clone())
-                .aligned()
-                .contained()
-                .with_background_color(theme.editor.background)
-                .flex(1., true)
-                .boxed()
+            MouseEventHandler::new::<Status, _, _>(0, cx, |_, _| {
+                Label::new(text.to_string(), theme.search.results_status.clone())
+                    .aligned()
+                    .contained()
+                    .with_background_color(theme.editor.background)
+                    .flex(1., true)
+                    .boxed()
+            })
+            .on_down(MouseButton::Left, |_, cx| {
+                cx.focus_parent_view();
+            })
+            .boxed()
         } else {
             ChildView::new(&self.results_editor).flex(1., true).boxed()
         }

crates/theme/src/theme.rs 🔗

@@ -38,8 +38,10 @@ pub struct Theme {
 pub struct Workspace {
     pub background: Color,
     pub titlebar: Titlebar,
-    pub tab: Tab,
-    pub active_tab: Tab,
+    pub active_pane_active_tab: Tab,
+    pub active_pane_inactive_tab: Tab,
+    pub inactive_pane_active_tab: Tab,
+    pub inactive_pane_inactive_tab: Tab,
     pub pane_button: Interactive<IconButton>,
     pub pane_divider: Border,
     pub leader_border_opacity: f32,

crates/workspace/src/pane.rs 🔗

@@ -136,6 +136,7 @@ pub enum Event {
 
 pub struct Pane {
     items: Vec<Box<dyn ItemHandle>>,
+    is_active: bool,
     active_item_index: usize,
     autoscroll: bool,
     nav_history: Rc<RefCell<NavHistory>>,
@@ -184,6 +185,7 @@ impl Pane {
         let split_menu = cx.add_view(|cx| ContextMenu::new(cx));
         Self {
             items: Vec::new(),
+            is_active: true,
             active_item_index: 0,
             autoscroll: false,
             nav_history: Rc::new(RefCell::new(NavHistory {
@@ -199,6 +201,11 @@ impl Pane {
         }
     }
 
+    pub fn set_active(&mut self, is_active: bool, cx: &mut ViewContext<Self>) {
+        self.is_active = is_active;
+        cx.notify();
+    }
+
     pub fn nav_history_for_item<T: Item>(&self, item: &ViewHandle<T>) -> ItemNavHistory {
         ItemNavHistory {
             history: self.nav_history.clone(),
@@ -865,26 +872,23 @@ impl Pane {
                 None
             };
 
+            let is_pane_active = self.is_active;
             let mut row = Flex::row().scrollable::<Tabs, _>(1, autoscroll, cx);
             for (ix, (item, detail)) in self.items.iter().zip(self.tab_details(cx)).enumerate() {
                 let detail = if detail == 0 { None } else { Some(detail) };
-                let is_active = ix == self.active_item_index;
+                let is_tab_active = ix == self.active_item_index;
 
                 row.add_child({
-                    let tab_style = if is_active {
-                        theme.workspace.active_tab.clone()
-                    } else {
-                        theme.workspace.tab.clone()
+                    let mut tab_style = match (is_pane_active, is_tab_active) {
+                        (true, true) => theme.workspace.active_pane_active_tab.clone(),
+                        (true, false) => theme.workspace.active_pane_inactive_tab.clone(),
+                        (false, true) => theme.workspace.inactive_pane_active_tab.clone(),
+                        (false, false) => theme.workspace.inactive_pane_inactive_tab.clone(),
                     };
                     let title = item.tab_content(detail, &tab_style, cx);
 
-                    let mut style = if is_active {
-                        theme.workspace.active_tab.clone()
-                    } else {
-                        theme.workspace.tab.clone()
-                    };
                     if ix == 0 {
-                        style.container.border.left = false;
+                        tab_style.container.border.left = false;
                     }
 
                     MouseEventHandler::new::<Tab, _, _>(ix, cx, |_, cx| {
@@ -894,9 +898,9 @@ impl Pane {
                                     Align::new({
                                         let diameter = 7.0;
                                         let icon_color = if item.has_conflict(cx) {
-                                            Some(style.icon_conflict)
+                                            Some(tab_style.icon_conflict)
                                         } else if item.is_dirty(cx) {
-                                            Some(style.icon_dirty)
+                                            Some(tab_style.icon_dirty)
                                         } else {
                                             None
                                         };
@@ -928,8 +932,8 @@ impl Pane {
                                     Container::new(Align::new(title).boxed())
                                         .with_style(ContainerStyle {
                                             margin: Margin {
-                                                left: style.spacing,
-                                                right: style.spacing,
+                                                left: tab_style.spacing,
+                                                right: tab_style.spacing,
                                                 ..Default::default()
                                             },
                                             ..Default::default()
@@ -947,10 +951,11 @@ impl Pane {
                                                 cx,
                                                 |mouse_state, _| {
                                                     if mouse_state.hovered {
-                                                        icon.with_color(style.icon_close_active)
+                                                        icon.with_color(tab_style.icon_close_active)
                                                             .boxed()
                                                     } else {
-                                                        icon.with_color(style.icon_close).boxed()
+                                                        icon.with_color(tab_style.icon_close)
+                                                            .boxed()
                                                     }
                                                 },
                                             )
@@ -969,27 +974,39 @@ impl Pane {
                                         } else {
                                             Empty::new().boxed()
                                         })
-                                        .with_width(style.icon_width)
+                                        .with_width(tab_style.icon_width)
                                         .boxed(),
                                     )
                                     .boxed(),
                                 )
                                 .boxed(),
                         )
-                        .with_style(style.container)
+                        .with_style(tab_style.container)
                         .boxed()
                     })
-                    .on_mouse_down(MouseButton::Left, move |_, cx| {
+                    .with_cursor_style(if is_tab_active && is_pane_active {
+                        CursorStyle::Arrow
+                    } else {
+                        CursorStyle::PointingHand
+                    })
+                    .on_down(MouseButton::Left, move |_, cx| {
                         cx.dispatch_action(ActivateItem(ix));
                     })
                     .boxed()
                 })
             }
 
+            let filler_style = if is_pane_active {
+                &theme.workspace.active_pane_inactive_tab
+            } else {
+                &theme.workspace.inactive_pane_inactive_tab
+            };
+
             row.add_child(
                 Empty::new()
                     .contained()
-                    .with_border(theme.workspace.tab.container.border)
+                    .with_style(filler_style.container)
+                    .with_border(theme.workspace.active_pane_active_tab.container.border)
                     .flex(0., true)
                     .named("filler"),
             );
@@ -1054,10 +1071,12 @@ impl View for Pane {
             .with_child(
                 EventHandler::new(if let Some(active_item) = self.active_item() {
                     Flex::column()
-                        .with_child(
-                            Flex::row()
-                                .with_child(self.render_tabs(cx).flex(1., true).named("tabs"))
-                                .with_child(
+                        .with_child({
+                            let mut tab_row = Flex::row()
+                                .with_child(self.render_tabs(cx).flex(1., true).named("tabs"));
+
+                            if self.is_active {
+                                tab_row.add_child(
                                     MouseEventHandler::new::<SplitIcon, _, _>(
                                         0,
                                         cx,
@@ -1080,7 +1099,7 @@ impl View for Pane {
                                         },
                                     )
                                     .with_cursor_style(CursorStyle::PointingHand)
-                                    .on_mouse_down(
+                                    .on_down(
                                         MouseButton::Left,
                                         |MouseButtonEvent { position, .. }, cx| {
                                             cx.dispatch_action(DeploySplitMenu { position });
@@ -1088,15 +1107,36 @@ impl View for Pane {
                                     )
                                     .boxed(),
                                 )
+                            }
+
+                            tab_row
                                 .constrained()
-                                .with_height(cx.global::<Settings>().theme.workspace.tab.height)
-                                .boxed(),
-                        )
+                                .with_height(
+                                    cx.global::<Settings>()
+                                        .theme
+                                        .workspace
+                                        .active_pane_active_tab
+                                        .height,
+                                )
+                                .boxed()
+                        })
                         .with_child(ChildView::new(&self.toolbar).boxed())
                         .with_child(ChildView::new(active_item).flex(1., true).boxed())
                         .boxed()
                 } else {
-                    Empty::new().boxed()
+                    enum EmptyPane {}
+                    let theme = cx.global::<Settings>().theme.clone();
+
+                    MouseEventHandler::new::<EmptyPane, _, _>(0, cx, |_, _| {
+                        Empty::new()
+                            .contained()
+                            .with_background_color(theme.workspace.background)
+                            .boxed()
+                    })
+                    .on_down(MouseButton::Left, |_, cx| {
+                        cx.focus_parent_view();
+                    })
+                    .boxed()
                 })
                 .on_navigate_mouse_down(move |direction, cx| {
                     let this = this.clone();

crates/workspace/src/sidebar.rs 🔗

@@ -187,7 +187,7 @@ impl Sidebar {
             ..Default::default()
         })
         .with_cursor_style(CursorStyle::ResizeLeftRight)
-        .on_mouse_down(MouseButton::Left, |_, _| {}) // This prevents the mouse down event from being propagated elsewhere
+        .on_down(MouseButton::Left, |_, _| {}) // This prevents the mouse down event from being propagated elsewhere
         .on_drag(
             MouseButton::Left,
             move |old_position,

crates/workspace/src/workspace.rs 🔗

@@ -1566,7 +1566,11 @@ impl Workspace {
 
     fn activate_pane(&mut self, pane: ViewHandle<Pane>, cx: &mut ViewContext<Self>) {
         if self.active_pane != pane {
+            self.active_pane
+                .update(cx, |pane, cx| pane.set_active(false, cx));
             self.active_pane = pane.clone();
+            self.active_pane
+                .update(cx, |pane, cx| pane.set_active(true, cx));
             self.status_bar.update(cx, |status_bar, cx| {
                 status_bar.set_active_pane(&self.active_pane, cx);
             });
@@ -1629,17 +1633,17 @@ impl Workspace {
         pane: ViewHandle<Pane>,
         direction: SplitDirection,
         cx: &mut ViewContext<Self>,
-    ) -> ViewHandle<Pane> {
-        let new_pane = self.add_pane(cx);
-        self.activate_pane(new_pane.clone(), cx);
-        if let Some(item) = pane.read(cx).active_item() {
+    ) -> Option<ViewHandle<Pane>> {
+        pane.read(cx).active_item().map(|item| {
+            let new_pane = self.add_pane(cx);
+            self.activate_pane(new_pane.clone(), cx);
             if let Some(clone) = item.clone_on_split(cx.as_mut()) {
                 Pane::add_item(self, new_pane.clone(), clone, true, true, cx);
             }
-        }
-        self.center.split(&pane, &new_pane, direction).unwrap();
-        cx.notify();
-        new_pane
+            self.center.split(&pane, &new_pane, direction).unwrap();
+            cx.notify();
+            new_pane
+        })
     }
 
     fn remove_pane(&mut self, pane: ViewHandle<Pane>, cx: &mut ViewContext<Self>) {
@@ -3045,16 +3049,17 @@ mod tests {
         //     multi-entry items:   (3, 4)
         let left_pane = workspace.update(cx, |workspace, cx| {
             let left_pane = workspace.active_pane().clone();
-            let right_pane = workspace.split_pane(left_pane.clone(), SplitDirection::Right, cx);
-
-            workspace.activate_pane(left_pane.clone(), cx);
             workspace.add_item(Box::new(cx.add_view(|_| item_2_3.clone())), cx);
             for item in &single_entry_items {
                 workspace.add_item(Box::new(cx.add_view(|_| item.clone())), cx);
             }
+            left_pane.update(cx, |pane, cx| {
+                pane.activate_item(2, true, true, false, cx);
+            });
 
-            workspace.activate_pane(right_pane.clone(), cx);
-            workspace.add_item(Box::new(cx.add_view(|_| single_entry_items[1].clone())), cx);
+            workspace
+                .split_pane(left_pane.clone(), SplitDirection::Right, cx)
+                .unwrap();
             workspace.add_item(Box::new(cx.add_view(|_| item_3_4.clone())), cx);
 
             left_pane

styles/src/styleTree/workspace.ts 🔗

@@ -14,7 +14,7 @@ export function workspaceBackground(theme: Theme) {
 }
 
 export default function workspace(theme: Theme) {
-  const tab = {
+  const activePaneInactiveTab = {
     height: 32,
     background: workspaceBackground(theme),
     iconClose: iconColor(theme, "muted"),
@@ -39,16 +39,27 @@ export default function workspace(theme: Theme) {
     }
   };
 
-  const activeTab = {
-    ...tab,
+  const activePaneActiveTab = {
+    ...activePaneInactiveTab,
     background: backgroundColor(theme, 500),
     text: text(theme, "sans", "active", { size: "sm" }),
     border: {
-      ...tab.border,
+      ...activePaneInactiveTab.border,
       bottom: false,
     },
   };
 
+  const inactivePaneInactiveTab = {
+    ...activePaneInactiveTab,
+    background: backgroundColor(theme, 100),
+    text: text(theme, "sans", "placeholder", { size: "sm" }),
+  };
+
+  const inactivePaneActiveTab = {
+    ...activePaneInactiveTab,
+    text: text(theme, "sans", "placeholder", { size: "sm" }),
+  }
+
   const titlebarPadding = 6;
 
   return {
@@ -63,15 +74,17 @@ export default function workspace(theme: Theme) {
     },
     leaderBorderOpacity: 0.7,
     leaderBorderWidth: 2.0,
-    tab,
-    activeTab,
+    activePaneActiveTab,
+    activePaneInactiveTab,
+    inactivePaneActiveTab,
+    inactivePaneInactiveTab,
     paneButton: {
       color: iconColor(theme, "secondary"),
       border: {
-        ...tab.border,
+        ...activePaneActiveTab.border,
       },
       iconWidth: 12,
-      buttonWidth: tab.height,
+      buttonWidth: activePaneActiveTab.height,
       hover: {
         color: iconColor(theme, "active"),
         background: backgroundColor(theme, 300),