Skip to content

Commit

Permalink
Fix following bugs (#3688)
Browse files Browse the repository at this point in the history 
* Follow command didn't work, because follow task was dropped
* An extra div prevented titlebar facepiles from rendering correctly
  • Loading branch information
@maxbrunsfeld
maxbrunsfeld authored Dec 16, 2023
2 parents 54eb452 + b5ae2f0 commit 29c6061
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 102 deletions.
4 changes: 3 additions & 1 deletion crates/collab2/src/tests/channel_buffer_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,9 @@ async fn test_following_to_channel_notes_without_a_shared_project(
// Client B follows client A.
workspace_b
.update(cx_b, |workspace, cx| {
workspace.follow(client_a.peer_id().unwrap(), cx).unwrap()
workspace
.start_following(client_a.peer_id().unwrap(), cx)
.unwrap()
})
.await
.unwrap();
Expand Down
51 changes: 24 additions & 27 deletions crates/collab_ui2/src/collab_titlebar_item.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -401,33 +401,30 @@ impl CollabTitlebarItem {
) -> Option<FacePile> {
let followers = project_id.map_or(&[] as &[_], |id| room.followers_for(peer_id, id));

let pile = FacePile::default().child(
div()
.child(
Avatar::new(user.avatar_uri.clone())
.grayscale(!is_present)
.border_color(if is_speaking {
gpui::blue()
} else if is_muted {
gpui::red()
} else {
Hsla::default()
}),
)
.children(followers.iter().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(div().child(Avatar::new(follower.avatar_uri.clone())))
})),
);
let pile = FacePile::default()
.child(
Avatar::new(user.avatar_uri.clone())
.grayscale(!is_present)
.border_color(if is_speaking {
gpui::blue()
} else if is_muted {
gpui::red()
} else {
Hsla::default()
}),
)
.children(followers.iter().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()))
}));

Some(pile)
}
Expand Down
124 changes: 50 additions & 74 deletions crates/workspace2/src/workspace2.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2292,7 +2292,7 @@ impl Workspace {
cx.notify();
}

fn start_following(
pub fn start_following(
&mut self,
leader_id: PeerId,
cx: &mut ViewContext<Self>,
Expand Down Expand Up @@ -2347,57 +2347,55 @@ impl Workspace {
}))
}

// pub fn follow_next_collaborator(
// &mut self,
// _: &FollowNextCollaborator,
// cx: &mut ViewContext<Self>,
// ) {
// let collaborators = self.project.read(cx).collaborators();
// let next_leader_id = if let Some(leader_id) = self.leader_for_pane(&self.active_pane) {
// let mut collaborators = collaborators.keys().copied();
// for peer_id in collaborators.by_ref() {
// if peer_id == leader_id {
// break;
// }
// }
// collaborators.next()
// } else if let Some(last_leader_id) =
// self.last_leaders_by_pane.get(&self.active_pane.downgrade())
// {
// if collaborators.contains_key(last_leader_id) {
// Some(*last_leader_id)
// } else {
// None
// }
// } else {
// None
// };

// let pane = self.active_pane.clone();
// let Some(leader_id) = next_leader_id.or_else(|| collaborators.keys().copied().next())
// else {
// return;
// };
// if Some(leader_id) == self.unfollow(&pane, cx) {
// return;
// }
// if let Some(task) = self.follow(leader_id, cx) {
// task.detach();
// }
// }

pub fn follow(
pub fn follow_next_collaborator(
&mut self,
leader_id: PeerId,
_: &FollowNextCollaborator,
cx: &mut ViewContext<Self>,
) -> Option<Task<Result<()>>> {
let room = ActiveCall::global(cx).read(cx).room()?.read(cx);
let project = self.project.read(cx);
) {
let collaborators = self.project.read(cx).collaborators();
let next_leader_id = if let Some(leader_id) = self.leader_for_pane(&self.active_pane) {
let mut collaborators = collaborators.keys().copied();
for peer_id in collaborators.by_ref() {
if peer_id == leader_id {
break;
}
}
collaborators.next()
} else if let Some(last_leader_id) =
self.last_leaders_by_pane.get(&self.active_pane.downgrade())
{
if collaborators.contains_key(last_leader_id) {
Some(*last_leader_id)
} else {
None
}
} else {
None
};

let pane = self.active_pane.clone();
let Some(leader_id) = next_leader_id.or_else(|| collaborators.keys().copied().next())
else {
return;
};
if Some(leader_id) == self.unfollow(&pane, cx) {
return;
}
self.start_following(leader_id, cx)
.map(|task| task.detach_and_log_err(cx));
}

pub fn follow(&mut self, leader_id: PeerId, cx: &mut ViewContext<Self>) {
let Some(room) = ActiveCall::global(cx).read(cx).room() else {
return;
};
let room = room.read(cx);
let Some(remote_participant) = room.remote_participant_for_peer_id(leader_id) else {
return None;
return;
};

let project = self.project.read(cx);

let other_project_id = match remote_participant.location {
call::ParticipantLocation::External => None,
call::ParticipantLocation::UnsharedProject => None,
Expand All @@ -2413,38 +2411,23 @@ impl Workspace {
// if they are active in another project, follow there.
if let Some(project_id) = other_project_id {
let app_state = self.app_state.clone();
return Some(crate::join_remote_project(
project_id,
remote_participant.user.id,
app_state,
cx,
));
crate::join_remote_project(project_id, remote_participant.user.id, app_state, cx)
.detach_and_log_err(cx);
}

// if you're already following, find the right pane and focus it.
for (pane, state) in &self.follower_states {
if leader_id == state.leader_id {
cx.focus_view(pane);
return None;
return;
}
}

// Otherwise, follow.
self.start_following(leader_id, cx)
.map(|task| task.detach_and_log_err(cx));
}

// // if you're already following, find the right pane and focus it.
// for (pane, state) in &self.follower_states {
// if leader_id == state.leader_id {
// cx.focus(pane);
// return None;
// }
// }

// // Otherwise, follow.
// self.start_following(leader_id, cx)
// }

pub fn unfollow(&mut self, pane: &View<Pane>, cx: &mut ViewContext<Self>) -> Option<PeerId> {
let state = self.follower_states.remove(pane)?;
let leader_id = state.leader_id;
Expand Down Expand Up @@ -2855,12 +2838,6 @@ impl Workspace {
if leader_in_this_project || !item.is_project_item(cx) {
items_to_activate.push((pane.clone(), item.boxed_clone()));
}
} else {
log::warn!(
"unknown view id {:?} for leader {:?}",
active_view_id,
leader_id
);
}
continue;
}
Expand Down Expand Up @@ -3255,6 +3232,7 @@ impl Workspace {
.on_action(cx.listener(Self::close_all_items_and_panes))
.on_action(cx.listener(Self::save_all))
.on_action(cx.listener(Self::add_folder_to_project))
.on_action(cx.listener(Self::follow_next_collaborator))
.on_action(cx.listener(|workspace, _: &Unfollow, cx| {
let pane = workspace.active_pane().clone();
workspace.unfollow(&pane, cx);
Expand Down Expand Up @@ -4240,9 +4218,7 @@ pub fn join_remote_project(
});

if let Some(follow_peer_id) = follow_peer_id {
workspace
.follow(follow_peer_id, cx)
.map(|follow| follow.detach_and_log_err(cx));
workspace.follow(follow_peer_id, cx);
}
}
})?;
Expand Down

0 comments on commit 29c6061

Please sign in to comment.