Joseph T. Lyons created
Closes https://github.com/zed-industries/zed/issues/32024
Release Notes:
- Fixed a bug where inactive tabs would be activated when pinning or
unpinning.crates/debugger_ui/src/session/running.rs | 1
crates/terminal_view/src/terminal_panel.rs | 1
crates/workspace/src/pane.rs | 226 ++++++++++++++++++++---
crates/workspace/src/workspace.rs | 17 +
4 files changed, 211 insertions(+), 34 deletions(-)
@@ -286,6 +286,7 @@ pub(crate) fn new_debugger_pane(
&new_pane,
item_id_to_move,
new_pane.read(cx).active_item_index(),
+ true,
window,
cx,
);
@@ -1062,6 +1062,7 @@ pub fn new_terminal_pane(
&new_pane,
item_id_to_move,
new_pane.read(cx).active_item_index(),
+ true,
window,
cx,
);
@@ -402,6 +402,12 @@ pub enum Side {
Right,
}
+#[derive(Copy, Clone)]
+enum PinOperation {
+ Pin,
+ Unpin,
+}
+
impl Pane {
pub fn new(
workspace: WeakEntity<Workspace>,
@@ -2099,53 +2105,66 @@ impl Pane {
}
fn pin_tab_at(&mut self, ix: usize, window: &mut Window, cx: &mut Context<Self>) {
+ self.change_tab_pin_state(ix, PinOperation::Pin, window, cx);
+ }
+
+ fn unpin_tab_at(&mut self, ix: usize, window: &mut Window, cx: &mut Context<Self>) {
+ self.change_tab_pin_state(ix, PinOperation::Unpin, window, cx);
+ }
+
+ fn change_tab_pin_state(
+ &mut self,
+ ix: usize,
+ operation: PinOperation,
+ window: &mut Window,
+ cx: &mut Context<Self>,
+ ) {
maybe!({
let pane = cx.entity().clone();
- let destination_index = self.pinned_tab_count.min(ix);
- self.pinned_tab_count += 1;
+
+ let destination_index = match operation {
+ PinOperation::Pin => self.pinned_tab_count.min(ix),
+ PinOperation::Unpin => self.pinned_tab_count.checked_sub(1)?,
+ };
+
let id = self.item_for_index(ix)?.item_id();
+ let should_activate = ix == self.active_item_index;
- if self.is_active_preview_item(id) {
+ if matches!(operation, PinOperation::Pin) && self.is_active_preview_item(id) {
self.set_preview_item_id(None, cx);
}
+ match operation {
+ PinOperation::Pin => self.pinned_tab_count += 1,
+ PinOperation::Unpin => self.pinned_tab_count -= 1,
+ }
+
if ix == destination_index {
cx.notify();
} else {
self.workspace
.update(cx, |_, cx| {
cx.defer_in(window, move |_, window, cx| {
- move_item(&pane, &pane, id, destination_index, window, cx)
+ move_item(
+ &pane,
+ &pane,
+ id,
+ destination_index,
+ should_activate,
+ window,
+ cx,
+ );
});
})
.ok()?;
}
- cx.emit(Event::ItemPinned);
- Some(())
- });
- }
-
- fn unpin_tab_at(&mut self, ix: usize, window: &mut Window, cx: &mut Context<Self>) {
- maybe!({
- let pane = cx.entity().clone();
- self.pinned_tab_count = self.pinned_tab_count.checked_sub(1)?;
- let destination_index = self.pinned_tab_count;
-
- let id = self.item_for_index(ix)?.item_id();
+ let event = match operation {
+ PinOperation::Pin => Event::ItemPinned,
+ PinOperation::Unpin => Event::ItemUnpinned,
+ };
- if ix == destination_index {
- cx.notify()
- } else {
- self.workspace
- .update(cx, |_, cx| {
- cx.defer_in(window, move |_, window, cx| {
- move_item(&pane, &pane, id, destination_index, window, cx)
- });
- })
- .ok()?;
- }
- cx.emit(Event::ItemUnpinned);
+ cx.emit(event);
Some(())
});
@@ -2898,7 +2917,7 @@ impl Pane {
})
}
} else {
- move_item(&from_pane, &to_pane, item_id, ix, window, cx);
+ move_item(&from_pane, &to_pane, item_id, ix, true, window, cx);
}
if to_pane == from_pane {
if let Some(old_index) = old_ix {
@@ -4006,13 +4025,13 @@ mod tests {
let ix = pane.index_for_item_id(item_b.item_id()).unwrap();
pane.pin_tab_at(ix, window, cx);
});
- assert_item_labels(&pane, ["C!", "B*!", "A"], cx);
+ assert_item_labels(&pane, ["C*!", "B!", "A"], cx);
pane.update_in(cx, |pane, window, cx| {
let ix = pane.index_for_item_id(item_a.item_id()).unwrap();
pane.pin_tab_at(ix, window, cx);
});
- assert_item_labels(&pane, ["C!", "B*!", "A!"], cx);
+ assert_item_labels(&pane, ["C*!", "B!", "A!"], cx);
}
#[gpui::test]
@@ -4161,6 +4180,151 @@ mod tests {
assert_item_labels(&pane, ["B*", "A", "C"], cx);
}
+ #[gpui::test]
+ async fn test_pinning_active_tab_without_position_change_maintains_focus(
+ cx: &mut TestAppContext,
+ ) {
+ init_test(cx);
+ let fs = FakeFs::new(cx.executor());
+
+ let project = Project::test(fs, None, cx).await;
+ let (workspace, cx) =
+ cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx));
+ let pane = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone());
+
+ // Add A
+ let item_a = add_labeled_item(&pane, "A", false, cx);
+ assert_item_labels(&pane, ["A*"], cx);
+
+ // Add B
+ add_labeled_item(&pane, "B", false, cx);
+ assert_item_labels(&pane, ["A", "B*"], cx);
+
+ // Activate A again
+ pane.update_in(cx, |pane, window, cx| {
+ let ix = pane.index_for_item_id(item_a.item_id()).unwrap();
+ pane.activate_item(ix, true, true, window, cx);
+ });
+ assert_item_labels(&pane, ["A*", "B"], cx);
+
+ // Pin A - remains active
+ pane.update_in(cx, |pane, window, cx| {
+ let ix = pane.index_for_item_id(item_a.item_id()).unwrap();
+ pane.pin_tab_at(ix, window, cx);
+ });
+ assert_item_labels(&pane, ["A*!", "B"], cx);
+
+ // Unpin A - remain active
+ pane.update_in(cx, |pane, window, cx| {
+ let ix = pane.index_for_item_id(item_a.item_id()).unwrap();
+ pane.unpin_tab_at(ix, window, cx);
+ });
+ assert_item_labels(&pane, ["A*", "B"], cx);
+ }
+
+ #[gpui::test]
+ async fn test_pinning_active_tab_with_position_change_maintains_focus(cx: &mut TestAppContext) {
+ init_test(cx);
+ let fs = FakeFs::new(cx.executor());
+
+ let project = Project::test(fs, None, cx).await;
+ let (workspace, cx) =
+ cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx));
+ let pane = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone());
+
+ // Add A, B, C
+ add_labeled_item(&pane, "A", false, cx);
+ add_labeled_item(&pane, "B", false, cx);
+ let item_c = add_labeled_item(&pane, "C", false, cx);
+ assert_item_labels(&pane, ["A", "B", "C*"], cx);
+
+ // Pin C - moves to pinned area, remains active
+ pane.update_in(cx, |pane, window, cx| {
+ let ix = pane.index_for_item_id(item_c.item_id()).unwrap();
+ pane.pin_tab_at(ix, window, cx);
+ });
+ assert_item_labels(&pane, ["C*!", "A", "B"], cx);
+
+ // Unpin C - moves after pinned area, remains active
+ pane.update_in(cx, |pane, window, cx| {
+ let ix = pane.index_for_item_id(item_c.item_id()).unwrap();
+ pane.unpin_tab_at(ix, window, cx);
+ });
+ assert_item_labels(&pane, ["C*", "A", "B"], cx);
+ }
+
+ #[gpui::test]
+ async fn test_pinning_inactive_tab_without_position_change_preserves_existing_focus(
+ cx: &mut TestAppContext,
+ ) {
+ init_test(cx);
+ let fs = FakeFs::new(cx.executor());
+
+ let project = Project::test(fs, None, cx).await;
+ let (workspace, cx) =
+ cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx));
+ let pane = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone());
+
+ // Add A, B
+ let item_a = add_labeled_item(&pane, "A", false, cx);
+ add_labeled_item(&pane, "B", false, cx);
+ assert_item_labels(&pane, ["A", "B*"], cx);
+
+ // Pin A - already in pinned area, B remains active
+ pane.update_in(cx, |pane, window, cx| {
+ let ix = pane.index_for_item_id(item_a.item_id()).unwrap();
+ pane.pin_tab_at(ix, window, cx);
+ });
+ assert_item_labels(&pane, ["A!", "B*"], cx);
+
+ // Unpin A - stays in place, B remains active
+ pane.update_in(cx, |pane, window, cx| {
+ let ix = pane.index_for_item_id(item_a.item_id()).unwrap();
+ pane.unpin_tab_at(ix, window, cx);
+ });
+ assert_item_labels(&pane, ["A", "B*"], cx);
+ }
+
+ #[gpui::test]
+ async fn test_pinning_inactive_tab_with_position_change_preserves_existing_focus(
+ cx: &mut TestAppContext,
+ ) {
+ init_test(cx);
+ let fs = FakeFs::new(cx.executor());
+
+ let project = Project::test(fs, None, cx).await;
+ let (workspace, cx) =
+ cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx));
+ let pane = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone());
+
+ // Add A, B, C
+ add_labeled_item(&pane, "A", false, cx);
+ let item_b = add_labeled_item(&pane, "B", false, cx);
+ let item_c = add_labeled_item(&pane, "C", false, cx);
+ assert_item_labels(&pane, ["A", "B", "C*"], cx);
+
+ // Activate B
+ pane.update_in(cx, |pane, window, cx| {
+ let ix = pane.index_for_item_id(item_b.item_id()).unwrap();
+ pane.activate_item(ix, true, true, window, cx);
+ });
+ assert_item_labels(&pane, ["A", "B*", "C"], cx);
+
+ // Pin C - moves to pinned area, B remains active
+ pane.update_in(cx, |pane, window, cx| {
+ let ix = pane.index_for_item_id(item_c.item_id()).unwrap();
+ pane.pin_tab_at(ix, window, cx);
+ });
+ assert_item_labels(&pane, ["C!", "A", "B*"], cx);
+
+ // Unpin C - moves after pinned area, B remains active
+ pane.update_in(cx, |pane, window, cx| {
+ let ix = pane.index_for_item_id(item_c.item_id()).unwrap();
+ pane.unpin_tab_at(ix, window, cx);
+ });
+ assert_item_labels(&pane, ["C", "A", "B*"], cx);
+ }
+
#[gpui::test]
async fn test_add_item_with_new_item(cx: &mut TestAppContext) {
init_test(cx);
@@ -3820,7 +3820,7 @@ impl Workspace {
};
let new_pane = self.add_pane(window, cx);
- move_item(&from, &new_pane, item_id_to_move, 0, window, cx);
+ move_item(&from, &new_pane, item_id_to_move, 0, true, window, cx);
self.center
.split(&pane_to_split, &new_pane, split_direction)
.unwrap();
@@ -7515,6 +7515,7 @@ pub fn move_item(
destination: &Entity<Pane>,
item_id_to_move: EntityId,
destination_index: usize,
+ activate: bool,
window: &mut Window,
cx: &mut App,
) {
@@ -7538,8 +7539,18 @@ pub fn move_item(
// This automatically removes duplicate items in the pane
destination.update(cx, |destination, cx| {
- destination.add_item(item_handle, true, true, Some(destination_index), window, cx);
- window.focus(&destination.focus_handle(cx))
+ destination.add_item_inner(
+ item_handle,
+ activate,
+ activate,
+ activate,
+ Some(destination_index),
+ window,
+ cx,
+ );
+ if activate {
+ window.focus(&destination.focus_handle(cx))
+ }
});
}