Merge pull request #2105 from zed-industries/fix-focus-stealing-when-collaborating

Kay Simmons created

Limit focus grabbing in followed pane

Change summary

crates/collab/src/tests/integration_tests.rs |  1 
crates/gpui/src/app.rs                       | 45 ++++++++++++++-------
crates/workspace/src/workspace.rs            | 18 +++++--
3 files changed, 42 insertions(+), 22 deletions(-)

Detailed changes

crates/collab/src/tests/integration_tests.rs 🔗

@@ -5625,7 +5625,6 @@ async fn test_following(
             .downcast::<Editor>()
             .unwrap()
     });
-    assert!(cx_b.read(|cx| editor_b2.is_focused(cx)));
     assert_eq!(
         cx_b.read(|cx| editor_b2.project_path(cx)),
         Some((worktree_id, "2.txt").into())

crates/gpui/src/app.rs 🔗

@@ -1414,21 +1414,6 @@ impl MutableAppContext {
         true
     }
 
-    /// Returns an iterator over all of the view ids from the passed view up to the root of the window
-    /// Includes the passed view itself
-    fn ancestors(&self, window_id: usize, mut view_id: usize) -> impl Iterator<Item = usize> + '_ {
-        std::iter::once(view_id)
-            .into_iter()
-            .chain(std::iter::from_fn(move || {
-                if let Some(ParentId::View(parent_id)) = self.parents.get(&(window_id, view_id)) {
-                    view_id = *parent_id;
-                    Some(view_id)
-                } else {
-                    None
-                }
-            }))
-    }
-
     fn actions_mut(
         &mut self,
         capture_phase: bool,
@@ -2733,6 +2718,32 @@ impl AppContext {
             panic!("no global has been added for {}", type_name::<T>());
         }
     }
+
+    /// Returns an iterator over all of the view ids from the passed view up to the root of the window
+    /// Includes the passed view itself
+    fn ancestors(&self, window_id: usize, mut view_id: usize) -> impl Iterator<Item = usize> + '_ {
+        std::iter::once(view_id)
+            .into_iter()
+            .chain(std::iter::from_fn(move || {
+                if let Some(ParentId::View(parent_id)) = self.parents.get(&(window_id, view_id)) {
+                    view_id = *parent_id;
+                    Some(view_id)
+                } else {
+                    None
+                }
+            }))
+    }
+
+    pub fn is_child_focused(&self, view: impl Into<AnyViewHandle>) -> bool {
+        let view = view.into();
+        if let Some(focused_view_id) = self.focused_view_id(view.window_id) {
+            self.ancestors(view.window_id, focused_view_id)
+                .skip(1) // Skip self id
+                .any(|parent| parent == view.view_id)
+        } else {
+            false
+        }
+    }
 }
 
 impl ReadModel for AppContext {
@@ -6383,6 +6394,8 @@ mod tests {
             cx.focus(&view_1);
             cx.focus(&view_2);
         });
+        assert!(cx.is_child_focused(view_1.clone()));
+        assert!(!cx.is_child_focused(view_2.clone()));
         assert_eq!(
             mem::take(&mut *view_events.lock()),
             [
@@ -6407,6 +6420,8 @@ mod tests {
         );
 
         view_1.update(cx, |_, cx| cx.focus(&view_1));
+        assert!(!cx.is_child_focused(view_1.clone()));
+        assert!(!cx.is_child_focused(view_2.clone()));
         assert_eq!(
             mem::take(&mut *view_events.lock()),
             ["view 2 blurred", "view 1 focused"],

crates/workspace/src/workspace.rs 🔗

@@ -2140,7 +2140,7 @@ impl Workspace {
         let call = self.active_call()?;
         let room = call.read(cx).room()?.read(cx);
         let participant = room.remote_participant_for_peer_id(leader_id)?;
-        let mut items_to_add = Vec::new();
+        let mut items_to_activate = Vec::new();
         match participant.location {
             call::ParticipantLocation::SharedProject { project_id } => {
                 if Some(project_id) == self.project.read(cx).remote_id() {
@@ -2149,12 +2149,12 @@ impl Workspace {
                             .active_view_id
                             .and_then(|id| state.items_by_leader_view_id.get(&id))
                         {
-                            items_to_add.push((pane.clone(), item.boxed_clone()));
+                            items_to_activate.push((pane.clone(), item.boxed_clone()));
                         } else {
                             if let Some(shared_screen) =
                                 self.shared_screen_for_peer(leader_id, pane, cx)
                             {
-                                items_to_add.push((pane.clone(), Box::new(shared_screen)));
+                                items_to_activate.push((pane.clone(), Box::new(shared_screen)));
                             }
                         }
                     }
@@ -2164,20 +2164,26 @@ impl Workspace {
             call::ParticipantLocation::External => {
                 for (pane, _) in self.follower_states_by_leader.get(&leader_id)? {
                     if let Some(shared_screen) = self.shared_screen_for_peer(leader_id, pane, cx) {
-                        items_to_add.push((pane.clone(), Box::new(shared_screen)));
+                        items_to_activate.push((pane.clone(), Box::new(shared_screen)));
                     }
                 }
             }
         }
 
-        for (pane, item) in items_to_add {
+        for (pane, item) in items_to_activate {
+            let active_item_was_focused = pane
+                .read(cx)
+                .active_item()
+                .map(|active_item| cx.is_child_focused(active_item.to_any()))
+                .unwrap_or_default();
+
             if let Some(index) = pane.update(cx, |pane, _| pane.index_for_item(item.as_ref())) {
                 pane.update(cx, |pane, cx| pane.activate_item(index, false, false, cx));
             } else {
                 Pane::add_item(self, &pane, item.boxed_clone(), false, false, None, cx);
             }
 
-            if pane == self.active_pane {
+            if active_item_was_focused {
                 pane.update(cx, |pane, cx| pane.focus_active_item(cx));
             }
         }