Danilo Leal created
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 |
|--------|--------|
| 
| 
|
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(-)
@@ -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": {
@@ -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,
@@ -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<Self>,
) -> 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<Self>) {
+ fn handle_filter_toggle_menu(
+ &mut self,
+ _: &ToggleFilterMenu,
+ window: &mut Window,
+ cx: &mut Context<Self>,
+ ) {
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>,
+ ) {
+ 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<FoundPath>,
separate_history: bool,
first_update: bool,
- popover_menu_handle: PopoverMenuHandle<ContextMenu>,
+ filter_popover_menu_handle: PopoverMenuHandle<ContextMenu>,
+ split_popover_menu_handle: PopoverMenuHandle<ContextMenu>,
focus_handle: FocusHandle,
include_ignored: Option<bool>,
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<Picker<Self>>) -> Option<AnyElement> {
- let context = self.focus_handle.clone();
+ fn render_footer(
+ &self,
+ window: &mut Window,
+ cx: &mut Context<Picker<Self>>,
+ ) -> Option<AnyElement> {
+ 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(),