Fix pinned tab becoming unpinned when dragged onto unpinned tab (#35539)

Joseph T. Lyons created 3 months ago

Case A - Correct:


https://github.com/user-attachments/assets/2ab943ea-ca5b-4b6b-a8ca-a0b02072293e

Case B - Incorrect:


https://github.com/user-attachments/assets/912be46a-73b2-48a8-b490-277a1e89d17d

Case B - Fixed:


https://github.com/user-attachments/assets/98c2311d-eebc-4091-ad7a-6cf857fda9c3

Release Notes:

- Fixed a bug where dragging a pinned tab onto an unpinned tab wouldn't
decrease the pinned tab count

Change summary

crates/workspace/src/pane.rs | 49 ++++++++++++++++++++++++++++++++++---
1 file changed, 45 insertions(+), 4 deletions(-)

Detailed changes

crates/workspace/src/pane.rs

@@ -3030,7 +3030,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| {
@@ -3062,9 +3062,13 @@ impl Pane {
                     }
                     to_pane.update(cx, |this, _| {
                         if to_pane == from_pane {
-                            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 to_ix = this
+                                .items
+                                .iter()
+                                .position(|item| item.item_id() == item_id)
+                                .unwrap_or(0);
+
+                            let is_pinned_in_to_pane = to_ix < this.pinned_tab_count;
 
                             if !was_pinned_in_from_pane && is_pinned_in_to_pane {
                                 this.pinned_tab_count += 1;
@@ -4950,6 +4954,43 @@ mod tests {
         assert_item_labels(&pane_a, ["B!", "A*!"], cx);
     }
 
+    #[gpui::test]
+    async fn test_dragging_pinned_tab_onto_unpinned_tab_reduces_unpinned_tab_count(
+        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 on top of B in the same pane, which changes tab order to B, A
+        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);
+        });
+
+        // Neither are 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,