Restore the colored background for collaborators that you are following (#4137)

Max Brunsfeld created

Change summary

crates/collab/.admins.default.json           |   9 +
crates/collab_ui/src/collab_panel.rs         |   7 
crates/collab_ui/src/collab_titlebar_item.rs | 134 ++++++++++++---------
crates/collab_ui/src/face_pile.rs            |  10 
script/zed-local                             |  70 +++++++---
5 files changed, 139 insertions(+), 91 deletions(-)

Detailed changes

crates/collab/.admins.default.json 🔗

@@ -1 +1,8 @@
-["nathansobo", "as-cii", "maxbrunsfeld", "iamnbutler"]
+[
+  "nathansobo",
+  "as-cii",
+  "maxbrunsfeld",
+  "iamnbutler",
+  "mikayla-maki",
+  "JosephTLyons"
+]

crates/collab_ui/src/collab_panel.rs 🔗

@@ -17,9 +17,8 @@ use gpui::{
     actions, canvas, div, fill, list, overlay, point, prelude::*, px, AnyElement, AppContext,
     AsyncWindowContext, Bounds, ClipboardItem, DismissEvent, Div, EventEmitter, FocusHandle,
     FocusableView, FontStyle, FontWeight, InteractiveElement, IntoElement, ListOffset, ListState,
-    Model, MouseDownEvent, ParentElement, Pixels, Point, PromptLevel, Render, RenderOnce,
-    SharedString, Styled, Subscription, Task, TextStyle, View, ViewContext, VisualContext,
-    WeakView, WhiteSpace,
+    Model, MouseDownEvent, ParentElement, Pixels, Point, PromptLevel, Render, SharedString, Styled,
+    Subscription, Task, TextStyle, View, ViewContext, VisualContext, WeakView, WhiteSpace,
 };
 use menu::{Cancel, Confirm, SelectNext, SelectPrev};
 use project::{Fs, Project};
@@ -2296,7 +2295,7 @@ impl CollabPanel {
                         h_flex()
                             .id(channel_id as usize)
                             .child(Label::new(channel.name.clone()))
-                            .children(face_pile.map(|face_pile| face_pile.render(cx))),
+                            .children(face_pile.map(|face_pile| face_pile.render().p_1())),
                     ),
             )
             .child(

crates/collab_ui/src/collab_titlebar_item.rs 🔗

@@ -57,6 +57,7 @@ impl Render for CollabTitlebarItem {
         let current_user = self.user_store.read(cx).current_user();
         let client = self.client.clone();
         let project_id = self.project.read(cx).remote_id();
+        let workspace = self.workspace.upgrade();
 
         h_flex()
             .id("titlebar")
@@ -100,6 +101,7 @@ impl Render for CollabTitlebarItem {
                                 true,
                                 room.is_speaking(),
                                 room.is_muted(),
+                                None,
                                 &room,
                                 project_id,
                                 &current_user,
@@ -113,6 +115,12 @@ impl Render for CollabTitlebarItem {
                             }))
                             .children(
                                 remote_participants.iter().filter_map(|collaborator| {
+                                    let player_color = player_colors
+                                        .color_for_participant(collaborator.participant_index.0);
+                                    let is_following = workspace
+                                        .as_ref()?
+                                        .read(cx)
+                                        .is_being_followed(collaborator.peer_id);
                                     let is_present = project_id.map_or(false, |project_id| {
                                         collaborator.location
                                             == ParticipantLocation::SharedProject { project_id }
@@ -124,6 +132,7 @@ impl Render for CollabTitlebarItem {
                                         is_present,
                                         collaborator.speaking,
                                         collaborator.muted,
+                                        is_following.then_some(player_color.selection),
                                         &room,
                                         project_id,
                                         &current_user,
@@ -134,13 +143,7 @@ impl Render for CollabTitlebarItem {
                                         v_flex()
                                             .id(("collaborator", collaborator.user.id))
                                             .child(face_pile)
-                                            .child(render_color_ribbon(
-                                                player_colors
-                                                    .color_for_participant(
-                                                        collaborator.participant_index.0,
-                                                    )
-                                                    .cursor,
-                                            ))
+                                            .child(render_color_ribbon(player_color.cursor))
                                             .cursor_pointer()
                                             .on_click({
                                                 let peer_id = collaborator.peer_id;
@@ -468,11 +471,12 @@ impl CollabTitlebarItem {
         is_present: bool,
         is_speaking: bool,
         is_muted: bool,
+        leader_selection_color: Option<Hsla>,
         room: &Room,
         project_id: Option<u64>,
         current_user: &Arc<User>,
         cx: &ViewContext<Self>,
-    ) -> Option<FacePile> {
+    ) -> Option<Div> {
         if room.role_for_user(user.id) == Some(proto::ChannelRole::Guest) {
             return None;
         }
@@ -481,56 +485,72 @@ impl CollabTitlebarItem {
         let followers = project_id.map_or(&[] as &[_], |id| room.followers_for(peer_id, id));
         let extra_count = followers.len().saturating_sub(FACEPILE_LIMIT);
 
-        let pile = FacePile::default()
-            .child(
-                Avatar::new(user.avatar_uri.clone())
-                    .grayscale(!is_present)
-                    .border_color(if is_speaking {
-                        cx.theme().status().info
-                    } else {
-                        // We draw the border in a transparent color rather to avoid
-                        // the layout shift that would come with adding/removing the border.
-                        gpui::transparent_black()
-                    })
-                    .when(is_muted, |avatar| {
-                        avatar.indicator(
-                            AvatarAudioStatusIndicator::new(ui::AudioStatus::Muted).tooltip({
-                                let github_login = user.github_login.clone();
-                                move |cx| Tooltip::text(format!("{} is muted", github_login), cx)
-                            }),
+        Some(
+            div()
+                .m_0p5()
+                .p_0p5()
+                // When the collaborator is not followed, still draw this wrapper div, but leave
+                // it transparent, so that it does not shift the layout when following.
+                .when_some(leader_selection_color, |div, color| {
+                    div.rounded_md().bg(color)
+                })
+                .child(
+                    FacePile::default()
+                        .child(
+                            Avatar::new(user.avatar_uri.clone())
+                                .grayscale(!is_present)
+                                .border_color(if is_speaking {
+                                    cx.theme().status().info
+                                } else {
+                                    // We draw the border in a transparent color rather to avoid
+                                    // the layout shift that would come with adding/removing the border.
+                                    gpui::transparent_black()
+                                })
+                                .when(is_muted, |avatar| {
+                                    avatar.indicator(
+                                        AvatarAudioStatusIndicator::new(ui::AudioStatus::Muted)
+                                            .tooltip({
+                                                let github_login = user.github_login.clone();
+                                                move |cx| {
+                                                    Tooltip::text(
+                                                        format!("{} is muted", github_login),
+                                                        cx,
+                                                    )
+                                                }
+                                            }),
+                                    )
+                                }),
                         )
-                    }),
-            )
-            .children(
-                followers
-                    .iter()
-                    .take(FACEPILE_LIMIT)
-                    .filter_map(|follower_peer_id| {
-                        let follower = room
-                            .remote_participants()
-                            .values()
-                            .find_map(|p| (p.peer_id == *follower_peer_id).then_some(&p.user))
-                            .or_else(|| {
-                                (self.client.peer_id() == Some(*follower_peer_id))
-                                    .then_some(current_user)
-                            })?
-                            .clone();
-
-                        Some(Avatar::new(follower.avatar_uri.clone()))
-                    }),
-            )
-            .children(if extra_count > 0 {
-                Some(
-                    div()
-                        .ml_1()
-                        .child(Label::new(format!("+{extra_count}")))
-                        .into_any_element(),
-                )
-            } else {
-                None
-            });
-
-        Some(pile)
+                        .children(followers.iter().take(FACEPILE_LIMIT).filter_map(
+                            |follower_peer_id| {
+                                let follower = room
+                                    .remote_participants()
+                                    .values()
+                                    .find_map(|p| {
+                                        (p.peer_id == *follower_peer_id).then_some(&p.user)
+                                    })
+                                    .or_else(|| {
+                                        (self.client.peer_id() == Some(*follower_peer_id))
+                                            .then_some(current_user)
+                                    })?
+                                    .clone();
+
+                                Some(Avatar::new(follower.avatar_uri.clone()))
+                            },
+                        ))
+                        .children(if extra_count > 0 {
+                            Some(
+                                div()
+                                    .ml_1()
+                                    .child(Label::new(format!("+{extra_count}")))
+                                    .into_any_element(),
+                            )
+                        } else {
+                            None
+                        })
+                        .render(),
+                ),
+        )
     }
 
     fn window_activation_changed(&mut self, cx: &mut ViewContext<Self>) {

crates/collab_ui/src/face_pile.rs 🔗

@@ -1,13 +1,13 @@
-use gpui::{div, AnyElement, IntoElement, ParentElement, RenderOnce, Styled, WindowContext};
+use gpui::{div, AnyElement, Div, IntoElement, ParentElement, Styled};
 use smallvec::SmallVec;
 
-#[derive(Default, IntoElement)]
+#[derive(Default)]
 pub struct FacePile {
     pub faces: SmallVec<[AnyElement; 2]>,
 }
 
-impl RenderOnce for FacePile {
-    fn render(self, _: &mut WindowContext) -> impl IntoElement {
+impl FacePile {
+    pub fn render(self) -> Div {
         let player_count = self.faces.len();
         let player_list = self.faces.into_iter().enumerate().map(|(ix, player)| {
             let isnt_last = ix < player_count - 1;
@@ -17,7 +17,7 @@ impl RenderOnce for FacePile {
                 .when(isnt_last, |div| div.neg_mr_1())
                 .child(player)
         });
-        div().p_1().flex().items_center().children(player_list)
+        div().flex().items_center().children(player_list)
     }
 }
 

script/zed-local 🔗

@@ -5,15 +5,15 @@ USAGE
   zed-local  [options]  [zed args]
 
 SUMMARY
-  Runs 1-4 instances of Zed using a locally-running collaboration server.
+  Runs 1-6 instances of Zed using a locally-running collaboration server.
   Each instance of Zed will be signed in as a different user specified in
   either \`.admins.json\` or \`.admins.default.json\`.
 
 OPTIONS
-  --help        Print this help message
-  --release     Build Zed in release mode
-  -2, -3, -4    Spawn 2, 3, or 4 Zed instances, with their windows tiled.
-  --top         Arrange the Zed windows so they take up the top half of the screen.
+  --help           Print this help message
+  --release        Build Zed in release mode
+  -2, -3, -4, ...  Spawn multiple Zed instances, with their windows tiled.
+  --top            Arrange the Zed windows so they take up the top half of the screen.
 `.trim();
 
 const { spawn, execFileSync } = require("child_process");
@@ -25,7 +25,9 @@ try {
   const customUsers = require("../crates/collab/.admins.json");
   assert(customUsers.length > 0);
   assert(customUsers.every((user) => typeof user === "string"));
-  users.splice(0, 0, ...customUsers);
+  users = customUsers.concat(
+    defaultUsers.filter((user) => !customUsers.includes(user)),
+  );
 } catch (_) {}
 
 const RESOLUTION_REGEX = /(\d+) x (\d+)/;
@@ -69,36 +71,43 @@ const mainDisplayResolution =
 if (!mainDisplayResolution) {
   throw new Error("Could not parse screen resolution");
 }
+const titleBarHeight = 24;
 const screenWidth = parseInt(mainDisplayResolution[1]);
-let screenHeight = parseInt(mainDisplayResolution[2]);
+let screenHeight = parseInt(mainDisplayResolution[2]) - titleBarHeight;
 
 if (isTop) {
   screenHeight = Math.floor(screenHeight / 2);
 }
 
 // Determine the window size for each instance
-let instanceWidth = screenWidth;
-let instanceHeight = screenHeight;
-if (instanceCount > 1) {
-  instanceWidth = Math.floor(screenWidth / 2);
-  if (instanceCount > 2) {
-    instanceHeight = Math.floor(screenHeight / 2);
-  }
+let rows;
+let columns;
+switch (instanceCount) {
+  case 1:
+    [rows, columns] = [1, 1];
+    break;
+  case 2:
+    [rows, columns] = [1, 2];
+    break;
+  case 3:
+  case 4:
+    [rows, columns] = [2, 2];
+    break;
+  case 5:
+  case 6:
+    [rows, columns] = [2, 3];
+    break;
 }
 
+const instanceWidth = Math.floor(screenWidth / columns);
+const instanceHeight = Math.floor(screenHeight / rows);
+
 // If a user is specified, make sure it's first in the list
 const user = process.env.ZED_IMPERSONATE;
 if (user) {
   users = [user].concat(users.filter((u) => u !== user));
 }
 
-const positions = [
-  "0,0",
-  `${instanceWidth},0`,
-  `0,${instanceHeight}`,
-  `${instanceWidth},${instanceHeight}`,
-];
-
 let buildArgs = ["build"];
 let zedBinary = "target/debug/Zed";
 if (isReleaseMode) {
@@ -106,20 +115,33 @@ if (isReleaseMode) {
   zedBinary = "target/release/Zed";
 }
 
-execFileSync("cargo", buildArgs, { stdio: "inherit" });
+try {
+  execFileSync("cargo", buildArgs, { stdio: "inherit" });
+} catch (e) {
+  process.exit(0);
+}
+
 setTimeout(() => {
   for (let i = 0; i < instanceCount; i++) {
+    const row = Math.floor(i / columns);
+    const column = i % columns;
+    const position = [
+      column * instanceWidth,
+      row * instanceHeight + titleBarHeight,
+    ].join(",");
+    const size = [instanceWidth, instanceHeight].join(",");
+
     spawn(zedBinary, i == 0 ? args : [], {
       stdio: "inherit",
       env: {
         ZED_IMPERSONATE: users[i],
-        ZED_WINDOW_POSITION: positions[i],
+        ZED_WINDOW_POSITION: position,
         ZED_STATELESS: "1",
         ZED_ALWAYS_ACTIVE: "1",
         ZED_SERVER_URL: "http://localhost:3000",
         ZED_RPC_URL: "http://localhost:8080/rpc",
         ZED_ADMIN_API_TOKEN: "secret",
-        ZED_WINDOW_SIZE: `${instanceWidth},${instanceHeight}`,
+        ZED_WINDOW_SIZE: size,
         PATH: process.env.PATH,
         RUST_LOG: process.env.RUST_LOG || "info",
       },