diff --git a/crates/debugger_ui/src/session/running.rs b/crates/debugger_ui/src/session/running.rs index 3151feeba4..ea6536e0d9 100644 --- a/crates/debugger_ui/src/session/running.rs +++ b/crates/debugger_ui/src/session/running.rs @@ -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, ); diff --git a/crates/terminal_view/src/terminal_panel.rs b/crates/terminal_view/src/terminal_panel.rs index 355e3328cf..dc9313a38f 100644 --- a/crates/terminal_view/src/terminal_panel.rs +++ b/crates/terminal_view/src/terminal_panel.rs @@ -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, ); diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 23d9f2cbf0..9fad4e8a5d 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -402,6 +402,12 @@ pub enum Side { Right, } +#[derive(Copy, Clone)] +enum PinOperation { + Pin, + Unpin, +} + impl Pane { pub fn new( workspace: WeakEntity, @@ -2099,53 +2105,66 @@ impl Pane { } fn pin_tab_at(&mut self, ix: usize, window: &mut Window, cx: &mut Context) { + 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.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, + ) { maybe!({ let pane = cx.entity().clone(); - let destination_index = self.pinned_tab_count.min(ix); - self.pinned_tab_count += 1; - let id = self.item_for_index(ix)?.item_id(); - if self.is_active_preview_item(id) { + 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 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(()) - }); - } + let event = match operation { + PinOperation::Pin => Event::ItemPinned, + PinOperation::Unpin => Event::ItemUnpinned, + }; - fn unpin_tab_at(&mut self, ix: usize, window: &mut Window, cx: &mut Context) { - 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(); - - 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); diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 86a6a3dadb..e16cf0038a 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -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, 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)) + } }); }