file_finder: Fix auto-jump to wrong file on hover via new set_hovered_index hook (#61716)
# Objective - Fixes #54158 ## Solution - The root cause is in `FileFinderDelegate::set_selected_index`: it always sets `has_changed_selected_index = true`, regardless of whether the selection change was triggered by keyboard navigation or mouse hover. - Fix: Added a separate `set_hovered_index` hook to the `PickerDelegate` trait with a default implementation that delegates to `set_selected_index`. The hover handler in Picker now calls `set_hovered_index` instead of `set_selected_index`. FileFinderDelegate overrides `set_hovered_index` to update `selected_index` without setting `has_changed_selected_index`, so hover-triggered selection changes no longer interfere with the `Cmd+P` auto-confirm logic. ## Testing - Added `test_hover_does_not_set_has_changed_selected_index` in `file_finder_tests.rs` ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable --- Release Notes: - Fixed file finder auto-jumping to the wrong file when the mouse hovered over a different entry after pressing `Cmd+P`
This commit is contained in:
1 parent
7f6677e795
commit
68bd0faf7e
3 files changed
+101
-1
No files matched your search
@@ -1847,6 +1847,11 @@ impl PickerDelegate for FileFinderDelegate {
|
||||
self.selected_index
|
||||
}
|
||||
|
||||
fn set_hovered_index(&mut self, ix: usize, _: &mut Window, cx: &mut Context<Picker<Self>>) {
|
||||
self.selected_index = ix;
|
||||
cx.notify();
|
||||
}
|
||||
|
||||
fn set_selected_index(&mut self, ix: usize, _: &mut Window, cx: &mut Context<Picker<Self>>) {
|
||||
self.has_changed_selected_index = true;
|
||||
self.selected_index = ix;
|
||||
|
||||
@@ -5186,3 +5186,64 @@ async fn test_exact_filename_with_directory_token(cx: &mut TestAppContext) {
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_hover_does_not_set_has_changed_selected_index(cx: &mut TestAppContext) {
|
||||
let app_state = init_test(cx);
|
||||
|
||||
app_state
|
||||
.fs
|
||||
.as_fake()
|
||||
.insert_tree(
|
||||
path!("/root"),
|
||||
json!({
|
||||
"a.rs": "",
|
||||
"b.rs": "",
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
|
||||
let project = Project::test(app_state.fs.clone(), [path!("/root").as_ref()], cx).await;
|
||||
let (multi_workspace, cx) =
|
||||
cx.add_window_view(|window, cx| MultiWorkspace::test_new(project, window, cx));
|
||||
let workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone());
|
||||
|
||||
open_close_queried_buffer("a", 1, "a.rs", &workspace, cx).await;
|
||||
open_close_queried_buffer("b", 1, "b.rs", &workspace, cx).await;
|
||||
|
||||
let picker = open_file_picker(&workspace, cx);
|
||||
|
||||
picker.update(cx, |picker, _| {
|
||||
assert!(
|
||||
picker.delegate.matches.len() >= 2,
|
||||
"need at least 2 matches"
|
||||
);
|
||||
});
|
||||
|
||||
picker.update_in(cx, |picker, window, cx| {
|
||||
picker.set_hovered_index(1, window, cx);
|
||||
});
|
||||
|
||||
picker.update(cx, |picker, _| {
|
||||
assert!(
|
||||
!picker.delegate.has_changed_selected_index,
|
||||
"hover should not set `has_changed_selected_index`"
|
||||
);
|
||||
assert_eq!(
|
||||
picker.delegate.selected_index(),
|
||||
1,
|
||||
"hover should change `selected_index`"
|
||||
);
|
||||
});
|
||||
|
||||
picker.update_in(cx, |picker, window, cx| {
|
||||
picker.cycle_selection(window, cx);
|
||||
});
|
||||
|
||||
picker.update(cx, |picker, _| {
|
||||
assert!(
|
||||
picker.delegate.has_changed_selected_index,
|
||||
"keyboard should set `has_changed_selected_index`"
|
||||
);
|
||||
});
|
||||
}
|
||||
@@ -172,6 +172,14 @@ pub trait PickerDelegate: Sized + 'static {
|
||||
fn separators_after_indices(&self) -> Vec<usize> {
|
||||
Vec::new()
|
||||
}
|
||||
fn set_hovered_index(
|
||||
&mut self,
|
||||
ix: usize,
|
||||
window: &mut Window,
|
||||
cx: &mut Context<Picker<Self>>,
|
||||
) {
|
||||
self.set_selected_index(ix, window, cx);
|
||||
}
|
||||
fn set_selected_index(
|
||||
&mut self,
|
||||
ix: usize,
|
||||
@@ -797,6 +805,32 @@ impl<D: PickerDelegate> Picker<D> {
|
||||
self.focus_handle(cx).focus(window, cx);
|
||||
}
|
||||
|
||||
pub fn set_hovered_index(&mut self, ix: usize, window: &mut Window, cx: &mut Context<Self>) {
|
||||
let match_count = self.delegate.match_count();
|
||||
if match_count == 0 {
|
||||
return;
|
||||
}
|
||||
|
||||
if !self.delegate.can_select(ix, window, cx) {
|
||||
return;
|
||||
}
|
||||
|
||||
let previous_index = self.delegate.selected_index();
|
||||
self.delegate.set_hovered_index(ix, window, cx);
|
||||
let current_index = self.delegate.selected_index();
|
||||
|
||||
if previous_index != current_index {
|
||||
if let Some(action) = self.delegate.selected_index_changed(ix, window, cx) {
|
||||
action(window, cx);
|
||||
}
|
||||
if let Some(preview) = &mut self.preview
|
||||
&& let Some(update) = self.delegate.try_get_preview_data_for_match(cx)
|
||||
{
|
||||
preview.update(update, window, cx);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Handles the selecting an index, and passing the change to the delegate.
|
||||
/// If `fallback_direction` is set to `None`, the index will not be selected
|
||||
/// if the element at that index cannot be selected.
|
||||
@@ -1414,7 +1448,7 @@ impl<D: PickerDelegate> Picker<D> {
|
||||
.when(self.delegate.select_on_hover(), |this| {
|
||||
this.on_hover(cx.listener(move |this, hovered: &bool, window, cx| {
|
||||
if *hovered {
|
||||
this.set_selected_index(ix, None, false, window, cx);
|
||||
this.set_hovered_index(ix, window, cx);
|
||||
cx.notify();
|
||||
}
|
||||
}))
|
||||
|
||||
Reference in new issue
Block a user