tab_switcher: Add tab close buttons (#9968)

Andrew Lygin created

Support for closing tabs from Tab Switcher:

- Close button color matches the indicator color to preserve the
information that the buffer is dirty (as in SublimeText).
- `ctrl-backspace` closes the currently selected item.


https://github.com/zed-industries/zed/assets/2101250/8ea33911-2f62-4199-826d-c17556db8e9a

Release Notes:

- N/A

Change summary

assets/keymaps/default-linux.json             |   5 
assets/keymaps/default-macos.json             |   5 
crates/picker/src/picker.rs                   |   7 +
crates/tab_switcher/src/tab_switcher.rs       | 105 +++++++++++++++++++-
crates/tab_switcher/src/tab_switcher_tests.rs |  45 +++++++++
crates/ui/src/components/indicator.rs         |   2 
6 files changed, 156 insertions(+), 13 deletions(-)

Detailed changes

assets/keymaps/default-linux.json 🔗

@@ -598,7 +598,10 @@
   },
   {
     "context": "TabSwitcher",
-    "bindings": { "ctrl-shift-tab": "menu::SelectPrev" }
+    "bindings": {
+      "ctrl-shift-tab": "menu::SelectPrev",
+      "ctrl-backspace": "tab_switcher::CloseSelectedItem"
+    }
   },
   {
     "context": "Terminal",

assets/keymaps/default-macos.json 🔗

@@ -606,7 +606,10 @@
   },
   {
     "context": "TabSwitcher",
-    "bindings": { "ctrl-shift-tab": "menu::SelectPrev" }
+    "bindings": {
+      "ctrl-shift-tab": "menu::SelectPrev",
+      "ctrl-backspace": "tab_switcher::CloseSelectedItem"
+    }
   },
   {
     "context": "Terminal",

crates/picker/src/picker.rs 🔗

@@ -61,6 +61,9 @@ pub trait PickerDelegate: Sized + 'static {
     fn set_selected_index(&mut self, ix: usize, cx: &mut ViewContext<Picker<Self>>);
 
     fn placeholder_text(&self, _cx: &mut WindowContext) -> Arc<str>;
+    fn no_matches_text(&self, _cx: &mut WindowContext) -> SharedString {
+        "No matches".into()
+    }
     fn update_matches(&mut self, query: String, cx: &mut ViewContext<Picker<Self>>) -> Task<()>;
 
     // Delegates that support this method (e.g. the CommandPalette) can chose to block on any background
@@ -524,7 +527,9 @@ impl<D: PickerDelegate> Render for Picker<D> {
                             .inset(true)
                             .spacing(ListItemSpacing::Sparse)
                             .disabled(true)
-                            .child(Label::new("No matches").color(Color::Muted)),
+                            .child(
+                                Label::new(self.delegate.no_matches_text(cx)).color(Color::Muted),
+                            ),
                     ),
                 )
             })

crates/tab_switcher/src/tab_switcher.rs 🔗

@@ -3,19 +3,19 @@ mod tab_switcher_tests;
 
 use collections::HashMap;
 use gpui::{
-    impl_actions, rems, Action, AppContext, DismissEvent, EventEmitter, FocusHandle, FocusableView,
-    Modifiers, ModifiersChangedEvent, ParentElement, Render, Styled, Task, View, ViewContext,
-    VisualContext, WeakView,
+    actions, impl_actions, rems, Action, AnyElement, AppContext, DismissEvent, EntityId,
+    EventEmitter, FocusHandle, FocusableView, Modifiers, ModifiersChangedEvent, MouseButton,
+    MouseUpEvent, ParentElement, Render, Styled, Task, View, ViewContext, VisualContext, WeakView,
 };
 use picker::{Picker, PickerDelegate};
 use serde::Deserialize;
 use std::sync::Arc;
-use ui::{prelude::*, ListItem, ListItemSpacing};
+use ui::{prelude::*, ListItem, ListItemSpacing, Tooltip};
 use util::ResultExt;
 use workspace::{
     item::ItemHandle,
     pane::{render_item_indicator, tab_details, Event as PaneEvent},
-    ModalView, Pane, Workspace,
+    ModalView, Pane, SaveIntent, Workspace,
 };
 
 const PANEL_WIDTH_REMS: f32 = 28.;
@@ -27,6 +27,7 @@ pub struct Toggle {
 }
 
 impl_actions!(tab_switcher, [Toggle]);
+actions!(tab_switcher, [CloseSelectedItem]);
 
 pub struct TabSwitcher {
     picker: View<Picker<TabSwitcherDelegate>>,
@@ -96,6 +97,14 @@ impl TabSwitcher {
             }
         }
     }
+
+    fn handle_close_selected_item(&mut self, _: &CloseSelectedItem, cx: &mut ViewContext<Self>) {
+        self.picker.update(cx, |picker, cx| {
+            picker
+                .delegate
+                .close_item_at(picker.delegate.selected_index(), cx)
+        });
+    }
 }
 
 impl EventEmitter<DismissEvent> for TabSwitcher {}
@@ -112,6 +121,7 @@ impl Render for TabSwitcher {
             .key_context("TabSwitcher")
             .w(rems(PANEL_WIDTH_REMS))
             .on_modifiers_changed(cx.listener(Self::handle_modifiers_changed))
+            .on_action(cx.listener(Self::handle_close_selected_item))
             .child(self.picker.clone())
     }
 }
@@ -154,9 +164,14 @@ impl TabSwitcherDelegate {
         cx.subscribe(&pane, |tab_switcher, _, event, cx| {
             match event {
                 PaneEvent::AddItem { .. } | PaneEvent::RemoveItem { .. } | PaneEvent::Remove => {
-                    tab_switcher
-                        .picker
-                        .update(cx, |picker, cx| picker.refresh(cx))
+                    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();
+                    })
                 }
                 _ => {}
             };
@@ -209,6 +224,38 @@ impl TabSwitcherDelegate {
             }
         }
     }
+
+    fn selected_item_id(&self) -> Option<EntityId> {
+        self.matches
+            .get(self.selected_index())
+            .map(|tab_match| tab_match.item.item_id())
+    }
+
+    fn select_item(
+        &mut self,
+        item_id: EntityId,
+        cx: &mut ViewContext<'_, Picker<TabSwitcherDelegate>>,
+    ) {
+        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 close_item_at(&mut self, ix: usize, cx: &mut ViewContext<'_, Picker<TabSwitcherDelegate>>) {
+        let Some(tab_match) = self.matches.get(ix) else {
+            return;
+        };
+        let Some(pane) = self.pane.upgrade() else {
+            return;
+        };
+        pane.update(cx, |pane, cx| {
+            pane.close_item_by_id(tab_match.item.item_id(), SaveIntent::Close, cx)
+                .detach_and_log_err(cx);
+        });
+    }
 }
 
 impl PickerDelegate for TabSwitcherDelegate {
@@ -218,6 +265,10 @@ impl PickerDelegate for TabSwitcherDelegate {
         "".into()
     }
 
+    fn no_matches_text(&self, _cx: &mut WindowContext) -> SharedString {
+        "No tabs".into()
+    }
+
     fn match_count(&self) -> usize {
         self.matches.len()
     }
@@ -275,6 +326,35 @@ impl PickerDelegate for TabSwitcherDelegate {
 
         let label = tab_match.item.tab_content(Some(tab_match.detail), true, cx);
         let indicator = render_item_indicator(tab_match.item.boxed_clone(), cx);
+        let indicator_color = if let Some(ref indicator) = indicator {
+            indicator.color
+        } else {
+            Color::default()
+        };
+        let indicator = h_flex()
+            .flex_shrink_0()
+            .children(indicator)
+            .child(div().w_2())
+            .into_any_element();
+        let close_button = div()
+            // We need this on_mouse_up here instead of on_click on the close
+            // button because Picker intercepts the same events and handles them
+            // as click's on list items.
+            // See the same handler in Picker for more details.
+            .on_mouse_up(
+                MouseButton::Right,
+                cx.listener(move |picker, _: &MouseUpEvent, cx| {
+                    cx.stop_propagation();
+                    picker.delegate.close_item_at(ix, cx);
+                }),
+            )
+            .child(
+                IconButton::new("close_tab", IconName::Close)
+                    .icon_size(IconSize::Small)
+                    .icon_color(indicator_color)
+                    .tooltip(|cx| Tooltip::text("Close", cx)),
+            )
+            .into_any_element();
 
         Some(
             ListItem::new(ix)
@@ -282,7 +362,14 @@ impl PickerDelegate for TabSwitcherDelegate {
                 .inset(true)
                 .selected(selected)
                 .child(h_flex().w_full().child(label))
-                .children(indicator),
+                .map(|el| {
+                    if self.selected_index == ix {
+                        el.end_slot::<AnyElement>(close_button)
+                    } else {
+                        el.end_slot::<AnyElement>(indicator)
+                            .end_hover_slot::<AnyElement>(close_button)
+                    }
+                }),
         )
     }
 }

crates/tab_switcher/src/tab_switcher_tests.rs 🔗

@@ -183,6 +183,51 @@ async fn test_open_with_single_item(cx: &mut gpui::TestAppContext) {
     });
 }
 
+#[gpui::test]
+async fn test_close_selected_item(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",
+            }),
+        )
+        .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;
+
+    cx.simulate_modifiers_change(Modifiers::control());
+    let tab_switcher = open_tab_switcher(false, &workspace, cx);
+    tab_switcher.update(cx, |tab_switcher, _| {
+        assert_eq!(tab_switcher.delegate.matches.len(), 2);
+        assert_match_at_position(tab_switcher, 0, tab_2.boxed_clone());
+        assert_match_selection(tab_switcher, 1, tab_1.boxed_clone());
+    });
+
+    cx.simulate_modifiers_change(Modifiers::control());
+    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_2);
+    });
+
+    // Still switches tab on modifiers release
+    cx.simulate_modifiers_change(Modifiers::none());
+    cx.read(|cx| {
+        let active_editor = workspace.read(cx).active_item_as::<Editor>(cx).unwrap();
+        assert_eq!(active_editor.read(cx).title(cx), "2.txt");
+    });
+    assert_tab_switcher_is_closed(workspace, cx);
+}
+
 fn init_test(cx: &mut TestAppContext) -> Arc<AppState> {
     cx.update(|cx| {
         let state = AppState::test(cx);

crates/ui/src/components/indicator.rs 🔗

@@ -13,7 +13,7 @@ pub enum IndicatorStyle {
 pub struct Indicator {
     position: Position,
     style: IndicatorStyle,
-    color: Color,
+    pub color: Color,
 }
 
 impl Indicator {