diff --git a/crates/tab_switcher/src/tab_switcher.rs b/crates/tab_switcher/src/tab_switcher.rs index 87a39f73080ecc4260f825394ccad25a75fe9baf..6291e69b423b851e42b9396beb547a95f5ad1c9f 100644 --- a/crates/tab_switcher/src/tab_switcher.rs +++ b/crates/tab_switcher/src/tab_switcher.rs @@ -185,11 +185,7 @@ impl TabSwitcherDelegate { PaneEvent::AddItem { .. } | PaneEvent::RemovedItem { .. } | PaneEvent::Remove { .. } => tab_switcher.picker.update(cx, |picker, cx| { - let selected_item_id = picker.delegate.selected_item_id(); picker.delegate.update_matches(cx); - if let Some(item_id) = selected_item_id { - picker.delegate.select_item(item_id, cx); - } cx.notify(); }), _ => {} @@ -199,6 +195,7 @@ impl TabSwitcherDelegate { } fn update_matches(&mut self, cx: &mut WindowContext) { + let selected_item_id = self.selected_item_id(); self.matches.clear(); let Some(pane) = self.pane.upgrade() else { return; @@ -236,13 +233,7 @@ impl TabSwitcherDelegate { a_score.cmp(&b_score) }); - if self.matches.len() > 1 { - if self.select_last { - self.selected_index = self.matches.len() - 1; - } else { - self.selected_index = 1; - } - } + self.selected_index = self.compute_selected_index(selected_item_id); } fn selected_item_id(&self) -> Option { @@ -251,17 +242,34 @@ impl TabSwitcherDelegate { .map(|tab_match| tab_match.item.item_id()) } - fn select_item( - &mut self, - item_id: EntityId, - cx: &mut ViewContext>, - ) { - let selected_idx = self - .matches - .iter() - .position(|tab_match| tab_match.item.item_id() == item_id) - .unwrap_or(0); - self.set_selected_index(selected_idx, cx); + fn compute_selected_index(&mut self, prev_selected_item_id: Option) -> usize { + if self.matches.is_empty() { + return 0; + } + + if let Some(selected_item_id) = prev_selected_item_id { + // If the previously selected item is still in the list, select its new position. + if let Some(item_index) = self + .matches + .iter() + .position(|tab_match| tab_match.item.item_id() == selected_item_id) + { + return item_index; + } + // Otherwise, try to preserve the previously selected index. + return self.selected_index.min(self.matches.len() - 1); + } + + if self.select_last { + return self.matches.len() - 1; + } + + if self.matches.len() > 1 { + // Index 0 is active, so don't preselect it for switching. + return 1; + } + + 0 } fn close_item_at(&mut self, ix: usize, cx: &mut ViewContext>) { diff --git a/crates/tab_switcher/src/tab_switcher_tests.rs b/crates/tab_switcher/src/tab_switcher_tests.rs index c4cb9d8ecf27e2f72efd7497fea77fa9dd50f31a..1ca4c446019076a629129bf9636964fc41fcd5a4 100644 --- a/crates/tab_switcher/src/tab_switcher_tests.rs +++ b/crates/tab_switcher/src/tab_switcher_tests.rs @@ -228,6 +228,53 @@ async fn test_close_selected_item(cx: &mut gpui::TestAppContext) { assert_tab_switcher_is_closed(workspace, cx); } +#[gpui::test] +async fn test_close_preserves_selected_position(cx: &mut gpui::TestAppContext) { + let app_state = init_test(cx); + app_state + .fs + .as_fake() + .insert_tree( + "/root", + json!({ + "1.txt": "First file", + "2.txt": "Second file", + "3.txt": "Third file", + }), + ) + .await; + + let project = Project::test(app_state.fs.clone(), ["/root".as_ref()], cx).await; + let (workspace, cx) = cx.add_window_view(|cx| Workspace::test_new(project.clone(), cx)); + + let tab_1 = open_buffer("1.txt", &workspace, cx).await; + let tab_2 = open_buffer("2.txt", &workspace, cx).await; + let tab_3 = open_buffer("3.txt", &workspace, cx).await; + + let tab_switcher = open_tab_switcher(false, &workspace, cx); + tab_switcher.update(cx, |tab_switcher, _| { + assert_eq!(tab_switcher.delegate.matches.len(), 3); + assert_match_at_position(tab_switcher, 0, tab_3.boxed_clone()); + assert_match_selection(tab_switcher, 1, tab_2.boxed_clone()); + assert_match_at_position(tab_switcher, 2, tab_1.boxed_clone()); + }); + + // Verify that if the selected tab was closed, tab at the same position is selected. + cx.dispatch_action(CloseSelectedItem); + tab_switcher.update(cx, |tab_switcher, _| { + assert_eq!(tab_switcher.delegate.matches.len(), 2); + assert_match_at_position(tab_switcher, 0, tab_3.boxed_clone()); + assert_match_selection(tab_switcher, 1, tab_1.boxed_clone()); + }); + + // But if the position is no longer valid, fall back to the position above. + cx.dispatch_action(CloseSelectedItem); + tab_switcher.update(cx, |tab_switcher, _| { + assert_eq!(tab_switcher.delegate.matches.len(), 1); + assert_match_selection(tab_switcher, 0, tab_3.boxed_clone()); + }); +} + fn init_test(cx: &mut TestAppContext) -> Arc { cx.update(|cx| { let state = AppState::test(cx);