From 4054d4a5b73be1829dc35b937ab25a6b94fbfb16 Mon Sep 17 00:00:00 2001 From: Agus Zubiaga Date: Mon, 13 Jan 2025 18:00:20 -0300 Subject: [PATCH] assistant2: Fix inline context picker and handle dismiss (#23081) The new `ContextMenu`-based `ContextPicker` requires initialization when opened, but we were only doing this for the `ContextStrip` picker, not the inline one. Additionally, because we have a wrapper element around ContextMenu, we need to propagate the `DismissEvent` so that it properly closes when Escape is pressed. Release Notes: - N/A --- crates/assistant2/src/context_picker.rs | 26 ++++++++++++++----- .../directory_context_picker.rs | 3 +-- .../context_picker/fetch_context_picker.rs | 3 +-- .../src/context_picker/file_context_picker.rs | 3 +-- .../context_picker/thread_context_picker.rs | 3 +-- crates/assistant2/src/context_strip.rs | 2 +- crates/assistant2/src/message_editor.rs | 8 +++++- crates/ui/src/components/context_menu.rs | 11 +++++++- 8 files changed, 42 insertions(+), 17 deletions(-) diff --git a/crates/assistant2/src/context_picker.rs b/crates/assistant2/src/context_picker.rs index 85fc282ca217770971d54798077d0feaf77be7ad..4f8e9fd3064f91cb859c5e4ac27fef600713cf18 100644 --- a/crates/assistant2/src/context_picker.rs +++ b/crates/assistant2/src/context_picker.rs @@ -65,14 +65,15 @@ impl ContextPicker { } } - pub fn reset_mode(&mut self, cx: &mut ViewContext) { - self.mode = ContextPickerMode::Default(self.build(cx)); + pub fn init(&mut self, cx: &mut ViewContext) { + self.mode = ContextPickerMode::Default(self.build_menu(cx)); + cx.notify(); } - fn build(&mut self, cx: &mut ViewContext) -> View { + fn build_menu(&mut self, cx: &mut ViewContext) -> View { let context_picker = cx.view().clone(); - ContextMenu::build(cx, move |menu, cx| { + let menu = ContextMenu::build(cx, move |menu, cx| { let kind_entry = |kind: &'static ContextKind| { let context_picker = context_picker.clone(); @@ -90,11 +91,24 @@ impl ContextPicker { .enumerate() .map(|(ix, entry)| self.recent_menu_item(context_picker.clone(), ix, entry)); - menu.when(has_recent, |menu| menu.label("Recent")) + let menu = menu + .when(has_recent, |menu| menu.label("Recent")) .extend(recent_entries) .when(has_recent, |menu| menu.separator()) - .extend(ContextKind::all().into_iter().map(kind_entry)) + .extend(ContextKind::all().into_iter().map(kind_entry)); + + match self.confirm_behavior { + ConfirmBehavior::KeepOpen => menu.keep_open_on_confirm(), + ConfirmBehavior::Close => menu, + } + }); + + cx.subscribe(&menu, move |_, _, _: &DismissEvent, cx| { + cx.emit(DismissEvent); }) + .detach(); + + menu } fn select_kind(&mut self, kind: ContextKind, cx: &mut ViewContext) { diff --git a/crates/assistant2/src/context_picker/directory_context_picker.rs b/crates/assistant2/src/context_picker/directory_context_picker.rs index 969e0e7f43bd4c8d7bef8ee9dd925df35dd405b5..fce39fec6d919d2cb8166325f74b90824cc41a00 100644 --- a/crates/assistant2/src/context_picker/directory_context_picker.rs +++ b/crates/assistant2/src/context_picker/directory_context_picker.rs @@ -221,8 +221,7 @@ impl PickerDelegate for DirectoryContextPickerDelegate { fn dismissed(&mut self, cx: &mut ViewContext>) { self.context_picker - .update(cx, |this, cx| { - this.reset_mode(cx); + .update(cx, |_, cx| { cx.emit(DismissEvent); }) .ok(); diff --git a/crates/assistant2/src/context_picker/fetch_context_picker.rs b/crates/assistant2/src/context_picker/fetch_context_picker.rs index dd23ed7c9fb3c7f8bf5b583f96c116a019a76834..7bb089e82b92086c72d0830ec90afd21ea87517e 100644 --- a/crates/assistant2/src/context_picker/fetch_context_picker.rs +++ b/crates/assistant2/src/context_picker/fetch_context_picker.rs @@ -222,8 +222,7 @@ impl PickerDelegate for FetchContextPickerDelegate { fn dismissed(&mut self, cx: &mut ViewContext>) { self.context_picker - .update(cx, |this, cx| { - this.reset_mode(cx); + .update(cx, |_, cx| { cx.emit(DismissEvent); }) .ok(); diff --git a/crates/assistant2/src/context_picker/file_context_picker.rs b/crates/assistant2/src/context_picker/file_context_picker.rs index 282c4c70b982b5e1a8fb16bd9955b6981b7fbdfb..533f793bed025387b3c42be29dcda43149e3ea92 100644 --- a/crates/assistant2/src/context_picker/file_context_picker.rs +++ b/crates/assistant2/src/context_picker/file_context_picker.rs @@ -239,8 +239,7 @@ impl PickerDelegate for FileContextPickerDelegate { fn dismissed(&mut self, cx: &mut ViewContext>) { self.context_picker - .update(cx, |this, cx| { - this.reset_mode(cx); + .update(cx, |_, cx| { cx.emit(DismissEvent); }) .ok(); diff --git a/crates/assistant2/src/context_picker/thread_context_picker.rs b/crates/assistant2/src/context_picker/thread_context_picker.rs index af52ec7172811b99432de3713c92493b0fc6e3ae..bea32d3d043ee8d14815b04ffc81c55a9d276b11 100644 --- a/crates/assistant2/src/context_picker/thread_context_picker.rs +++ b/crates/assistant2/src/context_picker/thread_context_picker.rs @@ -176,8 +176,7 @@ impl PickerDelegate for ThreadContextPickerDelegate { fn dismissed(&mut self, cx: &mut ViewContext>) { self.context_picker - .update(cx, |this, cx| { - this.reset_mode(cx); + .update(cx, |_, cx| { cx.emit(DismissEvent); }) .ok(); diff --git a/crates/assistant2/src/context_strip.rs b/crates/assistant2/src/context_strip.rs index 10ac4edd7e4ee8aa86128ee22c8201c2060bf118..d9689192c907e0ca8605c9fd1fa9360890aac228 100644 --- a/crates/assistant2/src/context_strip.rs +++ b/crates/assistant2/src/context_strip.rs @@ -168,7 +168,7 @@ impl Render for ContextStrip { PopoverMenu::new("context-picker") .menu(move |cx| { context_picker.update(cx, |this, cx| { - this.reset_mode(cx); + this.init(cx); }); Some(context_picker.clone()) diff --git a/crates/assistant2/src/message_editor.rs b/crates/assistant2/src/message_editor.rs index cd4c5899ee773d401846989883f91e6015f72428..f1092f16deb936fa79a22c4506c670b990480238 100644 --- a/crates/assistant2/src/message_editor.rs +++ b/crates/assistant2/src/message_editor.rs @@ -281,7 +281,13 @@ impl Render for MessageEditor { }) .child( PopoverMenu::new("inline-context-picker") - .menu(move |_cx| Some(inline_context_picker.clone())) + .menu(move |cx| { + inline_context_picker.update(cx, |this, cx| { + this.init(cx); + }); + + Some(inline_context_picker.clone()) + }) .attach(gpui::Corner::TopLeft) .anchor(gpui::Corner::BottomLeft) .offset(gpui::Point { diff --git a/crates/ui/src/components/context_menu.rs b/crates/ui/src/components/context_menu.rs index 53f2a62bb3b5086701d5462a5c7be944860adb06..38d151c8b0ed318bd4b7e9cdfa72290087dd6508 100644 --- a/crates/ui/src/components/context_menu.rs +++ b/crates/ui/src/components/context_menu.rs @@ -112,6 +112,7 @@ pub struct ContextMenu { delayed: bool, clicked: bool, _on_blur_subscription: Subscription, + keep_open_on_confirm: bool, } impl FocusableView for ContextMenu { @@ -144,6 +145,7 @@ impl ContextMenu { delayed: false, clicked: false, _on_blur_subscription, + keep_open_on_confirm: true, }, cx, ) @@ -304,6 +306,11 @@ impl ContextMenu { self } + pub fn keep_open_on_confirm(mut self) -> Self { + self.keep_open_on_confirm = true; + self + } + pub fn confirm(&mut self, _: &menu::Confirm, cx: &mut ViewContext) { let context = self.action_context.as_ref(); if let Some( @@ -318,7 +325,9 @@ impl ContextMenu { (handler)(context, cx) } - cx.emit(DismissEvent); + if !self.keep_open_on_confirm { + cx.emit(DismissEvent); + } } pub fn cancel(&mut self, _: &menu::Cancel, cx: &mut ViewContext) {