Activate screen-sharing when leader activates a panel (#2614)

Antonio Scandurra created 2 years ago

Fixes
https://linear.app/zed-industries/issue/Z-1875/screen-sharing-tab-is-not-activated-when-leader-is-on-a-panel

Release Notes:

- Fixed a bug that caused followers to not see the leader's screen when
they activated a panel.

Change summary

crates/collab/src/tests/integration_tests.rs | 50 ++++++++++++++++++---
crates/workspace/src/dock.rs                 |  4 
crates/workspace/src/item.rs                 |  4 
crates/workspace/src/workspace.rs            | 48 +++++++++++++-------
4 files changed, 77 insertions(+), 29 deletions(-)

Detailed changes

crates/collab/src/tests/integration_tests.rs

@@ -39,7 +39,12 @@ use std::{
     },
 };
 use unindent::Unindent as _;
-use workspace::{item::ItemHandle as _, shared_screen::SharedScreen, SplitDirection, Workspace};
+use workspace::{
+    dock::{test::TestPanel, DockPosition},
+    item::{test::TestItem, ItemHandle as _},
+    shared_screen::SharedScreen,
+    SplitDirection, Workspace,
+};
 
 #[ctor::ctor]
 fn init_logger() {
@@ -6847,12 +6852,43 @@ async fn test_basic_following(
         )
     });
 
-    // Client B activates an external window again, and the previously-opened screen-sharing item
-    // gets activated.
-    active_call_b
-        .update(cx_b, |call, cx| call.set_location(None, cx))
-        .await
-        .unwrap();
+    // Client B activates a panel, and the previously-opened screen-sharing item gets activated.
+    let panel = cx_b.add_view(workspace_b.window_id(), |_| {
+        TestPanel::new(DockPosition::Left)
+    });
+    workspace_b.update(cx_b, |workspace, cx| {
+        workspace.add_panel(panel, cx);
+        workspace.toggle_panel_focus::<TestPanel>(cx);
+    });
+    deterministic.run_until_parked();
+    assert_eq!(
+        workspace_a.read_with(cx_a, |workspace, cx| workspace
+            .active_item(cx)
+            .unwrap()
+            .id()),
+        shared_screen.id()
+    );
+
+    // Toggling the focus back to the pane causes client A to return to the multibuffer.
+    workspace_b.update(cx_b, |workspace, cx| {
+        workspace.toggle_panel_focus::<TestPanel>(cx);
+    });
+    deterministic.run_until_parked();
+    workspace_a.read_with(cx_a, |workspace, cx| {
+        assert_eq!(
+            workspace.active_item(cx).unwrap().id(),
+            multibuffer_editor_a.id()
+        )
+    });
+
+    // Client B activates an item that doesn't implement following,
+    // so the previously-opened screen-sharing item gets activated.
+    let unfollowable_item = cx_b.add_view(workspace_b.window_id(), |_| TestItem::new());
+    workspace_b.update(cx_b, |workspace, cx| {
+        workspace.active_pane().update(cx, |pane, cx| {
+            pane.add_item(Box::new(unfollowable_item), true, true, None, cx)
+        })
+    });
     deterministic.run_until_parked();
     assert_eq!(
         workspace_a.read_with(cx_a, |workspace, cx| workspace

crates/workspace/src/dock.rs

@@ -598,8 +598,8 @@ impl StatusItemView for PanelButtons {
     }
 }
 
-#[cfg(test)]
-pub(crate) mod test {
+#[cfg(any(test, feature = "test-support"))]
+pub mod test {
     use super::*;
     use gpui::{ViewContext, WindowContext};
 

crates/workspace/src/item.rs

@@ -710,8 +710,8 @@ impl<T: FollowableItem> FollowableItemHandle for ViewHandle<T> {
     }
 }
 
-#[cfg(test)]
-pub(crate) mod test {
+#[cfg(any(test, feature = "test-support"))]
+pub mod test {
     use super::{Item, ItemEvent};
     use crate::{ItemId, ItemNavHistory, Pane, Workspace, WorkspaceId};
     use gpui::{

crates/workspace/src/workspace.rs

@@ -919,6 +919,7 @@ impl Workspace {
                         this.zoomed = None;
                         this.zoomed_position = None;
                     }
+                    this.update_active_view_for_followers(cx);
                     cx.notify();
                 }
             }
@@ -1946,18 +1947,7 @@ impl Workspace {
             self.zoomed = None;
         }
         self.zoomed_position = None;
-
-        self.update_followers(
-            proto::update_followers::Variant::UpdateActiveView(proto::UpdateActiveView {
-                id: self.active_item(cx).and_then(|item| {
-                    item.to_followable_item_handle(cx)?
-                        .remote_id(&self.app_state.client, cx)
-                        .map(|id| id.to_proto())
-                }),
-                leader_id: self.leader_for_pane(&pane),
-            }),
-            cx,
-        );
+        self.update_active_view_for_followers(cx);
 
         cx.notify();
     }
@@ -2646,6 +2636,30 @@ impl Workspace {
         Ok(())
     }
 
+    fn update_active_view_for_followers(&self, cx: &AppContext) {
+        if self.active_pane.read(cx).has_focus() {
+            self.update_followers(
+                proto::update_followers::Variant::UpdateActiveView(proto::UpdateActiveView {
+                    id: self.active_item(cx).and_then(|item| {
+                        item.to_followable_item_handle(cx)?
+                            .remote_id(&self.app_state.client, cx)
+                            .map(|id| id.to_proto())
+                    }),
+                    leader_id: self.leader_for_pane(&self.active_pane),
+                }),
+                cx,
+            );
+        } else {
+            self.update_followers(
+                proto::update_followers::Variant::UpdateActiveView(proto::UpdateActiveView {
+                    id: None,
+                    leader_id: None,
+                }),
+                cx,
+            );
+        }
+    }
+
     fn update_followers(
         &self,
         update: proto::update_followers::Variant,
@@ -2693,12 +2707,10 @@ impl Workspace {
                             .and_then(|id| state.items_by_leader_view_id.get(&id))
                         {
                             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_activate.push((pane.clone(), Box::new(shared_screen)));
-                            }
+                        } else if let Some(shared_screen) =
+                            self.shared_screen_for_peer(leader_id, pane, cx)
+                        {
+                            items_to_activate.push((pane.clone(), Box::new(shared_screen)));
                         }
                     }
                 }