From 53abad5979cc8a7ec7483290bd62183722002904 Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Fri, 6 Jun 2025 05:09:48 -0400 Subject: [PATCH] Fixed more bugs around moving pinned tabs (#32228) Closes https://github.com/zed-industries/zed/issues/32199 https://github.com/zed-industries/zed/issues/32229 https://github.com/zed-industries/zed/issues/32230 https://github.com/zed-industries/zed/issues/32232 Release Notes: - Fixed a bug where if the last tab was a pinned tab and it was dragged to the right, resulting in a no-op, it would become unpinned - Fixed a bug where a pinned tab dragged just to the right of the end of the pinned tab region would become unpinned - Fixed a bug where dragging a pinned tab from one pane to another pane's pinned region could result in an existing pinned tab becoming unpinned when `max_tabs` was reached - Fixed a bug where moving an unpinned tab to the left, just to the end of the pinned region, would cause the pinned tabs to become unpinned. --- crates/workspace/src/pane.rs | 272 ++++++++++++++++++++++++++++++++++- 1 file changed, 265 insertions(+), 7 deletions(-) diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 685d0e6044..2fc87be2be 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -2889,6 +2889,7 @@ impl Pane { || cfg!(not(target_os = "macos")) && window.modifiers().control; let from_pane = dragged_tab.pane.clone(); + let from_ix = dragged_tab.ix; self.workspace .update(cx, |_, cx| { cx.defer_in(window, move |workspace, window, cx| { @@ -2919,15 +2920,26 @@ impl Pane { move_item(&from_pane, &to_pane, item_id, ix, true, window, cx); } to_pane.update(cx, |this, _| { - let is_pinned_in_to_pane = this.is_tab_pinned(ix); - if to_pane == from_pane { - if was_pinned_in_from_pane && !is_pinned_in_to_pane { - this.pinned_tab_count -= 1; - } else if !was_pinned_in_from_pane && is_pinned_in_to_pane { - this.pinned_tab_count += 1; + let moved_right = ix > from_ix; + let ix = if moved_right { ix - 1 } else { ix }; + let is_pinned_in_to_pane = this.is_tab_pinned(ix); + let is_at_same_position = ix == from_ix; + + if is_at_same_position + || (moved_right && is_pinned_in_to_pane) + || (!moved_right && !is_pinned_in_to_pane) + { + return; } - } else if this.items.len() > to_pane_old_length { + + if is_pinned_in_to_pane { + this.pinned_tab_count += 1; + } else { + this.pinned_tab_count -= 1; + } + } else if this.items.len() >= to_pane_old_length { + let is_pinned_in_to_pane = this.is_tab_pinned(ix); let item_created_pane = to_pane_old_length == 0; let is_first_position = ix == 0; let was_dropped_at_beginning = item_created_pane || is_first_position; @@ -4547,6 +4559,252 @@ mod tests { assert_item_labels(&pane_b, ["A*!", "B"], cx); } + #[gpui::test] + async fn test_drag_pinned_tab_into_existing_pane_at_max_capacity_closes_unpinned_tabs( + 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_a = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone()); + set_max_tabs(cx, Some(2)); + + // Add A, B to pane A. Pin both + let item_a = add_labeled_item(&pane_a, "A", false, cx); + let item_b = add_labeled_item(&pane_a, "B", false, cx); + pane_a.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); + + let ix = pane.index_for_item_id(item_b.item_id()).unwrap(); + pane.pin_tab_at(ix, window, cx); + }); + assert_item_labels(&pane_a, ["A!", "B*!"], cx); + + // Add C, D to pane B. Pin both + let pane_b = workspace.update_in(cx, |workspace, window, cx| { + workspace.split_pane(pane_a.clone(), SplitDirection::Right, window, cx) + }); + let item_c = add_labeled_item(&pane_b, "C", false, cx); + let item_d = add_labeled_item(&pane_b, "D", false, cx); + pane_b.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); + + let ix = pane.index_for_item_id(item_d.item_id()).unwrap(); + pane.pin_tab_at(ix, window, cx); + }); + assert_item_labels(&pane_b, ["C!", "D*!"], cx); + + // Add a third unpinned item to pane B (exceeds max tabs), but is allowed, + // as we allow 1 tab over max if the others are pinned or dirty + add_labeled_item(&pane_b, "E", false, cx); + assert_item_labels(&pane_b, ["C!", "D!", "E*"], cx); + + // Drag pinned A from pane A to position 0 in pane B + pane_b.update_in(cx, |pane, window, cx| { + let dragged_tab = DraggedTab { + pane: pane_a.clone(), + item: item_a.boxed_clone(), + ix: 0, + detail: 0, + is_active: true, + }; + pane.handle_tab_drop(&dragged_tab, 0, window, cx); + }); + + // E (unpinned) should be closed, leaving 3 pinned items + assert_item_labels(&pane_a, ["B*!"], cx); + assert_item_labels(&pane_b, ["A*!", "C!", "D!"], cx); + } + + #[gpui::test] + async fn test_drag_last_pinned_tab_to_same_position_stays_pinned(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_a = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone()); + + // Add A to pane A and pin it + let item_a = add_labeled_item(&pane_a, "A", false, cx); + pane_a.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, ["A*!"], cx); + + // Drag pinned A to position 1 (directly to the right) in the same pane + pane_a.update_in(cx, |pane, window, cx| { + let dragged_tab = DraggedTab { + pane: pane_a.clone(), + item: item_a.boxed_clone(), + ix: 0, + detail: 0, + is_active: true, + }; + pane.handle_tab_drop(&dragged_tab, 1, window, cx); + }); + + // A should still be pinned and active + assert_item_labels(&pane_a, ["A*!"], cx); + } + + #[gpui::test] + async fn test_drag_pinned_tab_beyond_last_pinned_tab_in_same_pane_stays_pinned( + 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_a = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone()); + + // Add A, B to pane A and pin both + let item_a = add_labeled_item(&pane_a, "A", false, cx); + let item_b = add_labeled_item(&pane_a, "B", false, cx); + pane_a.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); + + let ix = pane.index_for_item_id(item_b.item_id()).unwrap(); + pane.pin_tab_at(ix, window, cx); + }); + assert_item_labels(&pane_a, ["A!", "B*!"], cx); + + // Drag pinned A right of B in the same pane + pane_a.update_in(cx, |pane, window, cx| { + let dragged_tab = DraggedTab { + pane: pane_a.clone(), + item: item_a.boxed_clone(), + ix: 0, + detail: 0, + is_active: true, + }; + pane.handle_tab_drop(&dragged_tab, 2, window, cx); + }); + + // A stays pinned + assert_item_labels(&pane_a, ["B!", "A*!"], cx); + } + + #[gpui::test] + async fn test_drag_pinned_tab_beyond_unpinned_tab_in_same_pane_becomes_unpinned( + 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_a = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone()); + + // Add A, B to pane A and pin A + let item_a = add_labeled_item(&pane_a, "A", false, cx); + add_labeled_item(&pane_a, "B", false, cx); + pane_a.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, ["A!", "B*"], cx); + + // Drag pinned A right of B in the same pane + pane_a.update_in(cx, |pane, window, cx| { + let dragged_tab = DraggedTab { + pane: pane_a.clone(), + item: item_a.boxed_clone(), + ix: 0, + detail: 0, + is_active: true, + }; + pane.handle_tab_drop(&dragged_tab, 2, window, cx); + }); + + // A becomes unpinned + assert_item_labels(&pane_a, ["B", "A*"], cx); + } + + #[gpui::test] + async fn test_drag_unpinned_tab_in_front_of_pinned_tab_in_same_pane_becomes_pinned( + 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_a = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone()); + + // Add A, B to pane A and pin A + let item_a = add_labeled_item(&pane_a, "A", false, cx); + let item_b = add_labeled_item(&pane_a, "B", false, cx); + pane_a.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, ["A!", "B*"], cx); + + // Drag pinned B left of A in the same pane + pane_a.update_in(cx, |pane, window, cx| { + let dragged_tab = DraggedTab { + pane: pane_a.clone(), + item: item_b.boxed_clone(), + ix: 1, + detail: 0, + is_active: true, + }; + pane.handle_tab_drop(&dragged_tab, 0, window, cx); + }); + + // A becomes unpinned + assert_item_labels(&pane_a, ["B*!", "A!"], cx); + } + + #[gpui::test] + async fn test_drag_unpinned_tab_to_the_pinned_region_stays_pinned(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_a = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone()); + + // Add A, B, C to pane A and pin A + let item_a = add_labeled_item(&pane_a, "A", false, cx); + add_labeled_item(&pane_a, "B", false, cx); + let item_c = add_labeled_item(&pane_a, "C", false, cx); + pane_a.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, ["A!", "B", "C*"], cx); + + // Drag pinned C left of B in the same pane + pane_a.update_in(cx, |pane, window, cx| { + let dragged_tab = DraggedTab { + pane: pane_a.clone(), + item: item_c.boxed_clone(), + ix: 2, + detail: 0, + is_active: true, + }; + pane.handle_tab_drop(&dragged_tab, 1, window, cx); + }); + + // A stays pinned, B and C remain unpinned + assert_item_labels(&pane_a, ["A!", "C*", "B"], cx); + } + #[gpui::test] async fn test_drag_unpinned_tab_into_existing_panes_pinned_region(cx: &mut TestAppContext) { init_test(cx);