From a0f06508038fd89a39c56725dae6bb1a0e666e26 Mon Sep 17 00:00:00 2001 From: Dnreikronos Date: Wed, 4 Mar 2026 11:27:12 -0300 Subject: [PATCH 1/4] picker: Prevent clicking non-selectable entries from confirming selection --- crates/picker/src/picker.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/picker/src/picker.rs b/crates/picker/src/picker.rs index 716653d89642fe..596e77ff068793 100644 --- a/crates/picker/src/picker.rs +++ b/crates/picker/src/picker.rs @@ -619,6 +619,9 @@ impl Picker { ) { cx.stop_propagation(); window.prevent_default(); + if !self.delegate.can_select(ix, window, cx) { + return; + } self.set_selected_index(ix, None, false, window, cx); self.do_confirm(secondary, window, cx) } From 873e71d79aec8aaa57a19308f9b311dd86662c03 Mon Sep 17 00:00:00 2001 From: Dnreikronos Date: Thu, 5 Mar 2026 09:02:06 -0300 Subject: [PATCH 2/4] picker: Add tests and hide pointer cursor for non-selectable entries --- Cargo.lock | 1 + crates/picker/Cargo.toml | 1 + crates/picker/src/picker.rs | 183 +++++++++++++++++++++++++++++++++++- 3 files changed, 183 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1a192869d6fee6..6254c074faeb03 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12546,6 +12546,7 @@ dependencies = [ "schemars", "serde", "serde_json", + "settings", "theme", "ui", "ui_input", diff --git a/crates/picker/Cargo.toml b/crates/picker/Cargo.toml index f85c55b9f27bcb..fc46e4a8454667 100644 --- a/crates/picker/Cargo.toml +++ b/crates/picker/Cargo.toml @@ -33,3 +33,4 @@ editor = { workspace = true, features = ["test-support"] } env_logger.workspace = true gpui = { workspace = true, features = ["test-support"] } serde_json.workspace = true +settings.workspace = true diff --git a/crates/picker/src/picker.rs b/crates/picker/src/picker.rs index 596e77ff068793..94e6600b7b67fa 100644 --- a/crates/picker/src/picker.rs +++ b/crates/picker/src/picker.rs @@ -750,16 +750,17 @@ impl Picker { } fn render_element( - &self, + &mut self, window: &mut Window, cx: &mut Context, ix: usize, ) -> impl IntoElement + use { let item_bounds = self.item_bounds.clone(); + let selectable = self.delegate.can_select(ix, window, cx); div() .id(("item", ix)) - .cursor_pointer() + .when(selectable, |this| this.cursor_pointer()) .child( canvas( move |bounds, _window, _cx| { @@ -853,6 +854,184 @@ impl Picker { } } +#[cfg(test)] +mod tests { + use super::*; + use gpui::TestAppContext; + use std::cell::Cell; + + struct TestDelegate { + items: Vec, + selected_index: usize, + confirmed_index: Rc>>, + } + + impl TestDelegate { + fn new(items: Vec) -> Self { + Self { + items, + selected_index: 0, + confirmed_index: Rc::new(Cell::new(None)), + } + } + } + + impl PickerDelegate for TestDelegate { + type ListItem = ui::ListItem; + + fn match_count(&self) -> usize { + self.items.len() + } + + fn selected_index(&self) -> usize { + self.selected_index + } + + fn set_selected_index( + &mut self, + ix: usize, + _window: &mut Window, + _cx: &mut Context>, + ) { + self.selected_index = ix; + } + + fn can_select( + &mut self, + ix: usize, + _window: &mut Window, + _cx: &mut Context>, + ) -> bool { + self.items.get(ix).copied().unwrap_or(false) + } + + fn placeholder_text(&self, _window: &mut Window, _cx: &mut App) -> Arc { + "Test".into() + } + + fn update_matches( + &mut self, + _query: String, + _window: &mut Window, + _cx: &mut Context>, + ) -> Task<()> { + Task::ready(()) + } + + fn confirm( + &mut self, + _secondary: bool, + _window: &mut Window, + _cx: &mut Context>, + ) { + self.confirmed_index.set(Some(self.selected_index)); + } + + fn dismissed(&mut self, _window: &mut Window, _cx: &mut Context>) {} + + fn render_match( + &self, + ix: usize, + selected: bool, + _window: &mut Window, + _cx: &mut Context>, + ) -> Option { + Some( + ui::ListItem::new(ix) + .inset(true) + .toggle_state(selected) + .child(ui::Label::new(format!("Item {ix}"))), + ) + } + } + + #[ctor::ctor] + fn init_logger() { + env_logger::init(); + } + + fn init_test(cx: &mut TestAppContext) { + cx.update(|cx| { + let store = settings::SettingsStore::test(cx); + cx.set_global(store); + theme::init(theme::LoadThemes::JustBase, cx); + editor::init(cx); + }); + } + + #[gpui::test] + async fn test_clicking_non_selectable_item_does_not_confirm(cx: &mut TestAppContext) { + init_test(cx); + + let confirmed_index = Rc::new(Cell::new(None)); + let (picker, cx) = cx.add_window_view(|window, cx| { + let mut delegate = TestDelegate::new(vec![true, false, true]); + delegate.confirmed_index = confirmed_index.clone(); + Picker::uniform_list(delegate, window, cx) + }); + + picker.update(cx, |picker, _cx| { + assert_eq!(picker.delegate.selected_index(), 0); + }); + + picker.update_in(cx, |picker, window, cx| { + picker.handle_click(1, false, window, cx); + }); + assert!( + confirmed_index.get().is_none(), + "clicking a non-selectable item should not confirm" + ); + + picker.update_in(cx, |picker, window, cx| { + picker.handle_click(0, false, window, cx); + }); + assert_eq!( + confirmed_index.get(), + Some(0), + "clicking a selectable item should confirm" + ); + } + + #[gpui::test] + async fn test_keyboard_navigation_skips_non_selectable_items(cx: &mut TestAppContext) { + init_test(cx); + + let (picker, cx) = cx.add_window_view(|window, cx| { + Picker::uniform_list( + TestDelegate::new(vec![true, false, true]), + window, + cx, + ) + }); + + picker.update(cx, |picker, _cx| { + assert_eq!(picker.delegate.selected_index(), 0); + }); + + picker.update_in(cx, |picker, window, cx| { + picker.select_next(&menu::SelectNext, window, cx); + }); + picker.update(cx, |picker, _cx| { + assert_eq!( + picker.delegate.selected_index(), + 2, + "select_next should skip non-selectable item at index 1" + ); + }); + + picker.update_in(cx, |picker, window, cx| { + picker.select_previous(&menu::SelectPrevious, window, cx); + }); + picker.update(cx, |picker, _cx| { + assert_eq!( + picker.delegate.selected_index(), + 0, + "select_previous should skip non-selectable item at index 1" + ); + }); + } +} + impl EventEmitter for Picker {} impl ModalView for Picker {} From d05754ac58aa1d65860b2271c74517ef53901694 Mon Sep 17 00:00:00 2001 From: Dnreikronos Date: Thu, 5 Mar 2026 11:41:14 -0300 Subject: [PATCH 3/4] picker: Change can_select to take &self and remove unnecessary test deps --- Cargo.lock | 3 --- .../agent_ui/src/agent_configuration/tool_picker.rs | 2 +- crates/agent_ui/src/config_options.rs | 2 +- crates/agent_ui/src/language_model_selector.rs | 2 +- crates/agent_ui/src/model_selector.rs | 2 +- crates/agent_ui/src/profile_selector.rs | 2 +- crates/picker/Cargo.toml | 3 --- crates/picker/src/picker.rs | 11 +++-------- crates/recent_projects/src/recent_projects.rs | 2 +- crates/rules_library/src/rules_library.rs | 2 +- crates/sidebar/src/sidebar.rs | 2 +- 11 files changed, 11 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6254c074faeb03..26d9997d395349 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12538,14 +12538,11 @@ name = "picker" version = "0.1.0" dependencies = [ "anyhow", - "ctor", "editor", - "env_logger 0.11.8", "gpui", "menu", "schemars", "serde", - "serde_json", "settings", "theme", "ui", diff --git a/crates/agent_ui/src/agent_configuration/tool_picker.rs b/crates/agent_ui/src/agent_configuration/tool_picker.rs index 1c99f665ab1c8f..e937344bdf2b2f 100644 --- a/crates/agent_ui/src/agent_configuration/tool_picker.rs +++ b/crates/agent_ui/src/agent_configuration/tool_picker.rs @@ -173,7 +173,7 @@ impl PickerDelegate for ToolPickerDelegate { } fn can_select( - &mut self, + &self, ix: usize, _window: &mut Window, _cx: &mut Context>, diff --git a/crates/agent_ui/src/config_options.rs b/crates/agent_ui/src/config_options.rs index 458411d4d3af3f..1b0bbe5bc1c9e6 100644 --- a/crates/agent_ui/src/config_options.rs +++ b/crates/agent_ui/src/config_options.rs @@ -494,7 +494,7 @@ impl PickerDelegate for ConfigOptionPickerDelegate { } fn can_select( - &mut self, + &self, ix: usize, _window: &mut Window, _cx: &mut Context>, diff --git a/crates/agent_ui/src/language_model_selector.rs b/crates/agent_ui/src/language_model_selector.rs index 9205e21be1ab79..8d777b179a59bb 100644 --- a/crates/agent_ui/src/language_model_selector.rs +++ b/crates/agent_ui/src/language_model_selector.rs @@ -456,7 +456,7 @@ impl PickerDelegate for LanguageModelPickerDelegate { } fn can_select( - &mut self, + &self, ix: usize, _window: &mut Window, _cx: &mut Context>, diff --git a/crates/agent_ui/src/model_selector.rs b/crates/agent_ui/src/model_selector.rs index 307eda507410a0..78ad5b181970e4 100644 --- a/crates/agent_ui/src/model_selector.rs +++ b/crates/agent_ui/src/model_selector.rs @@ -213,7 +213,7 @@ impl PickerDelegate for ModelPickerDelegate { } fn can_select( - &mut self, + &self, ix: usize, _window: &mut Window, _cx: &mut Context>, diff --git a/crates/agent_ui/src/profile_selector.rs b/crates/agent_ui/src/profile_selector.rs index 45d7232e0dff8b..3443691ea523f6 100644 --- a/crates/agent_ui/src/profile_selector.rs +++ b/crates/agent_ui/src/profile_selector.rs @@ -444,7 +444,7 @@ impl PickerDelegate for ProfilePickerDelegate { } fn can_select( - &mut self, + &self, ix: usize, _window: &mut Window, _cx: &mut Context>, diff --git a/crates/picker/Cargo.toml b/crates/picker/Cargo.toml index fc46e4a8454667..8c76aa74645386 100644 --- a/crates/picker/Cargo.toml +++ b/crates/picker/Cargo.toml @@ -28,9 +28,6 @@ workspace.workspace = true zed_actions.workspace = true [dev-dependencies] -ctor.workspace = true editor = { workspace = true, features = ["test-support"] } -env_logger.workspace = true gpui = { workspace = true, features = ["test-support"] } -serde_json.workspace = true settings.workspace = true diff --git a/crates/picker/src/picker.rs b/crates/picker/src/picker.rs index 94e6600b7b67fa..c9429b94708d4a 100644 --- a/crates/picker/src/picker.rs +++ b/crates/picker/src/picker.rs @@ -114,7 +114,7 @@ pub trait PickerDelegate: Sized + 'static { None } fn can_select( - &mut self, + &self, _ix: usize, _window: &mut Window, _cx: &mut Context>, @@ -750,7 +750,7 @@ impl Picker { } fn render_element( - &mut self, + &self, window: &mut Window, cx: &mut Context, ix: usize, @@ -897,7 +897,7 @@ mod tests { } fn can_select( - &mut self, + &self, ix: usize, _window: &mut Window, _cx: &mut Context>, @@ -945,11 +945,6 @@ mod tests { } } - #[ctor::ctor] - fn init_logger() { - env_logger::init(); - } - fn init_test(cx: &mut TestAppContext) { cx.update(|cx| { let store = settings::SettingsStore::test(cx); diff --git a/crates/recent_projects/src/recent_projects.rs b/crates/recent_projects/src/recent_projects.rs index 110a702437d463..90c2ffb350af82 100644 --- a/crates/recent_projects/src/recent_projects.rs +++ b/crates/recent_projects/src/recent_projects.rs @@ -751,7 +751,7 @@ impl PickerDelegate for RecentProjectsDelegate { } fn can_select( - &mut self, + &self, ix: usize, _window: &mut Window, _cx: &mut Context>, diff --git a/crates/rules_library/src/rules_library.rs b/crates/rules_library/src/rules_library.rs index a89657e29680cc..73bf5fdd8fcaaf 100644 --- a/crates/rules_library/src/rules_library.rs +++ b/crates/rules_library/src/rules_library.rs @@ -222,7 +222,7 @@ impl PickerDelegate for RulePickerDelegate { cx.notify(); } - fn can_select(&mut self, ix: usize, _: &mut Window, _: &mut Context>) -> bool { + fn can_select(&self, ix: usize, _: &mut Window, _: &mut Context>) -> bool { match self.filtered_entries.get(ix) { Some(RulePickerEntry::Rule(_)) => true, Some(RulePickerEntry::Header(_)) | Some(RulePickerEntry::Separator) | None => false, diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index 24974512cda122..761ed5d793649d 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -387,7 +387,7 @@ impl PickerDelegate for WorkspacePickerDelegate { } fn can_select( - &mut self, + &self, ix: usize, _window: &mut Window, _cx: &mut Context>, From 39af4c605edcba4f05147659d80867df83bb0b40 Mon Sep 17 00:00:00 2001 From: Dnreikronos Date: Thu, 5 Mar 2026 15:46:41 -0300 Subject: [PATCH 4/4] picker: Fix rustfmt formatting for can_select signatures --- crates/agent_ui/src/agent_configuration/tool_picker.rs | 7 +------ crates/agent_ui/src/config_options.rs | 7 +------ crates/agent_ui/src/language_model_selector.rs | 7 +------ crates/agent_ui/src/model_selector.rs | 7 +------ crates/agent_ui/src/profile_selector.rs | 7 +------ crates/picker/src/picker.rs | 6 +----- crates/recent_projects/src/recent_projects.rs | 7 +------ crates/sidebar/src/sidebar.rs | 7 +------ 8 files changed, 8 insertions(+), 47 deletions(-) diff --git a/crates/agent_ui/src/agent_configuration/tool_picker.rs b/crates/agent_ui/src/agent_configuration/tool_picker.rs index e937344bdf2b2f..be6fcb5bd2b5ee 100644 --- a/crates/agent_ui/src/agent_configuration/tool_picker.rs +++ b/crates/agent_ui/src/agent_configuration/tool_picker.rs @@ -172,12 +172,7 @@ impl PickerDelegate for ToolPickerDelegate { self.selected_index = ix; } - fn can_select( - &self, - ix: usize, - _window: &mut Window, - _cx: &mut Context>, - ) -> bool { + fn can_select(&self, ix: usize, _window: &mut Window, _cx: &mut Context>) -> bool { let item = &self.filtered_items[ix]; match item { PickerItem::Tool { .. } => true, diff --git a/crates/agent_ui/src/config_options.rs b/crates/agent_ui/src/config_options.rs index 1b0bbe5bc1c9e6..6ec2595202490c 100644 --- a/crates/agent_ui/src/config_options.rs +++ b/crates/agent_ui/src/config_options.rs @@ -493,12 +493,7 @@ impl PickerDelegate for ConfigOptionPickerDelegate { cx.notify(); } - fn can_select( - &self, - ix: usize, - _window: &mut Window, - _cx: &mut Context>, - ) -> bool { + fn can_select(&self, ix: usize, _window: &mut Window, _cx: &mut Context>) -> bool { match self.filtered_entries.get(ix) { Some(ConfigOptionPickerEntry::Option(_)) => true, Some(ConfigOptionPickerEntry::Separator(_)) | None => false, diff --git a/crates/agent_ui/src/language_model_selector.rs b/crates/agent_ui/src/language_model_selector.rs index 8d777b179a59bb..e6e72b3197b410 100644 --- a/crates/agent_ui/src/language_model_selector.rs +++ b/crates/agent_ui/src/language_model_selector.rs @@ -455,12 +455,7 @@ impl PickerDelegate for LanguageModelPickerDelegate { cx.notify(); } - fn can_select( - &self, - ix: usize, - _window: &mut Window, - _cx: &mut Context>, - ) -> bool { + fn can_select(&self, ix: usize, _window: &mut Window, _cx: &mut Context>) -> bool { match self.filtered_entries.get(ix) { Some(LanguageModelPickerEntry::Model(_)) => true, Some(LanguageModelPickerEntry::Separator(_)) | None => false, diff --git a/crates/agent_ui/src/model_selector.rs b/crates/agent_ui/src/model_selector.rs index 78ad5b181970e4..89ed3e490b33ca 100644 --- a/crates/agent_ui/src/model_selector.rs +++ b/crates/agent_ui/src/model_selector.rs @@ -212,12 +212,7 @@ impl PickerDelegate for ModelPickerDelegate { cx.notify(); } - fn can_select( - &self, - ix: usize, - _window: &mut Window, - _cx: &mut Context>, - ) -> bool { + fn can_select(&self, ix: usize, _window: &mut Window, _cx: &mut Context>) -> bool { match self.filtered_entries.get(ix) { Some(ModelPickerEntry::Model(_, _)) => true, Some(ModelPickerEntry::Separator(_)) | None => false, diff --git a/crates/agent_ui/src/profile_selector.rs b/crates/agent_ui/src/profile_selector.rs index 3443691ea523f6..926549c22f88bc 100644 --- a/crates/agent_ui/src/profile_selector.rs +++ b/crates/agent_ui/src/profile_selector.rs @@ -443,12 +443,7 @@ impl PickerDelegate for ProfilePickerDelegate { cx.notify(); } - fn can_select( - &self, - ix: usize, - _window: &mut Window, - _cx: &mut Context>, - ) -> bool { + fn can_select(&self, ix: usize, _window: &mut Window, _cx: &mut Context>) -> bool { match self.filtered_entries.get(ix) { Some(ProfilePickerEntry::Profile(_)) => true, Some(ProfilePickerEntry::Header(_)) | None => false, diff --git a/crates/picker/src/picker.rs b/crates/picker/src/picker.rs index c9429b94708d4a..e87ec3415cf6d7 100644 --- a/crates/picker/src/picker.rs +++ b/crates/picker/src/picker.rs @@ -992,11 +992,7 @@ mod tests { init_test(cx); let (picker, cx) = cx.add_window_view(|window, cx| { - Picker::uniform_list( - TestDelegate::new(vec![true, false, true]), - window, - cx, - ) + Picker::uniform_list(TestDelegate::new(vec![true, false, true]), window, cx) }); picker.update(cx, |picker, _cx| { diff --git a/crates/recent_projects/src/recent_projects.rs b/crates/recent_projects/src/recent_projects.rs index 90c2ffb350af82..e7a8358da6f8da 100644 --- a/crates/recent_projects/src/recent_projects.rs +++ b/crates/recent_projects/src/recent_projects.rs @@ -750,12 +750,7 @@ impl PickerDelegate for RecentProjectsDelegate { self.selected_index = ix; } - fn can_select( - &self, - ix: usize, - _window: &mut Window, - _cx: &mut Context>, - ) -> bool { + fn can_select(&self, ix: usize, _window: &mut Window, _cx: &mut Context>) -> bool { matches!( self.filtered_entries.get(ix), Some(ProjectPickerEntry::OpenFolder { .. } | ProjectPickerEntry::RecentProject(_)) diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index 761ed5d793649d..e8fc3876e29aae 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -386,12 +386,7 @@ impl PickerDelegate for WorkspacePickerDelegate { self.selected_index = ix; } - fn can_select( - &self, - ix: usize, - _window: &mut Window, - _cx: &mut Context>, - ) -> bool { + fn can_select(&self, ix: usize, _window: &mut Window, _cx: &mut Context>) -> bool { match self.matches.get(ix) { Some(SidebarMatch { entry: SidebarEntry::Separator(_),