From 57a1b9b2cd573f36b3792aabb4be45c16996dac7 Mon Sep 17 00:00:00 2001 From: Andrew Lygin Date: Wed, 3 Apr 2024 18:28:51 +0300 Subject: [PATCH] tab_switcher: Add tab close buttons (#9968) 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 --- 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(-) diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index 8f7eb5b2c8..b1a00641d9 100644 --- a/assets/keymaps/default-linux.json +++ b/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", diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index d198f5bd44..0d252bc986 100644 --- a/assets/keymaps/default-macos.json +++ b/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", diff --git a/crates/picker/src/picker.rs b/crates/picker/src/picker.rs index 2679f21e1b..f3fbc4f111 100644 --- a/crates/picker/src/picker.rs +++ b/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>); fn placeholder_text(&self, _cx: &mut WindowContext) -> Arc; + fn no_matches_text(&self, _cx: &mut WindowContext) -> SharedString { + "No matches".into() + } fn update_matches(&mut self, query: String, cx: &mut ViewContext>) -> Task<()>; // Delegates that support this method (e.g. the CommandPalette) can chose to block on any background @@ -524,7 +527,9 @@ impl Render for Picker { .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), + ), ), ) }) diff --git a/crates/tab_switcher/src/tab_switcher.rs b/crates/tab_switcher/src/tab_switcher.rs index d7738ff4a6..913877af10 100644 --- a/crates/tab_switcher/src/tab_switcher.rs +++ b/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>, @@ -96,6 +97,14 @@ impl TabSwitcher { } } } + + fn handle_close_selected_item(&mut self, _: &CloseSelectedItem, cx: &mut ViewContext) { + self.picker.update(cx, |picker, cx| { + picker + .delegate + .close_item_at(picker.delegate.selected_index(), cx) + }); + } } impl EventEmitter 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 { + 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>, + ) { + 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>) { + 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::(close_button) + } else { + el.end_slot::(indicator) + .end_hover_slot::(close_button) + } + }), ) } } diff --git a/crates/tab_switcher/src/tab_switcher_tests.rs b/crates/tab_switcher/src/tab_switcher_tests.rs index b4a2a510bb..f5577878c9 100644 --- a/crates/tab_switcher/src/tab_switcher_tests.rs +++ b/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::(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 { cx.update(|cx| { let state = AppState::test(cx); diff --git a/crates/ui/src/components/indicator.rs b/crates/ui/src/components/indicator.rs index 97d38b1f22..5c548b26f2 100644 --- a/crates/ui/src/components/indicator.rs +++ b/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 {