branch_picker: Add design improvements (#60925)
This PR adds a series of design improvements to the branch picker, including: - Consistent icon used for filter actions - A keybinding/action to trigger the filter menu with the keyboard - Filter menu positioning depending on the picker's editor position - Error display that displays the full label without truncation/tooltip, and is placed accordingly also depending on the picker's editor position - List subheader in the search list depending on filtered option Here's a quick video: https://github.com/user-attachments/assets/70922daf-fc9f-4d94-bed8-c84d3ea73534 - - - Release Notes: - N/A
This commit is contained in:
1 parent
58f2b47005
commit
faa7c4f534
5 files changed
+169
-176
No files matched your search
@@ -1534,6 +1534,7 @@
|
||||
"ctrl-shift-backspace": "branch_picker::DeleteBranch",
|
||||
"ctrl-alt-shift-backspace": "branch_picker::ForceDeleteBranch",
|
||||
"ctrl-shift-i": "branch_picker::CycleBranchFilter",
|
||||
"ctrl-k": "branch_picker::ToggleFilterMenu",
|
||||
},
|
||||
},
|
||||
{
|
||||
|
||||
@@ -644,7 +644,7 @@
|
||||
"use_key_equivalents": true,
|
||||
"bindings": {
|
||||
"cmd-k v": "markdown::OpenPreviewToTheSide",
|
||||
"cmd-shift-v": "markdown::OpenPreview"
|
||||
"cmd-shift-v": "markdown::OpenPreview",
|
||||
},
|
||||
},
|
||||
{
|
||||
@@ -1588,6 +1588,7 @@
|
||||
"cmd-shift-backspace": "branch_picker::DeleteBranch",
|
||||
"cmd-alt-shift-backspace": "branch_picker::ForceDeleteBranch",
|
||||
"cmd-shift-i": "branch_picker::CycleBranchFilter",
|
||||
"cmd-k": "branch_picker::ToggleFilterMenu",
|
||||
},
|
||||
},
|
||||
{
|
||||
|
||||
@@ -1513,6 +1513,7 @@
|
||||
"ctrl-shift-backspace": "branch_picker::DeleteBranch",
|
||||
"ctrl-alt-shift-backspace": "branch_picker::ForceDeleteBranch",
|
||||
"ctrl-shift-i": "branch_picker::CycleBranchFilter",
|
||||
"ctrl-k": "branch_picker::ToggleFilterMenu",
|
||||
},
|
||||
},
|
||||
{
|
||||
|
||||
+164
-168
@@ -14,12 +14,12 @@ use picker::{Picker, PickerDelegate, PickerEditorPosition};
|
||||
use project::git_store::{Repository, RepositoryEvent};
|
||||
use project::project_settings::ProjectSettings;
|
||||
use settings::Settings;
|
||||
use std::rc::Rc;
|
||||
|
||||
use std::sync::Arc;
|
||||
use time::OffsetDateTime;
|
||||
use ui::{
|
||||
Banner, ContextMenu, ContextMenuEntry, Divider, HighlightedLabel, Indicator, KeyBinding,
|
||||
ListItem, ListItemSpacing, PopoverMenu, PopoverMenuHandle, Severity, Tooltip, prelude::*,
|
||||
Banner, ContextMenu, Divider, HighlightedLabel, Indicator, KeyBinding, ListItem,
|
||||
ListItemSpacing, ListSubHeader, PopoverMenu, PopoverMenuHandle, Severity, Tooltip, prelude::*,
|
||||
};
|
||||
use ui_input::ErasedEditor;
|
||||
use util::ResultExt;
|
||||
@@ -42,7 +42,9 @@ actions!(
|
||||
/// Show only remote branches.
|
||||
ShowRemoteBranches,
|
||||
/// Cycle through branch filters.
|
||||
CycleBranchFilter
|
||||
CycleBranchFilter,
|
||||
/// Toggles the branch filter menu.
|
||||
ToggleFilterMenu
|
||||
]
|
||||
);
|
||||
|
||||
@@ -446,7 +448,11 @@ impl BranchList {
|
||||
}
|
||||
|
||||
pub(crate) fn branch_filter_menu_open(&self, cx: &App) -> bool {
|
||||
self.picker.read(cx).delegate.branch_filter_menu_open
|
||||
self.picker
|
||||
.read(cx)
|
||||
.delegate
|
||||
.branch_filter_menu_handle
|
||||
.is_deployed()
|
||||
}
|
||||
|
||||
pub(crate) fn cycle_branch_filter(&mut self, window: &mut Window, cx: &mut Context<Self>) {
|
||||
@@ -490,13 +496,24 @@ impl Render for BranchList {
|
||||
this.cycle_branch_filter(window, cx);
|
||||
}),
|
||||
)
|
||||
.on_action(
|
||||
cx.listener(|this, _: &branch_picker::ToggleFilterMenu, window, cx| {
|
||||
let menu_handle = this
|
||||
.picker
|
||||
.read(cx)
|
||||
.delegate
|
||||
.branch_filter_menu_handle
|
||||
.clone();
|
||||
menu_handle.toggle(window, cx);
|
||||
}),
|
||||
)
|
||||
.child(self.picker.clone())
|
||||
.when(!self.embedded, |this| {
|
||||
this.on_mouse_down_out({
|
||||
cx.listener(move |this, _, window, cx| {
|
||||
// The filter menu is a deferred popover, so clicks within it are outside
|
||||
// the branch picker's bounds even though it is part of this interaction.
|
||||
if this.picker.read(cx).delegate.branch_filter_menu_open {
|
||||
if this.branch_filter_menu_open(cx) {
|
||||
return;
|
||||
}
|
||||
this.picker.update(cx, |this, cx| {
|
||||
@@ -572,6 +589,14 @@ impl BranchFilter {
|
||||
Self::Remote => Self::All,
|
||||
}
|
||||
}
|
||||
|
||||
fn label(self) -> &'static str {
|
||||
match self {
|
||||
Self::All => "All Branches",
|
||||
Self::Local => "Local Branches",
|
||||
Self::Remote => "Remote Branches",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
struct GlobalBranchFilter(BranchFilter);
|
||||
@@ -580,69 +605,35 @@ impl Global for GlobalBranchFilter {}
|
||||
|
||||
fn branch_filter_menu(
|
||||
branch_filter: BranchFilter,
|
||||
picker: WeakEntity<Picker<BranchListDelegate>>,
|
||||
focus_handle: FocusHandle,
|
||||
window: &mut Window,
|
||||
cx: &mut App,
|
||||
) -> Entity<ContextMenu> {
|
||||
let picker_for_menu = picker.clone();
|
||||
let menu = ContextMenu::build(window, cx, |menu, _, _| {
|
||||
menu.header("Show")
|
||||
.item(branch_filter_menu_entry(
|
||||
"All Branches",
|
||||
BranchFilter::All,
|
||||
branch_filter,
|
||||
picker_for_menu.clone(),
|
||||
))
|
||||
.item(branch_filter_menu_entry(
|
||||
"Local Branches",
|
||||
BranchFilter::Local,
|
||||
branch_filter,
|
||||
picker_for_menu.clone(),
|
||||
))
|
||||
.item(branch_filter_menu_entry(
|
||||
"Remote Branches",
|
||||
BranchFilter::Remote,
|
||||
branch_filter,
|
||||
picker_for_menu,
|
||||
))
|
||||
});
|
||||
window
|
||||
.subscribe(&menu, cx, move |_, _: &DismissEvent, window, cx| {
|
||||
let picker = picker.clone();
|
||||
window.defer(cx, move |_, cx| {
|
||||
picker
|
||||
.update(cx, |picker, _| {
|
||||
picker.delegate.branch_filter_menu_open = false;
|
||||
})
|
||||
.log_err();
|
||||
});
|
||||
})
|
||||
.detach();
|
||||
menu
|
||||
}
|
||||
ContextMenu::build(window, cx, |mut menu, _, _| {
|
||||
menu = menu.context(focus_handle.clone());
|
||||
|
||||
fn branch_filter_menu_entry(
|
||||
label: &'static str,
|
||||
branch_filter: BranchFilter,
|
||||
current_branch_filter: BranchFilter,
|
||||
picker: WeakEntity<Picker<BranchListDelegate>>,
|
||||
) -> ContextMenuEntry {
|
||||
ContextMenuEntry::new(label)
|
||||
.toggle(IconPosition::End, branch_filter == current_branch_filter)
|
||||
.handler(move |window, cx| {
|
||||
picker
|
||||
.update(cx, |picker, cx| {
|
||||
cx.set_global(GlobalBranchFilter(branch_filter));
|
||||
if picker.delegate.branch_filter == branch_filter {
|
||||
return;
|
||||
}
|
||||
picker.delegate.branch_filter = branch_filter;
|
||||
picker.update_matches(picker.query(cx), window, cx);
|
||||
picker.refresh_placeholder(window, cx);
|
||||
cx.notify();
|
||||
})
|
||||
.log_err();
|
||||
})
|
||||
let filter_actions: [(BranchFilter, Box<dyn Action>); 3] = [
|
||||
(BranchFilter::All, ShowAllBranches.boxed_clone()),
|
||||
(BranchFilter::Local, ShowLocalBranches.boxed_clone()),
|
||||
(BranchFilter::Remote, ShowRemoteBranches.boxed_clone()),
|
||||
];
|
||||
|
||||
for (filter, action) in filter_actions {
|
||||
let handler_focus = focus_handle.clone();
|
||||
let dispatched = action.boxed_clone();
|
||||
menu = menu.toggleable_entry(
|
||||
filter.label(),
|
||||
filter == branch_filter,
|
||||
IconPosition::End,
|
||||
Some(action),
|
||||
move |window, cx| {
|
||||
window.focus(&handler_focus, cx);
|
||||
window.dispatch_action(dispatched.boxed_clone(), cx);
|
||||
},
|
||||
);
|
||||
}
|
||||
menu
|
||||
})
|
||||
}
|
||||
|
||||
pub struct BranchListDelegate {
|
||||
@@ -658,7 +649,6 @@ pub struct BranchListDelegate {
|
||||
modifiers: Modifiers,
|
||||
branch_filter: BranchFilter,
|
||||
branch_filter_menu_handle: PopoverMenuHandle<ContextMenu>,
|
||||
branch_filter_menu_open: bool,
|
||||
state: PickerState,
|
||||
branch_selection_behavior: BranchSelectionBehavior,
|
||||
focus_handle: FocusHandle,
|
||||
@@ -986,7 +976,6 @@ impl BranchListDelegate {
|
||||
modifiers: Default::default(),
|
||||
branch_filter,
|
||||
branch_filter_menu_handle: PopoverMenuHandle::default(),
|
||||
branch_filter_menu_open: false,
|
||||
state: PickerState::List,
|
||||
branch_selection_behavior,
|
||||
focus_handle: cx.focus_handle(),
|
||||
@@ -1000,6 +989,27 @@ impl BranchListDelegate {
|
||||
self.branch_selection_behavior.is_select_only()
|
||||
}
|
||||
|
||||
fn branch_filter_trigger(&self) -> IconButton {
|
||||
IconButton::new("branch-filter", IconName::Filter)
|
||||
.icon_size(IconSize::Small)
|
||||
.toggle_state(self.branch_filter != BranchFilter::All)
|
||||
.when(self.branch_filter != BranchFilter::All, |this| {
|
||||
this.indicator(Indicator::dot().color(Color::Info))
|
||||
})
|
||||
}
|
||||
|
||||
fn branch_filter_tooltip(&self) -> impl Fn(&mut Window, &mut App) -> gpui::AnyView + 'static {
|
||||
let focus_handle = self.focus_handle.clone();
|
||||
move |_, cx| {
|
||||
Tooltip::for_action_in(
|
||||
"Filter Branches",
|
||||
&branch_picker::ToggleFilterMenu,
|
||||
&focus_handle,
|
||||
cx,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
fn is_force_delete_hovering_index(&self, index: usize) -> bool {
|
||||
self.modifiers.alt && self.hovered_delete_index == Some(index)
|
||||
}
|
||||
@@ -1200,81 +1210,81 @@ impl PickerDelegate for BranchListDelegate {
|
||||
&self,
|
||||
editor: &Arc<dyn ErasedEditor>,
|
||||
_window: &mut Window,
|
||||
cx: &mut Context<Picker<Self>>,
|
||||
_cx: &mut Context<Picker<Self>>,
|
||||
) -> Option<Div> {
|
||||
let picker = cx.weak_entity();
|
||||
let editor = editor.as_any().downcast_ref::<Entity<Editor>>().unwrap();
|
||||
let editor_start = matches!(self.editor_position(), PickerEditorPosition::Start);
|
||||
let editor_bottom = matches!(self.editor_position(), PickerEditorPosition::End);
|
||||
|
||||
let show_inline_filter =
|
||||
self.editor_position() == PickerEditorPosition::End || !self.show_footer;
|
||||
let warning_banner = || {
|
||||
self.branch_list_error.as_deref().map(|error| {
|
||||
let message = format!("Some branches could not be loaded: {error}");
|
||||
div().p_1p5().child(
|
||||
Banner::new()
|
||||
.severity(Severity::Warning)
|
||||
.child(div().min_w_0().flex_1().child(Label::new(message))),
|
||||
)
|
||||
})
|
||||
};
|
||||
|
||||
Some(
|
||||
v_flex()
|
||||
.when(
|
||||
self.editor_position() == PickerEditorPosition::End,
|
||||
|this| this.child(Divider::horizontal()),
|
||||
)
|
||||
.when_some(self.branch_list_error.clone(), |this, error| {
|
||||
let message = format!("Some branches could not be loaded: {error}");
|
||||
this.child(
|
||||
div()
|
||||
.id("branch-list-error")
|
||||
.p_1p5()
|
||||
.child(
|
||||
Banner::new().severity(Severity::Warning).child(
|
||||
Label::new(message.clone())
|
||||
.size(LabelSize::Small)
|
||||
.single_line()
|
||||
.truncate(),
|
||||
),
|
||||
)
|
||||
.tooltip(Tooltip::text(message)),
|
||||
)
|
||||
.w_full()
|
||||
.min_w_0()
|
||||
.when(editor_bottom, |this| {
|
||||
this.child(Divider::horizontal())
|
||||
.when_some(warning_banner(), |this, banner| this.child(banner))
|
||||
})
|
||||
.child(
|
||||
h_flex()
|
||||
.overflow_hidden()
|
||||
.flex_none()
|
||||
.h_9()
|
||||
.px_2p5()
|
||||
.flex_none()
|
||||
.overflow_hidden()
|
||||
.child(editor.clone())
|
||||
.when(show_inline_filter, |this| {
|
||||
.map(|this| {
|
||||
let branch_filter = self.branch_filter;
|
||||
let picker_for_menu_open = picker.clone();
|
||||
let focus_handle = self.focus_handle.clone();
|
||||
|
||||
this.gap_1().justify_between().child(
|
||||
PopoverMenu::new("branch-filter-menu")
|
||||
.with_handle(self.branch_filter_menu_handle.clone())
|
||||
.on_open(Rc::new(move |_, cx| {
|
||||
picker_for_menu_open
|
||||
.update(cx, |picker, _| {
|
||||
picker.delegate.branch_filter_menu_open = true;
|
||||
})
|
||||
.log_err();
|
||||
}))
|
||||
.trigger(
|
||||
IconButton::new("branch-filter", IconName::Filter)
|
||||
.toggle_state(branch_filter != BranchFilter::All)
|
||||
.when(branch_filter != BranchFilter::All, |this| {
|
||||
this.indicator(Indicator::dot().color(Color::Info))
|
||||
})
|
||||
.icon_size(IconSize::Small)
|
||||
.tooltip(Tooltip::text("Filter branches")),
|
||||
.trigger_with_tooltip(
|
||||
self.branch_filter_trigger(),
|
||||
self.branch_filter_tooltip(),
|
||||
)
|
||||
.menu(move |window, cx| {
|
||||
Some(branch_filter_menu(
|
||||
branch_filter,
|
||||
picker.clone(),
|
||||
focus_handle.clone(),
|
||||
window,
|
||||
cx,
|
||||
))
|
||||
})
|
||||
.map(|this| {
|
||||
if editor_bottom {
|
||||
this.anchor(gpui::Anchor::BottomRight)
|
||||
.attach(gpui::Anchor::TopRight)
|
||||
.offset(gpui::Point {
|
||||
x: px(0.0),
|
||||
y: px(-1.0),
|
||||
})
|
||||
} else {
|
||||
this.anchor(gpui::Anchor::TopRight)
|
||||
.attach(gpui::Anchor::BottomRight)
|
||||
.offset(gpui::Point {
|
||||
x: px(1.0),
|
||||
y: px(1.0),
|
||||
})
|
||||
}
|
||||
}),
|
||||
)
|
||||
}),
|
||||
)
|
||||
.when(
|
||||
self.editor_position() == PickerEditorPosition::Start,
|
||||
|this| this.child(Divider::horizontal()),
|
||||
),
|
||||
.when(editor_start, |this| {
|
||||
this.child(Divider::horizontal())
|
||||
.when_some(warning_banner(), |this, banner| this.child(banner))
|
||||
}),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -1289,6 +1299,29 @@ impl PickerDelegate for BranchListDelegate {
|
||||
}
|
||||
}
|
||||
|
||||
fn render_header(
|
||||
&self,
|
||||
_window: &mut Window,
|
||||
_cx: &mut Context<Picker<Self>>,
|
||||
) -> Option<AnyElement> {
|
||||
if self.branch_filter == BranchFilter::All {
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(
|
||||
div()
|
||||
.pt_1p5()
|
||||
.mb_neg_0p5()
|
||||
.child(ListSubHeader::new(self.branch_filter.label()).inset(true))
|
||||
.into_any_element(),
|
||||
)
|
||||
}
|
||||
|
||||
fn has_another_open_menu(&self, window: &Window, cx: &App) -> bool {
|
||||
self.branch_filter_menu_handle.is_deployed()
|
||||
|| self.branch_filter_menu_handle.is_focused(window, cx)
|
||||
}
|
||||
|
||||
fn match_count(&self) -> usize {
|
||||
self.matches.len()
|
||||
}
|
||||
@@ -1858,7 +1891,6 @@ impl PickerDelegate for BranchListDelegate {
|
||||
return None;
|
||||
}
|
||||
let focus_handle = self.focus_handle.clone();
|
||||
let picker = cx.weak_entity();
|
||||
|
||||
let footer_container = || {
|
||||
h_flex()
|
||||
@@ -1932,59 +1964,23 @@ impl PickerDelegate for BranchListDelegate {
|
||||
|
||||
Some(
|
||||
footer_container()
|
||||
.map(|this| {
|
||||
if branch_from_default_button.is_some() {
|
||||
this.justify_end().when_some(
|
||||
branch_from_default_button,
|
||||
|this, button| {
|
||||
this.child(button).child(
|
||||
Button::new("create", "Create")
|
||||
.key_binding(
|
||||
KeyBinding::for_action_in(
|
||||
&menu::Confirm,
|
||||
&focus_handle,
|
||||
cx,
|
||||
)
|
||||
.map(|kb| kb.size(rems_from_px(12.))),
|
||||
)
|
||||
.on_click(cx.listener(|this, _, window, cx| {
|
||||
this.delegate.confirm(false, window, cx);
|
||||
})),
|
||||
.justify_end()
|
||||
.map(|this| match branch_from_default_button {
|
||||
Some(button) => this.child(button).child(
|
||||
Button::new("create", "Create")
|
||||
.key_binding(
|
||||
KeyBinding::for_action_in(
|
||||
&menu::Confirm,
|
||||
&focus_handle,
|
||||
cx,
|
||||
)
|
||||
},
|
||||
)
|
||||
} else {
|
||||
this.justify_between()
|
||||
.child({
|
||||
let branch_filter = self.branch_filter;
|
||||
let picker_for_menu_open = picker.clone();
|
||||
PopoverMenu::new("branch-filter-footer-menu")
|
||||
.with_handle(self.branch_filter_menu_handle.clone())
|
||||
.on_open(Rc::new(move |_, cx| {
|
||||
picker_for_menu_open
|
||||
.update(cx, |picker, _| {
|
||||
picker.delegate.branch_filter_menu_open =
|
||||
true;
|
||||
})
|
||||
.log_err();
|
||||
}))
|
||||
.trigger(
|
||||
Button::new("branch-filter", "Filter…")
|
||||
.toggle_state(
|
||||
branch_filter != BranchFilter::All,
|
||||
),
|
||||
)
|
||||
.menu(move |window, cx| {
|
||||
Some(branch_filter_menu(
|
||||
branch_filter,
|
||||
picker.clone(),
|
||||
window,
|
||||
cx,
|
||||
))
|
||||
})
|
||||
})
|
||||
.child(delete_and_select_btns)
|
||||
}
|
||||
.map(|kb| kb.size(rems_from_px(12.))),
|
||||
)
|
||||
.on_click(cx.listener(|this, _, window, cx| {
|
||||
this.delegate.confirm(false, window, cx);
|
||||
})),
|
||||
),
|
||||
None => this.child(delete_and_select_btns),
|
||||
})
|
||||
.into_any_element(),
|
||||
)
|
||||
|
||||
@@ -208,13 +208,7 @@ impl<D: PickerDelegate> Picker<D> {
|
||||
Head::Editor(editor) => {
|
||||
if editor_position == PickerEditorPosition::Start {
|
||||
let editor = editor.clone();
|
||||
Some(
|
||||
h_flex().w_full().child(
|
||||
div()
|
||||
.flex_1()
|
||||
.child(self.render_editor(&editor, window, cx)),
|
||||
),
|
||||
)
|
||||
Some(self.render_editor(&editor, window, cx))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user