Pane focus working. Modals seem broken now

K Simmons created

Change summary

crates/diagnostics/src/diagnostics.rs      |  4 --
crates/editor/src/editor.rs                |  3 -
crates/editor/src/items.rs                 |  4 --
crates/editor/src/link_go_to_definition.rs |  5 +-
crates/editor/src/mouse_context_menu.rs    |  3 -
crates/gpui/src/app.rs                     | 14 +------
crates/search/src/project_search.rs        | 31 +-----------------
crates/terminal/src/terminal.rs            |  1 
crates/terminal/src/terminal_view.rs       |  9 -----
crates/workspace/src/pane.rs               | 34 ++++++++++++--------
crates/workspace/src/workspace.rs          | 38 ++++++++---------------
11 files changed, 43 insertions(+), 103 deletions(-)

Detailed changes

crates/diagnostics/src/diagnostics.rs 🔗

@@ -568,10 +568,6 @@ impl workspace::Item for ProjectDiagnosticsEditor {
         unreachable!()
     }
 
-    fn should_activate_item_on_event(event: &Self::Event) -> bool {
-        Editor::should_activate_item_on_event(event)
-    }
-
     fn should_update_tab_on_event(event: &Event) -> bool {
         Editor::should_update_tab_on_event(event)
     }

crates/editor/src/editor.rs 🔗

@@ -1561,7 +1561,6 @@ impl Editor {
     ) {
         if !self.focused {
             cx.focus_self();
-            cx.emit(Event::Activate);
         }
 
         let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx));
@@ -1623,7 +1622,6 @@ impl Editor {
     ) {
         if !self.focused {
             cx.focus_self();
-            cx.emit(Event::Activate);
         }
 
         let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx));
@@ -5969,7 +5967,6 @@ fn compute_scroll_position(
 
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub enum Event {
-    Activate,
     BufferEdited,
     Edited,
     Reparsed,

crates/editor/src/items.rs 🔗

@@ -469,10 +469,6 @@ impl Item for Editor {
         })
     }
 
-    fn should_activate_item_on_event(event: &Event) -> bool {
-        matches!(event, Event::Activate)
-    }
-
     fn should_close_item_on_event(event: &Event) -> bool {
         matches!(event, Event::Closed)
     }
@@ -8,8 +8,8 @@ use util::TryFutureExt;
 use workspace::Workspace;
 
 use crate::{
-    Anchor, DisplayPoint, Editor, EditorSnapshot, Event, GoToDefinition, GoToTypeDefinition,
-    Select, SelectPhase,
+    Anchor, DisplayPoint, Editor, EditorSnapshot, GoToDefinition, GoToTypeDefinition, Select,
+    SelectPhase,
 };
 
 #[derive(Clone, PartialEq)]
@@ -355,7 +355,6 @@ fn go_to_fetched_definition_of_kind(
         editor_handle.update(cx, |editor, cx| {
             if !editor.focused {
                 cx.focus_self();
-                cx.emit(Event::Activate);
             }
         });
 

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, Event, FindAllReferences, GoToDefinition, GoToTypeDefinition,
+    DisplayPoint, Editor, EditorMode, FindAllReferences, GoToDefinition, GoToTypeDefinition,
     Rename, SelectMode, ToggleCodeActions,
 };
 
@@ -25,7 +25,6 @@ pub fn deploy_context_menu(
 ) {
     if !editor.focused {
         cx.focus_self();
-        cx.emit(Event::Activate);
     }
 
     // Don't show context menu for inline editors

crates/gpui/src/app.rs 🔗

@@ -1239,7 +1239,7 @@ impl MutableAppContext {
         let mut view = self
             .cx
             .views
-            .remove(&(params.window_id, params.view_id))
+            .remove(&(window_id, view_id))
             .ok_or(anyhow!("view not found"))?;
         let element = view.render(params, self);
         self.cx.views.insert((window_id, view_id), view);
@@ -1781,7 +1781,7 @@ impl MutableAppContext {
                     .cx
                     .views
                     .get(&(window_id, view_id))
-                    .expect("View passed to visit does not exist")
+                    .unwrap()
                     .keymap_context(self.as_ref());
 
                 match self.keystroke_matcher.push_keystroke(
@@ -2548,11 +2548,6 @@ impl MutableAppContext {
 
             if let Some(blurred_id) = blurred_id {
                 for view_id in blurred_parents.iter().copied() {
-                    // We've reached a common anscestor. Break.
-                    if focused_parents.contains(&view_id) {
-                        break;
-                    }
-
                     if let Some(mut view) = this.cx.views.remove(&(window_id, view_id)) {
                         view.on_focus_out(this, window_id, view_id, blurred_id);
                         this.cx.views.insert((window_id, view_id), view);
@@ -2566,12 +2561,9 @@ impl MutableAppContext {
 
             if let Some(focused_id) = focused_id {
                 for view_id in focused_parents {
-                    if blurred_parents.contains(&view_id) {
-                        break;
-                    }
                     if let Some(mut view) = this.cx.views.remove(&(window_id, view_id)) {
                         view.on_focus_in(this, window_id, view_id, focused_id);
-                        this.cx.views.insert((window_id, focused_id), view);
+                        this.cx.views.insert((window_id, view_id), view);
                     }
                 }
 

crates/search/src/project_search.rs 🔗

@@ -73,7 +73,6 @@ pub struct ProjectSearchView {
     regex: bool,
     query_contains_error: bool,
     active_match_index: Option<usize>,
-    results_editor_was_focused: bool,
 }
 
 pub struct ProjectSearchBar {
@@ -197,12 +196,6 @@ impl View for ProjectSearchView {
                 .0
                 .insert(self.model.read(cx).project.downgrade(), handle)
         });
-
-        if self.results_editor_was_focused && !self.model.read(cx).match_ranges.is_empty() {
-            self.focus_results_editor(cx);
-        } else {
-            cx.focus(&self.query_editor);
-        }
     }
 }
 
@@ -330,14 +323,6 @@ impl Item for ProjectSearchView {
             .update(cx, |editor, cx| editor.navigate(data, cx))
     }
 
-    fn should_activate_item_on_event(event: &Self::Event) -> bool {
-        if let ViewEvent::EditorEvent(editor_event) = event {
-            Editor::should_activate_item_on_event(editor_event)
-        } else {
-            false
-        }
-    }
-
     fn should_update_tab_on_event(event: &ViewEvent) -> bool {
         matches!(event, ViewEvent::UpdateTab)
     }
@@ -385,12 +370,6 @@ impl ProjectSearchView {
             cx.emit(ViewEvent::EditorEvent(event.clone()))
         })
         .detach();
-        cx.observe_focus(&query_editor, |this, _, focused, _| {
-            if focused {
-                this.results_editor_was_focused = false;
-            }
-        })
-        .detach();
 
         let results_editor = cx.add_view(|cx| {
             let mut editor = Editor::for_multibuffer(excerpts, Some(project), cx);
@@ -399,12 +378,7 @@ impl ProjectSearchView {
         });
         cx.observe(&results_editor, |_, _, cx| cx.emit(ViewEvent::UpdateTab))
             .detach();
-        cx.observe_focus(&results_editor, |this, _, focused, _| {
-            if focused {
-                this.results_editor_was_focused = true;
-            }
-        })
-        .detach();
+
         cx.subscribe(&results_editor, |this, _, event, cx| {
             if matches!(event, editor::Event::SelectionsChanged { .. }) {
                 this.update_match_index(cx);
@@ -423,7 +397,6 @@ impl ProjectSearchView {
             regex,
             query_contains_error: false,
             active_match_index: None,
-            results_editor_was_focused: false,
         };
         this.model_changed(false, cx);
         this
@@ -905,6 +878,8 @@ impl ToolbarItemView for ProjectSearchBar {
         self.subscription = None;
         self.active_project_search = None;
         if let Some(search) = active_pane_item.and_then(|i| i.downcast::<ProjectSearchView>()) {
+            let query_editor = search.read(cx).query_editor.clone();
+            cx.reparent(query_editor);
             self.subscription = Some(cx.observe(&search, |_, _, cx| cx.notify()));
             self.active_project_search = Some(search);
             ToolbarItemLocation::PrimaryLeft {

crates/terminal/src/terminal_view.rs 🔗

@@ -153,10 +153,7 @@ impl View for TerminalView {
     }
 
     fn on_focus_in(&mut self, _: AnyViewHandle, cx: &mut ViewContext<Self>) {
-        cx.emit(Event::Activate);
-        cx.defer(|view, cx| {
-            cx.focus(view.content.handle());
-        });
+        cx.focus(self.content.handle());
     }
 
     fn keymap_context(&self, _: &gpui::AppContext) -> gpui::keymap::Context {
@@ -314,10 +311,6 @@ impl Item for TerminalView {
     fn should_close_item_on_event(event: &Self::Event) -> bool {
         matches!(event, &Event::CloseTerminal)
     }
-
-    fn should_activate_item_on_event(event: &Self::Event) -> bool {
-        matches!(event, &Event::Activate)
-    }
 }
 
 ///Get's the working directory for the given workspace, respecting the user's settings.

crates/workspace/src/pane.rs 🔗

@@ -13,9 +13,9 @@ use gpui::{
     },
     impl_actions, impl_internal_actions,
     platform::{CursorStyle, NavigationDirection},
-    AnyViewHandle, AppContext, AsyncAppContext, Entity, EventContext, ModelHandle, MouseButton,
-    MouseButtonEvent, MutableAppContext, PromptLevel, Quad, RenderContext, Task, View, ViewContext,
-    ViewHandle, WeakViewHandle,
+    AnyViewHandle, AnyWeakViewHandle, AppContext, AsyncAppContext, Entity, EventContext,
+    ModelHandle, MouseButton, MouseButtonEvent, MutableAppContext, PromptLevel, Quad,
+    RenderContext, Task, View, ViewContext, ViewHandle, WeakViewHandle,
 };
 use project::{Project, ProjectEntryId, ProjectPath};
 use serde::Deserialize;
@@ -132,7 +132,7 @@ pub fn init(cx: &mut MutableAppContext) {
 }
 
 pub enum Event {
-    Activate,
+    Focused,
     ActivateItem { local: bool },
     Remove,
     RemoveItem,
@@ -144,6 +144,7 @@ pub struct Pane {
     items: Vec<Box<dyn ItemHandle>>,
     is_active: bool,
     active_item_index: usize,
+    last_focused_view: Option<AnyWeakViewHandle>,
     autoscroll: bool,
     nav_history: Rc<RefCell<NavHistory>>,
     toolbar: ViewHandle<Toolbar>,
@@ -193,6 +194,7 @@ impl Pane {
             items: Vec::new(),
             is_active: true,
             active_item_index: 0,
+            last_focused_view: None,
             autoscroll: false,
             nav_history: Rc::new(RefCell::new(NavHistory {
                 mode: NavigationMode::Normal,
@@ -219,10 +221,6 @@ impl Pane {
         }
     }
 
-    pub fn activate(&self, cx: &mut ViewContext<Self>) {
-        cx.emit(Event::Activate);
-    }
-
     pub fn go_back(
         workspace: &mut Workspace,
         pane: Option<ViewHandle<Pane>>,
@@ -287,7 +285,7 @@ impl Pane {
         mode: NavigationMode,
         cx: &mut ViewContext<Workspace>,
     ) -> Task<()> {
-        workspace.activate_pane(pane.clone(), cx);
+        cx.focus(pane.clone());
 
         let to_load = pane.update(cx, |pane, cx| {
             loop {
@@ -523,7 +521,7 @@ impl Pane {
                 self.focus_active_item(cx);
             }
             if activate_pane {
-                self.activate(cx);
+                cx.emit(Event::Focused);
             }
             self.autoscroll = true;
             cx.notify();
@@ -830,7 +828,6 @@ impl Pane {
     pub fn focus_active_item(&mut self, cx: &mut ViewContext<Self>) {
         if let Some(active_item) = self.active_item() {
             cx.focus(active_item);
-            self.activate(cx);
         }
     }
 
@@ -1212,11 +1209,20 @@ impl View for Pane {
     }
 
     fn on_focus_in(&mut self, focused: AnyViewHandle, cx: &mut ViewContext<Self>) {
-        if cx.handle().id() == focused.id() {
-            self.focus_active_item(cx);
+        if cx.is_self_focused() {
+            if let Some(last_focused_view) = self
+                .last_focused_view
+                .as_ref()
+                .and_then(|handle| handle.upgrade(cx))
+            {
+                cx.focus(last_focused_view);
+            } else {
+                self.focus_active_item(cx);
+            }
         } else {
-            self.activate(cx);
+            self.last_focused_view = Some(focused.downgrade());
         }
+        cx.emit(Event::Focused);
     }
 }
 

crates/workspace/src/workspace.rs 🔗

@@ -297,9 +297,6 @@ pub trait Item: View {
         project: ModelHandle<Project>,
         cx: &mut ViewContext<Self>,
     ) -> Task<Result<()>>;
-    fn should_activate_item_on_event(_: &Self::Event) -> bool {
-        false
-    }
     fn should_close_item_on_event(_: &Self::Event) -> bool {
         false
     }
@@ -577,15 +574,6 @@ impl<T: Item> ItemHandle for ViewHandle<T> {
                 return;
             }
 
-            if T::should_activate_item_on_event(event) {
-                pane.update(cx, |pane, cx| {
-                    if let Some(ix) = pane.index_for_item(&item) {
-                        pane.activate_item(ix, true, true, false, cx);
-                        pane.activate(cx);
-                    }
-                });
-            }
-
             if T::should_update_tab_on_event(event) {
                 pane.update(cx, |_, cx| {
                     cx.emit(pane::Event::ChangeItemTitle);
@@ -1438,7 +1426,7 @@ impl Workspace {
         })
         .detach();
         self.panes.push(pane.clone());
-        self.activate_pane(pane.clone(), cx);
+        cx.focus(pane.clone());
         cx.emit(Event::PaneAdded(pane.clone()));
         pane
     }
@@ -1533,7 +1521,6 @@ impl Workspace {
             }
         });
         if let Some((pane, ix)) = result {
-            self.activate_pane(pane.clone(), cx);
             pane.update(cx, |pane, cx| pane.activate_item(ix, true, true, false, cx));
             true
         } else {
@@ -1544,7 +1531,7 @@ impl Workspace {
     fn activate_pane_at_index(&mut self, action: &ActivatePane, cx: &mut ViewContext<Self>) {
         let panes = self.center.panes();
         if let Some(pane) = panes.get(action.0).map(|p| (*p).clone()) {
-            self.activate_pane(pane, cx);
+            cx.focus(pane);
         } else {
             self.split_pane(self.active_pane.clone(), SplitDirection::Right, cx);
         }
@@ -1560,7 +1547,7 @@ impl Workspace {
             let next_ix = (ix + 1) % panes.len();
             panes[next_ix].clone()
         };
-        self.activate_pane(next_pane, cx);
+        cx.focus(next_pane);
     }
 
     pub fn activate_previous_pane(&mut self, cx: &mut ViewContext<Self>) {
@@ -1573,10 +1560,10 @@ impl Workspace {
             let prev_ix = if ix == 0 { panes.len() - 1 } else { ix - 1 };
             panes[prev_ix].clone()
         };
-        self.activate_pane(prev_pane, cx);
+        cx.focus(prev_pane);
     }
 
-    fn activate_pane(&mut self, pane: ViewHandle<Pane>, cx: &mut ViewContext<Self>) {
+    fn handle_pane_focused(&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));
@@ -1587,7 +1574,6 @@ impl Workspace {
                 status_bar.set_active_pane(&self.active_pane, cx);
             });
             self.active_item_path_changed(cx);
-            cx.focus(&self.active_pane);
             cx.notify();
         }
 
@@ -1614,8 +1600,8 @@ impl Workspace {
                 pane::Event::Remove => {
                     self.remove_pane(pane, cx);
                 }
-                pane::Event::Activate => {
-                    self.activate_pane(pane, cx);
+                pane::Event::Focused => {
+                    self.handle_pane_focused(pane, cx);
                 }
                 pane::Event::ActivateItem { local } => {
                     if *local {
@@ -1648,7 +1634,6 @@ impl Workspace {
     ) -> 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);
             }
@@ -1661,7 +1646,7 @@ impl Workspace {
     fn remove_pane(&mut self, pane: ViewHandle<Pane>, cx: &mut ViewContext<Self>) {
         if self.center.remove(&pane).unwrap() {
             self.panes.retain(|p| p != &pane);
-            self.activate_pane(self.panes.last().unwrap().clone(), cx);
+            cx.focus(self.panes.last().unwrap().clone());
             self.unfollow(&pane, cx);
             self.last_leaders_by_pane.remove(&pane.downgrade());
             cx.notify();
@@ -2490,7 +2475,10 @@ impl View for Workspace {
     }
 
     fn on_focus_in(&mut self, _: AnyViewHandle, cx: &mut ViewContext<Self>) {
-        cx.focus(&self.active_pane);
+        if cx.is_self_focused() {
+            println!("Active Pane Focused");
+            cx.focus(&self.active_pane);
+        }
     }
 }
 
@@ -3110,7 +3098,7 @@ mod tests {
         // once for project entry 0, and once for project entry 2. After those two
         // prompts, the task should complete.
         let close = workspace.update(cx, |workspace, cx| {
-            workspace.activate_pane(left_pane.clone(), cx);
+            cx.focus(left_pane.clone());
             Pane::close_items(workspace, left_pane.clone(), cx, |_| true)
         });