From cefa0cbed859f91cff07a16fa98668c9b40d0436 Mon Sep 17 00:00:00 2001 From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com> Date: Mon, 2 Jun 2025 09:54:15 -0300 Subject: [PATCH] Improve the file finder picker footer design (#31777) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current filter icon button in the file finder picker footer confused me because it is a really a toggleable button that adds a specific filter. From the icon used, I was expecting more configuration options rather than just one. Also, I was really wanting a way to trigger it with the keyboard, even if I need my mouse to initially learn about the keybinding. So, this PR transforms that icon button into an actual popover trigger, in which (for now) there's only one filter option. However, being a menu is cool because it allows to accomodate more items like, for example, "Include Git Submodule Files" and others, in the future. Also, there's now a keybinding that you can hit to open that popover, as well as an indicator that pops in to communicate that a certain item inside it has been toggled. Lastly, also added a keybinding to the "Split" menu in the spirit of making everything more keyboard accessible! | Before | After | |--------|--------| | ![CleanShot 2025-05-30 at 4  29 57@2x](https://github.com/user-attachments/assets/88a30588-289d-4d76-bb50-0a4e7f72ef84) | ![CleanShot 2025-05-30 at 4  24 31@2x](https://github.com/user-attachments/assets/30b8f3eb-4d5c-43e1-abad-59d32ed7c89f) | Release Notes: - Improved the keyboard navigability of the file finder filtering options. --- assets/keymaps/default-linux.json | 7 + assets/keymaps/default-macos.json | 8 ++ crates/file_finder/src/file_finder.rs | 196 ++++++++++++++++++++------ 3 files changed, 165 insertions(+), 46 deletions(-) diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index 73d49292c5..471bad98df 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -928,6 +928,13 @@ "tab": "channel_modal::ToggleMode" } }, + { + "context": "FileFinder", + "bindings": { + "ctrl-shift-a": "file_finder::ToggleSplitMenu", + "ctrl-shift-i": "file_finder::ToggleFilterMenu" + } + }, { "context": "FileFinder || (FileFinder > Picker > Editor) || (FileFinder > Picker > menu)", "bindings": { diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index 8b86268e98..701311f0f6 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -987,6 +987,14 @@ "tab": "channel_modal::ToggleMode" } }, + { + "context": "FileFinder", + "use_key_equivalents": true, + "bindings": { + "cmd-shift-a": "file_finder::ToggleSplitMenu", + "cmd-shift-i": "file_finder::ToggleFilterMenu" + } + }, { "context": "FileFinder || (FileFinder > Picker > Editor) || (FileFinder > Picker > menu)", "use_key_equivalents": true, diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index aa9ee4ca0a..bb49d7c147 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -38,8 +38,8 @@ use std::{ }; use text::Point; use ui::{ - ContextMenu, HighlightedLabel, IconButtonShape, ListItem, ListItemSpacing, PopoverMenu, - PopoverMenuHandle, Tooltip, prelude::*, + ButtonLike, ContextMenu, HighlightedLabel, Indicator, KeyBinding, ListItem, ListItemSpacing, + PopoverMenu, PopoverMenuHandle, TintColor, Tooltip, prelude::*, }; use util::{ResultExt, maybe, paths::PathWithPosition, post_inc}; use workspace::{ @@ -47,7 +47,10 @@ use workspace::{ notifications::NotifyResultExt, pane, }; -actions!(file_finder, [SelectPrevious, ToggleMenu]); +actions!( + file_finder, + [SelectPrevious, ToggleFilterMenu, ToggleSplitMenu] +); impl ModalView for FileFinder { fn on_before_dismiss( @@ -56,7 +59,14 @@ impl ModalView for FileFinder { cx: &mut Context, ) -> workspace::DismissDecision { let submenu_focused = self.picker.update(cx, |picker, cx| { - picker.delegate.popover_menu_handle.is_focused(window, cx) + picker + .delegate + .filter_popover_menu_handle + .is_focused(window, cx) + || picker + .delegate + .split_popover_menu_handle + .is_focused(window, cx) }); workspace::DismissDecision::Dismiss(!submenu_focused) } @@ -212,9 +222,30 @@ impl FileFinder { window.dispatch_action(Box::new(menu::SelectPrevious), cx); } - fn handle_toggle_menu(&mut self, _: &ToggleMenu, window: &mut Window, cx: &mut Context) { + fn handle_filter_toggle_menu( + &mut self, + _: &ToggleFilterMenu, + window: &mut Window, + cx: &mut Context, + ) { self.picker.update(cx, |picker, cx| { - let menu_handle = &picker.delegate.popover_menu_handle; + let menu_handle = &picker.delegate.filter_popover_menu_handle; + if menu_handle.is_deployed() { + menu_handle.hide(cx); + } else { + menu_handle.show(window, cx); + } + }); + } + + fn handle_split_toggle_menu( + &mut self, + _: &ToggleSplitMenu, + window: &mut Window, + cx: &mut Context, + ) { + self.picker.update(cx, |picker, cx| { + let menu_handle = &picker.delegate.split_popover_menu_handle; if menu_handle.is_deployed() { menu_handle.hide(cx); } else { @@ -345,7 +376,8 @@ impl Render for FileFinder { .w(modal_max_width) .on_modifiers_changed(cx.listener(Self::handle_modifiers_changed)) .on_action(cx.listener(Self::handle_select_prev)) - .on_action(cx.listener(Self::handle_toggle_menu)) + .on_action(cx.listener(Self::handle_filter_toggle_menu)) + .on_action(cx.listener(Self::handle_split_toggle_menu)) .on_action(cx.listener(Self::handle_toggle_ignored)) .on_action(cx.listener(Self::go_to_file_split_left)) .on_action(cx.listener(Self::go_to_file_split_right)) @@ -371,7 +403,8 @@ pub struct FileFinderDelegate { history_items: Vec, separate_history: bool, first_update: bool, - popover_menu_handle: PopoverMenuHandle, + filter_popover_menu_handle: PopoverMenuHandle, + split_popover_menu_handle: PopoverMenuHandle, focus_handle: FocusHandle, include_ignored: Option, include_ignored_refresh: Task<()>, @@ -758,7 +791,8 @@ impl FileFinderDelegate { history_items, separate_history, first_update: true, - popover_menu_handle: PopoverMenuHandle::default(), + filter_popover_menu_handle: PopoverMenuHandle::default(), + split_popover_menu_handle: PopoverMenuHandle::default(), focus_handle: cx.focus_handle(), include_ignored: FileFinderSettings::get_global(cx).include_ignored, include_ignored_refresh: Task::ready(()), @@ -1137,8 +1171,13 @@ impl FileFinderDelegate { fn key_context(&self, window: &Window, cx: &App) -> KeyContext { let mut key_context = KeyContext::new_with_defaults(); key_context.add("FileFinder"); - if self.popover_menu_handle.is_focused(window, cx) { - key_context.add("menu_open"); + + if self.filter_popover_menu_handle.is_focused(window, cx) { + key_context.add("filter_menu_open"); + } + + if self.split_popover_menu_handle.is_focused(window, cx) { + key_context.add("split_menu_open"); } key_context } @@ -1492,62 +1531,112 @@ impl PickerDelegate for FileFinderDelegate { ) } - fn render_footer(&self, _: &mut Window, cx: &mut Context>) -> Option { - let context = self.focus_handle.clone(); + fn render_footer( + &self, + window: &mut Window, + cx: &mut Context>, + ) -> Option { + let focus_handle = self.focus_handle.clone(); + Some( h_flex() .w_full() - .p_2() + .p_1p5() .justify_between() .border_t_1() .border_color(cx.theme().colors().border_variant) .child( - IconButton::new("toggle-ignored", IconName::Sliders) - .on_click({ - let focus_handle = self.focus_handle.clone(); - move |_, window, cx| { - focus_handle.dispatch_action(&ToggleIncludeIgnored, window, cx); - } + PopoverMenu::new("filter-menu-popover") + .with_handle(self.filter_popover_menu_handle.clone()) + .attach(gpui::Corner::BottomRight) + .anchor(gpui::Corner::BottomLeft) + .offset(gpui::Point { + x: px(1.0), + y: px(1.0), }) - .style(ButtonStyle::Subtle) - .shape(IconButtonShape::Square) - .toggle_state(self.include_ignored.unwrap_or(false)) - .tooltip({ - let focus_handle = self.focus_handle.clone(); + .trigger_with_tooltip( + IconButton::new("filter-trigger", IconName::Sliders) + .icon_size(IconSize::Small) + .icon_size(IconSize::Small) + .toggle_state(self.include_ignored.unwrap_or(false)) + .when(self.include_ignored.is_some(), |this| { + this.indicator(Indicator::dot().color(Color::Info)) + }), + { + let focus_handle = focus_handle.clone(); + move |window, cx| { + Tooltip::for_action_in( + "Filter Options", + &ToggleFilterMenu, + &focus_handle, + window, + cx, + ) + } + }, + ) + .menu({ + let focus_handle = focus_handle.clone(); + let include_ignored = self.include_ignored; + move |window, cx| { - Tooltip::for_action_in( - "Use ignored files", - &ToggleIncludeIgnored, - &focus_handle, - window, - cx, - ) + Some(ContextMenu::build(window, cx, { + let focus_handle = focus_handle.clone(); + move |menu, _, _| { + menu.context(focus_handle.clone()) + .header("Filter Options") + .toggleable_entry( + "Include Ignored Files", + include_ignored.unwrap_or(false), + ui::IconPosition::End, + Some(ToggleIncludeIgnored.boxed_clone()), + move |window, cx| { + window.focus(&focus_handle); + window.dispatch_action( + ToggleIncludeIgnored.boxed_clone(), + cx, + ); + }, + ) + } + })) } }), ) .child( h_flex() - .gap_2() + .gap_0p5() .child( - Button::new("open-selection", "Open").on_click(|_, window, cx| { - window.dispatch_action(menu::Confirm.boxed_clone(), cx) - }), - ) - .child( - PopoverMenu::new("menu-popover") - .with_handle(self.popover_menu_handle.clone()) - .attach(gpui::Corner::TopRight) - .anchor(gpui::Corner::BottomRight) + PopoverMenu::new("split-menu-popover") + .with_handle(self.split_popover_menu_handle.clone()) + .attach(gpui::Corner::BottomRight) + .anchor(gpui::Corner::BottomLeft) + .offset(gpui::Point { + x: px(1.0), + y: px(1.0), + }) .trigger( - Button::new("actions-trigger", "Split…") - .selected_label_color(Color::Accent), + ButtonLike::new("split-trigger") + .child(Label::new("Split…")) + .selected_style(ButtonStyle::Tinted(TintColor::Accent)) + .children( + KeyBinding::for_action_in( + &ToggleSplitMenu, + &focus_handle, + window, + cx, + ) + .map(|kb| kb.size(rems_from_px(12.))), + ), ) .menu({ + let focus_handle = focus_handle.clone(); + move |window, cx| { Some(ContextMenu::build(window, cx, { - let context = context.clone(); + let focus_handle = focus_handle.clone(); move |menu, _, _| { - menu.context(context) + menu.context(focus_handle.clone()) .action( "Split Left", pane::SplitLeft.boxed_clone(), @@ -1565,6 +1654,21 @@ impl PickerDelegate for FileFinderDelegate { })) } }), + ) + .child( + Button::new("open-selection", "Open") + .key_binding( + KeyBinding::for_action_in( + &menu::Confirm, + &focus_handle, + window, + cx, + ) + .map(|kb| kb.size(rems_from_px(12.))), + ) + .on_click(|_, window, cx| { + window.dispatch_action(menu::Confirm.boxed_clone(), cx) + }), ), ) .into_any(),